git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)
Date: Tue, 04 Jun 2013 14:55:01 -0700	[thread overview]
Message-ID: <7vd2s15yoa.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <3c8aaaa4ad606d2afb7c110a28c119792caa8aba.1370076477.git.j6t@kdbg.org> (Johannes Sixt's message of "Sat, 1 Jun 2013 11:34:24 +0200")

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.

> diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
> index 88be904..563ac7f 100755
> --- a/t/t3000-ls-files-others.sh
> +++ b/t/t3000-ls-files-others.sh
> @@ -19,12 +19,7 @@ filesystem.
>  
>  test_expect_success 'setup ' '
>  	date >path0 &&
> -	if test_have_prereq SYMLINKS
> -	then
> -		ln -s xyzzy path1
> -	else
> -		date >path1
> -	fi &&
> +	test_ln_s xyzzy path1 &&
>  	mkdir path2 path3 path4 &&
>  	date >path2/file2 &&
>  	date >path2-junk &&

This also is not appropriate, but it is not as bad as the one in
t2003 I earlier commented on.

This test wants an untracked symbolic link in the working tree as
path1 and wants to see "ls-files -o" report it as "other".  On a
filesystem that lack symbolic link, we obviously cannot have one,
so as a substitute we just create another regular file to make the
expected output and comparison simpler. 

> diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh
> index 2d0ff2d..310e0a2 100755
> --- a/t/t3010-ls-files-killed-modified.sh
> +++ b/t/t3010-ls-files-killed-modified.sh
> @@ -39,12 +39,7 @@ modified without reporting path9 and path10.
> ...
> +	test_ln_s_add xyzzy path1 &&
> ...
>  	date >path2/file2 &&
>  	date >path3/file3 &&

This one is correct; the test wants to make sure that path1 would be
checked out as a non-directory, killing any files in path1/ that is
a directory on the filesystem.

> @@ -52,20 +47,14 @@ test_expect_success 'git update-index --add to add various paths.' '
>  	date >path8 &&
>  	: >path9 &&
>  	date >path10 &&
> -	git update-index --add -- path0 path1 path?/file? path7 path8 path9 path10 &&
> +	git update-index --add -- path0 path?/file? path7 path8 path9 path10 &&

And exclusion of path1 here is a logical consequence of the previous
change, so it is good, too.

>  test_expect_success 'git ls-files -k to show killed files.' '
>  	date >path2 &&
> -	if test_have_prereq SYMLINKS
> -	then
> -		ln -s frotz path3 &&
> -		ln -s nitfol path5
> -	else
> -		date >path3 &&
> -		date >path5
> -	fi &&
> +	test_ln_s frotz path3 &&
> +	test_ln_s nitfol path5 &&

This falls into the same category as the one in t3000 above.  The
only thing that matters in this test is path3 and path5 are non
directories so that the former is killed when path3/file3 needs to
be checked out, while path5 will not appear in "ls-files -k" output.
Ideally we would want to test regular files and symlinks as two
different kinds of "non directory" filesystem entities, but on
platforms that lack symbolic links we cannot do so, hence we punt
and create a regular file.

> diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
> index f0d5041..3888519 100755
> --- a/t/t4011-diff-symlink.sh
> +++ b/t/t4011-diff-symlink.sh
> @@ -66,12 +69,12 @@ test_expect_success SYMLINKS 'diff removed symlink and file' '
>  	compare_diff_patch expected current
>  '
>  
> -test_expect_success SYMLINKS 'diff identical, but newly created symlink and file' '
> +test_expect_success 'diff identical, but newly created symlink and file' '
>  	>expected &&
>  	rm -f frotz nitfol &&
>  	echo xyzzy >nitfol &&
>  	test-chmtime +10 nitfol &&
> -	ln -s xyzzy frotz &&
> +	test_ln_s xyzzy frotz &&
>  	git diff-index -M -p $tree >current &&
>  	compare_diff_patch expected current &&

As the point of test is to compare $tree that has symlink with
another symlink that is identical but newly created one, I think
this _does_ want the filesystem entity to be a symbolic link, but
the index has frotz as a symlink and the mode propagates to what we
read from the filesystem on !has_symlinks systems, so this
conversion may be a correct one, though it is a bit tricky.

> @@ -80,7 +83,7 @@ test_expect_success SYMLINKS 'diff identical, but newly created symlink and file
>  	compare_diff_patch expected current
>  '
>  
> -test_expect_success SYMLINKS 'diff different symlink and file' '
> +test_expect_success 'diff different symlink and file' '
>  	cat >expected <<-\EOF &&
>  	diff --git a/frotz b/frotz
>  	index 7c465af..df1db54 120000
> @@ -100,7 +103,7 @@ test_expect_success SYMLINKS 'diff different symlink and file' '
>  	+yxyyz
>  	EOF
>  	rm -f frotz &&
> -	ln -s yxyyz frotz &&
> +	test_ln_s yxyyz frotz &&
>  	echo yxyyz >nitfol &&
>  	git diff-index -M -p $tree >current &&
>  	compare_diff_patch expected current

Likewise.

  parent reply	other threads:[~2013-06-04 21:55 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-01  9:34 [PATCH 00/11] Increase test coverage on Windows by removing SYMLINKS from many tests Johannes Sixt
2013-06-01  9:34 ` [PATCH 01/11] test-chmtime: Fix exit code on Windows Johannes Sixt
2013-06-01  9:34 ` [PATCH 02/11] t2100: modernize style and unroll a loop of test cases Johannes Sixt
2013-06-01  9:34 ` [PATCH 03/11] t3010: modernize style Johannes Sixt
2013-06-01  9:34 ` [PATCH 04/11] tests: introduce test_ln_s and test_ln_s_add Johannes Sixt
2013-06-01 11:11   ` Ramkumar Ramachandra
2013-06-01 15:06     ` Johannes Sixt
2013-06-01 17:10       ` Ramkumar Ramachandra
2013-06-01  9:34 ` [PATCH 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases) Johannes Sixt
2013-06-04 21:06   ` Junio C Hamano
2013-06-05 19:32     ` Johannes Sixt
2013-06-04 21:55   ` Junio C Hamano [this message]
2013-06-05 20:07     ` Johannes Sixt
2013-06-01  9:34 ` [PATCH 06/11] t0000: use test_ln_s_add to remove SYMLINKS prerequisite Johannes Sixt
2013-06-01  9:34 ` [PATCH 07/11] t2100: " Johannes Sixt
2013-06-04 22:04   ` Junio C Hamano
2013-06-05 20:23     ` Johannes Sixt
2013-06-01  9:34 ` [PATCH 08/11] t3030: " Johannes Sixt
2013-06-01  9:34 ` [PATCH 09/11] t3100: " Johannes Sixt
2013-06-01  9:34 ` [PATCH 10/11] t3509, t4023, t4114: " Johannes Sixt
2013-06-02 23:44   ` Junio C Hamano
2013-06-01  9:34 ` [PATCH 11/11] t6035: " Johannes Sixt
2013-06-07 20:53 ` [PATCH v2 00/10] Increase test coverage on Windows by removing SYMLINKS from many tests Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 01/10] test-chmtime: Fix exit code on Windows Johannes Sixt
2013-11-20 14:00     ` Erik Faye-Lund
2013-06-07 20:53   ` [PATCH v2 02/10] t3010: modernize style Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 03/10] tests: introduce test_ln_s_add Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 04/10] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases) Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 05/10] t0000: use test_ln_s_add to remove SYMLINKS prerequisite Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 06/10] t3030: " Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 07/10] t3100: " Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 08/10] t3509, t4023, t4114: " Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 09/10] t6035: " Johannes Sixt
2013-06-07 20:53   ` [PATCH v2 10/10] t4011: " Johannes Sixt
2013-06-07 22:04   ` [PATCH v2 00/10] Increase test coverage on Windows by removing SYMLINKS from many tests Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vd2s15yoa.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).