Re: [PATCH V3 1/2] patch-id: Fix antipatterns in tests

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 1, 2022, 23:16 UTC
Message-ID: <xmqqy22u9nzr.fsf@gitster.g>
In-Reply-To: <20220131235218.27392-1-jerry@skydio.com>

Jerry Zhang <jerry@skydio.com> writes:

Show 8 quoted lines

> Clean up the tests for patch-id by moving file preparation
> tasks inside the test body and redirecting files directly into
> stdin instead of using 'cat'.
>
> Signed-off-by: Jerry Zhang <jerry@skydio.com>
> ---
> V2->V3:
> - Quote the EOF marker

Yes but no.

>  test_expect_success 'patch-id handles no-nl-at-eof markers' '
> -	cat nonl | calc_patch_id nonl &&
> -	cat withnl | calc_patch_id withnl &&
> +	cat >nonl <<-'EOF' &&

We started the "executable" part of the test_expect_success as a single-quoted string, and then after writing <<-, we stepped out of that single-quote pair. Then we are writing E O F unquoted, and stepped back into another single-quote pair here. So, to the shell that runs this executable part, it is exactly the same as

	cat >nonl <<-EOF &&

side note: if it were not in a plain shell script (not the executable part that is passed as a single string to the test_expect_success function as an argument), what we see above, quoting EOF within a pair of single-quotes, is perfectly acceptable thing to do. But not here, for the reasons explained above.

Show 14 quoted lines

> +	diff --git i/a w/a
> +	index e69de29..2e65efe 100644
> +	--- i/a
> +	+++ w/a
> +	@@ -0,0 +1 @@
> +	+a
> +	\ No newline at end of file
> +	diff --git i/b w/b
> +	index e69de29..6178079 100644
> +	--- i/b
> +	+++ w/b
> +	@@ -0,0 +1 @@
> +	+b
> +	'EOF'

Same here. It is exactly the same as writing EOF without any quotes around it, just like the opening one we saw earlier.

In other words, the above is not quoting at all.

I think I demonstrated the way we should write this in my earlier review when I pointed out this exiting issue this step is fixing (https://lore.kernel.org/git/xmqqmtjbh5fu.fsf@gitster.g/):

	test_expect_success "title string" '
		...
		command <<-\EOF &&
		here document indented by tab
		more document
		EOF