Re: [PATCH] t3600: fix broken "choking git rm" test
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Oct 16, 2013, 00:18 UTC
- Message-ID
- <20131016001804.GJ9464@google.com>
- In-Reply-To
- <1381881687-11179-1-git-send-email-szeder@ira.uka.de>
SZEDER Gábor wrote:
Show 6 quoted lines
> 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.
Show 7 quoted lines
> --- 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.
[...]
Show 6 quoted lines
> - 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' '