Skip to content.

plope

Personal tools
You are here: Home » Members » chrism's Home » Writing Bad Unit Tests
 
 

Writing Bad Unit Tests

We write bad unit tests sometimes.

Lots of times you'll see people write unit tests in the form:

   class TestMyObject(unittest.TestCase):

       def test_my_object_add(self):
           from my.package import MyObject
           framework = self._setupTheUnderlyingFramework()
           object = MyObject('myobject')
           framework.add(object)
           self.assertEqual(framework.objectIds(), ['myobject'])

There are two reasons the above test is wrong:

  • It doesn't actually test your object. It's testing the return value of your framework's "objectIds" method, and to some extent, the "add" method of the framework. If it's a framework you didn't write, you don't need to test the framework. And even if you did write the framework, you're not supposed to be testing it here, you're supposed to be testing your MyObject. Unit tests are not supposed to be about testing how your object interacts with the framework you're using. They're supposed to about testing the implementation of the objects you create.
  • Unit tests (as opposed to functional or integration tests) should rely on as little as possible. When you write code that requires an entire framework to be initialized before it can be tested, you've written improperly factored code. If your objects can't be tested without using some actual framework implementation, you've already failed some unwritten fundamental factoring test (or at least the framework you're using has). For the record, it doesn't matter if the framework itself has all its tests written in this way either, that doesn't make it ok.

The reason I know these things is because I used to write tests just like the one I show above. For an example of my sheer incompetence, see the tests in Zope's Products.Sessions.tests. The setUp of these tests actually create an OFS.Application object. One of these tests actually creates a ZODB storage to test a simple object. I originally wrote/contributed to these somewhere around 2001, and I now know enough to be ashamed of myself. I'm not going to rewrite them (I'm not that ashamed of myself, and besides, I've already helped write a replacement implementation). But the tests are horrid. I just didn't know any better.

On the other hand, I wrote the tests for repoze.who more recently. While they're not the paragon of virtue, they are far less stupid. Each test tests only the implementation of the thing it claims it's testing. Even when an object relies on another object in the same module, but doesn't "own" that object, we use dummy implementations of things to ensure that we don't actually test more (or less) than we intend to.

The correct way to write unit tests is to create "mock" or "dummy" objects for each external object your implementation depends on. For example, if your object expects to take some complicated subobject in its __init__ method and consults it thereafter when you call one of its methods, instead of doing:

  def test_something(self):
      from some.real.implementation import ComplicatedThing
      c = ComplicatedThing()
      from my.package import MyObject
      myob = MyObject(c)
      self.assertEqual(myob.methodThatConsultsComplicatedThing(), True)

You actually want do do:

  def test_something(self):
      c = DummyComplicatedThing()
      from my.package import MyObject
      myob = MyObject(c)
      self.assertEqual(myob.methodThatConsultsComplicatedThing(), True)

  class DummyComplicatedThing:
      def consult(self):
          return True

Note that now you really are only testing and depending on code that you wrote yourself, and you're not depending on the real complicated thing. Why this this good? The real complicated thing is probably complicated. For example, its consult method might look like this:

  class ComplicatedThing:
      def consult(self):
          vector1 = self.frobozz.getFleebar()
          vector2 = getUtility(self, ICramItDown).getFuzbaz(self)
          return vector1 == vector2

If you actually use this implementation, you're going to need to deal with the fact that it needs a frobnozz who's getFleebar method is willing to return a particular value, and you'll need to figure out how to make getUtility(self, ICramItDown) return something that has a getFuzbaz method that is willing to return another particular value. Then you'll need to make sure that the comparison of "vector1 == vector2" returns soemthing sensible for your test. In short, it's going to turn into a clusterfuck. It is add-only-code-to-be.

It may seem like more work to set up the dummy objects instead of relying on actual implementations, and often it is initially. But later on when you have hundreds or thousands of unit tests, they take on the order of minutes (as opposed to seconds) to complete, and you can't really figure out the tests when you come back to them later because much of the noise in the tests goes towards wiggling wires to set up real implementations in such a way that the tests will pass, you'll be in a bright world of shit. The only thing you'll be able to do is reimplement the tests properly, and you might as well have done that in the first place.

For reasons I mention above, I must note that ZopeTestCase and PloneTestCase are canonizations of bad unit testing practice. When you need to set up the entirety of Zope and/or Plone to test the implementation of a small bit of custom code, you've just fed yourself and all others who need to deal with your code in the future a huge helping of fail. It takes forever to run these sorts of tests, and you're never actually testing just your object implementations, you're also testing and potentially dealing with hundreds upon hundreds of other implementations, each of which is arbitrarily complex. Likewise if your unit tests rely on an actual database connection, or an actual mailserver, etc.

All that said, this is a subtle, subtle thing. Half the time, I'm still not 100% confident that I'm "doing it the right way". Some of my tests still just plain suck.

Created by chrism
Last modified 2008-05-29 02:15 AM

yes, but...

Absolutely agreed -- proper unit tests should test one thing and one thing only. But, please don't stop with the unit tests. You can absolutely prove that your code interacts correctly with your understanding of the framework as you've manifested it in a mock object. But if that understanding is flawed, passing unit tests can still be associated with broken code, once it comes time to run under the real framework. Functional and integration tests are important too!

ZTC/PTC are integration tests with a naming problem ;)

In my opinion, PloneTestCase/ZopeTestCase is more a victim of the circumstances than anything else — they really aren't unit tests, they are integration tests. Which (at least in the case of Plone) is just as important.

That of course doesn't mean that we shouldn't have real, lightweight unit tests too, but that's exactly what you're pointing out. Of course, maybe we should call them by their correct name, but that's ultimately something others have to decide on. I don't actually know if they should all be under the same testrunner or not. :)

Unit, integration, mock

As Alex points out, PloneTestCase is about integration testing. The "official" Plone testing tutorial even says so - http://plone.org/documentation/tutorial/testing.

Quite often, integration tests are more important in Plone. The first tests you write tends to be:

- does the product install?
- did it set up the necessary bits (new workflows, say)?
- can I create an object of this type in the intended location?

It's quite hard to mock up the underlying bits of Zope and Plone in order to properly write unit test for things like Archetypes objects. For declarative GenericSetup stuff, say, it's impossible, and integration tests are the only meaningful way of testing.

I certainly advocate writing simpler tests if at all possible, though. I often find it easier to write doctests in docstrings for adapters and utilities that contain isolated logic. That implies that logic is isolated in the first place, which is another worthy goal.

Finally, I've had good experiences in Java with using a mock objects library (EasyMock, in this case). I don't often see them used in Python, but they can make writing proper unit tests much faster. Having to type out your own mocks can be fairly time consuming and requires, as David said, a deep understanding of the framework. A mock object needs only declare what methods it expects to have called on itself, in what order, with what parameters. Often, that's a more meaningful way of specifying the contract between your component and the mocked one.

Martin