Return value of subtest()

[guillaumeaubert]

I just noticed that subtest() in 0.94 returns a boolean indicating whether the subtests where successful, but 1.005000_002 doesn't. This feature is very useful for test modules to aggregate the results of a block of tests (I use it in Test::Dist::VersionSync for example).

Simple script to demonstrate the difference:

#!perl

use strict;
use warnings;

use Data::Dumper;
use Test::More tests => 2;

my $subtest1 = subtest(
        'A subtest that succeeds.',
        sub
        {
                ok( 1, 'Fine' );
                ok( 1, 'Fine' );
        }
);
print "Return value for successful subtests is: " . Dumper( $subtest1 ) . "\n";

my $subtest2 = subtest(
        'A subtest that fails.',
        sub
        {
                ok( 1, 'Fine' );
                ok( 0, 'Not fine' );
        }
);
print "Return value for failing subtests is: " . Dumper( $subtest2 ) . "\n";

With 0.94, this returns:

guillaume@avalon:~/Documents$ perl subtest.pl
    1..2
    ok 1 - Fine
    ok 2 - Fine
    1..2
ok 1 - A subtest that succeeds.
Return value for successful subtests is: $VAR1 = 1;

    ok 1 - Fine
    not ok 2 - Not fine
    #   Failed test 'Not fine'
    #   at subtest.pl line 24.
    1..2
    # Looks like you failed 1 test of 2.
not ok 2 - A subtest that fails.
#   Failed test 'A subtest that fails.'
#   at subtest.pl line 26.
Return value for failing subtests is: $VAR1 = 0;

# Looks like you failed 1 test of 2.

With 1.005000_002, this returns:

guillaume@avalon:~/Documents$ perl -Iuserlib/Test-Simple-1.005000_002/lib/ subtest.pl
TAP version 13
1..2
    TAP version 13
    ok 1 - Fine
    ok 2 - Fine
    1..2
ok 1 - A subtest that succeeds.
Return value for successful subtests is: $VAR1 = undef;

    TAP version 13
    ok 1 - Fine
    not ok 2 - Not fine
    #   Failed test 'Not fine'
    #   at subtest.pl line 24.
    1..2
    #1 test of 2 failed.
not ok 2 - A subtest that fails.
#   Failed test 'A subtest that fails.'
#   at subtest.pl line 26.
Return value for failing subtests is: $VAR1 = undef;

#1 test of 2 failed.

If this feature was not intentionally removed, I'm willing to take a look at https://github.com/schwern/test-more/blob/Test-Builder1.5/lib/Test/Builder.pm and see what could be returned at line 188 to make it work again. Let me know!

[schwern]

You're right, it was removed by accident. It's going to be a little more work
to get the subtest result, but I think that's only because the result is being
calculated in the wrong place. The calculation is being done in the TAP
formatter, but the TAP formatter should be asking SubtestEnd.

[guillaumeaubert]

Thank you for your reply! I dug deeper into the code using your precisions, and the following changes seem to fix the problem:

TB2::TestState::handle_subtest_end(): return whether the test was successful.

return $event->history->test_was_successful;

Test::Builder::subtest(): return the value returned by post_event().

return $self->post_event(
    TB2::Event::SubtestEnd->new(
        $self->_file_and_line,
    )
);

The test script previously mentioned then outputs the expected results:

guillaume@avalon:~/Documents$ perl -T -I userlib/Test-Simple-1.005000_002/lib/ subtest.pl
TAP version 13
1..2
    TAP version 13
    ok 1 - Fine
    ok 2 - Fine
    1..2
ok 1 - A subtest that succeeds.
Return value for successful subtests is: $VAR1 = 1;

    TAP version 13
    ok 1 - Fine
    not ok 2 - Not fine
    #   Failed test 'Not fine'
    #   at subtest.pl line 24.
    1..2
    # 1 test of 2 failed.
not ok 2 - A subtest that fails.
#   Failed test 'A subtest that fails.'
#   at subtest.pl line 26.
Return value for failing subtests is: $VAR1 = 0;

# 1 test of 2 failed.

Running the tests in t/ also doesn't seem to bring up any error that would be related to the change.

I'm not pretending to fully understand the overall architecture, so please correct me if I'm on the wrong path. Otherwise, I'll be happy to submit a patch for your consideration.

[schwern]

Thanks for digging into it!

The solution will work, but I'd like it if A) subtest returned a Result object, not just a boolean and B) if things could just ask the SubtestEnd event for a result. B is the important part, because right now the logic is in the TAP formatter and if other formatters want to implement subtests they'd have to reimplement that logic. It improves the design.

Lemme bang that together now. Thanks for inadvertently pushing me into it. :)

[schwern]

That should do it.

[guillaumeaubert]

Nice fix, thank you! I've just tested and it indeed solves the problem.