Re: [PATCH/RFC] test-lib: make --verbose work under prove

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Mar 01 2021, Jeff King wrote:

> On Sun, Feb 28, 2021 at 09:25:06PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> As explained in 452320f1f5 (test-lib: add --verbose-log option,
>> 2016-10-21) combining the "--verbose" option with the TAP-parsing
>> "prove" utility would result in broken tests.
>> 
>> This is because "prove" expects valid TAP on stdout, and if there's a
>> stray line starting with "ok", or other invalid TAP syntax it will
>> error on the test.
>> 
>> In 452320f1f5 numerous potential methods are discussed for dealing
>> with this problem, but not the one implemented here, which I think is
>> a much simpler approach.
>
> I guess I don't really have an objection to this method existing, but I
> don't see the point of it now. The original problem has been solved, and
> I don't find this any simpler than --verbose-log (on the contrary, I
> find it much more complicated). So I don't know why I would want to use
> this instead.

I guess that's the problem with picking something from the middle of a
local WIP series I have, I forgot to state the end goal of the whole
thing.

The point is to be able to run "prove -v <test> :: --verbose" and see
the verbose output. See the commit message for when TAP was added for
examples: 5099b99d25 (test-lib: Adjust output to be valid TAP format,
2010-06-24)[1].

Because it's valid TAP with this patch you can easily parse the output:

    HARNESS_ACTIVE=t ./t5547-push-quarantine.sh --verbose -x >t5547-push-quarantine.sh.tap
    perl -MTAP::Parser -MYAML::XS -wE '
        my $parser = TAP::Parser->new( { source => "t5547-push-quarantine.sh.tap" } );
        my @lines;
        while ( my $result = $parser->next ) {
            push @lines => $result;
        };
        say Dump \@lines
    ' >t5547-push-quarantine.sh.tap.parsed

Gives you a stream like:

    [...]
    - !!perl/hash:TAP::Parser::Result::Unknown
      raw: + chmod +x dest.git/hooks/pre-receive
      type: unknown
    - !!perl/hash:TAP::Parser::Result::Unknown
      raw: ''
      type: unknown
    - !!perl/hash:TAP::Parser::Result::Test
      description: '- create picky dest repo'
      directive: ''
      explanation: ''
      ok: ok
      raw: ok 1 - create picky dest repo
      test_num: 1
      type: test
    - !!perl/hash:TAP::Parser::Result::Unknown
      raw: ''
      type: unknown
    - !!perl/hash:TAP::Parser::Result::Unknown
      raw: 'expecting success of 5547.2 ''accepted objects work'': '
      type: unknown
    - !!perl/hash:TAP::Parser::Result::Unknown
      raw: "\ttest_commit ok &&"
      type: unknown
    - !!perl/hash:TAP::Parser::Result::Unknown
      raw: "\tgit push dest.git HEAD &&"
    [...]

The eventual goal was to make ci/print-test-failures.sh and the code
that cares about t/test-results/* capable of working with output like
that.

So e.g. you could optionally get summarized output about just the
specific tests that failed, not all the output for failing tests, as
ci/print-test-failures.sh is spewing out now.

I also wanted to have something similar to Documentation/doc-diff but
for smart test diffing. E.g. now we have an rc0 out, and the set of
tests broken on various test boxes I have access to is different than in
the last release.

But it's a pain to summarize that output currently (or at least I've
always written one-off tools for it).

By being able to parse the tap it's easier to massage that output and
e.g. spew it into a database, or into two text files friendly to "git
diff --no-index".

But yes, the implementation isn't any simpler, it's more complex than
just a "tee -a", but I think it's worth it for machine-readable verbose
test output.

1. As an aside this is a bit tricker/broken in the prove utility than
   when I wrote that code initially, sometimes it won't show you verbose
   output:
   https://github.com/Perl-Toolchain-Gang/Test-Harness/issues/105