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 07/11] t2100: use test_ln_s_add to remove SYMLINKS prerequisite
Date: Tue, 04 Jun 2013 15:04:59 -0700	[thread overview]
Message-ID: <7v8v2p5y7o.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <e232ae3efc6f7b282d4402e6e6976b34d34ab3e7.1370076477.git.j6t@kdbg.org> (Johannes Sixt's message of "Sat, 1 Jun 2013 11:34:26 +0200")

Johannes Sixt <j6t@kdbg.org> writes:

> @@ -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.

  reply	other threads:[~2013-06-04 22:05 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
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 [this message]
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=7v8v2p5y7o.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).