Subject: Re: [PATCH v2 1/7] t1300: test "set all" mode with value_regex

From: Emily Shaffer <[email-masked]>
Date: Mon, 23 Nov 2020 11:37:14 -0800

On Mon, Nov 23, 2020 at 04:05:01PM +0000, Derrick Stolee via GitGitGadget wrote:
> Add a test that demonstrates these options. Break from the existing
> pattern in t1300-config.sh to use 'git config --file=<file>' instead of
> modifying .git/config directly. Also use 'git config --file=<file>
> --list' for config state comparison instead of the config file format.

I'd normally say "but *why* do you want to do that?" but I think it's
actually pretty clear in the diff based on the naming of the config
files (and clever - I like it). I do think pointing out that you're
breaking from existing style is useful since the existing style isn't
contained in the diff alone.

> +test_expect_success 'set all config with value_regex' '
> +	git config --file=initial abc.key one &&
> +
> +	cp initial config &&
> +	git config --file=config abc.key two a+ &&
Make sure that if the value_regex misses existing keys, it can still add
normally.
> +	git config --file=config --list >actual &&
> +	cat >expect <<-\EOF &&
> +	abc.key=one
> +	abc.key=two
> +	EOF
> +	test_cmp expect actual &&
> +
> +	test_must_fail git config --file=config abc.key three o+ 2>err &&
> +	test_i18ngrep "has multiple values" err &&
Try to change the value of the config, but it's a multiconfig already.
Ok.

> +	git config --file=config abc.key three a+ &&
But /a+/ doesn't match any configs that already exist, so this one gets
added fine.

> +	git config --file=config --list >actual &&
> +	cat >expect <<-\EOF &&
> +	abc.key=one
> +	abc.key=two
> +	abc.key=three
> +	EOF
> +	test_cmp expect actual &&
> +
> +	cp initial config &&
Get back the config before the last exercise (with just 'one').
> +	git config --file=config abc.key three o+ &&
And if the value_regex matches exactly one config, then replace that
one. I could see this case being a little bit more compelling if the
value_regex was doing some lifting, e.g.

abc.key=nil
abc.key=one

git config abc.key two o+

abc.key=nil
abc.key=two

This criteria was kind of implied in #2 in your commit message - does it
work?

> +	git config --file=config --list >actual &&
> +	cat >expect <<-\EOF &&
> +	abc.key=three
> +	EOF
> +	test_cmp expect actual
> +'

Anyway, it's a pain, but I'd like to see either comments describing
which case each one is, or individual tests.

 - Emily