In documentation and lectures and books I've read that 100% test coverage does not mean that your code is bug-free.  Usually it's in the context of user input, or running all combinations of data through regular expression.  Yesterday, however, I fixed a bug that is an example of this working in a very different way.

WebGUI has a Storage class, that maps a base64 Md5 Hash into a filesystem location.  However, the base64 hash doesn't work very well on case insensitive filesystems, especially if you later decide to move the site onto a new machine where case does matter.  To address this problem, we began to use the hexadecimal version of the hash instead.

Inside the class, the hexadecimal version is calculated once, and then cached inside the object:

my $hexId = MD5::decode_base64($hash.'==');
$self->{_hexId} = $hexId;

The accessor for the hexId just returns the contents of the object's key in its hash:

sub getHexId {
     return $_[0]->{_hexId};
}

Storage locations come in two varieties, permanent (the most commonly used kind) and temporary.  Temporary locations are cleaned up in mass by a regularly scheduled workflow that runs outside of WebGUI, so you save a little request time by skipping the clean-up at the end.  Temporary locations are also stored in a different area of the file system from regular locations.  Because of this, they use an alternate contructor in the same class.  That constructor had this bit of code:

my $hexId = MD5::decode_base64($hash.'==');
$self->{_Hexid} = $hexId;

When I ran a coverage report, it reports that getHexId has 100% coverage, yet calls on a temporary storage location object returned undef instead of the hexadecimal id.

Fix the problem is as simple as fixing the hash key in the assignment above.  But it still leaves the problem of having code with questionable coverage.  Here are some ideas I've had about fixing that as well:

1. Don't cache the hexId, but calculate it in the accessor every time.
2. Instead of having an alternate constructor in the same class, creating temporary locations could be a subclass of WebGUI::Storage, where the constructor is overridden. Now, via Test::Class, I should get coverage for both classes.

I like the second idea better, but WebGUI has a five year API guarantee so I'd have to be careful about leaving the old method in place and having is call the one.

Category: perl