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