Archive for the ‘Unit Testing’ Category

Don’t write tests to test your code

Tuesday, April 22nd, 2008

Wisdom tells us what we should do, experience tells us what we should never do again

The theory goes that you should write tests to check the functionality of your code, then when you refactor your code or add new features you’ve got a safety net to check that what you’ve done hasn’t broken existing functionality.

The theory is good, but in practice it doesn’t work.*

If you write tests to test your code you won’t write your tests, at least you won’t write all of them and you’ll cut corners. Writing tests to test a piece of code you already have is tedious. You’ll end up compromising your tests. You won’t test the simple stuff, because, well it’s simple, what could go wrong? It may be simple, but that’s just the sort of thing you’ll change without a moments thought but somewhere that’ll have a consequence you didn’t think of. You might skip testing the obvious stuff, but what’s obvious to you isn’t obvious to the next person to look at the code. You won’t test the difficult stuff, because that’s too time consuming and you’ve tested most of the other stuff so you figure you’re fairly safe. You won’t test the stuff that’s ‘impossible’ to test. That safety net that makes you feel secure, the holes are too big - just when you really need it you’ll fall straight through it.

So don’t write tests to test your code, write the tests before you write the code. Write tests to drive the design of your code (TDD/BDD), write tests so you know when you’re done. Only write code to fix a failing test. Yes you’ll still find your tests becoming complicated and stuff that’s difficult or ‘impossible’ to test. Listen to your tests - they are telling you something about your design. And the great thing is at the end of it, a side effect almost, you end with that safety net that really does allow you to refactor or add functionality to your code.

I think TDD/BDD and automated acceptance testing has the potential to significantly change not only how we initially produce a piece of software but also more importantly how it is enhanced and maintained throughout its ‘working life’ and it’s that which is radically going to change the value the software can deliver to the business and so the test become crucial to the success of your business. More on that later.

* when blogging you’re suppose to go for big bold controversial statements aren’t you? ;-)

Improving Java unit test readability and maintainability

Tuesday, December 11th, 2007

The last year has seen me working with Ruby rather than Java and I have really come to like the the way tests (or rather specs) are structured in RSpec and the general way BDD approaches tests (specs).

RSpec splits the behaviour of a class up into what happens in different contexts. An example probably explains things better, taking a trivial case of a stack (I'll get to Java in a minute but even if you don't know Ruby I think this code should be understandable):

RUBY:
  1. require File.dirname(__FILE__) + "/stack"
  2.  
  3. context "A populated Stack" do
  4.   setup do
  5.     @stack = Stack.new
  6.     ["a","b","c"].each { |x| @stack.push x }
  7.   end
  8.  
  9.   specify "should add to the top when sent #push" do
  10.     @stack.push "d"
  11.     @stack.peek.should == "d"
  12.   end
  13.  
  14.   specify "should return the top item when sent #peek" do
  15.     @stack.peek.should == "c"
  16.   end
  17.  
  18.   specify "should NOT remove the top item when sent #peek" do
  19.     @stack.peek.should == "c"
  20.     @stack.peek.should == "c"
  21.   end
  22.  
  23.   specify "should return the top item when sent #pop" do
  24.     @stack.pop.should == "c"
  25.   end
  26.  
  27.   specify "should remove the top item when sent #pop" do
  28.     @stack.pop.should == "c"
  29.     @stack.pop.should == "b"
  30.   end
  31. end
  32.  
  33. context "An empty stack" do
  34.   setup do
  35.     @stack = Stack.new
  36.   end
  37.  
  38.   specify  "should be empty" do
  39.     @stack.should be_empty
  40.   end
  41.  
  42.   specify "should no longer be empty after #push" do
  43.     @stack.push "anything"
  44.     @stack.should_not be_empty
  45.   end
  46.  
  47.   specify "should complain when sent #peek" do
  48.     lambda { @stack.peek }.should raise_error(StackUnderflowError)
  49.   end
  50.  
  51.   specify "should complain when sent #pop" do
  52.     lambda { @stack.pop }.should raise_error(StackUnderflowError)
  53.   end
  54. end
  55.  
  56. context "An almost empty stack (with one item)" do
  57.   setup do
  58.     @stack = Stack.new
  59.     @stack.push 3
  60.   end
  61.  
  62.   specify "should not be empty" do
  63.     @stack.should_not be_empty
  64.   end
  65.  
  66.   specify "should remain not empty after #peek" do
  67.     @stack.peek
  68.     @stack.should_not be_empty
  69.   end
  70.  
  71.   specify "should become empty after #pop" do
  72.     @stack.pop
  73.     @stack.should be_empty
  74.   end
  75. end

