Thread (47 messages) 47 messages, 4 authors, 2021-12-11

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help