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

Andrei Rybak <rybak.a.v@gmail.com> writes:

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.