Adapt test2 api

[magnolia-k]

It was rewritten according to the latest Test 2 API.

However, some tests fail with messages output by Test2::API::Breakage.

[exodist]

I have not studied this pr for long, but I do have some comments.

For the Test2 implementation you still seem to monkeypatch Test::Builder. This will probably result in inconsistent results, and will likely break for any events that come from tools that bypass Test::Builder. With Test2 there is no requirements that Test::Builder ever enter the picture.

This is a huge step in the right direction, but I think you need to fully divorce the Test2 implementation of Test::Pretty from Test::Builder.

[magnolia-k]

Thanks for your comment.

The original Test::Pretty converts the hierarchized subtest into a flat tests and executes it (ignoring the subtest) when enabling the environment variable HARNESS_ACTIVE.

When trying to reproduce this behavior, at present, no method was found except to do monky patch to Test::Builder.

I think that this problem can not be solved by Test2 API, but is there any idea?

[exodist]

What is the intent of flattening the subtest? Is it flattened for aesthetic purposes, or is it flattened for a functional purpose? The answer to this question is important in determining how to proceed.

[exodist]

I have been thinking more about this. Right now I am thinking of 3 primary "problems" that Test::Pretty solves, please let me know if there are additional ones I have missed:

• Flattening subtests
• Injecting source line as a diagnostics on failed tests
• Pretty non-tap formatter when not inside a harness

Let me address each:

Flattening Subtests

Monkeypatching is asking for trouble, you will end up with fragile code that breaks whenever Test2 changes things. In addition there are now 3 subtest implementations, you would need to monkeypatch all 3. There is a better way.

Instead of monkeypatching, simply have Test::Pretty export its own subtest($name, $code) function that replaces the one Test::More exports.

sub import {
    my $caller = caller;

    {
        no warnings 'redefine';
        no strict 'refs';
        *{"$caller\::subtest"} = \&subtest;
    }
}

sub subtest { ... }

This would make all subtests inside the file use Test::Pretty's implementation instead of whatever one they previously loaded. This has the caveat of not replacing subtests generated by external tools, but I suspect those are a less common case. Also many tools that use subtests make assumptions about their independent state, some even make their own plans, monkeypatching breaks those assumptions, so this is a safer idea that still probably gets most of the ones you will see.

Injecting source line as a diagnostics on failed tests

Once again your patch turns to monkeypatching, which is fragile and likely to break, and will miss many sources of events. A better option is to make this a context callback in a plugin. Here is an example: https://metacpan.org/source/EXODIST/Test2-Suite-0.000067/lib/Test2/Plugin/DieOnFail.pm This plugin will throw an exception after a failed test (but waits for all the diagnostics to be output).

You can copy the DieOnFail code to write your own plugin that sends an additional diagnostics message containing the line of source that generated the failure. This plugin dies once, when the test goes from passing to failing, your plugin will instead need to check if the number of failures has increased.

Another option to accomplish this is to add a filter on the hub: https://metacpan.org/pod/Test2::Hub#ALTERING-OR-REMOVING-EVENTS use this to add a second event (A diag) with the source line for any event where $e->causes_fail is true. Or if you want to inject the source line even on passing tests you can add the diag/note for any event where $e->increments_count is true.

Pretty non-tap formatter when not inside a harness

You should not be copying/subclassing the TAP or Builder/Formatter formatters. These formatters have a lot of performance optimizations that are likely to fumble, these formatters are slated for replacement fairly soon. You should probably write your formatter to be independant, and it should probably make sure of the event API instead of looking at the event type (where possible):

https://metacpan.org/pod/Test2::Event#METHODS

Currently the Test2 team is working on extending the event API, so soon this list of methods will grow and you will have more to work with. In that change we will also be relying much less on event type (IE Test2::Event::Ok will not be the only event that should produce an OK line, etc). You can see my experimental branch on this here: https://github.com/Test-More/test-more/tree/new_event_api if you look the branch over you would quickly see that the changes would break this PR. However if you follow my recommendations above then your code would be resilient and not be effected by the change.

[magnolia-k]

Regarding the place where Monkey patch is applied to Test::Builder (the place where subtest is flattened), I confirmed it with @tokuhirom tokuhirom and found it to be unnecessary route so I decided to delete it.

For the rest of the comments, I need to redesign the design, so I'll think over it.