Re: [PATCH 01/23] t3600: clean up permissions test properly

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> 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@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.