Re: [PATCH v2 01/10] t/test-lib: teach --chain-lint to detect broken &&-chains in subshells

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 30, 2018, 20:25 UTC
Message-ID: <CAPig+cRFMKBQVVYjhS6-Yyy-aQCYXGiqG6XoqucJoedCvAzheQ@mail.gmail.com>
In-Reply-To: <20180730181356.GA156463@aiede.svl.corp.google.com>

On Mon, Jul 30, 2018 at 2:14 PM Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 7 quoted lines

> Eric Sunshine wrote:
> > Address this shortcoming by feeding the body of the test to a
> > lightweight "linter" which can peer inside subshells and identify broken
> > &&-chains by pure textual inspection.
>
> This is causing contrib/subtree tests to fail for me: running "make -C
> contrib/subtree test" produces

Thanks, I forgot that some of 'contrib' had bundled tests. (In fact, I just checked the other 'contrib' tests and found that a MediaWiki test has a broken top-level &&-chain.)

Show 10 quoted lines

> The problematic test code looks like this:
>
>         (
>                 chks_sub=$(cat <<TXT | sed '\''s,^,sub dir/,'\''
> $chks
> TXT
> ) &&
>
> Ugly quoting, useless use of "cat", etc, aside, I don't think it's
> missing any &&.  Hints?

Yes, it's a false positive.

The subshell linter would normally fold out the here-doc content, but 'sed' isn't a proper programming language, so the linter can't recognize arbitrary here-doc tags. Instead it has hard-coded knowledge of the tags commonly used in the Git tests, specifically EOF, EOT, and INPUT_END.

The linter also deals with multi-line $(...) expressions, however, it currently only recognizes them when the $( is on its own line.

Had this test used one of the common here-doc tags _or_ had it formatted the $(...) as described, then it wouldn't have misfired.

I could try to update the linter to not trip over this sort of input, however, this test code is indeed ugly and difficult to understand, and your rewrite[1] of it makes it far easier to grok, so I'm not sure the effort would be worthwhile. What do you think?

[1]: https://public-inbox.org/git/20180730190738.GD156463@aiede.svl.corp.google.com/