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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Oct 16, 2013, 00:38 UTC
Message-ID: <20131016003846.GU19704@goldbirke>
In-Reply-To: <20131016001804.GJ9464@google.com>

On Tue, Oct 15, 2013 at 05:18:04PM -0700, Jonathan Nieder wrote:

Show 11 quoted lines

> SZEDER Gábor wrote:
> > --- 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.

OK. My motivation for the ';' was that we should make sure that both steps of the cleanup are executed. However, now thinking about it if regular 'rm -f' can't remove the lock file then all bets are off anyway.

Show 13 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.)

I was not aware of the test_path_is_missing helper. I don't think it matters whether it's a file or a directory, because a stale .git/index.lock directory would be just as bad.