Friday, December 29, 2006

Boosting your test framework

I was reading through a discussion the other day on one of the agile development mailing lists and in response to a post about C++ unit test frameworks for Visual Studio, a poster mentioned that it’s possible to use NUnit to run unit tests on unmanaged C++ code.

This is cool because NUnit has some nifty features that the other C++ unit test framework that I’m familiar with (boost::test) appears to lack – namely Setup and Teardown methods. For the record, I introduced the boost test library at my company about two years ago – around the same time as introducing smart pointers, program options, lexical casts, and a few of the other boost library goodies. It's been great not having to reinvent tons of (buggy) code for going from ints to strings and back again, etc. not to mention the benefits of smart pointers, which we still don't use as much as we should.

Already having built and integrated boost was pretty much the driving force at the time for me in terms of selecting a unit test framework for our unmanaged C++ application. At the time, I don’t think I even considered shopping around to see if there was a better option – I was new to TDD and XP at the time, and applying the same techniques that I’d had success with in C# seemed more important than making sure I had exactly the right tool for the job. Besides which, I don’t remember struggling overly long in getting the environment set up and getting the code to build using boost::test.

However, I quickly discovered that unit testing in C++ has its own set of problems that need to be considered. These include the following:

  1. Asserts are usually implemented via macros, which are then expanded by the pre-processor and will typically cause the method call being asserted to be executed more than once. There are ways around this (I remember reading of at least one C++ unit test framework which subverted this problem), but at the time when I wrote a few assert wrappers on top of the boost::test ones, I didn’t realise that my "actuals" were going to be invoked twice, and I spent a fair bit of time in the debugger trying to figure out what was going on. See Listing 1 below which shows some code straight out of our solution.
  2. There’s no really simple way to do setup and teardown methods (that I’ve been able to find), and organising tests into hierarchies of fixtures was either not possible or too much effort at the time (I can’t remember which now).
My wrapper over boost's assert framework

As far as point number 2 goes, I tried a couple of options before settling for a solution that still seems very unsatisfactory to me. The first thing I tried was macro trickery - a newbie mistake. I had some tests which required a fairly elaborate setup (a smell in itself, I know) and I tried something like the following:

Listing 2 - When will we learn to stop leaning on the preprocessor? (Note: Production code retranslated into "pseudocode" to protect IP)

#ifndef SETUP_FOR_FIXTURE1
#define SETUP_FOR_FIXTURE1()                                                       \
                                                                                   \
// Declare an instance of the class under test                                     \
                                                                                   \
/* Comment line */                                                                 \
// Setup a temporary variable                                                      \
// Compute the sizeof a struct from our system                                     \
// Perform a calculation using a const and this struct size                        \
// 5 More lines of code!!! including setting up a boost::scoped_array<char>        \
// STT_CHECK_EQUAL(expected, actual);                                              \
#endif

There are still 5 tests in our system which call SETUP_FOR_FIXTURE1() at the top of the tests. In practice, this approach didn't hurt me too badly, mostly because it was only used in two "fixtures", and both pieces of code were fairly small - this one of 5 tests and another "fixture" with 3 tests (although there was one longish one with around 70 lines of code) and about 15 lines of setup code. However, just because I didn't get burned, it doesn't mean that I don't realize I was playing with fire. I quickly realised that debugging these things was difficult (the compiler would spew a message about an object that didn't even appear to be defined anywhere in the test), and the readability was pretty low (because the setups were defined in different header files in a different Visual Studio sub-folder).

So after writing in this style for a while, and struggling to reconcile the ease of setups and teardowns in NUnit with the monster I'd created in C++, I switched tack and tried a more object oriented approach - create one new class to act as a setup class (to setup our class under test), and another new class to act as a validator and overload methods on this class to assert the various properties of the class. However, instead of solving the setup/teardown problem, I'd just minimised its impact (I still had to declare the class under test (CUT), the setup class and the validator class in each test, and still had to make sure I passed the CUT around in the right order). I do have some excuses for doing this however:

  1. At the time, I was introducing my boss - the guy who'd basically written all the classes we were retrofitting tests for - to unit testing - so I was concentrating more on imparting the philosophy behind testing than making sure we had great code.
  2. In the heat of the moment, and due to the specific nature of the classes we were testing - I mostly forgot about the setup problem. The classes are a heirarchy about four or five layers deep (including a redundant, completely abstract base class) where most classes in the heirarchy are very similar and each only does a few things slightly different from its neighbours and ancestors. Also, there is no explicit (premeditated) use of the decorator pattern, but some of the classes aggregate others at different levels of the heirarchy and delegate first to this one, and then the result goes to that one, etc. - so I think I could refactor to decorators at some point in the future fairly easily. Because each of these classes was basically similar in most aspects, I was striving more not to repeat myself in terms of the asserts and validation at the bottom of the tests than in terms of setting the classes up. Overloaded validator methods would delegate back to the base case and then also validate whatever additional functionality the particular class in question was providing.
  3. The code itself was a real mess. For starters, it was hand-translated basically line for line from a Delphi prototype. Concepts which are taken for granted in Delphi which weren't readily available in C++ were written from scratch - including a horrendous container class to replace Delphi's StringList (If that even is a Delphi class), and we never once used the container to store simple strings. MFC code was scattered all over the place and had to be replaced with standard library equivalents or removed altogether. Use of the standard library was non-existent. In short, we were working without a safety net and making sure we didn't break anything and that was more important than not repeating ourselves.
  4. Refactoring in C++ is harder than in other languages. Although to be fair, most of my refactoring is still manual even in C#, doing it in C++ where #includes are all over the place and compile times take forever means longer feedback loops in the short term while you wait for the compiler to hook together 200K lines of C++ code.
  5. I simply didn't have the C++ experience that I do now (although I probably never would've admitted that at the time :))

