kentcowgill.org/blog

I'm really not understanding something. I've run into a module for which I had a lot of difficulty writing the first few tests. This usually happens when the module under scrutiny does something a little weird, so I'm used to it by now.

But this one is a lot weird.

The code for the module I'm looking at is structured exactly like this:

package foo;

use strict;
use warnings;

sub firstSub {
  my $string_orig = shift;

  # in the production code, this
  # somehow works and doesn't die
  my $string = secondSub $string_orig;

  return ( foo => $string, bar => $string_orig );
}

sub secondSub {
  my $string = shift;
  my $ret = join '', reverse split //, $string;
  return $ret;
}

1;

What's strange is that when I use the module and try to call "firstSub", I get a Can't locate method object secondSub via package VALUE_PASSED at line blah, of course substituting VALUE_PASSED for whatever value I actually pass :) But the module - as is - works on the development server, and has worked on the production server for years with this exact structure and syntax.

So for this example, this code (reduced to the smallest example that still exhibits the behavior, but essentially the same structure) tries to call the modules' subroutine via:

use foo;

my %q = foo::firstSub( 'MTFNPY' );
  
print "$_: $q{$_}" for keys %q;

And the output I get is:

A little boggling. I tried a few things, including this ugliness:

{
  no warnings 'once';
  *MTFNPY::secondSub = \&foo::secondSub;
}

... but of course that will only work as long as I'm passing the value of "MTFNPY".

Running the code through B::Deparse confirms that line is interpreted as my $string = $string_orig->secondSub.

I see I've got a couple of options:

