From: Phillip Wood <phillip.wood123@gmail.com>
Date: Fri, 02 Oct 2026 14:11:21 GMT
Subject: Re: [PATCH v9 4/4] var: add broken-out identity variables
Message-ID: <3509a23e-9de1-442c-a64c-bc33110f92e7@gmail.com>
In-Reply-To: <20260926162048.30853-5-andrewpleeter@gmail.com>

Hi Andrew

The implementation looks good, just one small comment on the tests.

On 26/09/2026 17:20, Andrew Pleeter wrote:
> +test_expect_success 'get author identity components' '
> +	test_tick &&
> +	echo "$GIT_AUTHOR_NAME" >expect.name &&
> +	echo "$GIT_AUTHOR_EMAIL" >expect.email &&
> +	echo "$GIT_AUTHOR_DATE" >expect.date &&
> +	git var GIT_AUTHOR_NAME >actual.name &&
> +	git var GIT_AUTHOR_EMAIL >actual.email &&
> +	git var GIT_AUTHOR_DATE >actual.date &&
> +	test_cmp expect.name actual.name &&
> +	test_cmp expect.email actual.email &&
> +	test_cmp expect.date actual.date
> +'

I think it would have been sufficient just to list all the identity 
components at once, rather than having separate tests for each one, but 
it is not worth re-rolling just for that.

Thanks

Phillip

> +test_expect_success 'get committer identity components' '
> +	test_tick &&
> +	echo "$GIT_COMMITTER_NAME" >expect.name &&
> +	echo "$GIT_COMMITTER_EMAIL" >expect.email &&
> +	echo "$GIT_COMMITTER_DATE" >expect.date &&
> +	git var GIT_COMMITTER_NAME >actual.name &&
> +	git var GIT_COMMITTER_EMAIL >actual.email &&
> +	git var GIT_COMMITTER_DATE >actual.date &&
> +	test_cmp expect.name actual.name &&
> +	test_cmp expect.email actual.email &&
> +	test_cmp expect.date actual.date
> +'
> +
> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'identity components are strict' '
> +	(
> +		sane_unset GIT_COMMITTER_NAME &&
> +		sane_unset GIT_COMMITTER_EMAIL &&
> +		test_must_fail git var GIT_COMMITTER_NAME
> +	)
> +'
> +
> +test_expect_success 'get several identity components at once' '
> +	test_tick &&
> +	cat >expect <<-EOF &&
> +	GIT_AUTHOR_NAME=$GIT_AUTHOR_NAME
> +	GIT_AUTHOR_EMAIL=$GIT_AUTHOR_EMAIL
> +	GIT_COMMITTER_NAME=$GIT_COMMITTER_NAME
> +	GIT_COMMITTER_EMAIL=$GIT_COMMITTER_EMAIL
> +	EOF
> +	git var GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'git var -l lists the identity components' '
> +	git var -l >actual &&
> +	test_grep "^GIT_AUTHOR_NAME=" actual &&
> +	test_grep "^GIT_AUTHOR_EMAIL=" actual &&
> +	test_grep "^GIT_AUTHOR_DATE=" actual &&
> +	test_grep "^GIT_COMMITTER_NAME=" actual &&
> +	test_grep "^GIT_COMMITTER_EMAIL=" actual &&
> +	test_grep "^GIT_COMMITTER_DATE=" actual
> +'
> +
>   test_done