On Sun, Feb 21, 2021 at 08:25:11PM +0100, SZEDER Gábor wrote:

The root of the issue stems from a5bf824f3b (t: prevent '-x' tracing
from interfering with test helpers' stderr, 2018-02-25), where we
started to use a couple of file descriptor duplications and
redirections to separate the standard error of git commands exercised
in test helper functions from the stderr containing the '-x' trace
output of said helper functions.  To achieve that the git command's
stderr is redirected to the test helper function's fd 7, which was
previously duplicated from the helper's stderr.  Alas, fd 7 was not
the right choice for this purpose, because fd 7 is the original
standard error of the test script, and, consequently, we now can't
send error messages from within such test helper functions directly to
the test script's stderr.  Since BUG() does want to send its error
message there it doesn't work as expected in such test helper
functions, because:

  - If the test helper's stderr were redirected to a file (as is often
    the case e.g. with 'test_must_fail'), then the "bug in the test
    script" error message would end up in that file.

  - If the test script is invoked without any of the verbose options,
    then that error message would get lost to /dev/null, leaving no
    clues about why the test script aborted so suddenly.

Makes sense. Well explained.

Use fd 6 instead of fd 7 for these '-x' tracing related duplications
and redirections.  It is a better choice for this purpose, because fd
6 is the test script's original standard input, and neither these test
helper functions not the git commands exercised by them should ever
read from the test scipt's stdin, see 781f76b158 (test-lib: redirect
stdin of tests, 2011-12-15).  Update the aforementioned error
reporting in 'test_must_fail' to send the error message to fd 6 as
well; the next patch will update it to use BUG() instead.

s/scipt/script/ in the paragraph above.

I agree that 6 is probably reasonable. I wonder if it is worth having a
master comment describing the function of various descriptors within the
test suite, so that people know which ones are available for which
purposes.  It is getting awfully crowded in that space. Sadly, I don't
think we can portably use numbers higher than 9 (bash is happy to, but
even dash cannot).

Of course people would have to know to look for said comment, which they
may not do. :)

-Peff