On Sat, Mar 3, 2018 at 8:12 AM, Jeff King <peff@xxxxxxxx> 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.