author tests: no more pointless test failures?

rjbs on 2008-02-25T03:47:11

The latest release of CPAN::Mini needs to be replaced, because it contains stupid OS X 10.5 resource fork crap. That's not why I'm going to replace it, though.

It also had a t/perl-critic.t file to check things against my Perl::Critic config. To keep other people from thinking things are busted, I put this in place:

if ($ENV{PERL_TEST_CRITIC}) {
  if (eval { require Test::Perl::Critic }) {
    Test::Perl::Critic::all_critic_ok();
  } else {
    plan skip_all => "couldn't load Test::Perl::Critic";
  }
} else {
  plan skip_all => "define PERL_TEST_CRITIC to run these tests";
}

Then, today, I got test failure report because somebody else happened to have that environment variable set. Oops! I could make more and more elaborate tests, but that just seems like a long battle with FAIL waiting to happen.

I've actually had this problem a few times, and it seemed like a nice solution would be the idea of an "extra tests" directory. This came up a few times on the perl-qa mailing list, and it seemed like a great 90% solution.

Today, I combined my problem, this idea, and my long-standing desire to write a Module::Install plugin. I uploaded Module::Install::AuthorTests earlier this evening.

Basically, it lets you add a line like this to your Makefile.PL (assuming that you use Module::Install):

author_tests('xt');

Then, if you're "the author" the tests are run during make test. If you're not, they aren't.

Module::Install has a pretty simple idea of whether you're the author. If an ./inc directory doesn't exist when you run the Makefile.PL, one is created, and all the required Module::Install code it put into it. Then, a directory is created called ./inc/author. If that directory exists, you're the author. If it doesn't -- because ./inc existed already when you ran the Makefile.PL, because you got the whole thing in a tarball from the CPAN -- then you are not the author.

So, for someone's smokebot to run my author tests, they'll need to create a directory in the unpacked dist before running the Makefile.PL. I think I'm safe.

Further, moving pod-coverage.t and friends to ./xt looks like it won't affect my kwalitee rating at CPANTS. A look at Module-CPANTS-Analyse makes it look like it will find the tests with no problem, but I won't have to worry about every old smoker on the internet running them and telling me that I forgot a =back when I didn't.

I think just about all my dists will get new releases in the next few weeks.

I really need to start using ShipIt or Joseki or something. I'm afraid I will end up writing my own. I should update my Module::Starter templates, too, which are wildly out of date with how I actually construct dists, now.

For now, though, I'm really happy with this plugin. Hooray!


aol!

jmason on 2008-02-25T10:13:15

I do this too, on some of my modules. A bit more simply, though -- instead of inferring whether the author tests should be run automatically, I just invoke them separately using something like:

prove -v author-tests/*.t

to be honest, the additional legwork of one commandline doesn't bother me ;)

Re:aol!

Aristotle on 2008-02-25T15:11:28

You can go one better: author tests are mostly things even the author doesn’t want to run all the time, but definitely wants to run before he ships a new release. So if you’re using Module::Build, you can override the dist and/or disttest action(s) to run your author tests automatically. I find this a better hook than RJBS’s, and unlike your suggestion you can’t forget to run the tests – so no brown bag bugs. :-)

"It's really quite simple..."

bart on 2008-02-25T12:47:06

Module::Install has a pretty simple idea of whether you're the author. If an ./inc directory doesn't exist when you run the Makefile.PL, one is created, and all the required Module::Install code it put into it. Then, a directory is created called ./inc/author. If that directory exists, you're the author. If it doesn't -- because ./inc existed already when you ran the Makefile.PL, because you got the whole thing in a tarball from the CPAN -- then you are not the author.
WTF?? I must have a different idea than you of what constitutes a simple test, then. Way too much voodoo!

I'd rather try something like this: if you're a CPAN author, you set an environment variable with your CPAN ID. For example, for you, you'd set it to "RJBS". At the time to run the tests, this author testing module test to see if $ENV{CPAN_ID} is set, and if it is, see if it's set to the name of the author of this particular module. Only then, you will be considered to be the module author, for this module.

Re:"It's really quite simple..."

rjbs on 2008-02-25T12:56:28

No, that wouldn't work for me.

I work on a number of distributions that have multiple authors, maintainers, and/or contributors. It would foolish if they had to set CPAN_ID=RJBS when working on my code. I'd also have to put my CPAN id somewhere in my code, either ina ll the "author tests" or in the Makefile.PL as an argument too some author test runner. That would need to be updated when the module changed hands.

I could set I_AUTHOR_Module-Starter=1 and have everyone who works on it do the same, but some would forget and then they'd check in changes that didn't pass the author tests.

This way, anyone who clones from my repository -- which does not contain "inc" -- will run the author tests as a matter of course by running make test.

I agree that the way Module::Install does it seems a bit convoluted, but it is very, very good at making sure the right thing happens. Also, neither I nor the user needs to know how it works. It just does.

Re:"It's really quite simple..."

Yanick on 2008-02-25T16:49:30

At the time to run the tests, this author testing module test to see if $ENV{CPAN_ID} is set

Personally, I prefer to have all my modules listed in TEST_AUTHOR (E.g., export TEST_AUTHOR=My::Module,My::Other::Module), and have the check be

if ( grep { $_ eq 'My::Module' } split ',' => $ENV{TEST_AUTHOR} ) {
...
}

That way, I have only one environment variable to take about, and it's not tied to me but rather to the modules of interest.

go ./xt!

Eric Wilhelm on 2008-02-25T17:29:38

Now if we can just get some metadata about the tests hammered into a standard...

>I really need to start using ShipIt or Joseki
>or something. I'm afraid I will end up writing
>my own.

If you want to ride along, the CPDK has a publish_module script. I haven't turned it on itself yet, but I've been using it for several months now.

http://scratchcomputing.com/svn/CPDK/trunk/

manifest?

pfig on 2008-02-26T17:02:19

unrelated to perl, but is there a reason why you don't generate the tarball from the manifest? that way the metadata files from os x wouldn't get in there.

Re:manifest?

rjbs on 2008-02-26T17:28:51

I "make dist." Maybe the manifest got them in it from a "make manifest" with an incomplete MANIFEST.SKIP.

Either way, easy to fix.