Re: [PATCH v3] tests: use "env" to run commands with temporary env-var settings

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 19, 2014, 19:55 UTC
Message-ID: <xmqqfvme2cbp.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1395168845-1972-1-git-send-email-unsignedzero@gmail.com>

David Tran <unsignedzero@gmail.com> writes:

Show 19 quoted lines

> Originally, we would use "VAR=VAL command" to execute a test command with
> environment variable(s) only for that command. This does not work for commands
> that are shell functions (most notably test functions like "test_must_fail");
> the result of the assignment is retained and affects later commands.
>
> To avoid this, we assigned and exported the environment variables and run
> the test(s) in a subshell like this,
>
> 	(
> 		VAR=VAL &&
> 		export VAR
> 		test_must_fail git command to be tested
> 	)
>
> Using the "env" utility, we should be able to say
>
> 	test_must_fail git command to be tested
>
> which is much short and easier to read.

Looks familiar ;-) but it seems the changes from the original you took it from all look worsening, not improvements, to me.

Show 7 quoted lines

>>Isn't GIT_CONFIG here another way of saying:
>>
>>	test_must_fail git config -f doesnotexist --list
>>
>>Perhaps that is shorter and more readable still (and there are a few
>>similar cases in this patch.
> I'll ignore this for now. If needed I can make another patch to resolve this.

Yes, I think that is sensible.  And it does not have to be done by you.

> Hopefully this should be all of it.

Seems to be well done.  Thanks.