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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 15, 2013 at 05:18:04PM -0700, Jonathan Nieder wrote:
> 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.

> [...]
> > -	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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]