Re: [PATCH 01/23] t3600: clean up permissions test properly
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 18, 2017, 04:10 UTC
- Message-ID
- <xmqqa86avm8j.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <bb244c72ec6e6ab56171d82f7379a75c4eb5481f.1495014840.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 26 quoted lines
> The test of failing `git rm -f` removes the write permissions on the > test directory, but fails to restore them if the test fails. This > means that the test temporary directory cannot be cleaned up, which > means that subsequent attempts to run the test fail mysteriously. > > Instead, do the cleanup in a `test_must_fail` block so that it can't > be skipped. > > Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> > --- > t/t3600-rm.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh > index 5f9913ba33..4a35c378c8 100755 > --- a/t/t3600-rm.sh > +++ b/t/t3600-rm.sh > @@ -98,8 +98,8 @@ embedded'" > > test_expect_success SANITY 'Test that "git rm -f" fails if its rm fails' ' > chmod a-w . && > - test_must_fail git rm -f baz && > - chmod 775 . > + test_when_finished "chmod 775 ." && > + test_must_fail git rm -f baz > '
Obviously a good idea.
In this case it would not matter very much, but I think it is a better style to have when-finished _before_ "chmod a-w ." that introduces the state we want to revert out of.