New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manage a distinct buffer for diagnostic messages. #14

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@merrilymeredith
Contributor

merrilymeredith commented Jan 29, 2015

Hi, one more PR for you here.

I accidentally branched this off my other changes so I had to get back around to rebasing it off of master so the PR would stay clean. This one probably needs a little more consideration than the others.

If you check out Issue #11, other than the misleading deprecation messages, there isn't actually any output indicating which tests failed or how. I took a look at the TAP spec, which does provide for in-band diagnostic output, then Test::Harness as well as the behavior of Test::More for precedent before working on this.

The changes below add an accessor named diag_buffer that lazily checks whether or not the test harness indicates it is in verbose mode -- if it is, we keep everything going to $self->{buffer}, as it will all be displayed in addition to the status of every test, and if it is not verbose, we open up STDERR so test output is concise but which tests failed and why can still be seen.

Thanks for your time!

@pvande

This comment has been minimized.

Show comment
Hide comment
@pvande

pvande Feb 5, 2015

Owner

My biggest concern with this would be that, architecturally, we're adding code to the top-level Logger class that is exclusively in support of a single subclass (this is the other logger I'm aware of). The diag method is definitely specific to the TAP logger, and the HARNESS_VERBOSE environment variable is specific to TAP-testing tools. I would also argue that the diag_buffer behavior of diverting diag output is also specific to the TAP logger...

Having said that, there's definitely a need to improve the TAP logger; while it was provided as a default, it was never actually a first-class citizen (since I never used it myself), and it didn't integrate with the existing Perl testing ecosystem nearly as well as it ought have. I'm glad to see someone taking an interest in bringing it up-to-standard.

Owner

pvande commented Feb 5, 2015

My biggest concern with this would be that, architecturally, we're adding code to the top-level Logger class that is exclusively in support of a single subclass (this is the other logger I'm aware of). The diag method is definitely specific to the TAP logger, and the HARNESS_VERBOSE environment variable is specific to TAP-testing tools. I would also argue that the diag_buffer behavior of diverting diag output is also specific to the TAP logger...

Having said that, there's definitely a need to improve the TAP logger; while it was provided as a default, it was never actually a first-class citizen (since I never used it myself), and it didn't integrate with the existing Perl testing ecosystem nearly as well as it ought have. I'm glad to see someone taking an interest in bringing it up-to-standard.

merrilymeredith added some commits Jan 11, 2015

Manage diagnostic output as a distinct buffer.
diag_buffer may be the same as the TAP output buffer, but this keeps the
harness from filtering which test actually failed when not in verbose
mode.
@merrilymeredith

This comment has been minimized.

Show comment
Hide comment
@merrilymeredith

merrilymeredith Apr 4, 2015

Contributor

Hi, and sorry for the delay -- it's been a heck of a year for me so far! It actually hadn't occurred to me that there were other backends, much less that they might not need a diag channel like that. I've updated the PR, and I'll be sure to check out ::Lapidary too.

Contributor

merrilymeredith commented Apr 4, 2015

Hi, and sorry for the delay -- it's been a heck of a year for me so far! It actually hadn't occurred to me that there were other backends, much less that they might not need a diag channel like that. I've updated the PR, and I'll be sure to check out ::Lapidary too.

@pvande

This comment has been minimized.

Show comment
Hide comment
@pvande

pvande Apr 4, 2015

Owner

Open question: does the IO::Handle package work and exist in Perl 5.8?

Owner

pvande commented Apr 4, 2015

Open question: does the IO::Handle package work and exist in Perl 5.8?

@merrilymeredith

This comment has been minimized.

Show comment
Hide comment
@merrilymeredith

merrilymeredith Apr 4, 2015

Contributor

It seems to be; I assumed it was since IO::Scalar was already use.

I built a copy of Perl 5.8.9 and came up with this:

