[PATCH] t3310: avoid hiding failures from rev-parse in command substitutions

[PATCH] t3310: avoid hiding failures from rev-parse in command substitutions

[Francesco Paparatto]

Running `git` commands inside command substitutions like

    test "$(git rev-parse A)" = "$(git rev-parse B)"

can hide failures from the `git` invocations. Extract the
`rev-parse` calls into variables so failures are not ignored.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
---
 t/t3310-notes-merge-manual-resolve.sh | 54 ++++++++++++++++++---------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index f0054b0a39..92a5951331 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -227,7 +227,8 @@ test_expect_success 'merge z into m (== y) with default ("manual") resolver => C
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	m=$(git rev-parse refs/notes/m) &&
+	test "$m" = "$(cat pre_merge_y)"
 '
 
 cat <<EOF | sort >expect_notes_z
@@ -375,8 +376,10 @@ EOF
 	git notes merge --commit &&
 	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
-	test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
-	test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
+	m1=$(git rev-parse refs/notes/m^1) &&
+	m2=$(git rev-parse refs/notes/m^2) &&
+	test "$m1" = "$(cat pre_merge_y)" &&
+	test "$m2" = "$(cat pre_merge_z)" &&
 	# Merge commit mentions the notes refs merged
 	git log -1 --format=%B refs/notes/m > merge_commit_msg &&
 	grep -q refs/notes/m merge_commit_msg &&
@@ -428,14 +431,16 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	m=$(git rev-parse refs/notes/m) &&
+	test "$m" = "$(cat pre_merge_y)"
 '
 
 test_expect_success 'abort notes merge' '
 	git notes merge --abort &&
 	notes_merge_files_gone &&
 	# m has not moved (still == y)
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
+	m=$(git rev-parse refs/notes/m) &&
+	test "$m" = "$(cat pre_merge_y)" &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -460,7 +465,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	m=$(git rev-parse refs/notes/m) &&
+	test "$m" = "$(cat pre_merge_y)"
 '
 
 cat <<EOF | sort >expect_notes_m
@@ -500,8 +506,10 @@ EOF
 	git notes merge --commit &&
 	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
-	test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
-	test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
+	m1=$(git rev-parse refs/notes/m^1) &&
+	m2=$(git rev-parse refs/notes/m^2) &&
+	test "$m1" = "$(cat pre_merge_y)" &&
+	test "$m2" = "$(cat pre_merge_z)" &&
 	# Merge commit mentions the notes refs merged
 	git log -1 --format=%B refs/notes/m > merge_commit_msg &&
 	grep -q refs/notes/m merge_commit_msg &&
@@ -539,7 +547,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	m=$(git rev-parse refs/notes/m) &&
+	test "$m" = "$(cat pre_merge_y)"
 '
 
 cp expect_notes_w expect_notes_m
@@ -548,7 +557,9 @@ cp expect_log_w expect_log_m
 test_expect_success 'reset notes ref m to somewhere else (w)' '
 	git update-ref refs/notes/m refs/notes/w &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
+	m=$(git rev-parse refs/notes/m) &&
+	w=$(git rev-parse refs/notes/w) &&
+	test "$m" = "$w"
 '
 
 test_expect_success 'fail to finalize conflicting merge if underlying ref has moved in the meantime (m != NOTES_MERGE_PARTIAL^1)' '
@@ -569,13 +580,17 @@ EOF
 	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
 	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
 	# Refs are unchanged
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
-	test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
-	test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
+	m=$(git rev-parse refs/notes/m) &&
+	w=$(git rev-parse refs/notes/w) &&
+	y=$(git rev-parse refs/notes/y) &&
+	p1=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
+	test "$m" = "$w" &&
+	test "$y" = "$p1" &&
+	test "$m" != "$p1" &&
 	# Mention refs/notes/m, and its current and expected value in output
 	test_grep -q "refs/notes/m" output &&
