Thread (9 messages) 9 messages, 3 authors, 2026-03-08

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

From: Eric Sunshine <hidden>
Date: 2026-03-05 22:34:34

Possibly related (same subject, not in this thread)

On Thu, Mar 5, 2026 at 2:13 PM Junio C Hamano [off-list ref] wrote:
quoted
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 <redacted>
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.
quoted
 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).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help