Re: [PATCH 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)

[Johannes Sixt <j6t@kdbg.org>]

Am 04.06.2013 23:06, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> There are many instances where the treatment of symbolic links in the
>> object model and the algorithms are tested, but where it is not
>> necessary to actually have a symbolic link in the worktree. Make
>> adjustments to the tests and remove the SYMLINKS prerequisite when
>> appropriate in trivial cases, where "trivial" means:
>>
>> - merely a replacement of 'ln -s a b' to test_ln_s or of
>>   'ln -s a b && git add b' to test_ln_s_add is needed;
>>
>> - a test for symbolic link on the file system can be split off (and
>>   remains protected by SYMLINKS);
>>
>> - existing code is equivalent to test_ln_s[_add].
> 
> This is too big to review in one go, so I may have separate messages
> later on this same patch.
> 
>> diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh
>> index ff163cf..bd17ba2 100755
>> --- a/t/t2003-checkout-cache-mkdir.sh
>> +++ b/t/t2003-checkout-cache-mkdir.sh
>> @@ -19,10 +19,10 @@ test_expect_success 'setup' '
>>  	git update-index --add path0 path1/file1
>>  '
>>  
>> -test_expect_success SYMLINKS 'have symlink in place where dir is expected.' '
>> +test_expect_success 'have symlink in place where dir is expected.' '
>>  	rm -fr path0 path1 &&
>>  	mkdir path2 &&
>> -	ln -s path2 path1 &&
>> +	test_ln_s path2 path1 &&
>>  	git checkout-index -f -a &&
>>  	test ! -h path1 && test -d path1 &&
>>  	test -f path1/file1 && test ! -f path2/file1
> 
> I do not think this hunk is correct.
[...]
> Under the precondition checkout-index runs in this test, a casual
> 
> 	echo rezrov >path1/file1
> 
> would leave path1 as a symlink without turning it into a real
> directory, and we will end up creating path2/file1.  We are making
> sure that checkout-index does not behave that way, and it is
> essential to have symlink support in the working tree for the "bug"
> to trigger.
> 
>> @@ -79,10 +79,10 @@ test_expect_success SYMLINKS 'use --prefix=tmp/orary- where tmp is a symlink' '
>>  	test -h tmp
>>  '
>>  
>> -test_expect_success SYMLINKS 'use --prefix=tmp- where tmp-path1 is a symlink' '
>> +test_expect_success 'use --prefix=tmp- where tmp-path1 is a symlink' '
>>  	rm -fr path0 path1 path2 tmp* &&
>>  	mkdir tmp1 &&
>> -	ln -s tmp1 tmp-path1 &&
>> +	test_ln_s tmp1 tmp-path1 &&
>>  	git checkout-index --prefix=tmp- -f -a &&
>>  	test -f tmp-path0 &&
>>  	test ! -h tmp-path1 &&
> 
> This change has the same issue, I think.

Yes, agreed. The converted tests -- when SYMLINKS are not available --
just repeat what is tested elsewhere.

Nice catch. After I've found a hammer (test_ln_s) I was mindlessly
looking for nails and found two of them in these two instances. ;-)

-- Hannes