[RFC,v2,3/3] t1300: add tests for missing keys

Commit Message

Andrei Rybak April 18, 2023, 5:50 p.m. UTC

There several tests in t1300-config.sh that validate failing invocations
of "git config".  However, there are no tests that check what happens
when "git config" is asked to retrieve a value for a missing key.

Add tests that check this for various combinations of "<section>.<key>".
---

The patch is marked as RFC, because I'm not sure if such a test is useful or
not.  Even if it is not useful as a test, maybe it would be useful as
documentation of behavior for people reading "t1300-config.sh"?

Having multiple tests for different kinds of keys might also be overkill, and
just one test for a single key is enough.  Conversely, there aren't enough
tests -- tests for keys with subsections are missing.

 t/t1300-config.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Junio C Hamano April 18, 2023, 7:31 p.m. UTC | #1

Andrei Rybak <[e-mail]> writes:

> +test_missing_key () {
> +	local key=$1
> +	local title=$2
> +	test_expect_success "value for $title is not printed" "
> +		test_must_fail git config $key >out 2>err &&
> +		test_must_be_empty out &&
> +		test_must_be_empty err
> +	"
> +}

In this case it would not make any difference only because no caller
will feed $key that can be split at $IFS, but it is a good practice
to enclose the executed body of test_expect_* script inside a single
quote *and* let variable substitution to happen inside eval; in
other words, you should write the above like so:

(GOOD)	test_expect_success "value for $title is not printed" '
		test_must_fail git config "$key" >out 2>err &&
		test_must_be_empty out &&
		test_must_be_empty err
	'

The difference is a bit subtle.  In the original version, the string
fed to "eval" when "test_expect_success" runs has $key already
substituted.  So if $key has $IFS whitespace in it, the command line
arguments of the "git config" would not be what you think your callers
are feeding.  An attempt to keep it together by enclosing inside a
double quote, still trying to salvage the pattern to enclose the
executed body inside a pair of double quotes, i.e.

(BAD)	test_expect_success "value for $title is not printed" "
		test_must_fail git config \"$key\" >out 2>err &&
		test_must_be_empty out &&
		test_must_be_empty err
	"

would fail when $key has an unbalanced double quotes and cause a
syntax error.

Again, in this case it would be OK due to the limited callers, but
it is a good discipline to follow, as others less familiar with our
tests scripts than you would copy from what you write in your
patches.

The title string (e.g. "value for $title is not printed") does not
go through eval, and it needs to be quoted with double quotes for
the $title to be substituted, by the way.

> +test_missing_key 'missingsection.missingkey' 'missing section and missing key'
> +test_missing_key 'missingsection.penguin' 'missing section and existing key'
> +test_missing_key 'section.missingkey' 'existing section and missing key'

Patch

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 20a15ede5c..a646ddd231 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -98,6 +98,20 @@  test_expect_success 'subsections are not canonicalized by git-config' '
 	test_cmp_config two section.SubSection.key
 '
 
+test_missing_key () {
+	local key=$1
+	local title=$2
+	test_expect_success "value for $title is not printed" "
+		test_must_fail git config $key >out 2>err &&
+		test_must_be_empty out &&
+		test_must_be_empty err
+	"
+}
+
+test_missing_key 'missingsection.missingkey' 'missing section and missing key'
+test_missing_key 'missingsection.penguin' 'missing section and existing key'
+test_missing_key 'section.missingkey' 'existing section and missing key'
+
 cat > .git/config <<\EOF
 [alpha]
 bar = foo