Re: [PATCH] t1450: fix quoting of NUL byte when corrupting pack

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 2, 2020, 17:57 UTC
Message-ID: <xmqqmu3drkvf.fsf@gitster.c.googlers.com>
In-Reply-To: <CAPx1GvdZNeuQqmYm8G62Zr02k=B5GK69xPw84WnvMCeJU7_amQ@mail.gmail.com>

Chris Torek <chris.torek@gmail.com> writes:

Show 24 quoted lines

> On Sun, Aug 2, 2020 at 7:35 AM Martin Ågren <martin.agren@gmail.com> wrote:
>> No worries! Thanks for having a look at the patch. Is there anything
>> that could be done to make this clearer in the commit message? (I find it
>> quite awkward to discuss quoting: will the reader understand which
>> quoting is part of my own formatting of the message vs which is part of
>> the quoting issue I want to get across!?)
>
> This is indeed a problem...
>
> Perhaps something along these lines (generic boilerplate
> for any single-quote fixes, that should be adjusted for the
> actual fix):
>
>     In the test scripts, the recommended style is, e.g.:
>
>         test_expect_success 'name' '
>             multi-line test
>             goes here
>         '
>
>     When using this style, any single quote in the multi-line
>     test section is actually closing the lone single quotes
>     that surround it.  To avoid confusion, minimize and/or
>     eliminate the use of single quotes here.

Another thing that falls into the same class and probably be a good addition to the above "tip" is how $variables are interpolated, i.e.

	test_expect_success 'test name' '
		test-that-references $variable &&
		another-test-that-references "$variable"
	'

are 99% of the time the right way to refer to variable that is assigned outside the test itself (e.g. the whole four lines shown above may be in a loop, "for variable in a b c; ... ;done").

	test_expect_success 'test name' '
		test-that-references '$variable' &&
		another-test-that-references '"$variable"'
	'

is most likely a wrong way to write for the first one (i.e. what if the value in $variable has $IFS whitespace) and "not wrong per-se but unnecessary" for the second one.

Same applies to $(command) substitution, but it is more important. "Step out of the quote, evaluate and step back into the quote" pattern would mean the command is evaluated while formulating the body of the test, not while running the test, which often is not what the author intended.

Thanks.