test-lib: silence "-x" cleanup under bash

When the test suite's "-x" option is used with bash, we end up seeing cleanup cruft in the output: $ bash t0001-init.sh -x [...] ++ diff -u expected actual + test_eval_ret_=0 + want_trace + test t = t + test t = t + set +x ok 42 - re-init from a linked worktree This ranges from mildly annoying (for a successful test) to downright confusing (when we say "last command exited with error", but it's really 5 commands back). We normally are able to suppress this cleanup. As the in-code comment explains, we can't convince the shell not to print it, but we can redirect its stderr elsewhere. But since d88785e424 (test-lib: set BASH_XTRACEFD automatically, 2016-05-11), that doesn't hold for bash. It sends the "set -x" output directly to descriptor 4, not to stderr. We can fix this by also redirecting descriptor 4, and paying close attention to which commands redirected and which are not (see the updated comment). Two alternatives I considered and rejected: - unsetting and setting BASH_XTRACEFD; doing so closes the descriptor, which we must avoid - we could keep everything in a single block as before, redirect 4>/dev/null there, but retain 5>&4 as a copy. And then selectively restore 4>&5 for commands which should be allowed to trace. This would work, but the descriptor swapping seems unnecessarily confusing. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 8, 2017 at 05:47 UTC 90c8a1db9d6e8f7d2e3e842142ba98f827d42800
1 file changed +24 -10
t/test-lib.sh
+24 -10
@@ -601,26 +601,40 @@ test_eval_inner_ () {
601 }
602
603 test_eval_ () {
604 - # We run this block with stderr redirected to avoid extra cruft
605 - # during a "-x" trace. Once in "set -x" mode, we cannot prevent
604 + # If "-x" tracing is in effect, then we want to avoid polluting stderr
605 + # with non-test commands. But once in "set -x" mode, we cannot prevent
606 # the shell from printing the "set +x" to turn it off (nor the saving
607 # of $? before that). But we can make sure that the output goes to
608 # /dev/null.
609 #
610 - # The test itself is run with stderr put back to &4 (so either to
611 - # /dev/null, or to the original stderr if --verbose was used).
610 + # There are a few subtleties here:
611 + #
612 + # - we have to redirect descriptor 4 in addition to 2, to cover
613 + # BASH_XTRACEFD
614 + #
615 + # - the actual eval has to come before the redirection block (since
616 + # it needs to see descriptor 4 to set up its stderr)
617 + #
618 + # - likewise, any error message we print must be outside the block to
619 + # access descriptor 4
620 + #
621 + # - checking $? has to come immediately after the eval, but it must
622 + # be _inside_ the block to avoid polluting the "set -x" output
623 + #
624 +
625 + test_eval_inner_ "$@" </dev/null >&3 2>&4
626 {
613 - test_eval_inner_ "$@" </dev/null >&3 2>&4
627 test_eval_ret_=$?
628 if want_trace
629 then
630 set +x
618 - if test "$test_eval_ret_" != 0
619 - then
620 - say_color error >&4 "error: last command exited with \$?=$test_eval_ret_"
621 - fi
631 fi
623 - } 2>/dev/null
632 + } 2>/dev/null 4>&2
633 +
634 + if test "$test_eval_ret_" != 0 && want_trace
635 + then
636 + say_color error >&4 "error: last command exited with \$?=$test_eval_ret_"
637 + fi
638 return $test_eval_ret_
639 }
640