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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 1, 2021, 13:54 UTC
Message-ID: <87r1kzj7xi.fsf@evledraar.gmail.com>
In-Reply-To: <YDy5LDsnF1THmvUw@coredump.intra.peff.net>

On Mon, Mar 01 2021, Jeff King wrote:

Show 19 quoted lines

> 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