So a stack should behave in a particular way:

A populated Stack
- should add to the top when sent #push
- should return the top item when sent #peek
- should NOT remove the top item when sent #peek
- should return the top item when sent #pop
- should remove the top item when sent #pop

An empty stack
- should be empty
- should no longer be empty after #push
- should complain when sent #peek
- should complain when sent #pop

An almost empty stack (with one item)
- should not be empty
- should remain not empty after #peek
- should become empty after #pop

(incidentally the above is output generated from the spec tool, a bit like generating JavaDocs from Java code)

I think splitting the behaviour up into these different contexts greatly improves the readability and maintainability of both the tests themselves and the code under test. But how to do it in Java?

It's possible to use RSpec to test Java code because you can run RSpec using JRuby, however I'm not sure if it is wise to drive the development of code in one language with tests in a different language, Ruby and Java approach numerous things very differently.

Now it would be possible to have numerous JUnit test classes for a given class, each one for a different context but this has it's drawbacks, when you make a change to your class you'd need to run numerous test cases, which you could get around by having a master testcase with a suite method that includes all the others, but then you have to maintain the list of testcases in the suite() method and it's all too easy for the list to get out of sync with the actual test cases. JUnit 4 provides a solution, you can use the Enclosed runner. The code below is an equivalent example to the RSpec one in Java.

JAVA:
  1. import static org.junit.Assert.assertThat;
  2.  
  3. import static org.hamcrest.CoreMatchers.is;
  4.  
  5. import java.util.EmptyStackException;
  6.  
  7. import org.junit.Before;
  8. import org.junit.Test;
  9. import org.junit.runner.RunWith;
  10. import org.junit.runners.Enclosed;
  11.  
  12. @RunWith(Enclosed.class)
  13. public class TestStack {
  14.  
  15.         public static class AnEmptyStack {
  16.  
  17.                 private Stack<Object> stack = null;
  18.  
  19.                 @Before
  20.                 public void setupAnEmptyStack() {
  21.                         stack = new Stack<Object>();
  22.                 }
  23.  
  24.                 @Test
  25.                 public void shouldBeEmpty() {
  26.                         assertThat(stack.isEmpty(), is(false));
  27.                 }
  28.  
  29.                 @Test(expected = EmptyStackException.class)
  30.                 public void shouldComplainWhenPeeked() {
  31.                         stack.peek();
  32.                 }
  33.  
  34.                 @Test(expected = EmptyStackException.class)
  35.                 public void shouldComplainWhenPoped() {
  36.                         stack.pop();
  37.                 }
  38.  
  39.                 @Test
  40.                 public void shouldNotBeEmptyAfterItemIsPushedOntoIt() {
  41.                         stack.push(new Object());
  42.                         assertThat(stack.isEmpty(), is(false));
  43.                 }
  44.         }
  45.         public static class AStackThatIsNeitherEmptyNorFull {
  46.                 private Stack<String> stack = null;
  47.  
  48.                 @Before
  49.                 public void setupAStackWithThreeItems() {
  50.                         stack = new Stack<String>();
  51.                         stack.push("one");
  52.                         stack.push("two");
  53.                         stack.push("three");
  54.                 }
  55.  
  56.                 @Test
  57.                 public void shouldAddToTheTopWhenSentAPush() {
  58.                         stack.push("four");
  59.                         assertThat(stack.peek(), is("four"));
  60.                 }
  61.  
  62.                 @Test
  63.                 public void shouldReturnTheTopItemWhenSentAPeek() {
  64.                         assertThat(stack.peek(), is("three"));
  65.                 }
  66.  
  67.                 @Test
  68.                 public void shouldNotRemoveTheTopItemWhenSentAPeek() {
  69.                         assertThat(stack.peek(), is("three"));
  70.                         assertThat(stack.peek(), is("three"));
  71.                 }
  72.  
  73.                 @Test
  74.                 public void shouldReturnTheTopItemWhenSentAPop() {
  75.                         assertThat(stack.peek(), is("three"));
  76.                 }
  77.  
  78.                 @Test
  79.                 public void shouldRemoveTheTopItemWhenSentAPop() {
  80.                         assertThat(stack.pop(), is("three"));
  81.                         assertThat(stack.peek(), is("two"));
  82.                 }
  83.         }
  84.         public static class AStackThatContainsOnlyOneItem {
  85.                 private Stack<String> stack = null;
  86.  
  87.                 @Before
  88.                 public void setupAStackWithOneItem() {
  89.                         stack = new Stack<String>();
  90.                         stack.push("one");
  91.                 }
  92.  
  93.                 @Test
  94.                 public void shouldNotBeEmpty() {
  95.                         assertThat(stack.isEmpty(), is(false));
  96.                 }
  97.  
  98.                 @Test
  99.                 public void shouldNotBeEmptyAfterAPeek() {
  100.                         stack.peek();
  101.                         assertThat(stack.isEmpty(), is(false));
  102.                 }
  103.  
  104.                 @Test
  105.                 public void shouldBeEmptyAfterAPop() {
  106.                         stack.pop();
  107.                         assertThat(stack.isEmpty(), is(true));
  108.                 }
  109.         }
  110. }

As an aside I've also used the new assertThat() method and Hamcrest matchers as you get significantly better error messages when your tests fail that way.

To get a plain test output you could probably write a XSLT stylesheet that parses the test report, the stylesheet Kerry produced for use with Junit 3.x tests maybe a good starting point, or write a custom formatter and specify that in your JUnit Ant task.

There are also the JBehave and JDave BDD frameworks for Java which would be worth looking at if you like the look of BDD.

Unit testing interface implementations

Sunday, February 25th, 2007

It's bugged me for a while about how best to go about unit testing classes that implement multiple interfaces where those interfaces are implemented by multiple classes or where a class extends another class and implements one or more interfaces.

What I want to avoid is in each test class implementing the the same set of tests for the behavior specified by the interface. That's not very DRY.

If your classes only implement a single interface then you can have a base class with the tests for the interface and then separate test classes derived from that for each class implementing the interface. The problem arises when your classes implement multiple interfaces - Java doesn't support multiple inheritance for classes so you can't have a base test case for each interface.

One answer with JUnit 4 is to use the parameterized test runner - I haven't thought about JUnit 3, if you're not using JUnit 4 because you're still using a JVM that doesn't support annotations JSE 6 is out now, catch up!

You need to have a test case for your interface and configure this to run with the parameterized runner. You also have a static factory method that provides the various implementations of the interface to be used in each test.

As an example, say the container I'm writing (I'm not really - I know there is Apache Geronimo, JBoss, Hivemind etc - this is just an example) supports services that it can start and stop, so I have an interface Service and the various services that I have, e.g. caching, lookup, logging etc, implement this interface.

JAVA:
  1. /**
  2. * Represents a service that the container can run.
  3. *
  4. */
  5. public interface Service {
  6.  
  7.     /**
  8.       * Starts the service, may only be called if the service is not started.
  9.       *
  10.       * @throws IllegalStateException
  11.       *             if called when the service is already started.
  12.       */
  13.     public void start();
  14.  
  15.     /**
  16.       * Indicates whether the service is started
  17.       *
  18.       * @return true if the service is started, false otherwise
  19.       */
  20.     public boolean isStarted();
  21. }

I have a test case for Service that tests that the various implementations of Service adhere to the general contract. I might have the following three tests:

JAVA:
  1. public class ServiceTest {
  2.  
  3.     private Service service;
  4.  
  5.     @Test
  6.     public void shouldNotIndicatedStartedForNewObject() {
  7.         assertFalse(service.isStarted());
  8.     }
  9.  
  10.     @Test
  11.     public void shouldStartIfNotAlreadyStarted() {
  12.         service.start();
  13.         assertTrue(service.isStarted());
  14.     }
  15.  
  16.     @Test(expected = IllegalStateException.class)
  17.     public void shouldThrowExceptionIfStartedCalledOnAStartedService() {
  18.         service.start();
  19.         assertTrue(service.isStarted());
  20.         service.start();
  21.     }
  22. }

If I have a caching and lookup service and I want to test that both adhere to the contract of the Service interface then I have to annotate the ServiceTest to indicate that it should run with the parameterized runner and provide an annotated factory method that returns a collection of arguments to run the tests with.

So the full testcase is:

JAVA:
  1. @RunWith(Parameterized.class)
  2. public class ServiceTest {
  3.  
  4.     private Service service;
  5.  
  6.     public ServiceTest(final Service service) {
  7.         this.service = service;
  8.     }
  9.  
  10.     @Parameters
  11.     public static Collection implementations() {
  12.         return Arrays.asList(new Object[][] { { new CachingService() }, { new LookupService() } });
  13.     }
  14.  
  15.     @Test
  16.     public void shouldNotIndicatedStartedForNewObject() {
  17.         assertThat(service, is(not(started())));
  18.     }
  19.  
  20.     @Test
  21.     public void shouldStartIfNotAlreadyStarted() {
  22.         service.start();
  23.         assertThat(service, is(started()));
  24.     }
  25.  
  26.     @Test(expected = IllegalStateException.class)
  27.     public void shouldThrowExceptionIfStartedCalledOnAStartedService() {
  28.         service.start();
  29.         assertThat(service, is(started()));
  30.         service.start();
  31.     }
  32. }

The test classes for CachingService and LookupService now only need deal with testing the specifics of caching or lookups rather than testing adherence to the service specification.

I have yet to use this in production code on a large scale so we'll have to see how it works out, but it seems far better than having to replicate test code amongst several test cases.

junit.jar and Ant

Tuesday, January 2nd, 2007

At last no more messing about copying junit.jar or modifying your system classpath to get JUnit to work with Ant. Version 1.7 allows you to specify its location in a classpath element in your build file.

See the last point from the manual page for the JUnit task:

Note: You must have junit.jar available. You can do one of:
1. Put both junit.jar and ant-junit.jar in ANT_HOME/lib.
2. Do not put either in ANT_HOME/lib, and instead include their locations in your CLASSPATH environment variable.
3. Add both JARs to your classpath using -lib.
4. Specify the locations of both JARs using a element in a in the build file.
5. Leave ant-junit.jar in its default location in ANT_HOME/lib but include junit.jar in the passed to . (since Ant 1.7)

One thing to be careful of, make sure you include the junit.jar file that you want, you probably want to make the classpath element specifying the location of the junit.jar file the first classpath element in your junit task - I spent quite a bit of time trying to work out why the tests annotated with the new JUnit 4 @Test annotation weren't getting run only to eventually find a JUnit 3.1 junit.jar file from the Spring Framework in the classpath before the JUnit 4 junit.jar file.

XP Day 2006 - Are your tests really driving your development?

Friday, December 29th, 2006

Are your tests really driving your development?

Nat Pryce and Steve Freeman

I won't write much about this because should this be presented again and you attend knowing too much about it, it will, I think, spoil it a bit - but if you do get a chance to see this do, Nat and Steve very effectively make their point.
So, hopefully, without giving things away...

Do your tests really communicate their intent?

  • don't use magic numbers in your tests just as you wouldn't in your production code
  • consider creating value types where you might typically use primitive types - a parameter to a method of type PhoneNumber rather than just a simple String conveys far more information, even if the PhoneNumber class is just a very simple wrapper around a String
  • a comment here and there can make a big difference
  • where a method under test is returning a numerical result based on values passed to the method, express the expected result in terms of the values passed to the method, e.g. assertEquals(2+2, billingService.caclculateCharge(2,2)) rather than assertEquals(4, billingService.caclculateCharge(2,2))