From: Taylor Blau <me@ttaylorr.com>
Date: Thu, 19 Jul 2018 15:10:28 GMT
Subject: Re: [PATCH] t/t5534: do not unset GIT_COMMITTER_EMAIL for other tests
Message-ID: <20180719151028.GA35774@syl.attlocal.net>
In-Reply-To: <20180719121409.6143-1-henning.schild@siemens.com>

On Thu, Jul 19, 2018 at 02:14:09PM +0200, Henning Schild wrote:
> Unsetting the varibale for good can have unwanted effects for new

s/varibale/variable

> tests added in the future It also meant we needed to hardcode the

s/future/&.

> value for "user.signingkey".
> Move the unset into a subshell, get rid of the hardcoded
> "committer@example.com", and switch the GPG variant to using test_config
> just like GPGSM.

OK, sounds good.

> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  t/t5534-push-signed.sh | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
> index 0cb88aa6f..f6d674156 100755
> --- a/t/t5534-push-signed.sh
> +++ b/t/t5534-push-signed.sh
> @@ -194,10 +194,12 @@ test_expect_success GPG 'fail without key and heed user.signingkey' '
>
>  	EOF
>
> -	unset GIT_COMMITTER_EMAIL &&
> -	git config user.email hasnokey@nowhere.com &&
> -	test_must_fail git push --signed dst noop ff +noff &&
> -	git config user.signingkey committer@example.com &&
> +	test_config user.email hasnokey@nowhere.com &&
> +	(
> +		unset GIT_COMMITTER_EMAIL &&
> +		test_must_fail git push --signed dst noop ff +noff
> +	) &&
> +	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
>  	git push --signed dst noop ff +noff &&

I think replacing the 'git config' invocations with their 'test_config'
equivalents in its own patch would be easier to grok, but I don't think
that splitting the change out is worth a re-roll on its own.

>  	(
> @@ -241,11 +243,14 @@ test_expect_success GPGSM 'fail without key and heed user.signingkey x509' '
>  	E_O_F
>
>  	EOF
> -	unset GIT_COMMITTER_EMAIL &&
> +

Nit; remove this newline.

>  	test_config user.email hasnokey@nowhere.com &&
>  	test_config user.signingkey "" &&
> -	test_must_fail git push --signed dst noop ff +noff &&
> -	test_config user.signingkey committer@example.com &&
> +	(
> +		unset GIT_COMMITTER_EMAIL &&
> +		test_must_fail git push --signed dst noop ff +noff
> +	) &&
> +	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
>  	git push --signed dst noop ff +noff &&

Looks good.

>  	(
> --
> 2.16.4

Thanks,
Taylor