❯ make test
cp lib/Test/Mini/Logger.pm blib/lib/Test/Mini/Logger.pm
Skip blib/lib/Test/README.pod (unchanged)
Skip blib/lib/Test/Mini/TestCase.pm (unchanged)
Skip blib/lib/Test/Mini/Runner.pm (unchanged)
Skip blib/lib/Test/Mini.pm (unchanged)
cp lib/Test/Mini/Logger/TAP.pm blib/lib/Test/Mini/Logger/TAP.pm
Skip blib/lib/Test/Mini/Assertions.pm (unchanged)
PERL_DL_NONLAZY=1 "/home/mhoward/.plenv/versions/5.8.9/bin/perl5.8.9" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/*/*.t t/*/*/*.t t/*/*/*/*.t t/*/*/*/*/*.t
t/01-Test-Mini.t .......... 1/15 
#   Failed test 'Exit code'
#   at t/01-Test-Mini.t line 112.
#          got: '1'
#     expected: '0'
# Looks like you failed 1 test of 15.
t/01-Test-Mini.t .......... Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/15 subtests 
t/Test/Mini/Assertions.t .. 1/? # Test Case: t::Test::Mini::Assertions
t/Test/Mini/Assertions.t .. ok    
t/Test/Mini/Logger.t ...... 1/? # Test Case: t::Test::Mini::Logger
t/Test/Mini/Logger.t ...... ok   
t/Test/Mini/Logger/TAP.t .. 1/? # Test Case: t::Test::Mini::Logger::TAP
t/Test/Mini/Logger/TAP.t .. ok    

Test Summary Report
-------------------
t/01-Test-Mini.t        (Wstat: 256 Tests: 15 Failed: 1)
  Failed test:  13
  Non-zero exit status: 1
Files=4, Tests=65,  3 wallclock secs ( 0.10 usr  0.08 sys +  1.62 cusr  0.26 csys =  2.06 CPU)
Result: FAIL
Failed 1/4 test programs. 1/65 subtests failed.
make: *** [test_dynamic] Error 255

❯ git checkout master
Switched to branch 'master'

❯ make test
cp lib/Test/Mini/Logger.pm blib/lib/Test/Mini/Logger.pm
cp README.pod blib/lib/Test/README.pod
cp lib/Test/Mini/TestCase.pm blib/lib/Test/Mini/TestCase.pm
cp lib/Test/Mini/Runner.pm blib/lib/Test/Mini/Runner.pm
cp lib/Test/Mini.pm blib/lib/Test/Mini.pm
cp lib/Test/Mini/Logger/TAP.pm blib/lib/Test/Mini/Logger/TAP.pm
cp lib/Test/Mini/Assertions.pm blib/lib/Test/Mini/Assertions.pm
PERL_DL_NONLAZY=1 "/home/mhoward/.plenv/versions/5.8.9/bin/perl5.8.9" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/*/*.t t/*/*/*.t t/*/*/*/*.t t/*/*/*/*/*.t
t/01-Test-Mini.t .......... 1/15 
#   Failed test 'Exit code'
#   at t/01-Test-Mini.t line 112.
#          got: '1'
#     expected: '0'
# Looks like you failed 1 test of 15.
t/01-Test-Mini.t .......... Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/15 subtests 
t/Test/Mini/Assertions.t .. ok    
t/Test/Mini/Logger.t ...... ok   
t/Test/Mini/Logger/TAP.t .. ok    

Test Summary Report
-------------------
t/01-Test-Mini.t        (Wstat: 256 Tests: 15 Failed: 1)
  Failed test:  13
  Non-zero exit status: 1
Files=4, Tests=64,  3 wallclock secs ( 0.10 usr  0.09 sys +  1.54 cusr  0.30 csys =  2.03 CPU)
Result: FAIL
Failed 1/4 test programs. 1/64 subtests failed.
make: *** [test_dynamic] Error 255
Contributor

merrilymeredith commented Apr 4, 2015

It seems to be; I assumed it was since IO::Scalar was already use.

