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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 22, 2020, 03:31 UTC
Message-ID: <xmqqlfeuqd8d.fsf@gitster.c.googlers.com>
In-Reply-To: <20201121222734.GG389879@camp.crustytoothpaste.net>

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Show 8 quoted lines

>> So that got a bit off-track, but I think:
>> 
>>   - t1300 already is very much like this, so it's not a new thing
>> 
>>   - but I would be happy not to see it go further in that direction,
>>     even if it means inconsistency with the rest of the script
>
> I agree we shouldn't make things worse.

I started looking at early parts of t1300 and here is how far I managed to get before I can no longer keep staring the existing tests without vomitting.

I am reasonably happy with the "let's keep the vanilla untouched one in .git/config-initial, refrain from using [core] and other sections that MUST be in the initial configuration for testing, and use a wrapper that reads expected addition to the initial one from the standard input for validation" approach I came up with, but I am not happy with the name 'compare_expect'; 'validate_config_result' might be a better name.

In any case, the reason I am sending this out early is if people find this approach to clean things up a sensible one. If we can find concensus, perhaps I (or somebody else---hint, hint) can find time to do the #leftoverbits following the approach after the ds/config-literal-value and ds/maintenance-part-3 topics graduate to 'master'.

 t/t1300-config.sh | 139 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 71 insertions(+), 68 deletions(-)

diff --git c/t/t1300-config.sh w/t/t1300-config.sh
index df13afaffd..c33520d7fa 100755
--- c/t/t1300-config.sh
+++ w/t/t1300-config.sh
@@ -7,80 +7,84 @@ test_description='Test git config in different settings'
 
 . ./test-lib.sh
 
-test_expect_success 'clear default config' '
-	rm -f .git/config
+test_expect_success 'save away default config' '
+	cp .git/config .git/config-initial
 '
 
-cat > expect << EOF
-[core]
-	penguin = little blue
-EOF
-test_expect_success 'initial' '
-	git config core.penguin "little blue" &&
+compare_expect () {
+	{
+		cat .git/config-initial &&
+		sed -e 's/^[|]//'
+	} >expect &&
 	test_cmp expect .git/config
+}
+
+test_expect_success 'initial' '
+	git config configtest.penguin "little blue" &&
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = little blue
+	EOF
 '
 
-cat > expect << EOF
-[core]
-	penguin = little blue
-	Movie = BadPhysics
-EOF
 test_expect_success 'mixed case' '
-	git config Core.Movie BadPhysics &&
-	test_cmp expect .git/config
+	git config ConfigTest.Movie BadPhysics &&
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = little blue
+	|	Movie = BadPhysics
+	EOF
 '
 
-cat > expect << EOF
-[core]
-	penguin = little blue
-	Movie = BadPhysics
-[Cores]
-	WhatEver = Second
-EOF
 test_expect_success 'similar section' '
-	git config Cores.WhatEver Second &&
-	test_cmp expect .git/config
+	git config ConfigTests.WhatEver Second &&
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = little blue
+	|	Movie = BadPhysics
+	[ConfigTests]
+	|	WhatEver = Second
+	EOF
 '
 
-cat > expect << EOF
-[core]
-	penguin = little blue
-	Movie = BadPhysics
-	UPPERCASE = true
-[Cores]
-	WhatEver = Second
-EOF
 test_expect_success 'uppercase section' '
-	git config CORE.UPPERCASE true &&
-	test_cmp expect .git/config
+	git config CONFIGTEST.UPPERCASE true &&
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = little blue
+	|	Movie = BadPhysics
+	|	UPPERCASE = true
+	[ConfigTests]
+	|	WhatEver = Second
+	EOF
 '
 
 test_expect_success 'replace with non-match' '
-	git config core.penguin kingpin !blue
+	git config configtest.penguin kingpin !blue
 '
 
 test_expect_success 'replace with non-match (actually matching)' '
-	git config core.penguin "very blue" !kingpin
+	git config configtest.penguin "very blue" !kingpin
 '
 
-cat > expect << EOF
-[core]
-	penguin = very blue
-	Movie = BadPhysics
-	UPPERCASE = true
-	penguin = kingpin
-[Cores]
-	WhatEver = Second
-EOF
-
-test_expect_success 'non-match result' 'test_cmp expect .git/config'
+test_expect_success 'non-match result' '
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = very blue
+	|	Movie = BadPhysics
+	|	UPPERCASE = true
+	|	penguin = kingpin
+	[ConfigTests]
+	|	WhatEver = Second
+	EOF
+'
 
 test_expect_success 'find mixed-case key by canonical name' '
-	test_cmp_config Second cores.whatever
+	test_cmp_config Second configtests.whatever
 '
 
 test_expect_success 'find mixed-case key by non-canonical name' '
-	test_cmp_config Second CoReS.WhAtEvEr
+	test_cmp_config Second CoNfIgTeSts.WhAtEvEr
 '
 
 test_expect_success 'subsections are not canonicalized by git-config' '
@@ -94,28 +98,27 @@ test_expect_success 'subsections are not canonicalized by git-config' '
 	test_cmp_config two section.SubSection.key
 '
 
-cat > .git/config <<\EOF
-[alpha]
-bar = foo
-[beta]
-baz = multiple \
-lines
-foo = bar
-EOF
-
 test_expect_success 'unset with cont. lines' '
-	git config --unset beta.baz
+	{
+		cat .git/config-initial &&
+		cat <<-\EOF
+		[alpha]
+		bar = foo
+		[beta]
+		baz = multiple \
+		lines
+		foo = bar
+		EOF
+	} >.git/config &&
+	git config --unset beta.baz &&
+	compare_expect <<-\EOF
+	[alpha]
+	bar = foo
+	[beta]
+	foo = bar
+	EOF
 '
 
-cat > expect <<\EOF
-[alpha]
-bar = foo
-[beta]
-foo = bar
-EOF
-
-test_expect_success 'unset with cont. lines is correct' 'test_cmp expect .git/config'
-
 cat > .git/config << EOF
 [beta] ; silly comment # another comment
 noIndent= sillyValue ; 'nother silly comment