Re: [PATCH 07/11] t2100: use test_ln_s_add to remove SYMLINKS prerequisite

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 4, 2013, 22:04 UTC
Message-ID: <7v8v2p5y7o.fsf@alter.siamese.dyndns.org>
In-Reply-To: <e232ae3efc6f7b282d4402e6e6976b34d34ab3e7.1370076477.git.j6t@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 13 quoted lines

> @@ -62,12 +57,7 @@ test_expect_success 'git update-index to add conflicting file path2 should fail'
>  
>  test_expect_success 'git update-index to add conflicting symlink path3 should fail' '
>  
> -	if test_have_prereq SYMLINKS
> -	then
> -		ln -s xyzzy path3
> -	else
> -		date >path3
> -	fi &&
> +	test_ln_s xyzzy path3 &&
>  	test_must_fail git update-index --add -- path3
>  '

This is also borderline questionable. With path2, we are already testing that adding a regular file (one variant of "non directory") at a path that the index expects to see a directory (the index has path2/file2 in it at this point) fails, and the test about path3 is to make sure that an attempt to add a symbolic link, the other variant of "non directory", is rejected the same way. So it may make more sense to skip this test when !SYMLINKS.

If we want to really test the equivalent on a filesystem without symbolic links, it would be a more faithful test to attempt to add it using "--add --cacheinfo" and see it fail, i.e.

   test_must_fail_to_ln_s_add xyzzy path3

which would be a copy of test_ln_s_add but has test_must_fail before two calls to git_update_index it makes.

I think all the test_ln_s_add conversion in the series make sense, but many uses of test_ln_s are questionable, and I suspect it would invite similar confusion down the road.