Re: git 2.10.1 test regression in t3700-add.sh

Jeremy Huddleston Sequoia [off-list ref] writes:

quoted

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 924a266126..53c0cb6dea 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh

@@ -350,6 +350,7 @@ test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x with symlinks' '

'

test_expect_success 'git add --chmod=[+-]x changes index with already added file' '
+	rm -f foo3 xfoo3 &&
	echo foo >foo3 &&
	git add foo3 &&
	git add --chmod=+x foo3 &&

I actually tried that, but the problem is that xfoo3 was
previously added as a symlink, so test_mode_in_index still reports
it as a symlink.

Ah, of course.  That "rm -f" needs to remove from the index and also
from the working tree, so has to be "git rm -f --ignore-unmatch" or
something like that.

It's fundamentally a question of who is responsible for cleanup.
Is the individual test responsible for cleaning up after itself
(such that later tests can rely on a clean state), or should
individual tests assume that the initial state might be undefined
and try to cleanup after earlier tests?

In modern tests, we strive to do the former with liberal use of
test_when_finished.  I think the one that creates xfoo[123] and
leaves them behind for a long time predates the modern practice.

A minimal fix with that approach may look like this:

 t/t3700-add.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 924a266126..80c7ee3e3b 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh

@@ -64,6 +64,7 @@ test_expect_success 'git add: filemode=0 should not get confused by symlink' '
 test_expect_success \
 	'git update-index --add: Test that executable bit is not used...' \
 	'git config core.filemode 0 &&
+	 test_when_finished "git rm -f xfoo3" &&
 	 test_ln_s_add xfoo2 xfoo3 &&	# runs git update-index --add
 	 test_mode_in_index 120000 xfoo3'