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

From: Junio C Hamano <[email-masked]>
Date: Tue, 24 Sep 2024 09:05:56 -0700

Patrick Steinhardt <[email-masked]> 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.