[Insight-developers] Test framework

[Steve M. Robbins]

On Tue, Dec 30, 2008 at 08:41:14AM -0500, Bill Lorensen wrote:
> I have looked a bit more at both proposed test frameworks. I'm not
> sure how helpful they will be for itk testing.
> For example, the google testing has, as Dan pointed out, several ctest
> like features. Well, we don't need ctest features in another test
> framework since we already have ctest.

Ctest is nothing but a test driver, or a "test runner".  That is the
least relevant part of a testing framework, from my point of view.  I
am confident a suitable test framework could be adapted to work with
ctest.

Most important for test-driven-development is the effort required to
write a test case and test suite.  The current ITK test suite is
horribly verbose, full of "if" statements, writing to std::cout,
try/catch blocks and the like.  This kind of code overhead (1)
distracts from the testing intent, (2) adds more non-test code into
which bugs creep, and (3) is simply a lot more code to write.  All of
these things add to the burden of producing tests, leading to less
coverage than you'd like.

The huge benefit of a test framework such as Googletest is that it has
a large and regular system of test assertions: EXPECT_EQ, EXPECT_TRUE,
etc.  This may seem a small benefit, at first glance, but in reality
it is paradigm-changing.  To see why, let's look at a current ITK
test from itkImageRegtionTest.cxx:

  // Take slices of a region
  try
    {
    SliceRegionType sliceA;
    sliceA = regionA.Slice(0);
    std::cout << "regionA.Slice(0): " << sliceA;
    }
  catch (itk::ExceptionObject &err)
    {
    std::cout << "Caught unexpected exception" << err;
    return EXIT_FAILURE;
    }

What is this really testing?  We take a slice from regionA and print
it to cout.  Is anyone reading cout?  How do we know the correct
answer was produced?  We don't.  All we really know is that it didn't
segfault or throw.  That's not a true regression test.

Compare the above to this:

    EXPECT_EQ( slice0, volume.Slice(0) );

Here we are still testing that no exception is thrown -- without the
try/catch baggage -- but more importantly we compare the result of
volume.Slice(0) to the EXPECTED RESULT slice0.  This is a much better
test and more concise to boot.

Oh, and the existing test aborts on failure so the following tests are
not run.  EXPECT_EQ() continues, letting the subsequent tests run.
This is helpful since it can point out several errors in one run
rather than having to fix one bug, rebuild, re-test, only to find a
second bug whose test was not run the first time.


So why is the test framework paradigm changing?  Because it is now
*SO* convenient to write a test, you are able to write many more tests
for less effort.  To continue the example, the current test creates a
volume and checks that slices can be obtained for axes 0, 1, and 2.
Then it tests that slicing on an illegal axis throws.  However, only
one out-of-bounds axis is tested.  When dealing with methods that take
a bounded integer, it is more prudent to check for off-by-one errors
on each end, like so:

    EXPECT_THROW( volume.Slice(-1), itk::ExceptionObject );
    EXPECT_THROW( volume.Slice(3), itk::ExceptionObject );

Notice how easy this is: one line for each test, rather than 12.
I'll wager that the cost of duplicating 12 lines often leads to
the decision not to test adequately.

Not only is there less code, but it is self documenting.  You can see
immediately the intent of the test.  It takes *much much longer* to
read the 12 lines in the current test and understand it is testing
that an exception is thrown for an out of bounds argument.

There really is no substitute for a well-designed test framework.


> Finally, for many of our classes, writing a test is very similar to
> using the class in an application. This is a good exercise for the
> developer and illustrates how the filter can be used. 

I agree completely that ONE test case is to use the class as it would
be used in an application.  But you really want to test lots of edge
cases and error cases as well.  As mentioned above, the verbosity of
the current situation can result in fewer tests than necesssary.  For
example, the current code also tests the method IsInside() -- but with
only one example where regionB is well inside regionA.  Where are the
tests for edge cases?

100% line coverage is a good start, but we really need to approach
100% code path coverage.


> I think the google tests appear a bit more cryptic.

It can appear so at first.  You definitely need to be familiar with
the test framework to read them.  But the good news is that there
isn't much to know: the large majority of the tests will be using
EXPECT_EQ, EXPECT_TRUE, or EXPECT_FALSE.  Those are pretty much
self-explanatory.

I would counter that itkImageRegtionTest is the cryptic one.  It
consists of a single 110 line function containing 6 tests.  That's a
lot of reading to parse out each of the tests and understand what is
being tested.


> Perhaps we should look at the two proposed frameworks and provide some
> of their ideas into our testing framework.

Perhaps.  But why reinvent the wheel?  Googletest, for example, is
about 67k LOC.  Rather than reproduce all that, it would take a
comparatively trivial amount of time to bolt Googletest into the
existing ITK test framework.

Elsewhere, someone was fretting that Google doesn't provide a
CMakeLists.txt file.  Well, you don't need one: they provide a single
file to build; quoting from the README:

  ### Using Your Own Build System ###
  If none of the build solutions we provide works for you, or if you
  prefer your own build system, you just need to compile
  src/gtest-all.cc into a library and link your tests with it.

Dan already said it, but I also believe adopting the work of other
smart people is the way to go.  Googletest is very much a live project
so, if selected, you get the added benefit of future bugfixes and
enhancements.

Best Regards,
-Steve

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20081230/571ee6a5/attachment.pgp>