Re: [PATCH 12/19] tests: fix broken &&-chains in `{...}` groups

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Dec 10, 2021 at 4:38 AM Fabian Stelzer <fs@gigacodes.de> wrote:
> On 09.12.2021 00:11, Eric Sunshine wrote:
> >Fix broken &&-chains in `{...}` groups in order to reduce the number of
> >possible lurking bugs.
> >       {
> >-          echo "*.t filter=rot13"
> >+          echo "*.t filter=rot13" &&
> >           echo "*.i ident"
> >       } >.gitattributes &&
> >       {
> >-              echo "expanded-keywords ident"
> >+              echo "expanded-keywords ident" &&
> >               echo "expanded-keywords-crlf ident text eol=crlf"
> >       } >>.gitattributes &&
> >
>
> Wouldn't some of these be better off as heredocs as well?
> There are a couple more below. I personally don't much mind either way but
> since you changed quite a few in an earlier commit why not these?

It's been months since I made these changes, but I think there were at
least a couple reasons for not converting these to here-docs. First,
in these cases, there were only one or two missing `&&` per block. Had
I bulk converted them to here-docs, it would have made for a much more
noisy patch, which would have taxed reviewers more, and
reviewer-fatigue is a real concern when crafting a lengthy patch
series like this one. In the "here-doc conversion" patch, on the other
hand, many of those cases involved a significant number of missing
`&&`; often every line was missing `&&`. So, the changes in that patch
was going to be very noisy anyhow, whether I added missing `&&` or
converted to here-docs.

Second...

> >       {
> >               echoid insert 444 1 2 3 4 5 a b c d e &&
> >-              echoid contains 44 441 440 444 4440 4444
> >+              echoid contains 44 441 440 444 4440 4444 &&
> >               echo clear
> >       } | test-tool oidtree >actual &&

... there are a number of cases like this which look like they could
easily be converted to here-doc, but in fact `echoid` is a function
call, so a here-doc wouldn't work. Also...

> >       {
> >               echo $(git rev-parse refs/tags/A) refs/tags/A &&
> >-              echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}"
> >+              echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}" &&
> >               echo $(git rev-parse refs/tags/C) refs/tags/C
> >       } >expect &&

... this sort of thing could certainly become a here-doc because
$(...) will work in a here-doc, but when there is a preponderance of
this sort of `{ echo && ... }` block in the test script, it would feel
inconsistent to convert a few of them to here-docs.