Re: [PATCH] test-lib: abort when can't remove trash directory

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

SZEDER Gábor <szeder.dev@gmail.com> writes:

> We had two similar bugs in the tests sporadically triggering error
> messages during the removal of the trash directory, see commits
> bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) and
> ef09036cf (t6500: wait for detached auto gc at the end of the test
> script, 2017-04-13).  The test script succeeded nonetheless, because
> these errors are ignored during housekeeping in 'test_done'.
>
> However, such an error is a sign that something is fishy in the test
> script.  Print an error message and abort the test script when the
> trash directory can't be removed successfully or is already removed,
> because that's unexpected and we woud prefer somebody notice and
> figure out why.

Makes sense to me, too.

>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>
> Note, that the commit message references ef09036cf (t6500: wait for
> detached auto gc at the end of the test script, 2017-04-13), which
> is still only in 'pu'.

I think that one is already part of 2.13-rc0 ;-)

>  t/test-lib.sh | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 13b569682..e9e6f677d 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -761,9 +761,12 @@ test_done () {
>  			say "1..$test_count$skip_all"
>  		fi
>  
> -		test -d "$remove_trash" &&
> +		test -d "$remove_trash" ||
> +		error "Tests passed but trash directory already removed before test cleanup; aborting"
> +
>  		cd "$(dirname "$remove_trash")" &&
> -		rm -rf "$(basename "$remove_trash")"
> +		rm -rf "$(basename "$remove_trash")" ||
> +		error "Tests passed but test cleanup failed; aborting"
>  
>  		test_at_end_hook_