improving unit test code

I ran into a problem with some of the unit tests last week
and ended up 
having to do more work than I wanted to in order to figure
out what was 
wrong and how to fix it.  Part of the problem I had to solve
was working 
around some quirks in the way the unit tests run and after
doing so I am 
currently of the opinion that we would be better served to
make some 
adjustments to the way the unit test code is currently layed
out.

I am not a unit test writing guru by any means, so if anyone
has more 
experience then feel free to chime in.  These are a few of
the things 
that I found to be troublesome with the current tests ...

1. Tests are too complex.  Many of the current unit test
methods are too 
complex and so it often requires extra effort to dig into a
unit test to 
figure out where it really failed.  This can become even
more 
troublesome when tests start throwing exceptions rather than
simply 
failing assertions.

2. Tests are not designed to properly make use of the
setUp() and 
tearDown() methods.  Many of the unit tests actually to work
in the test 
method which should ideally be part of the setup and
teardown process. 
This is part of what leads to #1, where tests end up being
more complex 
than needed, and it also leads to more problems when tests
fail for some 
reason.  Normally when a test fails at any point the test is
immediately 
terminated and therefore any teardown logic that was at the
bottom of 
the test will not actually be run.  At times this can cause
a simple 
failed assertion in a unit test to have a ripple effect
which causes 
problems in subsequent tests.

3. Poor exception handling and logging.  Another problem I
ran into was 
situations where we weren't properly handling exceptions and
logging 
messages which also made it more difficult to track down a
problem once 
it occurred.  In particular, throwing exceptions from the
setUp() and 
tearDown() methods is problematic because those 2 methods
should always 
be expected to complete properly.  If they do not then we
should handle 
those cases in a more appropriate fashion and log the
reasons why they 
failed so that a root cause can be more easily determined.

So, based on these issues I would like to make a few 
changes/recommendations on how the unit tests are written
moving forward ...

1. Write small and concise tests.  You should not be writing
much (if 
any) setup/teardown logic inside your test.  If you are then
it should 
be moved out into the setUp() and tearDown() methods.

2. Don't lump all tests into a single file.  Just because a
group of 
tests are all related to testing functionality of weblog
entries doesn't 
mean they have to be in the same file.  Separate out tests
into multiple 
class files particularly based on the setup and teardown
needs of the 
tests being run.  If you are trying to add a test to a file
which 
already has setUp() and tearDown() methods which don't fit
your needs, 
then put that test into a new file.

3. Don't throw exceptions from the setUp() and tearDown()
methods. 
Catch those exceptions and log them properly so that the
error can be 
quickly fixed.

4. Add logging messages to indicate the BEGIN & END of
each test.  This 
makes it *soooo* much easier for someone crawling through
the log file 
to tell exactly what has been happening and where.

do all those things make sense?

-- Allen

Can you give some specific test class examples?  I haven't
dug into
any tests in quite some time, so it probably wouldn't hurt
for me to
look at them.

Thanks,

Matt

On 1/29/07, Allen Gilliland <allen.gillilandsun.com> wrote:
> I ran into a problem with some of the unit tests last
week and ended up
> having to do more work than I wanted to in order to
figure out what was
> wrong and how to fix it.  Part of the problem I had to
solve was working
> around some quirks in the way the unit tests run and
after doing so I am
> currently of the opinion that we would be better served
to make some
> adjustments to the way the unit test code is currently
layed out.
>
> I am not a unit test writing guru by any means, so if
anyone has more
> experience then feel free to chime in.  These are a few
of the things
> that I found to be troublesome with the current tests
...
>
> 1. Tests are too complex.  Many of the current unit
test methods are too
> complex and so it often requires extra effort to dig
into a unit test to
> figure out where it really failed.  This can become
even more
> troublesome when tests start throwing exceptions rather
than simply
> failing assertions.
>
> 2. Tests are not designed to properly make use of the
setUp() and
> tearDown() methods.  Many of the unit tests actually to
work in the test
> method which should ideally be part of the setup and
teardown process.
> This is part of what leads to #1, where tests end up
being more complex
> than needed, and it also leads to more problems when
tests fail for some
> reason.  Normally when a test fails at any point the
test is immediately
> terminated and therefore any teardown logic that was at
the bottom of
> the test will not actually be run.  At times this can
cause a simple
> failed assertion in a unit test to have a ripple effect
which causes
> problems in subsequent tests.
>
> 3. Poor exception handling and logging.  Another
problem I ran into was
> situations where we weren't properly handling
exceptions and logging
> messages which also made it more difficult to track
down a problem once
> it occurred.  In particular, throwing exceptions from
the setUp() and
> tearDown() methods is problematic because those 2
methods should always
> be expected to complete properly.  If they do not then
we should handle
> those cases in a more appropriate fashion and log the
reasons why they
> failed so that a root cause can be more easily
determined.
>
> So, based on these issues I would like to make a few
> changes/recommendations on how the unit tests are
written moving forward ...
>
> 1. Write small and concise tests.  You should not be
writing much (if
> any) setup/teardown logic inside your test.  If you are
then it should
> be moved out into the setUp() and tearDown() methods.
>
> 2. Don't lump all tests into a single file.  Just
because a group of
> tests are all related to testing functionality of
weblog entries doesn't
> mean they have to be in the same file.  Separate out
tests into multiple
> class files particularly based on the setup and
teardown needs of the
> tests being run.  If you are trying to add a test to a
file which
> already has setUp() and tearDown() methods which don't
fit your needs,
> then put that test into a new file.
>
> 3. Don't throw exceptions from the setUp() and
tearDown() methods.
> Catch those exceptions and log them properly so that
the error can be
> quickly fixed.
>
> 4. Add logging messages to indicate the BEGIN & END
of each test.  This
> makes it *soooo* much easier for someone crawling
through the log file
> to tell exactly what has been happening and where.
>
> do all those things make sense?
>
> -- Allen
>
>


-- 
http://raibledesigns.com

certainly.  pretty much any of the business unit tests will
serve as an 
example, but some good ones are WeblogEntryTest and
WeblogCategoryTest. 
  Both of those have quite a few methods running complex
tests in them 
and many of the methods include some amount of setup and
teardown as 
indicated by comments like "// add some XXX to test
with".

-- Allen


Matt Raible wrote:
> Can you give some specific test class examples?  I
haven't dug into
> any tests in quite some time, so it probably wouldn't
hurt for me to
> look at them.
> 
> Thanks,
> 
> Matt
> 
> On 1/29/07, Allen Gilliland <allen.gillilandsun.com> wrote:
>> I ran into a problem with some of the unit tests
last week and ended up
>> having to do more work than I wanted to in order to
figure out what was
>> wrong and how to fix it.  Part of the problem I had
to solve was working
>> around some quirks in the way the unit tests run
and after doing so I am
>> currently of the opinion that we would be better
served to make some
>> adjustments to the way the unit test code is
currently layed out.
>>
>> I am not a unit test writing guru by any means, so
if anyone has more
>> experience then feel free to chime in.  These are a
few of the things
>> that I found to be troublesome with the current
tests ...
>>
>> 1. Tests are too complex.  Many of the current unit
test methods are too
>> complex and so it often requires extra effort to
dig into a unit test to
>> figure out where it really failed.  This can become
even more
>> troublesome when tests start throwing exceptions
rather than simply
>> failing assertions.
>>
>> 2. Tests are not designed to properly make use of
the setUp() and
>> tearDown() methods.  Many of the unit tests
actually to work in the test
>> method which should ideally be part of the setup
and teardown process.
>> This is part of what leads to #1, where tests end
up being more complex
>> than needed, and it also leads to more problems
when tests fail for some
>> reason.  Normally when a test fails at any point
the test is immediately
>> terminated and therefore any teardown logic that
was at the bottom of
>> the test will not actually be run.  At times this
can cause a simple
>> failed assertion in a unit test to have a ripple
effect which causes
>> problems in subsequent tests.
>>
>> 3. Poor exception handling and logging.  Another
problem I ran into was
>> situations where we weren't properly handling
exceptions and logging
>> messages which also made it more difficult to track
down a problem once
>> it occurred.  In particular, throwing exceptions
from the setUp() and
>> tearDown() methods is problematic because those 2
methods should always
>> be expected to complete properly.  If they do not
then we should handle
>> those cases in a more appropriate fashion and log
the reasons why they
>> failed so that a root cause can be more easily
determined.
>>
>> So, based on these issues I would like to make a
few
>> changes/recommendations on how the unit tests are
written moving 
>> forward ...
>>
>> 1. Write small and concise tests.  You should not
be writing much (if
>> any) setup/teardown logic inside your test.  If you
are then it should
>> be moved out into the setUp() and tearDown()
methods.
>>
>> 2. Don't lump all tests into a single file.  Just
because a group of
>> tests are all related to testing functionality of
weblog entries doesn't
>> mean they have to be in the same file.  Separate
out tests into multiple
>> class files particularly based on the setup and
teardown needs of the
>> tests being run.  If you are trying to add a test
to a file which
>> already has setUp() and tearDown() methods which
don't fit your needs,
>> then put that test into a new file.
>>
>> 3. Don't throw exceptions from the setUp() and
tearDown() methods.
>> Catch those exceptions and log them properly so
that the error can be
>> quickly fixed.
>>
>> 4. Add logging messages to indicate the BEGIN &
END of each test.  This
>> makes it *soooo* much easier for someone crawling
through the log file
>> to tell exactly what has been happening and where.
>>
>> do all those things make sense?
>>
>> -- Allen
>>
>>
> 
> 

