unit tests

Justin Harris jharris at redhat.com
Mon Jul 12 20:08:25 UTC 2010


----- "Adam Young" <ayoung at redhat.com> wrote:

> On 07/12/2010 03:47 PM, Jesus Rodriguez wrote:
> > While working in ConsumerResourceTest recently, I noticed that
> > it requires the DB. I started thinking (yeah imagine that) :)
> > that maybe we shouldn't require the DB for anything except
> > the curators.
> >
> > Another thing I noticed is we have 3 ways of creating a
> > test consumer:
> >
> > * direct ctor i.e. new Consumer(...)
> > * TestUtil.createConsumer
> > * consumerCurator.create(...)
> >
> > I prefer the first one or the second if it is used by many
> > places aside from this particular Test class.  The only
> > thing that should call the curators are the curator tests
> > the rest should use mockito to mock them out.
> >
> > The ConsumerResourceTest requires a lot of infrastructure,
> > like the database, curators, services etc. While guice makes
> > some of this easier, it makes for a slow and brittle test.
> >
> > I replaced the testCreateConsumer (and 2 others) with mocks.
> > http://pastie.org/1041394
> >
> > The original test 'looked' simpler: http://pastie.org/1041401
> > but it relied on a lot of stuff from the @Before method.
> > And the problem I ran into was that this test uses a
> > StubIdentityService which does not behave like our
> > DefaultIdentity Service adapter nor any other.
> >
> > My proposal is that we make the resource unit tests
> > use mocks as much as possible to test the interaction.
> > And the only thing that should use the DB should be the
> > Curator tests.
> >
> > Thoughts? comments?
> >
> >    
> I'm not on the project anymore, but I heartily endorse this approach.
> 
> Testability is one of the best side effects of using GUice.

Yes absolutely - I think this is a big point that Dmitri made a few weeks back, as well.
We also get the benefit of faster tests, and hopefully more loosely coupled classes.

Along these lines, there are a few other things I would suggest (warning: personal opinion)

* Don't just autopilot a test<method_name> for single method - this is unlikely to get all
  branching logic and doesn't take multiple inputs into account.  Especially if there are 
  null input cases or any arithmetic that has to happen, write a bunch of different test cases
  with crazy inputs so that we know that the method behaves appropriately.

* Along those lines - we don't have to keep a 1-to-1 mapping between test class and class.  It could make more
  sense to split tests between multiple test classes to make the setup/teardown more appropriate to similar
  situations.

* As much as possible, make a single assertion per test case.  I know this sounds crazy at first, but
  it really does make test cases more useful when a test fails - there is seldom a need to go through
  and debug the test case to find which assertion is failing and why.

 - Justin

> _______________________________________________
> candlepin mailing list
> candlepin at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/candlepin



More information about the candlepin mailing list