Thursday, October 18, 2012

Test-driven development

To my mind, the most important software develoment tool, next to the compiler, I suppose, is a unit test framework.  I wrote one of these back in 1985 and I regret that I couldn't persuade the company to develop it further, perhaps even replacing our actual product.  But back in those days, the other tools just weren't ready for unit testing -- everything took far too long and we didn't have anything like continuous integration or even an IDE.  There was a powerful Q/A lobby which saw unit testing as trying to put them out of work!

Fast forward to 2012 and not only do we have unit testing, we have mockers and test-code generators.  I will admit right now that I am not an expert (barely a novice) in the realm of mocking.  It may be that some of what I am about to describe could be done better a different way.  No matter, onward.

I like to use a little tool called JUnitHelper which is an Eclipse plugin.  Now, it certainly has its quirks and idiosyncracies.  But it does do a basic first cut at a set of methods that "cover" all of the (non-private) methods declared in a source file.  And it makes it easy to hop back and forth between test class and test target class (Alt 9 and Alt 8).  When you enter Alt 9 (you can of course do it via menu commands too) it will add any test methods that have become necessary since you last did an Alt 9.  If there is no test class at all (defined by adding "Test" to type name), it will create one for you.  There are options so you can vary (a little) the various things that it does for you.

So here's what it produces for the Randomizer class that we talked about in the last blog (assuming we have it set to generate JUnit 4 tests):
package phasmid;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.junit.Assert.assertThat;

import org.apache.commons.math3.random.RandomGenerator;
import org.junit.Test;

public class RandomizerTest {

 @Test
 public void type() throws Exception {
  // TODO auto-generated by JUnit Helper.
  assertThat(Randomizer.class, notNullValue());
 }

 @Test
 public void instantiation() throws Exception {
  // TODO auto-generated by JUnit Helper.
  RandomGenerator random = null;
  Randomizer target = new Randomizer(random);
  assertThat(target, notNullValue());
 }

 @Test
 public void nextBoolean_A$() throws Exception {
  // TODO auto-generated by JUnit Helper.
  RandomGenerator random = null;
  Randomizer target = new Randomizer(random);
  boolean actual = target.nextBoolean();
  boolean expected = false;
  assertThat(actual, is(equalTo(expected)));
 }
etc. etc.....
I'm not in love with having those // TODO lines added but I suppose that's their way of advertising.  I'd prefer it to use more readable assertions such as:
  assertNotNull(Randomizer.class);

  instead of
  assertThat(Randomizer.class, notNullValue());


or 
  boolean actual = target.nextBoolean();
  assertFalse(actual);

  instead of


  boolean actual = target.nextBoolean();
  boolean expected = false;
  assertThat(actual, is(equalTo(expected)));

  or, better yet,
  assertFalse(target.nextBoolean());


But these details aren't too significant.  Another possible improvement might be to create a @Before setup method which reinitializes a field to a given state before each test.  Then you would, typically, only need to have one initializer for each field, instead of an initializer per field per method.  But, again, small potatoes.

I will actually create that method just so that the code is a little more readable.

So, now let's get to some real work.  We want to be able to set an array of relative frequencies for the various "bands" of the wheel of fortune.  So we create a test method to set those bands.  We will also create a test method to get the next index.  We will try to name these tests according to the scheme specified in the options for JUnitHelper (but if we don't, it won't be the end of the world, we will just ultimately end up with two test methods for one target method).

This is how the start of our test class looks now:
 private RandomGenerator random;

 @Before
 public void setup() {
  setRandom(new JDKRandomGenerator());
  getRandom().setSeed(0);
 }
 
 @Test
 public void setBands_A$() {
  Randomizer target = new Randomizer(getRandom());
  target.setBands(new int[]{1,2});
 }

 @Test
 public void nextIndex_A$() {
  Randomizer target = new Randomizer(getRandom());
  int actual = target.nextIndex();
 }

 @Test
 public void type() throws Exception {
  assertNotNull(Randomizer.class);
 }


Errors appear for setBands(...) and nextIndex() because these methods don't yet exist (so you also don't get any auto-complete when typing those names, obviously).

So, we opt for the appropriate quick fix: create the methods in the target class. But before fleshing these new method bodies out, let's actually define the behavior that we want right now.

We want the default behavior (if no bands are set) of nextIndex() to match what nextInt() would return if its parameter was Integer.MAX_VALUE.  So, let's code that in our test method for nextIndex.  But we will also add logic to test the invocation of setBands() and a subsequent call to getIndex() [actually, we will make many such calls].

We also check that the delegated methods behave as expected.

This is what (the start of) our test class looks like now:
 private static final int SEED = 0;

 private RandomGenerator random;

 @Before
 public void setup() {
  setRandom(new JDKRandomGenerator());
  getRandom().setSeed(SEED);
 }

 @Test
 public void setBands_A$() {
  Randomizer target = new Randomizer(getRandom());
  target.setBands(new int[] { 1, 2 });
  int[] counts = new int[] { 0, 0};
  final int n = 100000;
  for (int i = 0; i < n; i++) {
   int nextIndex = target.nextIndex();
   assertTrue(nextIndex == 1 || nextIndex == 0);
   counts[nextIndex]++;
  }
  assertTrue(counts[0] + counts[1] == n);
  assertTrue(Math.abs(counts[1] - 2 * counts[0]) < 500);
 }

 @Test
 public void nextIndex_A$() {
  Randomizer target = new Randomizer(getRandom());
  int actual = target.nextIndex();
  getRandom().setSeed(SEED);
  int nextInt = target.nextInt(Integer.MAX_VALUE);
  assertEquals(nextInt, actual);
 }

 @Test
 public void type() throws Exception {
  assertNotNull(Randomizer.class);
 }

 @Test
 public void instantiation() throws Exception {
  Randomizer target = new Randomizer(getRandom());
  assertThat(target, notNullValue());
 }

 @Test
 public void nextBoolean_A$() throws Exception {
  Randomizer target = new Randomizer(getRandom());
  final boolean actual = target.nextBoolean();
  boolean expected = new Random(SEED).nextBoolean();
  assertEquals(expected, actual);
 }

 @Test
 public void nextBytes_A$byteArray() throws Exception {
  Randomizer target = new Randomizer(getRandom());
  byte[] actual = new byte[] {};
  target.nextBytes(actual);
  byte[] expected = new byte[] {};
  new Random(SEED).nextBytes(expected);
  assertTrue(Arrays.equals(expected, actual));
 }
Notice that we've completely define the desired behavior of the setBands() and nextIndex() methods but we still haven't actually coded the case where there are bands set.  Not surprisingly, the unit test runs excatp that there is an assertion error in the setBands_A$() method.

Wouldn't it be nice if we could get that written for us.  Well, on second thoughts, wouldn't we be out of a job then?  Actually, there are methods to create such conforming code, via evolutionary programming but for now, we'll just do it our own way.

This is what we end up with for the two methods in question:
 /**
  * @param bands
  *            a set of relative frequencies for the index values 0, 1, ...
  */
 public void setBands(int... bands) {
  this._bands = bands;
  int count = 0;
  for (int i = 0; i < getBands().length; i++)
   count += getBands()[i];
  setTotal(count);
 }

 /**
  * The behavior of this method depends on whether any bands were defined for
  * this randomizer. If bands were defined, the result is an index into the
  * bands. If bands were not defined, the result is a number uniformly
  * distributed between 0 and {@link Integer#MAX_VALUE}. In both cases, the
  * superclass method {@link RandomGenerator#nextInt(int)} is called.
  * 
  * @return the "index" as defined above.
  */
 public int nextIndex() {
  if (getTotal() > 0) {
   int randomizer = nextInt(getTotal());
   for (int i = 0; i < getBands().length; i++) {
    randomizer -= getBands()[i];
    if (randomizer < 0)
     return i;
   }
   return -1; // logic error
  }

  return nextInt(Integer.MAX_VALUE);
 }


Of couse it isn't necessary to do test-driven development as shown above.  It's perfectly valid to write the method and then do the testing.  But it can actually be easier and more obvious what's going on when you do it this way.  And, moreover, your unit tests then actually become documentation on how your methods should be used.

Finally, you need to test for coverage.  The full suite of tests that I've defined above is not really a complete set.  There is a lot more we could test for.  But let's see what sort of coverage we get, by running EclEmma, another Eclipse plugin that I find works very well.  You might prefer Cobertura or one of the others.  Come to think of it you might prefer other test runners than JUnit.

EclEmma tells us that things aren't too bad: 356 covered instructions, 4 missed.  I think we could do rather more testing of the nextIndex() method, perhaps using a greater number of trials than 100,000, maybe with a greater number of bands.

Tomorrow we're going to look at unit testing private methods and also search for bugs and code smells.

OK, back to work!

No comments:

Post a Comment