-	test_grep -q "$(git rev-parse refs/notes/m)" output &&
-	test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
+	test_grep -q "$m" output &&
+	test_grep -q "$p1" output &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -587,7 +602,9 @@ test_expect_success 'resolve situation by aborting the notes merge' '
 	git notes merge --abort &&
 	notes_merge_files_gone &&
 	# m has not moved (still == w)
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
+	m=$(git rev-parse refs/notes/m) &&
+	w=$(git rev-parse refs/notes/w) &&
+	test "$m" = "$w" &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -606,8 +623,9 @@ test_expect_success 'switch cwd before committing notes merge' '
 	test_must_fail git notes merge refs/notes/other &&
 	(
 		cd .git/NOTES_MERGE_WORKTREE &&
-		echo "foo" > $(git rev-parse HEAD) &&
-		echo "bar" >> $(git rev-parse HEAD) &&
+		oid=$(git rev-parse HEAD) &&
+		echo "foo" >"$oid" &&
+		echo "bar" >>"$oid" &&
 		git notes merge --commit
 	) &&
 	git notes show HEAD > actual_notes &&
-- 
2.52.0

Re: [PATCH] t3310: avoid hiding failures from rev-parse in command substitutions

[Eric Sunshine]

On Wed, Mar 4, 2026 at 7:20 AM Francesco Paparatto
<francescopaparatto@gmail.com> wrote:
> Running `git` commands inside command substitutions like
>
>     test "$(git rev-parse A)" = "$(git rev-parse B)"
>
> can hide failures from the `git` invocations. Extract the
> `rev-parse` calls into variables so failures are not ignored.

Okay, surfacing failures of `git` invocations is a laudable goal. However...

> Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
> ---
> diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
> @@ -227,7 +227,8 @@ test_expect_success 'merge z into m (== y) with default ("manual") resolver => C
> -       test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
> +       m=$(git rev-parse refs/notes/m) &&
> +       test "$m" = "$(cat pre_merge_y)"
>  '

...a failure exposed by `test` is not very developer-friendly since it
doesn't give any indication about what went wrong. Since the pre-merge
value of "y" (and also "z" in subsequent tests) is already in a file,
we can make the failure mode much more helpful by using `test_cmp
<expect> <actual>` which will show both the expected and actual values
when they don't match. Thus, the above transformation would be better
stated along these lines:

    git rev-parse refs/notes/m >actual &&
    test_cmp pre_merge_y actual

The same comment applies to other changes in this patch.

> @@ -569,13 +580,17 @@ EOF
>         # Refs are unchanged
> -       test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
> -       test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
> -       test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
> +       m=$(git rev-parse refs/notes/m) &&
> +       w=$(git rev-parse refs/notes/w) &&
> +       y=$(git rev-parse refs/notes/y) &&
> +       p1=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
> +       test "$m" = "$w" &&
> +       test "$y" = "$p1" &&
> +       test "$m" != "$p1" &&

In this case we can do even better by taking advantage of
`test_cmp_rev`, which would allow you to express the above more simply
along these lines:

    test_cmp_rev refs/notes/m rev-parse refs/notes/w &&
    test_cmp_rev refs/notes/y NOTES_MERGE_PARTIAL^1 &&
    test_cmp_rev ! refs/notes/m NOTES_MERGE_PARTIAL^1 &&

Note the "!" for negation in the third line.

The same comment applies to other changes in this patch.

[PATCH v2] t3310: avoid hiding failures from rev-parse in command substitutions

[Francesco Paparatto]

Running `git` commands inside command substitutions like

	test "$(git rev-parse A)" = "$(git rev-parse B)"

can hide failures from the `git` invocations and provide little
diagnostic information when `test` fails.

Use `test_cmp` when comparing against a stored expected value so
mismatches show both expected and actual output. Use `test_cmp_rev`
when comparing two revisions. These helpers produce clearer failure
output, making it easier to understand what went wrong.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
---
 t/t3310-notes-merge-manual-resolve.sh | 60 ++++++++++++---------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index 92a5951331..64c0a753ff 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -227,8 +227,8 @@ test_expect_success 'merge z into m (== y) with default ("manual") resolver => C
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	m=$(git rev-parse refs/notes/m) &&
-	test "$m" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cat <<EOF | sort >expect_notes_z
@@ -376,10 +376,10 @@ EOF
 	git notes merge --commit &&
 	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
-	m1=$(git rev-parse refs/notes/m^1) &&
-	m2=$(git rev-parse refs/notes/m^2) &&
-	test "$m1" = "$(cat pre_merge_y)" &&
-	test "$m2" = "$(cat pre_merge_z)" &&
+	git rev-parse refs/notes/m^1 >actual &&
+	test_cmp pre_merge_y actual &&
+	git rev-parse refs/notes/m^2 >actual &&
+	test_cmp pre_merge_z actual &&
 	# Merge commit mentions the notes refs merged
 	git log -1 --format=%B refs/notes/m > merge_commit_msg &&
 	grep -q refs/notes/m merge_commit_msg &&
@@ -431,16 +431,16 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	m=$(git rev-parse refs/notes/m) &&
-	test "$m" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 test_expect_success 'abort notes merge' '
 	git notes merge --abort &&
 	notes_merge_files_gone &&
 	# m has not moved (still == y)
-	m=$(git rev-parse refs/notes/m) &&
-	test "$m" = "$(cat pre_merge_y)" &&
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -465,8 +465,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	m=$(git rev-parse refs/notes/m) &&
-	test "$m" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cat <<EOF | sort >expect_notes_m
@@ -506,10 +506,10 @@ EOF
 	git notes merge --commit &&
 	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
-	m1=$(git rev-parse refs/notes/m^1) &&
-	m2=$(git rev-parse refs/notes/m^2) &&
-	test "$m1" = "$(cat pre_merge_y)" &&
-	test "$m2" = "$(cat pre_merge_z)" &&
+	git rev-parse refs/notes/m^1 >actual &&
+	test_cmp pre_merge_y actual &&
+	git rev-parse refs/notes/m^2 >actual &&
+	test_cmp pre_merge_z actual &&
 	# Merge commit mentions the notes refs merged
 	git log -1 --format=%B refs/notes/m > merge_commit_msg &&
 	grep -q refs/notes/m merge_commit_msg &&
@@ -547,8 +547,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	m=$(git rev-parse refs/notes/m) &&
-	test "$m" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cp expect_notes_w expect_notes_m
@@ -557,9 +557,7 @@ cp expect_log_w expect_log_m
 test_expect_success 'reset notes ref m to somewhere else (w)' '
 	git update-ref refs/notes/m refs/notes/w &&
 	verify_notes m &&
-	m=$(git rev-parse refs/notes/m) &&
-	w=$(git rev-parse refs/notes/w) &&
-	test "$m" = "$w"
+	test_cmp_rev refs/notes/m refs/notes/w
 '
 
 test_expect_success 'fail to finalize conflicting merge if underlying ref has moved in the meantime (m != NOTES_MERGE_PARTIAL^1)' '
@@ -580,17 +578,15 @@ EOF
 	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
 	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
 	# Refs are unchanged
-	m=$(git rev-parse refs/notes/m) &&
-	w=$(git rev-parse refs/notes/w) &&
-	y=$(git rev-parse refs/notes/y) &&
-	p1=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
-	test "$m" = "$w" &&
-	test "$y" = "$p1" &&
-	test "$m" != "$p1" &&
+	test_cmp_rev refs/notes/m refs/notes/w &&
+	test_cmp_rev refs/notes/y NOTES_MERGE_PARTIAL^1 &&
+	test_cmp_rev ! refs/notes/m NOTES_MERGE_PARTIAL^1 &&
 	# Mention refs/notes/m, and its current and expected value in output
 	test_grep -q "refs/notes/m" output &&
-	test_grep -q "$m" output &&
-	test_grep -q "$p1" output &&
+	git rev-parse refs/notes/m >actual &&
+	test_grep -q "$(cat actual)" output &&
+	git rev-parse NOTES_MERGE_PARTIAL^1 >actual &&
+	test_grep -q "$(cat actual)" output &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -602,9 +598,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '
 	git notes merge --abort &&
 	notes_merge_files_gone &&
 	# m has not moved (still == w)
-	m=$(git rev-parse refs/notes/m) &&
-	w=$(git rev-parse refs/notes/w) &&
-	test "$m" = "$w" &&
+	test_cmp_rev refs/notes/m refs/notes/w &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
-- 
2.52.0

Re: [PATCH v2] t3310: avoid hiding failures from rev-parse in command substitutions

[Junio C Hamano]

Francesco Paparatto <francescopaparatto@gmail.com> writes:

> Running `git` commands inside command substitutions like
>
> 	test "$(git rev-parse A)" = "$(git rev-parse B)"
>
> can hide failures from the `git` invocations and provide little
> diagnostic information when `test` fails.
>
> Use `test_cmp` when comparing against a stored expected value so
> mismatches show both expected and actual output. Use `test_cmp_rev`
> when comparing two revisions. These helpers produce clearer failure
> output, making it easier to understand what went wrong.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>

Hmph, did I suggest this?  I know Eric had comments on a previous
round, and the improvements in this patch seems to be influenced a
lot stronger by his input than whatever I may have said.

> Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
> ---
>  t/t3310-notes-merge-manual-resolve.sh | 60 ++++++++++++---------------
>  1 file changed, 27 insertions(+), 33 deletions(-)
>
> diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
> index 92a5951331..64c0a753ff 100755

On top of what commit is this patch designed to apply?

Thanks.

Re: [PATCH v2] t3310: avoid hiding failures from rev-parse in command substitutions

[Eric Sunshine]

On Thu, Mar 5, 2026 at 2:13 PM Junio C Hamano <gitster@pobox.com> wrote:
> > Running `git` commands inside command substitutions like
> >
> >       test "$(git rev-parse A)" = "$(git rev-parse B)"
> >
> > can hide failures from the `git` invocations and provide little
> > diagnostic information when `test` fails.
> >
> > Use `test_cmp` when comparing against a stored expected value so
> > mismatches show both expected and actual output. Use `test_cmp_rev`
> > when comparing two revisions. These helpers produce clearer failure
> > output, making it easier to understand what went wrong.
> >
> > Suggested-by: Junio C Hamano <gitster@pobox.com>
>
> Hmph, did I suggest this?  I know Eric had comments on a previous
> round, and the improvements in this patch seems to be influenced a
> lot stronger by his input than whatever I may have said.

Indeed. The use of test_cmp and test_cmp_rev makes this version much
more developer-friendly than v1. Nice.

> >  t/t3310-notes-merge-manual-resolve.sh | 60 ++++++++++++---------------
> >  1 file changed, 27 insertions(+), 33 deletions(-)
> >
> > diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
> > index 92a5951331..64c0a753ff 100755
>
> On top of what commit is this patch designed to apply?

What Junio probably means is that you appear to have based v2 atop v1,
but instead you should squash v1 and v2 into a single patch, and send
that as v3 so that when the patch is finally accepted into his tree,
it will appear to have been perfect from the start (because v1 and v2
will only exist in the mailing list archive, not in the Git project
history).

Re: [PATCH v2] t3310: avoid hiding failures from rev-parse in command substitutions

[Francesco Paparatto]

Junio C Hamano <gitster@pobox.com> writes:

> Hmph, did I suggest this?  I know Eric had comments on a previous
> round, and the improvements in this patch seems to be influenced a
> lot stronger by his input than whatever I may have said.

Sorry about the Suggested-by line. I added it because of your earlier
comment here:

    https://public-inbox.org/git/xmqqv7fioueg.fsf@gitster.g/

but you're right that the concrete changes in this version were mostly
influenced by Eric's review, so I'll drop that trailer.

> On top of what commit is this patch designed to apply?

This patch is based on top of:

    b3ec5aec2367262f464a33d6eab7a9f49fd413f1
    ("t3310: replace test -f/-d with test_path_is_file/test_path_is_dir")

I'll reroll and send a v3 of the patch shortly.

Thanks,
Francesco

Re: [PATCH v2] t3310: avoid hiding failures from rev-parse in command substitutions

[Francesco Paparatto]

Eric Sunshine <sunshine@sunshineco.com> writes:

> What Junio probably means is that you appear to have based v2 atop v1,
> but instead you should squash v1 and v2 into a single patch, and send
> that as v3 so that when the patch is finally accepted into his tree,
> it will appear to have been perfect from the start (because v1 and v2
> will only exist in the mailing list archive, not in the Git project
> history).

Sorry about that, and thanks for the clarification.

I've squashed the changes and rerolled the patch based on your
suggestions. I've just sent v3 to the list.

Thanks,
Francesco

[PATCH v3] t3310: avoid hiding failures from rev-parse in command substitutions

[Francesco Paparatto]

Running `git` commands inside command substitutions like

    test "$(git rev-parse A)" = "$(git rev-parse B)"

can hide failures from the `git` invocations and provide little
diagnostic information when `test` fails.