1. Fix the code minimally - re-order the subroutines inside the module so the second subroutine is defined first, so it will be correctly interpreted as a function call.
2. Fix the code a little less minimally - add parens around the argument to secondSub.
3. Fix* my tests and don't touch the original code until my tests are complete so I am 100% certain I don't break anything else. (I'm fairly sure either of these changes won't, but "fairly" isn't 100%).

* By which I mean 'just make it work'

So, I add this near the top of my test file:

BEGIN {
  package foo;
  use subs 'secondSub';
}
use_ok( 'foo' );

... which basically tells the interpreter that package foo will be defining a subroutine called secondSub, which is to say predeclaring them - before the foo module has a chance to load, by taking place inside a BEGIN block.

And lo, testing successful.

But what I still don't understand is how exactly the code in the original module works at all! I'm testing on the same box where the code lives - where the code runs day in and day out without bringing the test server down to a screeching halt. I know - I added trace debugging statements all around, thinking that perhaps the code has always been silently failing and the failure was just dealt with. But no, the code was making it through that step and doing exactly what my predecessor(s) expected it to do. It's just doing it wrong.

So, I'm dutifully going through some 129 modules in our codebase, creating documentation and tests for everything I find.

I mean absolutely everything.

But I wonder if I'm going too far?

For instance, I happened upon a module called Boolean.pm. Seems like it ought to be fairly low-hanging fruit, so I open it up to find that the whole of the module looks like this:

package Boolean;
use strict;
use base q(Exporter);
@Boolean::EXPORT = qw(TRUE FALSE);

use constant TRUE  => 1;
use constant FALSE => 0;

1;
__END__

No joke.

Fine, the tests will be easy enough. I won't repeat them here for brevity, but suffice it to say that the only thing that threw me for a loop is that use_ok didn't seem to import the exported constants into the namespace of my test file - after testing the module, I just use Boolean; and then I can test the constants. Could I have just called import as the module inherited from Exporter? Probably, but I didn't.

The next module I come across is another top level namespace module named Database.pm. Whose primary purpose is to figure out which database server to have the rest of the code connect to, depending on a few external conditions. Part of the module declares a number of constants - the database connection strings - for internal (to the module) use only, and doesn't use base 'Exporter' and subsequently export those constants. However, it makes sense to me that I ought to test those constants as part of my test suite.

So, fresh with my knowledge that constants aren't imported into the test file's namespace via use_ok, I add a use Database; and expect my tests to pass. Oops, not so fast - because I'm testing the constants as barewords, and they're not exported via @EXPORT, they're not recognized as constants and my test file (with use strict at the top) fails miserably complaining about my usage of barewords.

So, what to do?

I decided I would force the Database module to be @ISA = 'Exporter' and add the constants to @EXPORT myself, on behalf of the module, in the test file. So now my test file looks a bit like this:

use strict;
use warnings;

use Test::More tests => 2;

BEGIN {
  use Exporter;
  @Database::ISA = ( 'Exporter' );
  @Database::EXPORT = qw/PROD DEV/;
}

use_ok( 'Database' );

use Database;

is( PROD, 'production:dsn:etc',
    'PROD points to correct db' );
is( DEV, 'development:dsn:etc',
    'DEV points to correct db' );

... but I have to wonder if this is worth all the bother. I suppose for now I'll leave it.

&qidYou may have guessed by now, but I've taken up a particularly onerous gauntlet at my job. I'm afraid to touch any modules. But holy cow, do they need touching.

What do we learn from Refactoring: Improving the Design of Existing Code ? Well, we learn that without tests, you can't refactor. Well, you can - it's really not wise to do so, however. It will be difficult to know if you've broken something or slightly altered a particular functionality.

Of course, tests also help you prove what you've got is a set of modules that performs to their specifications - or in the absense of a specification - proving that the code does what it looks like it's supposed to do based on certain circumstance.

Given that our codebase is huge, undocumented, and more important untested, I've come up with a few snippets of shell code to really move me pretty far forward with my task.

First, to create a directory hierarchy to house all my new tests:

for i in $(find . -name "*.pm")
  do touch $(echo $i | \
           sed -e 's/\./\/path\/to\/dir/' | \
           sed -e 's/\.pm$/.t/')
done

And to give the files some default content (it's ugly but it sorta works, YMMV):

for i in $(find . -size 0)
  do echo '#!/usr/bin/perl' >> $i
    echo "" >> $i
    echo "use strict;" >> $i
    echo "use warnings;" >> $i
    echo "" >> $i
    echo "Test::More 'no_plan';" >> $i
    echo "" >> $i
    echo "use_ok( '`sed -e 's/\//::/' | \
      sed -e 's/\.t//'`' );" >> $i
  done

These snippets, plus my previously mentioned vim tip are really getting me pretty fair along in creating a comprehensive test suite and a good set of documentation for all of our legacy code.

Oh, and as a side benefit, I'm getting to know the modules pretty well while I'm at it :)

I wrote a few days ago about writing my own Test::Harness parser since I had been unable to install the module that does that, and does it well.

On my machine, Test::TAP::HTMLMatrix is beautifully installed - however where this module could do me the most good - as in having tests run all the time for me - I have historically had issues getting modules installed. At the very least, I need to identify all the various dependencies of every module and their dependencies in order for the SAs to search for .rpm files for ease in making the modules easily installable on every other production machine in the cluster. Which makes a certain amount of sense. As long as you don't remember that this test reporting module has no business on production anyhow.

But a brief perusal of the source for HTMLMatrix, and I'm starting to get the impression that having the SAs install it, plus all it's dependencies, plus all their dependencies, might be more hassle and take longer than I'd like.

Since I got my html harness pretty much working before trying to install HTML Matrix again, I might as well start using that at work to keep at-a-glace test results at a handy web page. Without all those dependencies.

So I did. And I copied all the tests over to our dev server. And I tweaked the hell out of them to make them actually run. More on that later.

The above picture is just a snippet of it to be able to fit into my blog, but you can see a fuller version here.

The code to create this is pretty simple. Here are the important bits of it:

#!/usr/bin/perl

use strict;
use warnings;

use Test::Harness::Straps;

my $strap = Test::Harness::Straps->new();

my $graph_width    = 600;
my $filename_width = 150;
my $percent_width  =  75;

print report_head();

my $cnt = 1;
my $tot = 0;
my $ok  = 0;

for my $file( @ARGV ){
  next unless -f $file;
  $cnt++;
  my $style = $cnt % 2 ? 'blue' : 'tan';
  my $result = $strap->analyze_file( $file );
  my( $this_tot, $this_ok )
    = ( $result->seen, $result->ok );
  $tot += $this_tot;
  $ok += $this_ok;
  my $graph
    = return_graph( $this_tot, $this_ok );
  my $percent
    = int( $this_ok / $this_tot * 100 );
  my $status = $percent < 100
      ? '<span style="color:red;">NOK</span>'
      : 'OK';
  my $perstyle = get_percent_style( $percent );
  printf <<END_REPORT, $style, $file,
    $file, $status, $graph, $perstyle, $percent;
  <tr class="row">
    <td class="%s">
      <a href="%s">%s</a>
    </td>
    <td class="status">%s</td>
    <td>
      <table class="graph">
        <tr>
%s
       </tr>
     </table>
   </td>
   <td class="%s ctr">%d\%</td>
 </tr>
END_REPORT
}

print report_end();

sub return_graph {
  my( $cnt, $run ) = @_;
  my $width = int( $graph_width / $cnt );
  my $leftover = $cnt - $run;
  my $bad_cell
    = qq{<td class="nok" style="width: }
    . qq{${width}px"> </td>};
  my $good_cell
    = qq{<td class="ok" style="width: }
    . qq{${width}px"> </td>};
  my $cells = $good_cell x $run;
  $cells .= $bad_cell x $leftover;
  return $cells;
}

sub get_percent_style {
  my $pct = shift;
  return
         $pct == 100    ? 'hundred'
       : $pct >= 95     ? 'ninetyfive'
       : $pct >= 90     ? 'ninety'
       : $pct >= 80     ? 'eighty'
       : $pct >= 70     ? 'seventy'
       : $pct >= 60     ? 'sixty'
       : $pct >= 50     ? 'fifty'
       : $pct >= 40     ? 'forty'
       : $pct >= 30     ? 'thirty'
       : $pct >= 20     ? 'twenty'
       :                  'ten';
}

The missing report_header() and report_end() routines add the head, title, stylesheet, body tags, etc. to turn the meat of the output into a web page.

Btw, if you have 1) unresponsive SAs or 2) too much bureaucracy to get modules installed, and 3) have an improper Scalar::Util installation, and 4) need to use Test::MockObject (or some other module that 'optionally' uses Scalar::Util::weaken - and some Enterprise Linux Distributions ship a broken version of Scalar::Util) you can use this sneaky workaround:

BEGIN {
  # We're not using feature XYZ in
  # Obj::Using::Weaken, so don't
  # care about our improperly
  # installed Scalar::Util
  require Scalar::Util;  # load it into memory
  # we know this causes warnings
  no warnings 'redefine';
  # load our own versions that do nothing.
  *Scalar::Util::weaken = sub { return @_ };
  *Scalar::Util::export_fail = sub { return };
}

Wrote a quick little perl wrapper around vim to automate posting entries to my blog.

Really threw it together pretty quickly, but wanted to save myself the trouble of opening a web browser, typing in the site name, scrolling down, clicking on "login", logging in, etc. Just a whole lot of work. Quite honestly, no one should have to put up with that.

Maybe I should see about writing a plugin for vim, since the perl wrapper is a little hackish.

Ok, so most of my historical (2006-2007) posts have reasonable dates and times assigned to them, but they're completely fabricated at this point.

Is that a big deal?

I don't think so. It's just a blog.

Would I approach Real Data^™ with such a blasé attitude? Not on your life.

I don't really have anything to say, but it's easier to post a test post than to read the docs for the database regarding inserting dates :)