Re: [PATCH] t: avoid sed-based chain-linting in some expensive cases

From: Martin Ågren <martin.agren@gmail.com>
Date: May 13, 2021, 11:05 UTC
Message-ID: <CAN0heSp3mXQeqeC_Zd==bBoJCCWe-NzJsomuUf6MTxy7+WZ1wA@mail.gmail.com>
In-Reply-To: <YJzGcZpZ+E9R0gYd@coredump.intra.peff.net>

On Thu, 13 May 2021 at 08:29, Jeff King <peff@peff.net> wrote:

Show 10 quoted lines

> This patch introduces a mechanism for controlling when those extra
> lint checks are run, at two levels:
>
>   - a user can ask to disable or to force-enable the checks by setting
>     GIT_TEST_CHAIN_LINT_HARDER
>
>   - if the user hasn't specified a preference, individual scripts can
>     disable the checks by setting GIT_TEST_CHAIN_LINT_HARDER_DEFAULT;
>     scripts which don't set that get the current behavior of enabling
>     them.

> +# Disable expensive chain-lint tests; all of the tests in this script
> +# are variants of a few trivial test-tool invocations, and there are a lot of
> +# them.
> +GIT_TEST_CHAIN_LINT_HARDER_DEFAULT=0

Devil's advocate: Who do we expect to turn GIT_TEST_CHAIN_LINT_HARDER on, and when? If no one ever does it then we might as well drop the "default" thing and just go "we won't bother linting these particular tests, ever". But as long as "someone" does it "sometimes", it's not like it's a very complex mechanism to carry around.

I seem to have 140 tests that haven't changed on disk since I did this particular clone in 2017. 235 haven't changed this calendar year. Maybe we could skip linting those tests that haven't been modified for several weeks on the basis that they can't reasonably have newly-introduced syntax mistakes. I guess it gets tricky where the t????-*.sh file doesn't change in a long time, but it sources tests from other places, such as a lib-foo.sh helper. We'd have to be a bit more clever there. That's all just thinking out loud, and definitely not something that should hold up your patch.

Martin