threads / patch / 54922

patch, 3 partst1300: minor cleanups

Subject: [PATCH 0/3] t1300: minor cleanups

## tl;dr

5 messages between Jan 3, 2021 and Jan 6, 2021. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Martin Ågren· Jan 3, 2021, 09:36 UTC · lore

There was some discussion around t1300 a while back and how it could need some major cleanup [1]. This patch series is not that. I'll post an RFC series on that later. These initial cleanups -- or something like them -- should be useful regardless of the fate of that future RFC, so I'm posting them early.

[1] https://lore.kernel.org/git/xmqqlfeuqd8d.fsf@gitster.c.googlers.com/
Martin Ågren (3):
  t1300: remove duplicate test for `--file ../foo`
  t1300: remove duplicate test for `--file no-such-file`
  t1300: don't needlessly work with `core.foo` configs
 t/t1300-config.sh | 72 +++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 40 deletions(-)
-- 
2.30.0
Martin Ågren· Jan 3, 2021, 09:36 UTC · re: Martin Ågren · lore

[PATCH 1/3] t1300: remove duplicate test for `--file ../foo`

We have two tests for checking that we can handle `git config --file ../other-config ...`. One, using `--file`, was introduced in 65807ee697 ("builtin-config: Fix crash when using "-f <relative path>" from non-root dir", 2010-01-26), then another, using `GIT_CONFIG`, came about in 270a34438b ("config: stop using config_exclusive_filename", 2012-02-16).

The latter of these was then converted to use `--file` in f7e8714101 ("t: prefer "git config --file" to GIT_CONFIG", 2014-03-20). Both where then simplified in a5db0b77b9 ("t1300: extract and use test_cmp_config()", 2018-10-21).

These two tests differ slightly in the order of the options used, but other than that, they are identical. Let's drop one. As noted in f7e8714101, we do still have a test for `GIT_CONFIG` and it shares the implementation with `--file`.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 t/t1300-config.sh | 4 ----
 1 file changed, 4 deletions(-)
Show changes to t/t1300-config.sh +0 −4
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 97a04c6cc2..302821fb02 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -506,10 +506,6 @@ test_expect_success 'editing stdin is an error' '
 
 test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
-	test_cmp_config -C x strasse --get --file ../other-config ein.bahn
-'
-
-test_expect_success 'refer config from subdirectory via --file' '
 	test_cmp_config -C x strasse --file=../other-config --get ein.bahn
 '
 

-- 
2.30.0
Martin Ågren· Jan 3, 2021, 09:36 UTC · re: Martin Ågren · lore

[PATCH 2/3] t1300: remove duplicate test for `--file no-such-file`

We test that we can handle `git config --file symlink` and the error case of `git config --file symlink-to-missing-file`. For good measure, we also throw in a test to check that we correctly handle referencing a missing regular file. But we have such a test earlier in this script. They both check that we fail to use `--file no-such-file --list`.

Drop the latter of these and keep the one that is in the general area where we test `--file` and `GIT_CONFIG`. The one we're dropping also checks that we can't even get a specific key from the missing file -- let's make sure we check that in the test we keep.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 t/t1300-config.sh | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
Show changes to t/t1300-config.sh +2 −6
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 302821fb02..1439d870d6 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,7 +469,8 @@ test_expect_success 'new variable inserts into proper section' '
 '
 
 test_expect_success 'alternative --file (non-existing file should fail)' '
-	test_must_fail git config --file non-existing-config -l
+	test_must_fail git config --file non-existing-config -l &&
+	test_must_fail git config --file non-existing-config test.xyzzy
 '
 
 cat > other-config << EOF
@@ -1032,11 +1033,6 @@ test_expect_success SYMLINKS 'symlinked configuration' '
 	test_cmp expect actual
 '
 
