Re: [PATCH v2 2/2] var: allow GIT_EDITOR to return null

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 25, 2022, 22:48 UTC
Message-ID: <221125.86pmdamyv5.gmgdl@evledraar.gmail.com>
In-Reply-To: <427cb7b55ac3fead1651cbad7318b9c0bb454b08.1669395151.git.gitgitgadget@gmail.com>

On Fri, Nov 25 2022, Sean Allred via GitGitGadget wrote:

> From: Sean Allred <allred.sean@gmail.com>

Show 7 quoted lines

> +test_expect_success 'get GIT_EDITOR without configuration' '
> +	(
> +		sane_unset GIT_EDITOR &&
> +		sane_unset VISUAL &&
> +		sane_unset EDITOR &&
> +		>expect &&
> +		! git var GIT_EDITOR >actual &&

Negate git with "test_must_fail", not "!", this would e.g. hide segfaults. See t/README's discussion about it.

> +		test_cmp expect actual

Looks like this should be:

	test_must_fail git ... >out &&
	test_must_be_empty out

Show 10 quoted lines

> +test_expect_success 'get GIT_EDITOR with configuration and environment variable EDITOR' '
> +	test_config core.editor foo &&
> +	(
> +		sane_unset GIT_EDITOR &&
> +		sane_unset VISUAL &&
> +		sane_unset EDITOR &&
> +		echo foo >expect &&
> +		EDITOR=bar git var GIT_EDITOR >actual &&
> +		test_cmp expect actual
> +	)

Perhaps these can all be factored into a helper to hide this repetition in a function, but maybe not. E.g:

	test_git_var () {
		cat >expect &&
		(
			[...common part of subshell ...]
		        "$@" >actual &&
			test_cmp expect actual
		)
	}

(untested)