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

> > diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
> > @@ -569,13 +578,15 @@ EOF
> >         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 &&
> > +       git rev-parse refs/notes/m >actual &&
> > +       test_grep -q "$(cat actual)" output &&
> > +       git rev-parse NOTES_MERGE_PARTIAL^1 >actual &&
> > +       test_grep -q "$(cat actual)" output &&
>
> Storing the output of git-rev-parse in a file only to read it back out
> of that file a moment later is unnecessarily roundabout. It would
> instead be cleaner to do it this way:
>
>     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 &&
>
> Unlike this original in which git-rev-parse's exit code was lost due
> to being embedded in the test_grep invocation, this rewrite is safe
> because the exit code of git-rev-parse becomes the exit code of the
> variable assignment, thus correctly aborts the test (due to the
> &&-chain) if git-rev-parse fails.
>
> > @@ -606,8 +617,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" &&
>
> This is purely subjective and you don't have to take the suggestion,
> but although yours is a faithful rewrite (which is good), I probably
> would have simplified this to:
>
>     oid=$(git rev-parse HEAD) &&
>     test_write_lines foo bar >"$oid" &&