Re: [PATCH] t3600: fix broken "choking git rm" test

[Jonathan Nieder <jrnieder@gmail.com>]

SZEDER Gábor wrote:

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

Fun. :)  Makes sense.

> --- 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'd use "&&" here --- the test_cleanup checks the exit status from
this scriptlet, so it's a good habit.

[...]
> -	test -f .git/index.lock
> -	status=$?
> -	rm -f .git/index.lock
> -	git reset -q --hard
> -	test "$status" != 0
> +	test ! -f .git/index.lock

Gah.  Thanks for cleaning it up.

Maybe test_path_is_missing would make sense here?  (It would notice a
.git/index.lock directory, which is not very likely :), but more
importantly, it says why it is failing the test when it fails.)

With or without the changes mentioned above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

diff --git i/t/t3600-rm.sh w/t/t3600-rm.sh
index 8386b54..540c49b 100755
--- i/t/t3600-rm.sh
+++ w/t/t3600-rm.sh
@@ -240,7 +240,7 @@ 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" &&
+	test_when_finished "rm -f .git/index.lock && git reset -q --hard" &&
 	i=0 &&
 	while test $i -lt 12000
 	do
@@ -248,7 +248,7 @@ test_expect_success 'choking "git rm" should not let it die with cruft' '
 	    i=$(( $i + 1 ))
 	done | git update-index --index-info &&
 	git rm -n "some-file-*" | : &&
-	test ! -f .git/index.lock
+	test_path_is_missing .git/index.lock
 '
 
 test_expect_success 'rm removes subdirectories recursively' '