On 1/29/07, Allen Gilliland <allen.gillilandsun.com> wrote:
> I ran into a problem with some of the unit tests last
week and ended up
> having to do more work than I wanted to in order to
figure out what was
> wrong and how to fix it.  Part of the problem I had to
solve was working
> around some quirks in the way the unit tests run and
after doing so I am
> currently of the opinion that we would be better served
to make some
> adjustments to the way the unit test code is currently
layed out.

... etc. ...

> So, based on these issues I would like to make a few
> changes/recommendations on how the unit tests are
written moving forward ...
>
> 1. Write small and concise tests.  You should not be
writing much (if
> any) setup/teardown logic inside your test.  If you are
then it should
> be moved out into the setUp() and tearDown() methods.
>
> 2. Don't lump all tests into a single file.  Just
because a group of
> tests are all related to testing functionality of
weblog entries doesn't
> mean they have to be in the same file.  Separate out
tests into multiple
> class files particularly based on the setup and
teardown needs of the
> tests being run.  If you are trying to add a test to a
file which
> already has setUp() and tearDown() methods which don't
fit your needs,
> then put that test into a new file.
>
> 3. Don't throw exceptions from the setUp() and
tearDown() methods.
> Catch those exceptions and log them properly so that
the error can be
> quickly fixed.
>
> 4. Add logging messages to indicate the BEGIN & END
of each test.  This
> makes it *soooo* much easier for someone crawling
through the log file
> to tell exactly what has been happening and where.
>
> do all those things make sense?

+1

I think you've correctly identified (some of the) problems
with the
unit tests and all 4 of those recommendations make sense.
The thing
that bugs me the most is when a test fails, tearDown fails
and you
then get a string of usernameInUse errors for the rest of
the tests
instead of valid test results.

- Dave

Dave wrote:
> On 1/29/07, Allen Gilliland <allen.gillilandsun.com> wrote:
>> I ran into a problem with some of the unit tests
last week and ended up
>> having to do more work than I wanted to in order to
figure out what was
>> wrong and how to fix it.  Part of the problem I had
to solve was working
>> around some quirks in the way the unit tests run
and after doing so I am
>> currently of the opinion that we would be better
served to make some
>> adjustments to the way the unit test code is
currently layed out.
> 
> ... etc. ...
> 
>> So, based on these issues I would like to make a
few
>> changes/recommendations on how the unit tests are
written moving 
>> forward ...
>>
>> 1. Write small and concise tests.  You should not
be writing much (if
>> any) setup/teardown logic inside your test.  If you
are then it should
>> be moved out into the setUp() and tearDown()
methods.
>>
>> 2. Don't lump all tests into a single file.  Just
because a group of
>> tests are all related to testing functionality of
weblog entries doesn't
>> mean they have to be in the same file.  Separate
out tests into multiple
>> class files particularly based on the setup and
teardown needs of the
>> tests being run.  If you are trying to add a test
to a file which
>> already has setUp() and tearDown() methods which
don't fit your needs,
>> then put that test into a new file.
>>
>> 3. Don't throw exceptions from the setUp() and
tearDown() methods.
>> Catch those exceptions and log them properly so
that the error can be
>> quickly fixed.
>>
>> 4. Add logging messages to indicate the BEGIN &
END of each test.  This
>> makes it *soooo* much easier for someone crawling
through the log file
>> to tell exactly what has been happening and where.
>>
>> do all those things make sense?
> 
> +1
> 
> I think you've correctly identified (some of the)
problems with the
> unit tests and all 4 of those recommendations make
sense. The thing
> that bugs me the most is when a test fails, tearDown
fails and you
> then get a string of usernameInUse errors for the rest
of the tests
> instead of valid test results.

yep.  that's the one that caused me the most problems this
last time i 
was working on the testing code and i would love to see that
problem go 
away.

-- Allen


> 
> - Dave