From: Francesco Paparatto <francescopaparatto@gmail.com>
To: git@vger.kernel.org
Cc: Francesco Paparatto <francescopaparatto@gmail.com>,
Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] t3310: avoid hiding failures from rev-parse in command substitutions
Date: Wed, 4 Mar 2026 13:19:55 +0100 [thread overview]
Message-ID: <20260304121955.73794-1-francescopaparatto@gmail.com> (raw)
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
next reply other threads:[~2026-03-04 12:20 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-04 12:19 Francesco Paparatto [this message]
2026-03-04 22:21 ` [PATCH] t3310: avoid hiding failures from rev-parse in command substitutions 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 ` [PATCH v3] " Francesco Paparatto
2026-03-07 6:29 ` 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=20260304121955.73794-1-francescopaparatto@gmail.com \
--to=francescopaparatto@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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