public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Francesco Paparatto <francescopaparatto@gmail.com>
To: francescopaparatto@gmail.com
Cc: git@vger.kernel.org, gitster@pobox.com, sunshine@sunshineco.com
Subject: [PATCH v3] t3310: avoid hiding failures from rev-parse in command substitutions
Date: Thu,  5 Mar 2026 23:51:28 +0100	[thread overview]
Message-ID: <20260305225128.54283-1-francescopaparatto@gmail.com> (raw)
In-Reply-To: <CAEaT9_-h2MEshMHoyoW9kWQgt_EfQJXcxWSn+cXTSL4mKME=5w@mail.gmail.com>

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


  reply	other threads:[~2026-03-05 22:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-04 12:19 [PATCH] t3310: avoid hiding failures from rev-parse in command substitutions Francesco Paparatto
2026-03-04 22:21 ` Eric Sunshine
2026-03-05  9:06   ` [PATCH v2] " Francesco Paparatto
2026-03-05 19:13     ` Junio C Hamano
2026-03-05 22:34       ` Eric Sunshine
2026-03-05 22:49         ` Francesco Paparatto
2026-03-05 23:06         ` Junio C Hamano
2026-03-05 23:14           ` Junio C Hamano
2026-03-05 22:42       ` Francesco Paparatto
2026-03-05 22:51         ` Francesco Paparatto [this message]
2026-03-07  6:29           ` [PATCH v3] " Eric Sunshine
2026-03-07 10:17             ` Francesco Paparatto
2026-03-07 10:36           ` [PATCH v4] " Francesco Paparatto
2026-03-08  4:13             ` Eric Sunshine
2026-03-08  6:04               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260305225128.54283-1-francescopaparatto@gmail.com \
    --to=francescopaparatto@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox