Re: [PATCH 1/2] t1305: exercise edge cases of "onbranch" includes

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 24, 2024, 16:05 UTC
Message-ID: <xmqqed59vxy3.fsf@gitster.g>
In-Reply-To: <615df98339e9451bc237decea087716ca15d157d.1727171197.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> +test_expect_success 'onbranch without repository' '
> +	test_when_finished "rm -f .gitconfig config.inc" &&
> +	git config set -f .gitconfig "includeIf.onbranch:**.path" config.inc &&
> +	git config set -f config.inc foo.bar baz &&

This assumes that the $(pwd) is the $HOME; so .gitconfig is the per-user configuration that ought to apply everywhere; since includeIf.<condition>.path that is relative is relative to the including file, config.inc would be in cluded when the condition holds in $HOME/.gitconfig. OK.

> +	git config get foo.bar &&

This assumes that the $(pwd) that is $HOME is a valid repository, and checks if includeIf.onbranch works from within a repository. OK.

> +	test_must_fail nongit git config get foo.bar
> +'

> +test_expect_failure 'onbranch without repository but explicit nonexistent Git directory' '
> +	test_when_finished "rm -f .gitconfig config.inc" &&
> +	git config set -f .gitconfig "includeIf.onbranch:**.path" config.inc &&
> +	git config set -f config.inc foo.bar baz &&

The same set-up.

> +	git config get foo.bar &&
> +	test_must_fail nongit git --git-dir=nonexistent config get foo.bar

It has to work when $(pwd) is outside a repository, but is "nongit" strictly necessary? IOW, even when we _could_ discover the top level of a git-controlled working tree, wouldn't presence of --git-dir that points at elsewhere make $(pwd) and the repository there irrelevant to the operation?

I am not suggesting to just drop "nongit" from this test. I am wondering if this is better split into two tests, with and without "nongit" to test different situations.

> +'
> +
>  test_done

Other than that, looks like good additions to the test coverage.

Thanks.