t3310: avoid hiding failures from rev-parse in command substitutions
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> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Francesco Paparatto committed
Mar 7, 2026 at 11:36 UTC
d3edca979a1e916518bc2376e468609ddae2a217
1 file changed
+29
-18
t/t3310-notes-merge-manual-resolve.sh
+29
-18
@@ -227,7 +227,8 @@ test_expect_success 'merge z into m (== y) with default ("manual") resolver => C
227
# Verify that current notes tree (pre-merge) has not changed (m == y)
228
verify_notes y &&
229
verify_notes m &&
230
- test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
230
+ git rev-parse refs/notes/m >actual &&
231
+ test_cmp pre_merge_y actual
232
'
233
234
cat <<EOF | sort >expect_notes_z
@@ -375,8 +376,10 @@ EOF
376
git notes merge --commit &&
377
notes_merge_files_gone &&
378
# Merge commit has pre-merge y and pre-merge z as parents
378
- test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
379
- test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
379
+ git rev-parse refs/notes/m^1 >actual &&
380
+ test_cmp pre_merge_y actual &&
381
+ git rev-parse refs/notes/m^2 >actual &&
382
+ test_cmp pre_merge_z actual &&
383
# Merge commit mentions the notes refs merged
384
git log -1 --format=%B refs/notes/m > merge_commit_msg &&
385
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
431
# Verify that current notes tree (pre-merge) has not changed (m == y)
432
verify_notes y &&
433
verify_notes m &&
431
- test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
434
+ git rev-parse refs/notes/m >actual &&
435
+ test_cmp pre_merge_y actual
436
'
437
438
test_expect_success 'abort notes merge' '
439
git notes merge --abort &&
440
notes_merge_files_gone &&
441
# m has not moved (still == y)
438
- test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
442
+ git rev-parse refs/notes/m >actual &&
443
+ test_cmp pre_merge_y actual &&
444
# Verify that other notes refs has not changed (w, x, y and z)
445
verify_notes w &&
446
verify_notes x &&
@@ -460,7 +465,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
465
# Verify that current notes tree (pre-merge) has not changed (m == y)
466
verify_notes y &&
467
verify_notes m &&
463
- test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
468
+ git rev-parse refs/notes/m >actual &&
469
+ test_cmp pre_merge_y actual
470
'
471
472
cat <<EOF | sort >expect_notes_m
@@ -500,8 +506,10 @@ EOF
506
git notes merge --commit &&
507
notes_merge_files_gone &&
508
# Merge commit has pre-merge y and pre-merge z as parents
503
- test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
504
- test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
509
+ git rev-parse refs/notes/m^1 >actual &&
510
+ test_cmp pre_merge_y actual &&
511
+ git rev-parse refs/notes/m^2 >actual &&
512
+ test_cmp pre_merge_z actual &&
513
# Merge commit mentions the notes refs merged
514
git log -1 --format=%B refs/notes/m > merge_commit_msg &&
515
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
547
# Verify that current notes tree (pre-merge) has not changed (m == y)
548
verify_notes y &&
549
verify_notes m &&
542
- test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
550
+ git rev-parse refs/notes/m >actual &&
551
+ test_cmp pre_merge_y actual
552
'
553
554
cp expect_notes_w expect_notes_m
@@ -548,7 +557,7 @@ cp expect_log_w expect_log_m
557
test_expect_success 'reset notes ref m to somewhere else (w)' '
558
git update-ref refs/notes/m refs/notes/w &&
559
verify_notes m &&
551
- test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
560
+ test_cmp_rev refs/notes/m refs/notes/w
561
'
562
563
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
578
test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
579
test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
580
# Refs are unchanged
572
- test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
573
- test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
574
- test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
581
+ test_cmp_rev refs/notes/m refs/notes/w &&
582
+ test_cmp_rev refs/notes/y NOTES_MERGE_PARTIAL^1 &&
583
+ test_cmp_rev ! refs/notes/m NOTES_MERGE_PARTIAL^1 &&
584
# Mention refs/notes/m, and its current and expected value in output
585
test_grep -q "refs/notes/m" output &&
577
- test_grep -q "$(git rev-parse refs/notes/m)" output &&
578
- test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output &&
586
+ oid=$(git rev-parse refs/notes/m) &&
587
+ test_grep -q "$oid" output &&
588
+ oid=$(git rev-parse NOTES_MERGE_PARTIAL^1) &&
589
+ test_grep -q "$oid" output &&
590
# Verify that other notes refs has not changed (w, x, y and z)
591
verify_notes w &&
592
verify_notes x &&
@@ -587,7 +598,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '
598
git notes merge --abort &&
599
notes_merge_files_gone &&
600
# m has not moved (still == w)
590
- test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
601
+ test_cmp_rev refs/notes/m refs/notes/w &&
602
# Verify that other notes refs has not changed (w, x, y and z)
603
verify_notes w &&
604
verify_notes x &&
@@ -606,8 +617,8 @@ test_expect_success 'switch cwd before committing notes merge' '
617
test_must_fail git notes merge refs/notes/other &&
618
(
619
cd .git/NOTES_MERGE_WORKTREE &&
609
- echo "foo" > $(git rev-parse HEAD) &&
610
- echo "bar" >> $(git rev-parse HEAD) &&
620
+ oid=$(git rev-parse HEAD) &&
621
+ test_write_lines foo bar >"$oid" &&
622
git notes merge --commit
623
) &&
624
git notes show HEAD > actual_notes &&