Re: [PATCH 2/3] t/t5400-send-pack.sh: add test_when_finished commands

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 18, 2011, 23:10 UTC
Message-ID: <7vfwspkdtq.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1295386896-21828-2-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 12 quoted lines

> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index 656ab1a..0357610 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -90,7 +90,8 @@ test_expect_success 'refuse deleting push with denyDeletes' '
>  	    git config receive.denyDeletes true &&
>  	    git branch extra master
>  	) &&
> -	test_must_fail git send-pack ./victim :extra master
> +	test_must_fail git send-pack ./victim :extra master &&
> +	test_when_finished "(cd victim && git config --unset receive.denyDeletes)"
>  '

I agree that you are trying to do a good thing, but does this implementation work in practice? If somebody breaks send-pack in such a way that this push succeeds, doesn't the &&-chain will prevent your "when-finished" hook to be registered to begin with? IOW, wouldn't you want to have when-finished as _early_ as possible, before any test that might fail to help us identify a new breakage?

The same comment applies to other two hunks in the patch.