Re: [PATCH v5 2/2] test-config: Add tests for the config_set API

Tanay Abhra [off-list ref] writes:

+test_expect_success 'get value for a simple key' '
+	echo "very blue" >expect &&
+	test-config get_value core.penguin >actual &&
+	test_cmp expect actual
+'

All these tests would greatly benefit from a helper like

test_expect_config () {
	echo "1" >expect &&
	test-config get_value "$2" >actual &&
	test_cmp expect actual
}

Then, all the 3-liners below would become 1-liners.

Should not block inclusion, but may be worth considering.

+test_expect_success 'get value for a key with value as an empty string' '
+	echo "" >expect &&
+	test-config get_value core.my >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'get value for a key with value as NULL' '
+	echo "(NULL)" >expect &&
+	test-config get_value core.foo >actual &&
+	test_cmp expect actual
+'

[...]

+test_expect_success 'key with case sensitive subsection' '
+	echo "mixed-case" >expect &&
+	echo "upper-case" >>expect &&
+	echo "lower-case" >>expect &&
+	test-config get_value "my.Foo bAr.hi" >actual &&
+	test-config get_value "my.FOO BAR.hi" >>actual &&
+	test-config get_value "my.foo bar.hi" >>actual &&
+	test_cmp expect actual
+'

This would become a 3-liner with my helper.

+test_expect_success 'key with case insensitive section header' '
+	echo "ball" >expect &&
+	echo "ball" >>expect &&
+	echo "ball" >>expect &&
+	test-config get_value cores.baz >actual &&
+	test-config get_value Cores.baz >>actual &&
+	test-config get_value CORES.baz >>actual &&
+	test_cmp expect actual
+'

I think you miss a simple case: get_value with a case that doesn't exist
in the config file, like "get_value coreS.baz".

+test_expect_success 'find value with the highest priority' '
+	echo hask >expect &&
+	test-config get_value "core.baz">actual &&

Space before >.

No time for a real review of this file, but from a quick look, it seems
OK.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/