Ovid / test--most

Test::Most -- The most commonly needed testing functionality in Perl
9 stars 14 forks source link

ensure Test::Builder's original DESTROY is still called #10

Closed karenetheridge closed 4 years ago

karenetheridge commented 9 years ago

This is very important for cleaning up after exceptions that occurred within subtests.

I'm not sure how to test this with your current framework, but you can see the difference in outcome here:

use strict;
use warnings;
use Test::More tests => 1;
#use Test::Most;

subtest foo => sub {
 is( die('oh noes'), 1, 'this is going to blow up');
};

without Test::Most, this prints:

1..1
    # Subtest: foo
oh noes at subtest.t line 7.
    # Child (foo) exited without calling finalize()
not ok 1 - foo
#   Failed test 'foo'
#   at /Users/ether/.perlbrew/libs/21.5@std/lib/perl5/Test/Builder.pm line 279.
# Looks like you failed 1 test of 1.
# Looks like your test exited with 255 just after 1.

and with Test::Most:

1..1
    # Subtest: foo
oh noes at subtest.t line 7.
# Looks like your test exited with 255 before it could output anything.
exodist commented 9 years ago

+1

karenetheridge commented 9 years ago

ping

barefootcoder commented 6 years ago

@Ovid, what do you need to move forward on this? Does it just need a failing test case and a unit test, perhaps?

Ovid commented 4 years ago

Oy! I had totally forgotten about this.

This is possibly the longest I've ever taken to merge a request. It's on its way to the CPAN now as 0.36. Thank you, @karenetheridge and everyone else.

Ovid commented 4 years ago

For what it's worth, this patch is causing the following warnings:

t/00-load.t ..................... 1/9 # Testing Test::Most 0.36, Perl 5.026002, /Users/ovid/perl5/perlbrew/perls/perl-5.26.2/bin/perl
#     Exception::Class version is 1.44
#     Test::Deep version is 1.128
#     Test::Differences version is 0.67
#     Test::Exception version is 0.43
#     Test::Harness version is 3.38
#     Test::More version is 1.302162
#     Test::Warn version is 0.36
    (in cleanup)    (in cleanup)  at /Users/ovid/Dropbox/Mine/projects/perl/test-most/lib/Test/Most.pm line 290 during global destruction.
t/00-load.t ..................... ok
t/bug_duplicate_plan.t .......... 1/?   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/bug_duplicate_plan.t .......... ok
t/common.t ...................... 1/6   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/common.t ...................... ok
t/defer_plan.t .................. 1/?   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/defer_plan.t .................. ok
t/env_bail.t .................... 1/7   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/env_bail.t .................... ok
t/env_die.t ..................... 1/7   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/env_die.t ..................... ok
t/exception.t ................... 1/4   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/exception.t ................... ok
t/exclude_test_modules.t ........ 1/4   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/exclude_test_modules.t ........ ok
t/exclude_test_symbols.t ........ 1/5   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/exclude_test_symbols.t ........ ok
t/explain.t ..................... 1/8   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/explain.t ..................... ok
t/function_bail.t ............... 1/?   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/function_bail.t ............... ok
t/function_die.t ................ 1/7   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/function_die.t ................ ok
t/import_bail.t ................. 1/7   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/import_bail.t ................. ok
t/import_die.t .................. 1/7   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/import_die.t .................. ok
t/import_strict_and_warnings.t .. 1/2   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/import_strict_and_warnings.t .. ok
t/timex.t ....................... 1/?   (in cleanup)    (in cleanup)  at lib/Test/Most.pm line 290 during global destruction.
t/timex.t ....................... ok
All tests successful.
Files=16, Tests=88,  1 wallclock secs ( 0.04 usr  0.02 sys +  1.14 cusr  0.18 csys =  1.38 CPU)
Result: PASS

It's the final $orig_destroy->(@_); which causes the warning. Putting a breakpoint there in the debugger doesn't seem to work, so my goto debugging tool is useless here (presumably because this is global destruction).

karenetheridge commented 4 years ago

It looks like Test::Builder::DESTROY doesn't exist anymore in modern Test::More (i.e. Test2::*). So, making this change will fix up the warning:

-        $orig_destroy->(@_);
+        $orig_destroy->(@_) if $orig_destroy;
Ovid commented 4 years ago

Thanks, @karenetheridge . Fixed and uploaded as 0.37!