Implemented new Test::* paradigm

lejeunerenard commented 9 years ago

I rewrote Test::Pretty to use the new Test::Stream API in Test::More's alphas. So far all tests pass with Test::More 1.301001_084. Note that the rewrite is not backwards compatible. If it needs to be, let me know and I'll update the PR.

The code has been structured so that all events emitted by a stream are processed via stream_listener(). This way tests can be printed recursively (e.g. subtests). stream_listener() adds "pretty" output for 3 different events:

subtest events are printed as "pretty" ok()s if subtests are not set to 'delayed'. Otherwise they print their name and then recurse through their child tests. They do not end with an ok() anymore.
ok events are printed same as before.
plan events are printed if they are a SKIP. The rest are ignored.

Everything else is delegated to it's default rendering.

Closes #25

Notes:

Added TEST_PRETTY_INDENT env variable to set indention
subtest names are no longer half indented.
To differentiate between a "pretty" ok and the dummy tap, I created a new event called Test::Stream::Event::DummyTap which outputs the same dummy tap as before. This utilizes the same filehandle as Test::More instead of directly writing to STDOUT.
Removed code that set the exit code so that Test::More can do that for us.
Todo items inside a subtest do not know they are in todo given the context is hidden by Test::Stream.

tokuhirom commented 9 years ago

Great work!! Hm. Backward compatibility is not a critical issue for this module.

And so, test case was fafiled XD

exodist commented 9 years ago

Overall this is definitely a step forward, good job studying Test::Stream. Just yesterday I added a module that may be able to reduce your code a bit: https://metacpan.org/pod/release/EXODIST/Test-Simple-1.301001_086/lib/Test/Stream/API.pm but it is not a requirement, what you have should work fine as-is.

I have no objections to the use of the Test::Stream::Event::* namespace, it is open for additions in other dists.

I notice that the module is not currently tested, I recommend using Test::Stream::Tester to test this package.

Thanks for being the first non-me consumer of these enhancement, please do report any suggestions or pain-points!

exodist commented 9 years ago

looking at the current lib/Test/Pretty.pm as ti would be after this merge, it seems you did leave some old code laying around, stuff that modified Test::Builder. I suspect this is where the failure lies. I have not studied the module to any depth, but at a glance that is the only issue I noted.

exodist commented 9 years ago

https://github.com/Test-More/test-more/commit/0850e52eb74e4049bd0cfc36d29b5638b321e73b

Noting this here as it may effect you. Follow-ups had a bug that caused them to run at the end of every subtest (oops) and also had a bug when they ran without using done_testing. This commit fixes both, I will release later tonight (088/rc8) assuming my downstream tests all pass.

lejeunerenard commented 9 years ago

@exodist thanks for all the tips. I will definitely look into the Follow-up changes and write tests for Test::Stream::Event::DummyTap.

As for the old code I left in the module, I don't know why its there. I understand it's for use with Test::Harness, but as I don't know anything about Test::Harness, I decided to leave it as is. If @tokuhirom could explain why he added it, I can keep its original functionality. If not, I will take a stab trying to understand why its necessary/unnecessary. It monkey patches Test::Builder so if I do keep it, I'll make sure it uses the new methods.

I have never used Travis CI but it seems the failures are because its using Test::More 0.98.

@tokuhirom, in the future, I'd like to make this backward compatible. But for now it will be bleeding edge. :)

@exodist how would you like me to submit suggestions? Via issues/PRs?

Hopefully I will update this PR by the end of tomorrow.

exodist commented 9 years ago

@lejeunerenard, Issues and pull requests are perfect.

lejeunerenard commented 9 years ago

Updated the code with suggested changes.

Summary:

Wrote tests for Test::Stream::Event::DummyTap
Adjusted for changes to subtests

zoffixznet commented 9 years ago

Hey all. I got this dist assigned to me as part of the CPAN PR Challenge.

This seems to be a large PR. Is there any way to have it merged before I work through the open Issues?

Travis failure on this PR is due to too low version of the prereq specified.

Let me know. Thanks!

lejeunerenard commented 9 years ago

@zoffixznet I believe @tokuhirom hasn't merged the PR because it relies on an alpha version of the Test::More API. I am not sure when the new API is going to be released, but it makes sense to me to not merge this PR into master until then. The PR breaks backwards compatibility.