I guess what I should have done was to use less of an "object-oriented" approach, and more of a "Resource Acquisition is Initialisation" (RAII) approach. An ideal solution would be to have a fixture class, and have its constructor setup the CUT, have the destructor do any cleanup (a-la a teardown method) and then have methods to test each piece of functionality - like a [Test] method in NUnit. In fact, this might have worked really well if we'd integrated templates into the picture. One test fixture class, templated based on the particular class we were testing, and coupled with specialization, might have solved a whole host of problems. In this case, the test "function" would instantiate the fixture, and then just call the correct test method on it - delegating immediately to the fixture class. This might have looked something like the following:

Listing 3 - A much more C++-like solution to the setup/teardown problem
Shapes.h

#ifndef SHAPES_H
#define SHAPES_H

struct Shape
{
  virtual int sides()=0;
  virtual int width()=0;
  virtual int height()=0;
  virtual int area()=0;
};

struct Triangle : public Shape
{
  virtual int sides() { return 3; }
  virtual int width() { return 50; }
  virtual int height() { return 100; }
  virtual int area() { return width() * height() / 2; }
};

struct Rectangle : public Shape
{
  virtual int sides() { return 4; }
  virtual int width() { return 100; }
  virtual int height() { return 50; }
  virtual int area() { return width() * height(); }
};

struct Square : public Rectangle
{
  virtual int width() { return height(); }
};

#endif
ShapeTestFixture.h

#ifndef SHAPE_TEST_FIXTURE_H
#define SHAPE_TEST_FIXTURE_H

#include <cassert>
#include "Shapes.h"

template <class T>
class ShapeTestFixture
{
private:
  T* shape;
public:
  ShapeTestFixture() { shape = new T(); }
  ~ShapeTestFixture() { delete shape; }
  void TestShapeSides() { std::cout << "!!!! Untested shape sides !!!! \n"; }
  void TestShapeArea() { std::cout << "!!!! Untested shape area !!!! \n"; }
};

void ShapeTestFixture<Rectangle>::TestShapeSides()
{
  std::cout << "Checking rectangle\n";
  assert(shape->sides() == 4);
  assert(shape->width() == 100);
  assert(shape->height() == 50);
}

void ShapeTestFixture<Rectangle>::TestShapeArea()
{
  assert(shape->area() == 5000);
}

void ShapeTestFixture<Square>::TestShapeSides()
{
  std::cout < "Checking square\n";
  assert(shape->sides() == 4);
  assert(shape->width() == 50);
  assert(shape->height() == 50);
}

void ShapeTestFixture<Square>::TestShapeArea()
{
  assert(shape->area() == 2500);
}

void ShapeTestFixture<Triangle>::TestShapeSides()
{
  std::cout << "Checking triangle\n";
  assert(shape->sides() == 3);
  assert(shape->width() == 50);
  assert(shape->height() == 100);
}

void ShapeTestFixture<Triangle>::TestShapeArea()
{
  assert(shape->area() == 2500);
}
#endif
RAIIUnitTest.cpp

#include "stdafx.h"
#include "ShapeTestFixture.h"
#include "Shapes.h"

int main(int argc, char* argv[])
{
  ShapeTestFixture<Rectangle> rectangleFixture;
  rectangleFixture.TestShapeSides();
  rectangleFixture.TestShapeArea();

  ShapeTestFixture<Square> squareFixture;
  squareFixture.TestShapeSides();
  squareFixture.TestShapeArea();

  ShapeTestFixture<Triangle> triangleFixture;
  triangleFixture.TestShapeSides();
  triangleFixture.TestShapeArea();

  std::cout << "Finished all tests\n";
}

Pretty simple right? There is still some duplication here - we need to manually instantiate the fixture we're interested in and then also manually call the correct test method, but the scope of duplication has been reduced greatly, and not by sacrificing readability.

There are however still a few improvements that we can make to this code - namely to drop boost::test completely in favour of NUnit. I've gotten that to work and I'm impressed by how quickly I was able to adapt my way of working to the way I do things in C#. I'll talk about that next time.