Skip to content

Commit 82f6c0c

Browse files
frapaparattogitster
authored andcommitted
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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 67ad421 commit 82f6c0c

1 file changed

Lines changed: 30 additions & 18 deletions

File tree

t/t3310-notes-merge-manual-resolve.sh

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,8 @@ test_expect_success 'merge z into m (== y) with default ("manual") resolver => C
227227
# Verify that current notes tree (pre-merge) has not changed (m == y)
228228
verify_notes y &&
229229
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
231232
'
232233

233234
cat <<EOF | sort >expect_notes_z
@@ -375,8 +376,10 @@ EOF
375376
git notes merge --commit &&
376377
notes_merge_files_gone &&
377378
# 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 &&
380383
# Merge commit mentions the notes refs merged
381384
git log -1 --format=%B refs/notes/m > merge_commit_msg &&
382385
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
428431
# Verify that current notes tree (pre-merge) has not changed (m == y)
429432
verify_notes y &&
430433
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
432436
'
433437

434438
test_expect_success 'abort notes merge' '
435439
git notes merge --abort &&
436440
notes_merge_files_gone &&
437441
# 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 &&
439444
# Verify that other notes refs has not changed (w, x, y and z)
440445
verify_notes w &&
441446
verify_notes x &&
@@ -460,7 +465,8 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
460465
# Verify that current notes tree (pre-merge) has not changed (m == y)
461466
verify_notes y &&
462467
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
464470
'
465471

466472
cat <<EOF | sort >expect_notes_m
@@ -500,8 +506,10 @@ EOF
500506
git notes merge --commit &&
501507
notes_merge_files_gone &&
502508
# 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 &&
505513
# Merge commit mentions the notes refs merged
506514
git log -1 --format=%B refs/notes/m > merge_commit_msg &&
507515
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
539547
# Verify that current notes tree (pre-merge) has not changed (m == y)
540548
verify_notes y &&
541549
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
543552
'
544553

545554
cp expect_notes_w expect_notes_m
@@ -548,7 +557,7 @@ cp expect_log_w expect_log_m
548557
test_expect_success 'reset notes ref m to somewhere else (w)' '
549558
git update-ref refs/notes/m refs/notes/w &&
550559
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
552561
'
553562

554563
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
569578
test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
570579
test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
571580
# 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 &&
575584
# Mention refs/notes/m, and its current and expected value in output
576585
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+
git rev-parse refs/notes/m >actual &&
587+
test_grep -q "$(cat actual)" output &&
588+
git rev-parse NOTES_MERGE_PARTIAL^1 >actual &&
589+
test_grep -q "$(cat actual)" output &&
579590
# Verify that other notes refs has not changed (w, x, y and z)
580591
verify_notes w &&
581592
verify_notes x &&
@@ -587,7 +598,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '
587598
git notes merge --abort &&
588599
notes_merge_files_gone &&
589600
# 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 &&
591602
# Verify that other notes refs has not changed (w, x, y and z)
592603
verify_notes w &&
593604
verify_notes x &&
@@ -606,8 +617,9 @@ test_expect_success 'switch cwd before committing notes merge' '
606617
test_must_fail git notes merge refs/notes/other &&
607618
(
608619
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+
echo "foo" >"$oid" &&
622+
echo "bar" >>"$oid" &&
611623
git notes merge --commit
612624
) &&
613625
git notes show HEAD > actual_notes &&

0 commit comments

Comments
 (0)