[PATCH 4/3] test-lib: bail out when "-v" used under "prove"
- From
Jeff King <peff@peff.net>
- Date
- Oct 22, 2016, 04:45 UTC
- Message-ID
- <20161022044506.vba6g2q25yxa2air@sigill.intra.peff.net>
- In-Reply-To
- <CA+P7+xp8TDL59tQgCpmMDJ-BspA1CF6YsnuGMXas1bx_X0qJoA@mail.gmail.com>
On Fri, Oct 21, 2016 at 09:15:28AM -0700, Jacob Keller wrote:
Show 12 quoted lines
> >> > For $HARNESS_ACTIVE with _just_ "--verbose", I don't think it would be a > >> > good idea to activate it. We should either silently ignore --verbose > >> > then, or complain and die. > >> > >> We should probably do that to make sure people realize what might > >> happen. I read your series and it has a good explanation of the > >> possible alternatives. I like the approach you chose. > > > > Thanks. Do you want to make a patch on top of my series? > > I am not sure I will find time to do it today, so it wouldn't be for a > few more days.
I thought I'd just knock this out in 5 minutes before I forgot about it. But as with so many things, getting it right proved slightly harder than I thought. But I did learn about TAP's "Bail out!" directive. And apparently you can pass it back arbitrary YAML (!). And the "--verbose" output really is violating the spec, and they claim that Test::Harness will eventually be tightened to complain (though that was in 2007, and it still hasn't happened, so...).
Anyway. Here is the patch I came up with (on top of the others).
-- >8 -- Subject: test-lib: bail out when "-v" used under "prove"
When there is a TAP harness consuming the output of our test scripts, the "--verbose" breaks the output by mingling test command output with TAP. Because the TAP::Harness module used by "prove" is fairly lenient, this _usually_ works, but it violates the spec, and things get very confusing if the commands happen to output a line that looks like TAP (e.g., the word "ok" on its own line).
Let's detect this situation and complain. Just calling error() isn't great, though; prove will tell us that the script failed, but the message doesn't make it through to the user. Instead, we can use the special TAP signal "Bail out!". This not only shows the message to the user, but instructs the harness to stop running the tests entirely. This is exactly what we want here, as the problem is in the command-line options, and every test script would produce the same error.
The result looks like this (the first "Bailout called" line is in red if prove uses color on your terminal):
$ make GIT_TEST_OPTS='--verbose --tee' rm -f -r 'test-results' *** prove *** Bailout called. Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log FAILED--Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log Makefile:39: recipe for target 'prove' failed make: *** [prove] Error 255
Signed-off-by: Jeff King <peff@peff.net> --- t/test-lib.sh | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/t/test-lib.sh b/t/test-lib.sh index 85946ec40d..b859db61ac 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -321,6 +321,16 @@ say () { say_color info "$*" } +if test -n "$HARNESS_ACTIVE" +then + if test "$verbose" = t || test -n "$verbose_only" + then + printf 'Bail out! %s\n' \ + 'verbose mode forbidden under TAP harness; try --verbose-log' + exit 1 + fi +fi + test "${test_description}" != "" || error "Test script did not set test_description."
-- 2.10.1.776.ge0e381e