If you make any changes to master that can be applied to this PR as well, I'd be willing to help integrate them so it's not lost if this PR is merged.

zoffixznet commented 9 years ago

Sounds good!

preaction commented 8 years ago

Is this ready to go? The Test2 module is out, and #36 and #37 are reported failures to install Test::Pretty. Is there anything I can do to help get this out the door?

tokuhirom commented 8 years ago

hmm the test suite is still failing.

tokuhirom commented 8 years ago

Travis says following comment. Did you forgot to add the deps?

Test/Stream.pm in @INC (@INC contains: /home/travis/build/tokuhirom/Test-Pretty/blib/lib /home/travis/build/tokuhirom/Test-Pretty/blib/arch /home/travis/build/tokuhirom/Test-Pretty/_build/lib /home/travis/perl5/perlbrew/perls/5.16/lib/site_perl/5.16.3/x86_64-linux /home/travis/perl5/perlbrew/perls/5.16/lib/site_perl/5.16.3 /home/travis/perl5/perlbrew/perls/5.16/lib/5.16.3/x86_64-linux /home/travis/perl5/perlbrew/perls/5.16/lib/5.16.3 .) at /home/travis/build/tokuhirom/Test-Pretty/blib/lib/Test/Pretty.pm

exodist commented 8 years ago

You need to use Test2::API not Test::Stream. On Jun 17, 2016 5:15 PM, "Tokuhiro Matsuno" notifications@github.com wrote:

Travis says following comment. Did you forgot to add the deps?

Test/Stream.pm in @INC (@INC contains: /home/travis/build/tokuhirom/Test-Pretty/blib/lib /home/travis/build/tokuhirom/Test-Pretty/blib/arch /home/travis/build/tokuhirom/Test-Pretty/_build/lib /home/travis/perl5/perlbrew/perls/5.16/lib/site_perl/5.16.3/x86_64-linux /home/travis/perl5/perlbrew/perls/5.16/lib/site_perl/5.16.3 /home/travis/perl5/perlbrew/perls/5.16/lib/5.16.3/x86_64-linux /home/travis/perl5/perlbrew/perls/5.16/lib/5.16.3 .) at /home/travis/build/tokuhirom/Test-Pretty/blib/lib/Test/Pretty.pm

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tokuhirom/Test-Pretty/pull/28#issuecomment-226909066, or mute the thread https://github.com/notifications/unsubscribe/AAEgumKDx-JzKvSuQyUD67GiyqSZwGepks5qMziFgaJpZM4DHho4 .

preaction commented 8 years ago

I can take a shot at making that change unless someone with more experience has some time

lejeunerenard commented 8 years ago

@preaction feel free to tackle this. I haven't touched Test2 since it was Test::Stream.

karupanerura commented 7 years ago

How is the progress?

tokuhirom commented 7 years ago

@exodist how do i use Test2::API?

exodist commented 7 years ago

I think at this point the best thing to do is write a new formatter. Look at https://metacpan.org/source/EXODIST/Test-Simple-1.302067/lib/Test2/Formatter/TAP.pm as an example implementation.

All you, or anyone else would have to do is load Test2::Formatter::YourFormatter, and it would do the job.

The formatter receives events. Each event will implement the following methods to help you decide what to display (Taken from https://metacpan.org/source/EXODIST/Test-Simple-1.302067/lib/Test2/Event.pm)

sub causes_fail      { return $bool }       # not ok
sub increments_count { return $bool } # Bumps the test count
sub diagnostics      { return $bool }      # Is a diagnostics message (STDERR)
sub no_display       { return $bool }      # Should not be displayed by most formatters

# Plan details: $max is the count, #directive is something like 'skip all' if set, and 'reason' is the reason to skip all
sub sets_plan { return ($max, $directive, $reason) }

# Basically an $event->to_string that a formatter can use to decide what to show, for diag events this is the diagnostics message.
sub summary { return "display this" }

So you just write a formatter that has a write($event, $number) method, that prints the right thing to STDERR/STDOUT when the event comes in. Loading the formatter is usually enough to make it be THE formatter.

magnolia-k commented 7 years ago

rewrote according to Test2::Formatter's API.

https://github.com/tokuhirom/Test-Pretty/pull/38

tokuhirom / Test-Pretty

Implemented new Test::* paradigm #28