Jun 21, 2008

You are not paid to introduce bugs :)

Hi, Today's post is in the series "what is wrong with this code" but before presenting the actual problematic code I want to give some background, so bear with me. At Db4o we run migration tests (regularly) in order to guarantee that we are able to load (and query) databases created by older versions of Db4o engine. The way it's accomplished is very simple:
  • for each configured version, create an database using it and perform a series of tests with the trunk version.
Some time ago, I was asked to fix an annoying bug in these migration tests. In order minimize the time required to run these tests, we do not run then against every existing Db4o version. To allow this selective running we introduced a way to specify the minimum version to be tested so, if one want to test versions 6.0 and above it is a simple matter of setting a constant in a C# source:
private const double MinimumVersionToTest = 6.0;
At some point in the code we called the following function (wrong version):
double VersionFromVersionName(string versionName)
{
Version version = new Version(versionName);
return version.Major + version.Minor / 10;
}
This function is supposed to return the double representation of an assembly version (string) but it fails miserably (this code works correctly only for versions with a decimal value 0 (zero); for instance, 6.0, 7.0, etc). Now the question is, what's is wrong with this function? To be fair I have to admit that it's not a hard to spot bug, but when you are in a hurry and have no test for your test code... The problem is that version.Minor is an integer field and so,
version.Minor / 10
will result in a integer number and the decimal part will be happily discarded. The developer who wrote this code (me) is aware of language type conversions but he didn't notice the error
(shame on me) ; so whenever we passed a version number not finished in 0 (zero) we got the wrong double representation for it!
Actual VersionFunction Return
6.06
6.16
77
7.27
The fix? Pretty easy: Just add a ".0" to the divisor :) (I could also add an F: 10F)
double VersionFromVersionName(string versionName)
{
 Version version = new Version(versionName);
 return version.Major + version.Minor / 10.0;
}
The bottom line? T
his episode just highlights (and reminds me about) the importance of double checking our test code! after all, as a friend of mine likes to ask:
who test that the tests test what they should test?
Have fun. Adriano

No comments: