Re: [PATCH 12/19] tests: fix broken &&-chains in `{...}` groups
From: Eric Sunshine <hidden>
Date: 2021-12-11 07:32:32
On Fri, Dec 10, 2021 at 4:38 AM Fabian Stelzer [off-list ref] wrote:
On 09.12.2021 00:11, Eric Sunshine wrote:quoted
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...
quoted
{ 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...
quoted
{ 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.