Re: [PATCH v3 3/4] t7005: sanitize test environment for subsequent tests

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 12, 2025, 17:13 UTC
Message-ID: <CAPig+cRRBDqewrqUZGS=EPJgSkYmX-zD_qrqbRXzXq59Ub2m_g@mail.gmail.com>
In-Reply-To: <CALnO6CDaHZuC0=_Saccj3DuW-vaWs30PmwXf9dw1+AfsC34wWg@mail.gmail.com>

On Tue, Aug 12, 2025 at 12:41 PM D. Ben Knoble <ben.knoble+github@gmail.com> wrote:

Show 12 quoted lines

> On Mon, Aug 11, 2025 at 6:34 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Mon, Aug 11, 2025 at 6:17 PM D. Ben Knoble
> > <ben.knoble+github@gmail.com> wrote:
> > > +       TERM=dumb test_must_fail git commit --amend
> >
> > Don't use one-shot environment variable assignments when calling shell
> > functions. Instead, you can do this:
> >
> >     test_env TERM=dumb test_must_fail git commit --amend
>
> Yep ;) I had the latter, switched (see range-diff, I think), and then
> CI caught me. Why doesn't the local test run catch it, though?

Although the `chainlint` linter gets wired into each test script, thus is invoked automatically when manually running any individual test script, the `check-non-portable-shell` linter does not get wired into each test script, thus does not get run automatically when manually running a test script.

Almost all of the "lints" performed by `check-non-portable` could/should eventually be folded into `chainlint`, thus eliminating this difference in behavior, but the "shell-function-one-shot-variable-assignment" lint is an outlier because it can't be easily localized. Specifically, whereas all the other lints can operate just by consulting local context, that one lint can't because it can't know what command names are in fact shell functions without looking at other test scripts, as well. Hence, the operation of check-non-portable-shell.sh is two-phase: (1) it first scans all scripts to find all shell function definitions, and then (2) it "lints" each test script individually, in the process consulting the table of function names compiled in step 1.

When you run a test script manually, the script triggers a run of `chainlint`, but `chainlint` only knows about that one script it's checking, thus it can't compile a table of shell functions which might be defined in other scripts. Obviously, there is no technical reason that this couldn't be implemented directly in `chainlint`; for instance, `chainlint` could detect other scripts which are pulled in by the script it is testing and compile a table of names of shell functions defined in those scripts. Eventually, I think it would be nice for `chainlint` to subsume all the checks performed by `check-non-portable-shell`, thus eliminating the latter script and (hopefully) speeding up linting altogether (at least a bit) but the work to do so simply hasn't been done yet.