-test_expect_success 'nonexistent configuration' '
-	test_must_fail git config --file=doesnotexist --list &&
-	test_must_fail git config --file=doesnotexist test.xyzzy
-'
-
 test_expect_success SYMLINKS 'symlink to nonexistent configuration' '
 	ln -s doesnotexist linktonada &&
 	ln -s linktonada linktolinktonada &&

-- 
2.30.0
Martin Ågren· Jan 3, 2021, 09:36 UTC · re: Martin Ågren · lore

[PATCH 3/3] t1300: don't needlessly work with `core.foo` configs

We use various made-up config keys in the "core" section for no real reason. Change them to work in the "section" section instead and be careful to also change "cores" to "sections". Make sure to also catch "Core", "CoReS" and similar.

There are a few instances that actually want to work with a real "core" config such as `core.bare` or `core.editor`. After this, it's clearer that they work with "core" for a reason.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 t/t1300-config.sh | 60 +++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)
Show changes to t/t1300-config.sh +30 −30
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 1439d870d6..1a4156c704 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -12,75 +12,75 @@ test_expect_success 'clear default config' '
 '
 
 cat > expect << EOF
-[core]
+[section]
 	penguin = little blue
 EOF
 test_expect_success 'initial' '
-	git config core.penguin "little blue" &&
+	git config section.penguin "little blue" &&
 	test_cmp expect .git/config
 '
 
 cat > expect << EOF
-[core]
+[section]
 	penguin = little blue
 	Movie = BadPhysics
 EOF
 test_expect_success 'mixed case' '
-	git config Core.Movie BadPhysics &&
+	git config Section.Movie BadPhysics &&
 	test_cmp expect .git/config
 '
 
 cat > expect << EOF
-[core]
+[section]
 	penguin = little blue
 	Movie = BadPhysics
-[Cores]
+[Sections]
 	WhatEver = Second
 EOF
 test_expect_success 'similar section' '
-	git config Cores.WhatEver Second &&
+	git config Sections.WhatEver Second &&
 	test_cmp expect .git/config
 '
 
 cat > expect << EOF
-[core]
+[section]
 	penguin = little blue
 	Movie = BadPhysics
 	UPPERCASE = true
-[Cores]
+[Sections]
 	WhatEver = Second
 EOF
 test_expect_success 'uppercase section' '
-	git config CORE.UPPERCASE true &&
+	git config SECTION.UPPERCASE true &&
 	test_cmp expect .git/config
 '
 
 test_expect_success 'replace with non-match' '
-	git config core.penguin kingpin !blue
+	git config section.penguin kingpin !blue
 '
 
 test_expect_success 'replace with non-match (actually matching)' '
-	git config core.penguin "very blue" !kingpin
+	git config section.penguin "very blue" !kingpin
 '
 
 cat > expect << EOF
-[core]
+[section]
 	penguin = very blue
 	Movie = BadPhysics
 	UPPERCASE = true
 	penguin = kingpin
-[Cores]
+[Sections]
 	WhatEver = Second
 EOF
 
 test_expect_success 'non-match result' 'test_cmp expect .git/config'
 
 test_expect_success 'find mixed-case key by canonical name' '
-	test_cmp_config Second cores.whatever
+	test_cmp_config Second sections.whatever
 '
 
 test_expect_success 'find mixed-case key by non-canonical name' '
-	test_cmp_config Second CoReS.WhAtEvEr
+	test_cmp_config Second SeCtIoNs.WhAtEvEr
 '
 
 test_expect_success 'subsections are not canonicalized by git-config' '
@@ -1057,12 +1057,12 @@ test_expect_success 'git -c "key=value" support' '
 	true
 	EOF
 	{
-		git -c core.name=value config core.name &&
+		git -c section.name=value config section.name &&
 		git -c foo.CamelCase=value config foo.camelcase &&
 		git -c foo.flag config --bool foo.flag
 	} >actual &&
 	test_cmp expect actual &&
-	test_must_fail git -c name=value config core.name
+	test_must_fail git -c name=value config section.name
 '
 
 # We just need a type-specifier here that cares about the