Use `test_cmp` when comparing against a stored expected value so
mismatches show both expected and actual output. Use `test_cmp_rev`
when comparing two revisions. These helpers produce clearer failure
output, making it easier to understand what went wrong.

Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
---
 t/t3310-notes-merge-manual-resolve.sh | 48 +++++++++++++++++----------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index f0054b0a39..64c0a753ff 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -227,7 +227,8 @@ test_expect_success 'merge z into m (== y) with default ("manual") resolver => C
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cat <<EOF | sort >expect_notes_z
@@ -375,8 +376,10 @@ EOF
 	git notes merge --commit &&
 	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
-	test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
-	test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
+	git rev-parse refs/notes/m^1 >actual &&
+	test_cmp pre_merge_y actual &&
+	git rev-parse refs/notes/m^2 >actual &&
+	test_cmp pre_merge_z actual &&
 	# Merge commit mentions the notes refs merged
 	git log -1 --format=%B refs/notes/m > merge_commit_msg &&
 	grep -q refs/notes/m merge_commit_msg &&
@@ -428,14 +431,16 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 test_expect_success 'abort notes merge' '
 	git notes merge --abort &&
 	notes_merge_files_gone &&
 	# m has not moved (still == y)
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -460,7 +465,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cat <<EOF | sort >expect_notes_m
@@ -500,8 +506,10 @@ EOF
 	git notes merge --commit &&
 	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
-	test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
-	test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
+	git rev-parse refs/notes/m^1 >actual &&
+	test_cmp pre_merge_y actual &&
+	git rev-parse refs/notes/m^2 >actual &&
+	test_cmp pre_merge_z actual &&
 	# Merge commit mentions the notes refs merged
 	git log -1 --format=%B refs/notes/m > merge_commit_msg &&
 	grep -q refs/notes/m merge_commit_msg &&
@@ -539,7 +547,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cp expect_notes_w expect_notes_m
@@ -548,7 +557,7 @@ cp expect_log_w expect_log_m
 test_expect_success 'reset notes ref m to somewhere else (w)' '
 	git update-ref refs/notes/m refs/notes/w &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
+	test_cmp_rev refs/notes/m refs/notes/w
 '
 
 test_expect_success 'fail to finalize conflicting merge if underlying ref has moved in the meantime (m != NOTES_MERGE_PARTIAL^1)' '
@@ -569,13 +578,15 @@ EOF
 	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
 	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
 	# Refs are unchanged
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
-	test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
-	test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
+	test_cmp_rev refs/notes/m refs/notes/w &&
+	test_cmp_rev refs/notes/y NOTES_MERGE_PARTIAL^1 &&
+	test_cmp_rev ! refs/notes/m NOTES_MERGE_PARTIAL^1 &&
 	# Mention refs/notes/m, and its current and expected value in output
 	test_grep -q "refs/notes/m" output &&
-	test_grep -q "$(git rev-parse refs/notes/m)" output &&
-	test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
+	git rev-parse refs/notes/m >actual &&
+	test_grep -q "$(cat actual)" output &&
+	git rev-parse NOTES_MERGE_PARTIAL^1 >actual &&
+	test_grep -q "$(cat actual)" output &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -587,7 +598,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '
 	git notes merge --abort &&
 	notes_merge_files_gone &&
 	# m has not moved (still == w)
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
+	test_cmp_rev refs/notes/m refs/notes/w &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -606,8 +617,9 @@ test_expect_success 'switch cwd before committing notes merge' '
 	test_must_fail git notes merge refs/notes/other &&
 	(
 		cd .git/NOTES_MERGE_WORKTREE &&
-		echo "foo" > $(git rev-parse HEAD) &&
-		echo "bar" >> $(git rev-parse HEAD) &&
+		oid=$(git rev-parse HEAD) &&
+		echo "foo" >"$oid" &&
+		echo "bar" >>"$oid" &&
 		git notes merge --commit
 	) &&
 	git notes show HEAD > actual_notes &&
-- 
2.52.0

Re: [PATCH v2] t3310: avoid hiding failures from rev-parse in command substitutions

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> > diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
>> > index 92a5951331..64c0a753ff 100755
>>
>> On top of what commit is this patch designed to apply?
>
> What Junio probably means is that you appear to have based v2 atop v1,
> but instead you should squash v1 and v2 into a single patch, and send
> that as v3 so that when the patch is finally accepted into his tree,
> it will appear to have been perfect from the start (because v1 and v2
> will only exist in the mailing list archive, not in the Git project
> history).

No.  The v1 and this one touch separate areas and can go
independently.  The thing I had trouble with was that this did not
apply to either on top of v1 (which by the way is already in 'next')
nor on top of 'master'.

