Re: [PATCH v9 3/4] var: accept more than one variable

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Andrew

This all looks fine, though we typically avoid using test_env() because 
it introduces a hidden subshell. I've left a couple of suggestions 
below, but unless there is another reason to re-roll I wouldn't worry 
too much.

On 26/09/2026 17:20, Andrew Pleeter wrote:

> +test_expect_success 'variable without a value is omitted but is not an error' '
> +	test_tick &&
> +	cat >expect <<-EOF &&
> +	GIT_AUTHOR_IDENT=$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE
> +	GIT_COMMITTER_IDENT=$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
> +	EOF
> +	test_env GIT_CONFIG_GLOBAL= \
> +		git var GIT_AUTHOR_IDENT GIT_CONFIG_GLOBAL GIT_COMMITTER_IDENT >actual &&

There is no need to use test_env here, "GIT_CONFIG_GLOBAL= git var ..." 
is all that's needed.

> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'a single variable without a value still exits with 1' '
> +	test_env GIT_CONFIG_GLOBAL= test_expect_code 1 git var GIT_CONFIG_GLOBAL >out &&

Here test_env is also not needed, "env GIT_CONFIG_GLOBAL= 
test_expect_code 1 git var ..." would be our typical style.

Thanks

Phillip


> +	test_must_be_empty out
> +'
> +
> +test_expect_success 'unknown variable is a usage error' '
> +	test_must_fail git var GIT_AUTHOR_IDENT NO_SUCH_VARIABLE 2>err &&
> +	test_grep usage err
> +'
> +
>   test_done