Re: [PATCH 00/11] Make the test suite pass with '-x' and /bin/sh

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Mar 5, 2018, 21:18 UTC
Message-ID: <CAM0VKjmaL16M87SVt9xh3nrHBT49Gygr7=9RqvfxA6E9D6OzmA@mail.gmail.com>
In-Reply-To: <20180303071259.GB17312@sigill.intra.peff.net>

On Sat, Mar 3, 2018 at 8:12 AM, Jeff King <peff@peff.net> wrote:

Show 11 quoted lines

> On Sat, Feb 24, 2018 at 12:39:40AM +0100, SZEDER Gábor wrote:
>
>> The first patch is the most important: with a couple of well-placed file
>> descriptor redirections it ensures that the stderr of the test helper
>> functions running git commands only contain the stderr of the tested
>> command, thereby resolving over 90% of the failures resulting from
>> running the test suite with '-x' and /bin/sh.
>
> I dunno. It seems like this requires a lot of caveats for people using
> subshells and shell functions, and I suspect it's going to be an
> on-going maintenance burden.

After finally figuring out the redirections in the first patch, I was quite surprised by how few failing tests remained. We only gathered 28 such tests over all these years; if it continues at this rate, that probably won't be that much of a burden. And the second patch provides an escape hatch, should it ever be needed.

The current situation, however, is a burden much more frequently, because the idiosyncrasies of TEST_SHELL_PATH and/or '--verbose-log' pop up whenever trying to run any test script with '-x' that has such a test in it.

I think this is the right tradeoff.

Show 8 quoted lines

> That said, I'm not opposed if you want to do the work to try to get the
> whole test-suite clean, and we can see how it goes from there. It
> shouldn't be hurting anything, I don't think, aside from some
> mysterious-looking redirects (but your commit messages seem to explain
> it, so anybody can dig).
>
> Does it make descriptor 7 magical, and something that scripts should
> avoid touching? That would mean we have 2 magical descriptors now.

Tests can still use fd 7 as long as they don't intend to attach it directly to that particular git command that is run inside one of these test helper functions.

I settled on fd 7 because that fd is already used as stderr for the 'test_pause' and 'debug' helper functions and it isn't used in any of our tests.