Spot the Bug!

Ovid on 2007-06-06T07:33:13

The module:

package Some::Module;

use strict;
use warnings;

BEGIN {
    unless ( $ENV{'REMOTE_USER'} ) {
        die "REMOTE_USER environment variable not defined";
    }
}

use ProdDev::DBConnection qw;   # this is not the bug!
use Exporter::NoWork;

sub dbh { db('manage') }

1;

The (beginnings of the) tests:

#!/usr/bin/perl

use strict;
use warnings;

use lib 'lib', 't/lib';
use Test::More 'no_plan';    # tests => 1;

my $MODULE = 'Some::Module';

eval "use $MODULE";
ok $@,   'Trying to use $MODULE without a remote user should fail';
like $@, qr/^REMOTE_USER environment variable not defined/,
  '... with an appropriate error message';

$ENV{REMOTE_USER} = 'john q. Public';
use_ok $MODULE, qw/dbh/;

isa_ok dbh(), 'DBI::db', 'The object dbh() returns';

The failure:

t/manage......ok 1 - Trying to use Some::Module without a remote user should fail
ok 2 - ... with an appropriate error message
ok 3 - use Some::Module;
Undefined subroutine &main::dbh called at t/manage.t line 17.
ok 4 - no warnings
1..4
# Looks like your test died just after 4.

Yesterday I wrote this code right before I left and I blew quite a chunk of time trying to figure it out, even calling in another programmer who was totally mystified.

Today I sat down at my desk, opened up the code with trepidation, and said "oh, duh!", and immediately fixed the error. I was really, really stupid yesterday.


D’uh

Aristotle on 2007-06-06T08:41:11

Your “minimal” example was far from minimal so I suspected the wrong things for a long time… I guess that was your point. This was compounded by the fact that I have neither ProdDev::DBConnection nor a desire to install Exporter::NoWork, so I could only read and guess run than run it and poke around. Once I suspected the right thing I found the problem in 30 seconds; then I had to spend another 5 to rewrite to an actual minimal example to confirm the fix (ROT13ed):

qryrgr$VAP{qb{zl$x=$ZBQHYR;$x=~f!::!/!t;$x.".cz"}};

Re:D’uh

Ovid on 2007-06-06T09:05:26

You're correct about my posting the other stuff to make it a bit more obscure since that's how things work in real life. I could have just posted the tests and said "that's enough to find the bug", but that would have made it too easy.

Your solution, by the way, is correct. Of course, you knew that :)

Re:D’uh

Aristotle on 2007-06-06T09:56:32

Yeah. I admit that I might have been stumped far longer if I hadn’t read about this particular trap before, though I would have found it eventually. With knowledge of how the machinery in Perl works, it’s tractable, if not obvious.

The average J. Random Perlhacker doesn’t know enough though, I suspect, that they could track this down without help. I have often wondered if my fix shouldn’t be applied automatically on failures of the kind you deliberately caused. It also leads me to think that such checks should be delayed from compile time if you can at all help it – if yours were in import, say, you’d have avoided this headscratching moment.

(On an unrelated note, Exporter::NoWork makes me shudder… way too implicit. Give me Exporter::Tidy any day…)

Re:D’uh

Ovid on 2007-06-06T10:24:57

I feel a bit odd about using Exporter::NoWork, but it's standard here, so that's what I go with unless I need fine-grained control.

Re:D’uh

barbie on 2007-06-06T11:52:28

I'm glad I got it right after only glancing at it, but I am surprised that anyone used to writing tests would have not have spotted it. You actually made me doubt myself as I thought it couldn't be that obvious and I was missing something. It was only on checking Aristotle's post that confirmed that I was right.

Personally I would consider it bad form to have that sort of test script, as you're not cleanly testing the interface, as you've discovered :) It also means you're not truly testing a user situation, which ultimately is what the test scripts should be testing.

(Brainos)

Aristotle on 2007-06-06T09:34:32

(Another 5 minutes, that is. Also, that was supposed to say “read and guess rather than run it”.)