@@ -1107,7 +1107,7 @@ test_expect_success 'aliases can be CamelCased' '
 
 test_expect_success 'git -c does not split values on equals' '
 	echo "value with = in it" >expect &&
-	git -c core.foo="value with = in it" config core.foo >actual &&
+	git -c section.foo="value with = in it" config section.foo >actual &&
 	test_cmp expect actual
 '
 
@@ -1838,53 +1838,53 @@ do
 done
 
 cat >.git/config <<-\EOF &&
-[core]
+[section]
 foo = true
 number = 10
 big = 1M
 EOF
 
 test_expect_success 'identical modern --type specifiers are allowed' '
-	test_cmp_config 1048576 --type=int --type=int core.big
+	test_cmp_config 1048576 --type=int --type=int section.big
 '
 
 test_expect_success 'identical legacy --type specifiers are allowed' '
-	test_cmp_config 1048576 --int --int core.big
+	test_cmp_config 1048576 --int --int section.big
 '
 
 test_expect_success 'identical mixed --type specifiers are allowed' '
-	test_cmp_config 1048576 --int --type=int core.big
+	test_cmp_config 1048576 --int --type=int section.big
 '
 
 test_expect_success 'non-identical modern --type specifiers are not allowed' '
-	test_must_fail git config --type=int --type=bool core.big 2>error &&
+	test_must_fail git config --type=int --type=bool section.big 2>error &&
 	test_i18ngrep "only one type at a time" error
 '
 
 test_expect_success 'non-identical legacy --type specifiers are not allowed' '
-	test_must_fail git config --int --bool core.big 2>error &&
+	test_must_fail git config --int --bool section.big 2>error &&
 	test_i18ngrep "only one type at a time" error
 '
 
 test_expect_success 'non-identical mixed --type specifiers are not allowed' '
-	test_must_fail git config --type=int --bool core.big 2>error &&
+	test_must_fail git config --type=int --bool section.big 2>error &&
 	test_i18ngrep "only one type at a time" error
 '
 
 test_expect_success '--type allows valid type specifiers' '
-	test_cmp_config true  --type=bool core.foo
+	test_cmp_config true  --type=bool section.foo
 '
 
 test_expect_success '--no-type unsets type specifiers' '
-	test_cmp_config 10 --type=bool --no-type core.number
+	test_cmp_config 10 --type=bool --no-type section.number
 '
 
 test_expect_success 'unset type specifiers may be reset to conflicting ones' '
-	test_cmp_config 1048576 --type=bool --no-type --type=int core.big
+	test_cmp_config 1048576 --type=bool --no-type --type=int section.big
 '
 
 test_expect_success '--type rejects unknown specifiers' '
-	test_must_fail git config --type=nonsense core.foo 2>error &&
+	test_must_fail git config --type=nonsense section.foo 2>error &&
 	test_i18ngrep "unrecognized --type argument" error
 '
 

-- 
2.30.0
Junio C Hamano· Jan 6, 2021, 06:05 UTC · re: Martin Ågren · lore

Re: [PATCH 0/3] t1300: minor cleanups

Martin Ågren <martin.agren@gmail.com> writes:
Show 15 quoted lines
> There was some discussion around t1300 a while back and how it could
> need some major cleanup [1]. This patch series is not that. I'll post an
> RFC series on that later. These initial cleanups -- or something like
> them -- should be useful regardless of the fate of that future RFC, so
> I'm posting them early.
>
> [1] https://lore.kernel.org/git/xmqqlfeuqd8d.fsf@gitster.c.googlers.com/
>
> Martin Ågren (3):
>   t1300: remove duplicate test for `--file ../foo`
>   t1300: remove duplicate test for `--file no-such-file`
>   t1300: don't needlessly work with `core.foo` configs
>
>  t/t1300-config.sh | 72 +++++++++++++++++++++--------------------------
>  1 file changed, 32 insertions(+), 40 deletions(-)
All look sensible.  Thanks.

← back to recent threads