Re: [PATCH 4/3] test-lib: bail out when "-v" used under "prove"

From: Jacob Keller <jacob.keller@gmail.com>
Date: Oct 22, 2016, 05:25 UTC
Message-ID: <CA+P7+xqwLiwSWAwsreW=nCMU5QiNq4AUGPMawzLofB3vyAqYYw@mail.gmail.com>
In-Reply-To: <20161022044506.vba6g2q25yxa2air@sigill.intra.peff.net>

On Fri, Oct 21, 2016 at 9:45 PM, Jeff King <peff@peff.net> wrote:

> 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.

Always seems to be that way, doesn't it?

Show 8 quoted lines

> 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).
>

Nice.

Show 32 quoted lines

> -- >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
>

Nice that makes sense.

Show 23 quoted lines

> 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
> +

Not too much code, so that's good. I like it.

Thanks, Jake

Show 6 quoted lines

>  test "${test_description}" != "" ||
>  error "Test script did not set test_description."
>
> --
> 2.10.1.776.ge0e381e
>