MooseX obscure error and importance of Unit Testing

[greengaroo]

Let's meditate for a while. I just spend half a day trying to figure this out, and I finally resolved the problem easily, after creating a Unit Test script! If I had created the Unit Test script in the first place, I would have saved some very valuable time! That's why I want to share this with you, so maybe you will someday find yourself in a similar situation and decide to create a Unit Test script and save hours, unlike me!

Imagine I have two classes, using MooseX-Declare, one extends the other.

First class:

use MooseX::Declare;

class Utils::File::Section {
  
  use Utils::TypeConstraints;

  has 'id' => (
    is => 'ro',
    isa => 'Str',
    required => 1,
  );
  
  has 'entities' => (
    is => 'rw',
    isa => 'HashRef[Entity]',
    default => sub { {} }
  );
  
  # "Entity" is a Type Constraint defined in the Utils::TypeConstraint
+s
  method add(Entity $entity) {
    
    # To access the section from the entity
    $entity->section( $self );
    
    $self->{'entities'}->{$entity->id} = $entity;
  }
  
  # This returns the entity object corresponding to $id
  method entity(Str $id) {
    
    return $self->{'entities'}->{$id};
  }

}

Second class:

use MooseX::Declare;

class Utils::File::Macro
  extends Utils::File::Section {
  
  use Utils::TypeConstraints;
  
  has '+id' => (
    default => 'macro'
  );
  
  # Just adding an extra layer of validation
  method add(DefaultEntity|MacrosEntity $entity) {
    
    $self->SUPER::add($entity);
  }
  
}

I also have a third class called Utils::File::SectionFactory. In a few words, what I am doing is simple, I have files in which I have sections, and inside sections I have entities. While reading the file, I use the Section Factory to create a Section object based on the section type, and I skip the rest of the story. Pretty simple. My instantiation method, from the Section Factory, looks like this:

  method instantiate(Str $type) {
    
    # _getclass will return something like 'Utils::File::Something'
    my $class = $self->_getclass($type);
    
    eval("use $class");
    my $obj = $class->new( {'id' => $type} );
     
    if ( $obj->isa('Utils::File::Section') ) {
      
      return $obj;
    }
    else {
      
      die("The $class class is not a type of Section!");
    }
  }

Basically, what it does is using the right Class, creating an instance of that, and making sure it is a type of Section. Since Utils::File::Macro extends Utils::File::Section, if I create a Macro object, it will pass the validation as being also a Section object. I've done this hundreds of time, it always work.

But not this time! My code dies with:

The Utils::File::Macro class is not a type of Section!

That's impossible! I say to myself. So after adding some prints here and there, I see that everything seems to be fine, but still, the Macro object is not considered to be a Section! Since I am running this from the SectionFactory Unit Test script, I decided to add this line just before the validation:

Test::More::isa_ok( $obj, 'Utils::File::Section', '$obj' );

And I get:

not ok 19 - $obj isa Utils::File::Section
#   Failed test '$obj isa Utils::File::Section'
#   in /.../Utils/File/SectionFactory.pm at line 29.
#     $obj isn't a 'Utils::File::Section' it's a 'Utils::File::Macro'

Still, I don't understand what is going on! Now I am thinking that I have a problem in the Utils::File::Macro class, so i revise the syntax, line by line. Nothing wrong. But I have some doubts. I revise the documentation on ISA, just to make sure I am not imagining things. I know that the "isa" method comes from UNIVERSAL, so I go to CPAN and:

...isa returns true if CLASS inherits from (or is itself) the name of the package TYPE or inherits from package TYPE.

Faith restored! So, after all this struggle, and several cups of coffee, I finally decide to create a simple Unit Test script for the Utils::File::Macro class:

use strict;
use Test::More qw( no_plan );

use_ok( 'Utils::File::Macro' );

my $macro = Utils::File::Macro->new( {'id' => 'test'} );

isa_ok( $macro, 'Utils::File::Macro', '$macro' );
isa_ok( $macro, 'Utils::File::Section', '$macro' );

This cannot do wrong! I will get to the bottom of this! Finally! The script throws:

not ok 1 - use Utils::File::Macro;
#   Failed test 'use Utils::File::Macro;'
#   in Macro.t at line 12.
#     Tried to use 'Utils::File::Macro'.
#     Error:  Can't locate object method "name" via package "MacrosEnt
+ity" (perhaps you forgot to load "MacrosEntity"?) at /.../lib/perl5/M
+oose/Meta/TypeConstraint/Union.pm line 28.
# Compilation failed in require at (eval 46) line 2.

The problem was that one of the Type Constraints, called 'MacrosEntity', used in Utils::File::Macro in the 'Add' method, was not defined properly in Utils::TypeConstraints!!! This was undetectable from the Section Factory Unit Test script! Even with years of experience with Perl, learning how to interpret obscure error message to be able to pinpoint the problems quickly, using Moose and MooseX will give you an extra level of challenge!

So the moral of the story, something I already knew but sometimes forget, Unit Test scripts should really test "units" or small bits of code. You should always test the smallest components first, then the more complex ones.

Now, your turn to meditate...

[tobyink]

The problem is, this is sloppy:

ClassName->new

It will behave differently depending on whether or not there is a sub called ClassName in the current package. This is a pretty hard to debug error, and is not really Moose's fault. The following snippet illustrates the problem and doesn't use Moose anywhere...

use 5.010;
use strict;
use LWP::UserAgent;

{
  package ClassName;
  sub new { bless [@_], $_[0] }
}
{
  package main;
  sub ClassName () { 'LWP::UserAgent' };
  my $obj = ClassName->new;
  say ref $obj;  # says "LWP::UserAgent"  
}

The solution is to institute a policy of never using the BareWord->method syntax. Better, and unambiguous ways of writing ClassName->new are:

ClassName::->new
# or...
'ClassName'->new

Update: aliased is also a rather nice solution.

[Anonymous Monk]

The solution is to institute a policy of never using the BareWord->method syntax

LOL

[chromatic]

Do you know a more reliable solution, other than constructing optrees by hand?

[sundialsvc4]

/me nods vigorously ...

I very vividly remember the first time I really embraced unit testing and validation testing as an integral part of the development process.   I was writing a set of classes which had to operate smoothly, both within a Mojolicious-based web site, and outside of it.   And, this time, I decided to build with tests at every step.   I decided not only to test the code’s behavior in normal cases, but its detection of error cases, as well.

O_o !! !! !!

First off, I was amazed at how bad each iteration of the code actually turned out to be when I put it to the test.   I discovered errors in my code and errors in my thinking.   It was like pulling back the covers of a nice warm cuddly bed and finding bed-bugs everywhere.   Ewwwwwwww....!   But even more surprisingly, as I built the application piece by piece and constantly ran and re-ran the tests while doing so, I was confronted with how slight of a change can cause functionality that had been working perfectly for weeks, to now break in subtle and unexpected ways.

But I persevered, and here’s what the outcome turned out to be:   bulletproof code; a robust web-site; back-end code whose results matched it exactly (after it quit throwing all those exceptions ... all of which I unfortunately knew to be valid throws).   I could walk anywhere on that platform; hell, I could dance on it and it wouldn’t break.   During debugging I could quickly zero-in on a problem because I knew that the problem could not be here, here, or there.

I wound up building Test::Most tests for the interior and Selenium tests for the user interface (using every browser imaginable).   It was hell-on-wheels to get everything to run clean, but when it finally did, it was able to deploy into production with virtually zero defects.   Yes, it took considerably longer than I had thought it would, but in retrospect I realize that I really didn’t know, all things properly considered, how long it would take.   Furthermore, when the code was done, it was “Big-D” Done.   As in, “done for good... really.”

This immediately became our standard business practice, and it has both cut costs and improved customer satisfaction.   We frankly had not realized how much profit was “slipping through our fingers” in post-deployment service issues.

We owe a big debt of gratitude to the fact that CPAN modules are accompanied by all those hundreds of tests and won’t install without them.   Apply the same principles to your own code.   You’ll work a lot harder up front, and get a lot better outcomes in the end.

[greengaroo]

I cannot agree more! Thank you for your input!