On Fri, Oct 21, 2016 at 09:15:28AM -0700, Jacob Keller wrote:
> >> > 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@xxxxxxxx>
---
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