E. Choroba

My August assignment (yes, I had to "stick" with it for September) in the CPAN Pull Request Challenge was Test::Aggregate. In August, I went on vacation, and CPAN Testers' reporting was down, so I decided to solve the task after I return from the YAPC in Granada and ask for a new distribution for September with a two-week delay. It wasn't that easy, though.

Adding Local Lib to Git

It was known the module would break with the new major changes to Test::Builder four years ago. When I started playing with it, I wanted to stay sure I didn't break anything, i.e. by fixing an issue under the new Test::Builder, I didn't want to make it fail under the old one.

I needed two different versions of a distribution installed in Perl. As a quick hack, I just went to my local lib, created a new git repository there, and added the current state to it.

cd perl5/lib
git init .
git add .
git commit -m 'old version'

Then, I installed the development version of Test::Simple (Test::Builder is part of it) and added the new state into a branch.

git checkout -b new
git add .
git commit -m 'installed 1.302007_004'

Switching the versions was now just a matter of

git checkout master

git checkout new

First Attempt

I started with the fix recommended in the bugreport mentioned above. It decreased the number of failures in one of the tests, and removed two "Bad plan" failures in other ones. Nevertheless, there were still many tests failing, even for the Test::Aggregate::Nested, which presents itself as "superior to Test::Aggregate" and "the preferred way of aggregating tests in the future". I wanted to fix it at least.

In one of the tests, the Test::Builder::diag method was aliased to intercept diagnostic messages:

local *Test::Builder::diag = sub {
  my ($self, $msg) = @_;
  push @{ $tb->{diag} }, $msg;
};

After running a test file, the number of diagnostics recorded was compared to the expected number, and the messages were matched against regular expressions. The test was failing, because a "No tests run" message from a skip wasn't captured in the array.

I examined the Test::Builder code and discovered the "No tests run" message could be emitted not only by Test::Builder, but also by Test::Stream::Context. I aliased its diagnostic subroutine in the same way

local *Test::Stream::Context::diag = *Test::Builder::diag{CODE};

and the number of failing tests decreased again!

Error Message

Some of the tests were still failing with a strange error, though:

Couldn't parse 'aggtests/skip_all.t':
  Label not found for "last TS_SUBTEST_WRAPPER" at
  /home/choroba/perl5/lib/perl5/Test/Builder.pm line 529

The line contained die and was preceded by a comment:

# HACK!

I wasn't sure what to do, so I logged into the IRC and started asking in the #pr-challenge channel. Fortunately, Ether was there and recommended me to ask in #perl-qa, where I started talking with Exodist, the author of the new Test::Builder. Together, we realized Test::Builder::diag didn't need to be aliased under the new code, as it called Test::Steam::Context::diag anyway. But when we turned to the TS_SUBTEST_WRAPPER label, there was no similar simple solution: Test::Stream was still in an alpha state, the current CPAN version was quite different to the latest one on GitHub, and API might still change. The discussion continued in the #test-stream channel, where I finally decided to postpone the hard work and just make the module fail with a meaningful message under the new Test::Builder.

The Real Outcome

Test::Aggregate contains both Build.PL and Makefile.PL. In the former, you can easily say

my $builder = Module::Build->new(
    # ...
    requires => {
        'Test::More' => '< 1.3',
    }

but I don't know how to do the same in the Makefile.PL. Therefore, I moved the logic to the first test to run: 00-load.t.

my $v = $Test::More::VERSION;
cmp_ok($v, '<', 1.3, 'Compatible Test::Builer')
    or BAIL_OUT(<< "__BAIL_OUT__");
This module only works with Test::More version < 1.3, but you have $v.
__BAIL_OUT__

Randy Stauner, the current maintainer of Test::Aggregate, appeared in the channel during the conversation, and we decided I'd join them again once Test::Stream is stable to make Test::Aggregate work with it.

I think that what Neil wanted to achieve with the Challenge was to make us involved. In this particular case, he succeeded.