Re: [PATCH 01/23] t3600: clean up permissions test properly
- Date: Thu, 18 May 2017 13:10:20 +0900
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH 01/23] t3600: clean up permissions test properly
Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
> 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@xxxxxxxxxxxx>
> 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.