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

[Michael Haggerty <mhagger@alum.mit.edu>]

On 05/18/2017 06:10 AM, Junio C Hamano wrote:
> 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.

OK, I'll change this in v2.

Michael