From: Eric Sunshine Date: Sun, 08 Mar 2026 04:13:00 GMT Subject: Re: [PATCH v4] t3310: avoid hiding failures from rev-parse in command substitutions Message-ID: In-Reply-To: <20260307103631.89829-1-francescopaparatto@gmail.com> On Sat, Mar 7, 2026 at 5:36 AM Francesco Paparatto wrote: > 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 > Signed-off-by: Francesco Paparatto > --- > @@ -569,13 +578,15 @@ EOF > - test_grep -q "$(git rev-parse refs/notes/m)" output && > - test_grep -q "$(git rev-parse NOTES_MERGE_PARTIAL^1)" output && > + oid=$(git rev-parse refs/notes/m) && > + test_grep -q "$oid" output && > + oid=$(git rev-parse NOTES_MERGE_PARTIAL^1) && > + test_grep -q "$oid" output && > @@ -606,8 +617,8 @@ test_expect_success 'switch cwd before committing notes merge' ' > - echo "foo" > $(git rev-parse HEAD) && > - echo "bar" >> $(git rev-parse HEAD) && > + oid=$(git rev-parse HEAD) && > + test_write_lines foo bar >"$oid" && Thank you, this version (v4) looks good; it addresses all my review comments. For what it's worth: Reviewed-by: Eric Sunshine