Re: [PATCH v2] t3310: avoid hiding failures from rev-parse in command substitutions

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>>> > diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
>>> > index 92a5951331..64c0a753ff 100755
>>>
>>> On top of what commit is this patch designed to apply?
>>
>> What Junio probably means is that you appear to have based v2 atop v1,
>> but instead you should squash v1 and v2 into a single patch, and send
>> that as v3 so that when the patch is finally accepted into his tree,
>> it will appear to have been perfect from the start (because v1 and v2
>> will only exist in the mailing list archive, not in the Git project
>> history).
>
> No.  The v1 and this one touch separate areas and can go
> independently.  The thing I had trouble with was that this did not
> apply to either on top of v1 (which by the way is already in 'next')
> nor on top of 'master'.

Ah, sorry, no.  I was utterly confused.  Somehow I mixed two
unrelated patches on this same t3310 script.  What went to 'next'
was the other unrelated one, and I did not even take the v1 of this
topic.

I'll look at v3 now.

Sorry for the confusion, and thanks for helping.

Re: [PATCH v3] t3310: avoid hiding failures from rev-parse in command substitutions

[Eric Sunshine]

On Thu, Mar 5, 2026 at 5:51 PM Francesco Paparatto
<francescopaparatto@gmail.com> wrote:
> Running `git` commands inside command substitutions like
>
>     test "$(git rev-parse A)" = "$(git rev-parse B)"
>
> can hide failures from the `git` invocations and provide little
> diagnostic information when `test` fails.
>
> Use `test_cmp` when comparing against a stored expected value so
> mismatches show both expected and actual output. Use `test_cmp_rev`
> when comparing two revisions. These helpers produce clearer failure
> output, making it easier to understand what went wrong.
>
> Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
> ---

Thank you. This version looks much better and addresses my review
comments on the previous round. I do have one actionable
recommendation and one subjective comment, though...

> diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
> @@ -569,13 +578,15 @@ EOF
>         test_grep -q "refs/notes/m" output &&
> -       test_grep -q "$(git rev-parse refs/notes/m)" output &&
> -       test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
> +       git rev-parse refs/notes/m >actual &&
> +       test_grep -q "$(cat actual)" output &&
> +       git rev-parse NOTES_MERGE_PARTIAL^1 >actual &&
> +       test_grep -q "$(cat actual)" output &&

Storing the output of git-rev-parse in a file only to read it back out
of that file a moment later is unnecessarily roundabout. It would
instead be cleaner to do it this way:

    oid=$(git rev-parse refs/notes/m) &&
    test_grep -q "$oid" output &&
    oid=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
    test_grep -q "$oid" output &&

Unlike this original in which git-rev-parse's exit code was lost due
to being embedded in the test_grep invocation, this rewrite is safe
because the exit code of git-rev-parse becomes the exit code of the
variable assignment, thus correctly aborts the test (due to the
&&-chain) if git-rev-parse fails.

