[v2,1/7] t1300: test "set all" mode with value_regex

Commit Message

Derrick Stolee Nov. 23, 2020, 4:05 p.m. UTC

From: Derrick Stolee <[e-mail]>

Without additional modifiers, 'git config' attempts to set a single
value in the .git/config file. When the value_regex parameter is
supplied, this command behaves in a non-trivial manner.

Consider 'git config key value value_regex'. The expected behavior
is as follows:

1. If there are multiple existing values that match 'value_regex',
   then the command fails. Users should use --replace-all instead.

2. If there is one existing value that matches 'value_regex', then
   the new config has one entry where 'key=value'.

3. If there is no existing values match 'value_regex', then the
   'key=value' pair is appended, making this 'key' a multi-valued
   config setting.

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.

Signed-off-by: Derrick Stolee <[e-mail]>
---
 t/t1300-config.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Emily Shaffer Nov. 23, 2020, 7:37 p.m. UTC | #1

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

Patch

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 97ebfe1f9d..19836ec83b 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1914,4 +1914,36 @@  test_expect_success '--replace-all does not invent newlines' '
 	test_cmp expect .git/config
 '
 
+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+ &&
+	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 &&
+	git config --file=config abc.key three a+ &&
+	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 &&
+	git config --file=config abc.key three o+ &&
+	git config --file=config --list >actual &&
+	cat >expect <<-\EOF &&
+	abc.key=three
+	EOF
+	test_cmp expect actual
+'
+
 test_done