On Thu, Jul 29, 2010 at 01:09, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>
>> Change the tests that skipped due to unavailable SYMLINKS support to
>> use the three-arg prereq form of test_expect_success.
>>
>> This is like the "tests: implicitly skip SYMLINKS tests using
>> <prereq>" change, but I needed to create an additional test for some
>> setup code. It's in a separate change as suggested by Jonathan Nieder
>> for ease of reviewing.
>
> Hmm, I still don’t understand this. Do you mean that there is
> some setup that needs to be run before these commands, and so
> the patch fails if that change is not included?
>
> Or is it a matter of "while at it, fix this other problem
> I noticed" (which would be fine, but it is clearer to
> present it as such if so)?
The setup code needs to be inside a test so that it'll only run if we
have SYMLINKS support.
I could also have done:
if test_have_prereq PERL
then
..setup code..
fi
But setup code should be inside tests, so that we'll get failure
reporting.
>> diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
>> index 1a09e8d..92a65f4 100755
>> --- a/t/t4004-diff-rename-symlink.sh
>> +++ b/t/t4004-diff-rename-symlink.sh
>> @@ -40,8 +34,9 @@ test_expect_success \
>> # rezrov and nitfol are rename/copy of frotz and bozbar should be
>> # a new creation.
>>
>> -GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current
>> -cat >expected <<\EOF
>> +test_expect_success SYMLINKS 'setup diff output' "
>> + GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current &&
>> + cat >expected <<\EOF
>> diff --git a/bozbar b/bozbar
>> new file mode 120000
>> --- /dev/null
>> @@ -65,8 +60,9 @@ deleted file mode 100644
>> -xyzzy
>> \ No newline at end of file
>> EOF
>> +"
>
> Tip for the future: if you use 'straight quotes', then readers
> can avoid carefully scanning through for $ and similar oddities
> (and the test script presented with the "expecting success:"
> prompt will use the friendlier $tree instead of 76c98ds89).
Ah, thanks.
> The patch looks good; my only remaining concern is the log
> message as mentioned above.
Hopefully that's cleared up now.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html