From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 15 Apr 2010 00:56:25 GMT
Subject: Re: [PATCH] t7006: guard cleanup with test_expect_success
Message-ID: <7v7ho9mt3a.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100415003803.GB14151@progeny.tock>

Jonathan Nieder <jrnieder@gmail.com> writes:

> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
> index d9202d5..62595ab 100755
> --- a/t/t7006-pager.sh
> +++ b/t/t7006-pager.sh
> @@ -4,17 +4,24 @@ test_description='Test automatic use of a pager.'
>  
>  . ./test-lib.sh
>  
> -rm -f stdout_is_tty
> +cleanup_fail() {
> +	echo >&2 cleanup failed
> +	exit 1
> +}

I think you meant to say "false" or "(exit 1)" here.  To see why...

>  test_expect_success 'set up terminal for tests' '
> +	rm -f stdout_is_tty ||
> +	cleanup_fail &&
> +

... try your patch with "rm -f stdout_is_tty" replaced with "(exit 1)" to
see how your cleanup_fail behaves.

Other than that I like the general idea, especially "might-fail" is cute.