> @@ -606,8 +617,9 @@ test_expect_success 'switch cwd before committing notes merge' '
>         test_must_fail git notes merge refs/notes/other &&
>         (
>                 cd .git/NOTES_MERGE_WORKTREE &&
> -               echo "foo" > $(git rev-parse HEAD) &&
> -               echo "bar" >> $(git rev-parse HEAD) &&
> +               oid=$(git rev-parse HEAD) &&
> +               echo "foo" >"$oid" &&
> +               echo "bar" >>"$oid" &&

This is purely subjective and you don't have to take the suggestion,
but although yours is a faithful rewrite (which is good), I probably
would have simplified this to:

    oid=$(git rev-parse HEAD) &&
    test_write_lines foo bar >"$oid" &&

Re: [PATCH v3] t3310: avoid hiding failures from rev-parse in command substitutions

[Francesco Paparatto]

Eric Sunshine <sunshine@sunshineco.com> writes:

> > diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
> > @@ -569,13 +578,15 @@ EOF
> >         test_grep -q "refs/notes/m" output &&
> > -       test_grep -q "$(git rev-parse refs/notes/m)" output &&
> > -       test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
> > +       git rev-parse refs/notes/m >actual &&
> > +       test_grep -q "$(cat actual)" output &&
> > +       git rev-parse NOTES_MERGE_PARTIAL^1 >actual &&
> > +       test_grep -q "$(cat actual)" output &&
>
> Storing the output of git-rev-parse in a file only to read it back out
> of that file a moment later is unnecessarily roundabout. It would
> instead be cleaner to do it this way:
>
>     oid=$(git rev-parse refs/notes/m) &&
>     test_grep -q "$oid" output &&
>     oid=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
>     test_grep -q "$oid" output &&
>
> Unlike this original in which git-rev-parse's exit code was lost due
> to being embedded in the test_grep invocation, this rewrite is safe
> because the exit code of git-rev-parse becomes the exit code of the
> variable assignment, thus correctly aborts the test (due to the
> &&-chain) if git-rev-parse fails.
>
> > @@ -606,8 +617,9 @@ test_expect_success 'switch cwd before committing notes merge' '
> >         test_must_fail git notes merge refs/notes/other &&
> >         (
> >                 cd .git/NOTES_MERGE_WORKTREE &&
> > -               echo "foo" > $(git rev-parse HEAD) &&
> > -               echo "bar" >> $(git rev-parse HEAD) &&
> > +               oid=$(git rev-parse HEAD) &&
> > +               echo "foo" >"$oid" &&
> > +               echo "bar" >>"$oid" &&
>
> This is purely subjective and you don't have to take the suggestion,
> but although yours is a faithful rewrite (which is good), I probably
> would have simplified this to:
>
>     oid=$(git rev-parse HEAD) &&
>     test_write_lines foo bar >"$oid" &&

Thanks for the review. Both suggestions make sense. I'll use the
variable assignment for the rev-parse cases and test_write_lines for
the foo/bar case. I will send v4 shortly.

[PATCH v4] t3310: avoid hiding failures from rev-parse in command substitutions

[Francesco Paparatto]

Running `git` commands inside command substitutions like

    test "$(git rev-parse A)" = "$(git rev-parse B)"

can hide failures from the `git` invocations and provide little
diagnostic information when `test` fails.

Use `test_cmp` when comparing against a stored expected value so
mismatches show both expected and actual output. Use `test_cmp_rev`
when comparing two revisions. These helpers produce clearer failure
output, making it easier to understand what went wrong.

Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
---
 t/t3310-notes-merge-manual-resolve.sh | 47 +++++++++++++++++----------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index f0054b0a39..0bb366fdb8 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -227,7 +227,8 @@ test_expect_success 'merge z into m (== y) with default ("manual") resolver => C
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cat <<EOF | sort >expect_notes_z
@@ -375,8 +376,10 @@ EOF
 	git notes merge --commit &&
 	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
-	test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
-	test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
+	git rev-parse refs/notes/m^1 >actual &&
+	test_cmp pre_merge_y actual &&
+	git rev-parse refs/notes/m^2 >actual &&
+	test_cmp pre_merge_z actual &&
 	# Merge commit mentions the notes refs merged
 	git log -1 --format=%B refs/notes/m > merge_commit_msg &&
 	grep -q refs/notes/m merge_commit_msg &&
@@ -428,14 +431,16 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 test_expect_success 'abort notes merge' '
 	git notes merge --abort &&
 	notes_merge_files_gone &&
 	# m has not moved (still == y)
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -460,7 +465,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cat <<EOF | sort >expect_notes_m
@@ -500,8 +506,10 @@ EOF
 	git notes merge --commit &&
 	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
-	test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
-	test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
+	git rev-parse refs/notes/m^1 >actual &&
+	test_cmp pre_merge_y actual &&
+	git rev-parse refs/notes/m^2 >actual &&
+	test_cmp pre_merge_z actual &&
 	# Merge commit mentions the notes refs merged
 	git log -1 --format=%B refs/notes/m > merge_commit_msg &&
 	grep -q refs/notes/m merge_commit_msg &&
@@ -539,7 +547,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 	# Verify that current notes tree (pre-merge) has not changed (m == y)
 	verify_notes y &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+	git rev-parse refs/notes/m >actual &&
+	test_cmp pre_merge_y actual
 '
 
 cp expect_notes_w expect_notes_m
@@ -548,7 +557,7 @@ cp expect_log_w expect_log_m
 test_expect_success 'reset notes ref m to somewhere else (w)' '
 	git update-ref refs/notes/m refs/notes/w &&
 	verify_notes m &&
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
+	test_cmp_rev refs/notes/m refs/notes/w
 '
 
 test_expect_success 'fail to finalize conflicting merge if underlying ref has moved in the meantime (m != NOTES_MERGE_PARTIAL^1)' '
@@ -569,13 +578,15 @@ EOF
 	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
 	test_path_is_file .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
 	# Refs are unchanged
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
-	test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
-	test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
+	test_cmp_rev refs/notes/m refs/notes/w &&
+	test_cmp_rev refs/notes/y NOTES_MERGE_PARTIAL^1 &&
+	test_cmp_rev ! refs/notes/m NOTES_MERGE_PARTIAL^1 &&
 	# Mention refs/notes/m, and its current and expected value in output
 	test_grep -q "refs/notes/m" output &&
-	test_grep -q "$(git rev-parse refs/notes/m)" output &&
-	test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
+	oid=$(git rev-parse refs/notes/m) &&
+	test_grep -q "$oid" output &&
+	oid=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
+	test_grep -q "$oid" output &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -587,7 +598,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '
 	git notes merge --abort &&
 	notes_merge_files_gone &&
 	# m has not moved (still == w)
-	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
+	test_cmp_rev refs/notes/m refs/notes/w &&
 	# Verify that other notes refs has not changed (w, x, y and z)
 	verify_notes w &&
 	verify_notes x &&
@@ -606,8 +617,8 @@ test_expect_success 'switch cwd before committing notes merge' '
 	test_must_fail git notes merge refs/notes/other &&
 	(
 		cd .git/NOTES_MERGE_WORKTREE &&
-		echo "foo" > $(git rev-parse HEAD) &&
-		echo "bar" >> $(git rev-parse HEAD) &&
+		oid=$(git rev-parse HEAD) &&
+		test_write_lines foo bar >"$oid" &&
 		git notes merge --commit
 	) &&
 	git notes show HEAD > actual_notes &&
-- 
2.52.0

Re: [PATCH v4] t3310: avoid hiding failures from rev-parse in command substitutions

[Eric Sunshine]

On Sat, Mar 7, 2026 at 5:36 AM Francesco Paparatto
<francescopaparatto@gmail.com> wrote:
> Running `git` commands inside command substitutions like
>
>     test "$(git rev-parse A)" = "$(git rev-parse B)"
>
> can hide failures from the `git` invocations and provide little
> diagnostic information when `test` fails.
>
> Use `test_cmp` when comparing against a stored expected value so
> mismatches show both expected and actual output. Use `test_cmp_rev`
> when comparing two revisions. These helpers produce clearer failure
> output, making it easier to understand what went wrong.
>
> Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Francesco Paparatto <francescopaparatto@gmail.com>
> ---
> @@ -569,13 +578,15 @@ EOF
> -       test_grep -q "$(git rev-parse refs/notes/m)" output &&
> -       test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
> +       oid=$(git rev-parse refs/notes/m) &&
> +       test_grep -q "$oid" output &&
> +       oid=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
> +       test_grep -q "$oid" output &&
> @@ -606,8 +617,8 @@ test_expect_success 'switch cwd before committing notes merge' '
> -               echo "foo" > $(git rev-parse HEAD) &&
> -               echo "bar" >> $(git rev-parse HEAD) &&
> +               oid=$(git rev-parse HEAD) &&
> +               test_write_lines foo bar >"$oid" &&

Thank you, this version (v4) looks good; it addresses all my review
comments. For what it's worth:

    Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

Re: [PATCH v4] t3310: avoid hiding failures from rev-parse in command substitutions

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> @@ -569,13 +578,15 @@ EOF
>> -       test_grep -q "$(git rev-parse refs/notes/m)" output &&
>> -       test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
>> +       oid=$(git rev-parse refs/notes/m) &&
>> +       test_grep -q "$oid" output &&
>> +       oid=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
>> +       test_grep -q "$oid" output &&
>> @@ -606,8 +617,8 @@ test_expect_success 'switch cwd before committing notes merge' '
>> -               echo "foo" > $(git rev-parse HEAD) &&
>> -               echo "bar" >> $(git rev-parse HEAD) &&
>> +               oid=$(git rev-parse HEAD) &&
>> +               test_write_lines foo bar >"$oid" &&
>
> Thank you, this version (v4) looks good; it addresses all my review
> comments. For what it's worth:
>
>     Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

Yup, looking good.