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

Jerry Zhang [off-list ref] writes:

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 <redacted>
---
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.

+	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/ (local)):

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