git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] t3600: fix broken "choking git rm" test
@ 2013-10-16  0:48 SZEDER Gábor
  0 siblings, 0 replies; 4+ messages in thread
From: SZEDER Gábor @ 2013-10-16  0:48 UTC (permalink / raw)
  To: Junio C Hamano, Jonathan Nieder; +Cc: git, SZEDER Gábor

The test 'choking "git rm" should not let it die with cruft' is
supposed to check 'git rm's behavior when interrupted by provoking a
SIGPIPE while 'git rm' is busily deleting files from a specially
crafted index.

This test is silently broken for the following reasons:

- The test crafts a special index by feeding a large number of index
  entries with null shas to 'git update-index --index-info'.  It was
  OK back then when this test was introduced in commit 0693f9ddad
  (Make sure lockfiles are unlocked when dying on SIGPIPE,
  2008-12-18), but since commit 4337b5856f (do not write null sha1s to
  on-disk index, 2012-07-28) null shas are not allowed in the on-disk
  index causing 'git update-index' to error out.

- The barfing 'git update-index --index-info' should fail the test,
  but it remains unnoticed because of the severely broken && chain:
  the test's result depends solely on whether there is a stale lock
  file left behind, but after 'git update-index' errors out 'git rm'
  won't be executed at all.

To fix this test feed only non-null shas to 'git update-index' and
restore the && chain (partly by adding a missing && and by using the
test_when_finished helper instead of manual cleanup).

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t3600-rm.sh | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 824342f413..ad30a61f9e 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -230,18 +230,15 @@ test_expect_success 'refresh index before checking if it is up-to-date' '
 
 test_expect_success 'choking "git rm" should not let it die with cruft' '
 	git reset -q --hard &&
+	test_when_finished "rm -f .git/index.lock && git reset -q --hard" &&
 	i=0 &&
 	while test $i -lt 12000
 	do
-	    echo "100644 $_z40 0	some-file-$i"
+	    echo "100644 1234567890123456789012345678901234567890 0	some-file-$i"
 	    i=$(( $i + 1 ))
 	done | git update-index --index-info &&
-	git rm -n "some-file-*" | :;
-	test -f .git/index.lock
-	status=$?
-	rm -f .git/index.lock
-	git reset -q --hard
-	test "$status" != 0
+	git rm -n "some-file-*" | : &&
+	test_path_is_missing .git/index.lock
 '
 
 test_expect_success 'rm removes subdirectories recursively' '
-- 
1.8.4.1.495.gd8d272e

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH] t3600: fix broken "choking git rm" test
@ 2013-10-16  0:01 SZEDER Gábor
  2013-10-16  0:18 ` Jonathan Nieder
  0 siblings, 1 reply; 4+ messages in thread
From: SZEDER Gábor @ 2013-10-16  0:01 UTC (permalink / raw)
  To: git; +Cc: SZEDER Gábor

The test 'choking "git rm" should not let it die with cruft' is
supposed to check 'git rm's behavior when interrupted by provoking a
SIGPIPE while 'git rm' is busily deleting files from a specially
crafted index.

This test is silently broken for the following reasons:

- The test crafts a special index by feeding a large number of index
  entries with null shas to 'git update-index --index-info'.  It was
  OK back then when this test was introduced in commit 0693f9ddad
  (Make sure lockfiles are unlocked when dying on SIGPIPE,
  2008-12-18), but since commit 4337b5856f (do not write null sha1s to
  on-disk index, 2012-07-28) null shas are not allowed in the on-disk
  index causing 'git update-index' to error out.

- The barfing 'git update-index --index-info' should fail the test,
  but it remains unnoticed because of the severely broken && chain:
  the test's result depends solely on whether there is a stale lock
  file left behind, but after 'git update-index' errors out 'git rm'
  won't be executed at all.

To fix this test feed only non-null shas to 'git update-index' and
restore the && chain (partly by adding a missing && and by using the
test_when_finished helper instead of manual cleanup).

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
A particularly funny one from the fallout of gmane/236183

 t/t3600-rm.sh | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 85854f338f..4dd0130dd9 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -240,18 +240,15 @@ test_expect_success 'refresh index before checking if it is up-to-date' '
 
 test_expect_success 'choking "git rm" should not let it die with cruft' '
 	git reset -q --hard &&
+	test_when_finished "rm -f .git/index.lock ; git reset -q --hard" &&
 	i=0 &&
 	while test $i -lt 12000
 	do
-	    echo "100644 $_z40 0	some-file-$i"
+	    echo "100644 1234567890123456789012345678901234567890 0	some-file-$i"
 	    i=$(( $i + 1 ))
 	done | git update-index --index-info &&
-	git rm -n "some-file-*" | :;
-	test -f .git/index.lock
-	status=$?
-	rm -f .git/index.lock
-	git reset -q --hard
-	test "$status" != 0
+	git rm -n "some-file-*" | : &&
+	test ! -f .git/index.lock
 '
 
 test_expect_success 'rm removes subdirectories recursively' '
-- 
1.8.4.1.495.gd8d272e

^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-16  0:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-16  0:48 [PATCH] t3600: fix broken "choking git rm" test SZEDER Gábor
  -- strict thread matches above, loose matches on Subject: below --
2013-10-16  0:01 SZEDER Gábor
2013-10-16  0:18 ` Jonathan Nieder
2013-10-16  0:38   ` SZEDER Gábor

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 NNTP newsgroup(s).