What a tangled web Kwalitee culting has accidentally woven

Alias on 2009-08-13T17:47:17

As I mentioned yesterday, I now have a new shiny analysis toy in the form of downstream dependency graphing.

So now we can tackle some issues that have been hard to get traction on until now.

The first one I'd like your help dealing with is some fallout from Kwalitee and Perl::Critic, unintended consequences of superficially positive metrics targeted by contestants in a game without access to clear best practices resulting in damage to the CPAN graph.

(Don't get me wrong, I'm a huge supported of CPANTS and Dom did a lot to help me with it, including creating the original version of the awesome red and green box author page basically to my spec. This is just about two particular metrics)

The two big problems are caused by Test::Pod and Test::Pod::Coverage.

It's not so much the use of them at all, since they are quite safe to use when applied with author test best practices.

It's that many (alas, very many) people used these modules in ways that made their use compulsory, imposing hard dependencies instead of soft dependencies (only running them if already installs).

Test::Pod imposes a Perl 5.8 dependency, blowing any chance at back-compatibility for anything that uses it.

And Test::Pod::Coverage has a history of failing to get to 100% PASS rates, the current release has 30 odd failures.

So, what's the shape of the problem we've got here?

Let's start with Test::Pod::Coverage.

http://svn.ali.as/graph/Test-Pod-Coverage

(Don't forget to use CTRL-minus to zoom out) :)

As you can see, there are a couple of large nasty tangles of dependencies, and a number of high-dependency explosions.

And even though Test::Pod::Coverage has 650 dependencies (resulting in 18,500 FAILure points) it's obvious from the graph that several hundred dependencies can be removed by fixing just three modules

Test::Compile Module::Find WWW::Search

There's one more critical high-dependency node there (Test::Distribution) but as a Test:: module depending on another Test:: distribution, it's far more likely it is a legitimate dependency.

But even without that last one, we've got some very juicy low-hanging fruit to go after here. And once these three have been fixed, we can rerun the graphs and see if anything else stands out.

So now we move on to Test::Pod, which is bigger again.

http://svn.ali.as/graph/Test-Pod

As with Test::Pod::Coverage, the three previous modules (Test::Compile, Module::Find and WWW::Search) show up again.

And this graph also adds some more interesting targets.

MooseX::Types::Structured MIME::Types MailTools Test::Strict

So that's no more than around 7 modules that we need to tweak and rerelease to unwind half or more of the dependencies, and reduce the dependency weight of many of the Heavy 100 by 5 dependencies each.

By doing this reduction of false dependencies, we reduce the time it takes to install CPAN software, and reduce false failures due to failing dependency modules for things that never should have been dependencies at all.


Okay author of MooseX::Types::Structured listening

jjn1056 on 2009-08-13T19:00:27

So, not so clear what you need me to do (too little caffeine today). You want me to drop the Test::Pod stuff? Sorry, but that link you gave regarding author best practices it broken for me. Just let me know what to do and I will cut a release asap.

John

Re:Okay author of MooseX::Types::Structured listen

barbie on 2009-08-13T19:48:51

In your Makefile.PL change..

build_requires 'Test::Pod' => '1.14';
build_requires 'Test::Pod::Coverage' => '1.08';

..to..

recommends 'Test::Pod' => '1.14';
recommends 'Test::Pod::Coverage' => '1.08';

You already have the appropriate eval/skip in both respective test files.

Re:Okay author of MooseX::Types::Structured listen

jjn1056 on 2009-08-13T21:28:24

okay, I'll do this today or tomorrow then

Re:Okay author of MooseX::Types::Structured listen

Alias on 2009-08-14T01:30:47

Actually, it shouldn't be in recommends either.

Firstly, recommends isn't actually implemented.

Secondly, once the META.yml upgrade process does reach the point of dealing with this, it's likely to mean either "Should be installed for all users except in resource-constrained environment" or possibly "Enhances the run-time functionality of the module".

Re:Okay author of MooseX::Types::Structured listen

barbie on 2009-08-14T07:42:12

Should there be a build_recommends then? Although I don't reference them in my Makefile.PL, I do reference them in the 'recommends' section of my META.yml. I'd rather mention them somewhere, in the event that someone might be interested :)

Re:Okay author of MooseX::Types::Structured listen

jjn1056 on 2009-08-14T14:12:05

Well,

That sucks. I changed it, tagged it and released it. "recommends" didn't give any errors when I ran perl Makefile.PL, and the rest.

Now I have to go figure out what I need to change and change it again :(

Re:Okay author of MooseX::Types::Structured listen

Alias on 2009-08-14T02:02:35

1. Remove all mention of the author test modules from META.yml

2. As a bonus, you can change pod.t to something like this

#!/usr/bin/perl

# Test that the syntax of our POD documentation is valid
use strict;
BEGIN {
        $| = 1;
        $^W = 1;
}

my @MODULES = (
        'Pod::Simple 3.07',
        'Test::Pod 1.26',
);

# Don't run tests during end-user installs
use Test::More;
unless ( $ENV{AUTOMATED_TESTING} or $ENV{RELEASE_TESTING} ) {
        plan( skip_all => "Author tests not required for installation" );
}

# Load the testing modules
foreach my $MODULE ( @MODULES ) {
        eval "use $MODULE";
        if ( $@ ) {
                $ENV{RELEASE_TESTING}
                ? die( "Failed to load required release-testing module $MODULE" )
                : plan( skip_all => "$MODULE not available for testing" );
        }
}

all_pod_files_ok();

Blargh

chromatic on 2009-08-14T03:48:21

Speaking of cargo-cult code....

Re:Blargh

Alias on 2009-08-14T16:51:06

It's good enough until Module::Install (and other things) have built in xt support

Re:Okay author of MooseX::Types::Structured listen

jjn1056 on 2009-08-14T14:38:10

well, META.yml gets generated automatically, I guess you want me to editted manually?

I guess I don't really get it, I just patterned my Makefile.PL against some other big distros, like DBIC, Moose, etc and just assumed they knew what that are doing.

Re:Okay author of MooseX::Types::Structured listen

Alias on 2009-08-14T16:52:12

You remove it manually from the Makefile.PL so it's removed automatically from the META.yml.

Re:Okay author of MooseX::Types::Structured listen

tgape on 2009-08-15T03:03:42

I realize you are only suggesting this for the pod tests.  For those tests, this is innocuous enough.  However, as chromatic pointed out, cargo cult programming exists.  I'd prefer to give the cargo cult programmers as few chances to happen across code that skips all tests for people installing via CPAN/CPANPLUS, especially since some of them may not run any tests on their own boxes (even though they have them.)

Re:Okay author of MooseX::Types::Structured listen

Alias on 2009-08-15T13:25:11

You prefer to prioritise the people that would copy and paste author tests into their own modules ahead of everyone everywhere that installs modules for any reason at all?

That seems like prioritising a small group with a rare situation ahead of an enormous group with common situation.

Broken link

mauzo on 2009-08-13T21:10:43

I believe your 'author best practices' link is supposed to go to Test::XT rather than Test::XS.

Re:Broken link

Alias on 2009-08-14T01:32:01

Fixed, thank you.