I built a copy of Perl 5.8.9 and came up with this:

❯ make test
cp lib/Test/Mini/Logger.pm blib/lib/Test/Mini/Logger.pm
Skip blib/lib/Test/README.pod (unchanged)
Skip blib/lib/Test/Mini/TestCase.pm (unchanged)
Skip blib/lib/Test/Mini/Runner.pm (unchanged)
Skip blib/lib/Test/Mini.pm (unchanged)
cp lib/Test/Mini/Logger/TAP.pm blib/lib/Test/Mini/Logger/TAP.pm
Skip blib/lib/Test/Mini/Assertions.pm (unchanged)
PERL_DL_NONLAZY=1 "/home/mhoward/.plenv/versions/5.8.9/bin/perl5.8.9" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/*/*.t t/*/*/*.t t/*/*/*/*.t t/*/*/*/*/*.t
t/01-Test-Mini.t .......... 1/15 
#   Failed test 'Exit code'
#   at t/01-Test-Mini.t line 112.
#          got: '1'
#     expected: '0'
# Looks like you failed 1 test of 15.
t/01-Test-Mini.t .......... Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/15 subtests 
t/Test/Mini/Assertions.t .. 1/? # Test Case: t::Test::Mini::Assertions
t/Test/Mini/Assertions.t .. ok    
t/Test/Mini/Logger.t ...... 1/? # Test Case: t::Test::Mini::Logger
t/Test/Mini/Logger.t ...... ok   
t/Test/Mini/Logger/TAP.t .. 1/? # Test Case: t::Test::Mini::Logger::TAP
t/Test/Mini/Logger/TAP.t .. ok    

Test Summary Report
-------------------
t/01-Test-Mini.t        (Wstat: 256 Tests: 15 Failed: 1)
  Failed test:  13
  Non-zero exit status: 1
Files=4, Tests=65,  3 wallclock secs ( 0.10 usr  0.08 sys +  1.62 cusr  0.26 csys =  2.06 CPU)
Result: FAIL
Failed 1/4 test programs. 1/65 subtests failed.
make: *** [test_dynamic] Error 255

❯ git checkout master
Switched to branch 'master'

❯ make test
cp lib/Test/Mini/Logger.pm blib/lib/Test/Mini/Logger.pm
cp README.pod blib/lib/Test/README.pod
cp lib/Test/Mini/TestCase.pm blib/lib/Test/Mini/TestCase.pm
cp lib/Test/Mini/Runner.pm blib/lib/Test/Mini/Runner.pm
cp lib/Test/Mini.pm blib/lib/Test/Mini.pm
cp lib/Test/Mini/Logger/TAP.pm blib/lib/Test/Mini/Logger/TAP.pm
cp lib/Test/Mini/Assertions.pm blib/lib/Test/Mini/Assertions.pm
PERL_DL_NONLAZY=1 "/home/mhoward/.plenv/versions/5.8.9/bin/perl5.8.9" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/*/*.t t/*/*/*.t t/*/*/*/*.t t/*/*/*/*/*.t
t/01-Test-Mini.t .......... 1/15 
#   Failed test 'Exit code'
#   at t/01-Test-Mini.t line 112.
#          got: '1'
#     expected: '0'
# Looks like you failed 1 test of 15.
t/01-Test-Mini.t .......... Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/15 subtests 
t/Test/Mini/Assertions.t .. ok    
t/Test/Mini/Logger.t ...... ok   
t/Test/Mini/Logger/TAP.t .. ok    

Test Summary Report
-------------------
t/01-Test-Mini.t        (Wstat: 256 Tests: 15 Failed: 1)
  Failed test:  13
  Non-zero exit status: 1
Files=4, Tests=64,  3 wallclock secs ( 0.10 usr  0.09 sys +  1.54 cusr  0.30 csys =  2.03 CPU)
Result: FAIL
Failed 1/4 test programs. 1/64 subtests failed.
make: *** [test_dynamic] Error 255
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment