Subject: Re: [RFC PATCH v2 3/3] t1300: add tests for missing keys

From: Junio C Hamano <[email-masked]>
Date: Tue, 18 Apr 2023 12:31:12 -0700

Andrei Rybak <[email-masked]> 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'