Re: [PATCH v4] branch: introduce --show-current display option

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Oct 18, 2018 at 5:51 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Wed, 17 Oct 2018, Eric Sunshine wrote:
> > On Wed, Oct 17, 2018 at 6:18 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > My suspicion: it is essentially the `(exit 117)` that adds about 100ms to
> > > every of those 67 test cases.
> >
> > The subshell chain-linter adds a 'sed' and 'grep' invocation to each test which doesn't help. (v1 of the subshell chain-linter only added a 'sed', but that changed with v2.)
> > You could disable the subshell chain-linter like this if you want test the (exit 117) goop in isolation:
>
> You're right! This is actually responsible for about five of those seven
> seconds. The subshell still hurts a little, as it means that every single
> of the almost 20,000 test cases we have gets slowed down by ~0.03s, which
> amounts to almost 10 minutes.
>
> This is "only" for the Windows phase of our Continuous Testing, of course.
> Yet I think we can do better than this.
>
> How difficult/involved, do you think, would it be to add a t/helper/
> command for chain linting?

Probably more effort than it's worth, and it would only save one
process invocation.

Since the  subshell portion of the chain-linting is done by pure
textual inspection, an alternative I had considered was to just
perform it as a preprocess over the entire test suite, much like the
other t/Makefile "test-lint" targets. In other words, the entire test
suite might be tested in one go with something like this:

    sed -f chainlint.sed t*.sh | grep -q '?![A-Z][A-Z]*?!' &&
        echo "BROKEN &&-chain"

That won't work today since chainlint.sed isn't written to understand
everything which we might see outside of a test_expect_*, but doing it
that way is within the realm of possibility. There were two reasons
why I didn't pursue that approach.

First, although I was expecting Windows folks to complain (or at least
speak up) about the extra 'sed' and 'grep', nobody did, so my
impression was that those two extra commands were likely lost in the
noise of the rest of the boilerplate commands invoked by
test_expect_success(), test_run_(), test_eval_(), etc., and by
whatever expensive commands are invoked by each test itself. Second,
the top-level &&-chain "(exit 117)" linting kicks in even when you run
a single test script manually, say after editing a test, which is
exactly when you want to discover that you botched a &&-chain, so it
seemed a good idea for the subshell &&-chain linter to follow suit.
The t/Makefile "test-lint" targets, on the other hand, don't kick in
when running test scripts in isolation.

However, a pragmatic way to gain back those 10 minutes might be simply
to disable the chain-linter for continuous integration testing on
Windows, but leave it enabled on other platforms. This way, we'd still
catch broken &&-chains, with the exception of tests which are specific
to Windows, of which I think there are very few.