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

2018-03-05 Thread SZEDER Gábor
On Sat, Mar 3, 2018 at 8:12 AM, Jeff King  wrote:
> 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.

> 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.


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

2018-03-02 Thread Jeff King
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.

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.

-Peff


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

2018-03-02 Thread SZEDER Gábor
On Sat, Feb 24, 2018 at 12:39 AM, SZEDER Gábor  wrote:
> This patch series makes '-x' tracing of tests work reliably even when
> running them with /bin/sh, and setting TEST_SHELL_PATH=bash will be
> unnecessary.
>
>   make GIT_TEST_OPTS='-x --verbose-log' test
>
> passes on my setup and on all Travis CI build jobs (though neither me
> nor Travis CI run all tests, e.g. CVS).

I installed 'cvs' and whatnot to run t94* and t96* tests, and sure
enough, 5 tests in 2 test scripts fail with '-x' tracing and /bin/sh.
I think I will be able to get around to send v2 during the weekend.