
Unit testing best practices - jrabone
http://howtodoinjava.com/2012/11/05/unit-testing-best-practices-junit-reference-guide/
======
ShaunCodeweaver
This article is wrong in numerous cases.

Testing private methods? Wrong. Don't do this. They'll be covered by your
public API, if they aren't, delete them. If you need to test code that is
private, you've got a missing abstraction. Pull that code out into a separate
class, make the method public.

Testing you get a null reference when you send in null to a method? So what?
You're testing Java/C#/etc... if you do this. Test for behaviour.

Need to use tear down methods? You're not unit testing anymore.

At least the article never recommended testing accessors... Again, please
don't.

~~~
CJefferson
I don't get some of your complaints.

I just had a quick look through my unit tests for place where I test a private
method.

I have a method which looks through an array for a pair of adjacent variables,
with a particular property (they have p < V1 > V2 < n for variables V1 and V2,
relating to the previous variable and next variable, and where < is a more
complicated operator).

Anyway, this gets used lots of times by my algorithm. However, I added some
unit tests because I wanted to make sure that the obvious corner cases
(beginning and end of array, not occurring) get checked correctly. Hopefully
these cases are hit by my input data, but it is hard to construct input data
that has particular properties.

I could pull this out into another class, but the method is actually fairly
small and not used anywhere else, so that seems like a bit of a waste really.

On checking you get a null reference when you pass null into a method. I make
this part of the API of some of my functions (in my case C++, so null pointer
actually). I want to check that actually happens, as opposed to some nasty
crash. Isn't that checking for behaviour? What else should I do instead?

~~~
ShaunCodeweaver
"Anyway, this gets used lots of times by my algorithm"

You've got a missing class. The fact you're so concerned with testing it
highlights this.

The fact it's private I should be free to rename, add or remove parameters. If
I did this to your private method, I'd break unit tests, despite the code
being private.

For the null reference problem, it depends on your language. But the point
remains the same. For public API's, e.g something you let the outside world
consume, checking for null etc.. will be required. For internal code, don't
bother. Go as high as you can in the stack and fix the root cause of the null
reference in the first place.

~~~
CJefferson
I've got a missing class, for a 5 line function called from one place? I
suppose I can see your point of view, but I'm not sure I agree.

On your second point, I do disagree. I like to view all functions, even
internals one, as having a documented API, covering things like bad inputs.
This means that all functions (in debugging/assertion mode at least) promise
to sanity-check their inputs for null pointers, etc.

In C++ at least I find this a necessity, as a single wrong pointer can cause
horrible memory corruption all over my code. The whole point of these
assertions is to allow me to find the place in the stack where the null
pointers are coming from without pulling all my hair out in the process!

~~~
lmm
That's not the right way to think about internal APIs. To make changes
effectively you need to be willing to redefine those boundaries with a minimum
of friction. If you make each function check its inputs then you won't be able
to decompose your problem into small enough functions (a good function is
usually <10 lines), and you'll mostly be duplicating your own work.

Null is probably the worst mistake in programming history; never use it
internally, never call an internal function with it, and then you don't need
to make your functions check for it. If you're really paranoid you can use a
static analysis tool to enforce that you're doing it right.

~~~
CJefferson
Just Null might be a bit limited, however there are many other similar issues
(some examples I can think of include functions which take a positive integer,
or a non-empty container, or a sorted container, or two containers of equal
size, or an integer which is smaller than the size of another argument, which
is a container).

I used to be very slack on internal checks. As times goes by I have pushed
more of these into the type system where appropriate, and as checks. While
unit tests on your external API can be useful, it can be hard to ensure
coverage of all the nasty corner cases of your internals, some of which are
only hit one time a million, or hundred million.

I find having all these checks makes me more confident about changing my
internal APIs. If anyone is calling a function inappropriately for it's new
interface, I will quickly get an assertion flagged up, as opposed to code
carrying on calling a function incorrectly.

Just so we aren't talking at cross purposes, I feel I should say most of my
programming is algorithmic, particularly in AI. In this field it can be
devilishly difficult to construct good unit tests and ways of hitting bits of
algorithms -- I am currently working on an algorithm which has function which
I believe is required but which I cannot construct test data for my external
API which causes it be called. Always a little worrying!

~~~
lmm
>As times goes by I have pushed more of these into the type system where
appropriate

Yeah, the type system is the appropriate place for most of these (including
null even).

>I find having all these checks makes me more confident about changing my
internal APIs. If anyone is calling a function inappropriately for it's new
interface, I will quickly get an assertion flagged up, as opposed to code
carrying on calling a function incorrectly.

I think we may be talking at cross purposes wrt "public". I don't mean "only
ever test your external API". I do mean "only ever test public methods of a
class, in the technical sense of public".

Of course a large project will end up with lower-level and higher-level
layers, and eventually you do need to define more rigid boundaries between the
two to avoid everything becoming an unmaintainable mess, and testing at these
boundaries is entirely appropriate. But a single class should belong to one
layer or another.

>I am currently working on an algorithm which has function which I believe is
required but which I cannot construct test data for my external API which
causes it be called. Always a little worrying!

More than a little. My advice (not that you need it) would be to try and
encode the constraint that leads to it not being called in the type system,
then push it up through the layers.

------
programminggeek
I'll add a couple of my own "best practices".

Use Guard to run tests after each file change - Constantly running and re-
running your tests in the background lets you know faster when you break
something, but also forces you to keep your tests fast.

Use a visual test runner - Using guard is great, but I also like a nice HTML
page I can reload that ends up being red/green based on your tests. Going from
red to green can be deeply satisfying for reasons I don't fully understand.
RSpec has a nice HTML output for ruby.

Use a visual code coverage tool - Having 80% or 90% code coverage is a useless
statistic, but having a tool that shows you what part of the code is being
tested and what isn't is extremely useful. If nothing else it tells you where
you need to add tests or what part of your code is hard/impossible to test. I
like SimpleCov for ruby.

------
beothorn
"Create unit tests that target exceptions" ,
@Test(expected=NullPointerException.class) Don't do this, any point of your
test can throw this exception. A try/catch with an assert avoids false
negatives and documents where you were expecting the exception to be thrown.
edit: typo

~~~
ajanuary
I'd suggest that if you need to document where is throwing the exception,
you're doing too much work in the test.

Setup code should be pretty simple and stable, so should very rarely suddenly
start throwing new exceptions.

If you need to pinpoint which bit of code under test threw the exception,
you're probably testing too much in one test.

Of course, the advice changes when you're using a unit testing framework as a
runner for integration tests.

------
ajanuary
> I will recommend to use Exception class and do not use specific subclasses
> of Exception. This will increase the test coverage also.

Firstly, it doesn't increase test coverage. You're still executing the same
code paths, just weakening your assertions about the methods behavior.

Secondly, even if it did, what's the point of increasing test coverage if
you're not actually saying anything useful about the contracts of your
methods?

Coverage isn't the end goal that you slavishly need to satisfy, it's a useful
indicator of how thorough your tests are. You don't make your tests more
thorough by weakening your assertions.

------
ExpiredLink
> Unit testing is not about finding bugs

Of course it is!

> Mock out all external services and state

... unless those 'external services' are an essential part of you application
(e.g. databases).

> Don’t unit-test configuration settings

Unit-test anything that needs to be unit-tested.

> All methods, regardless of visibility, should have appropriate unit tests

Capable of being misunderstood ...

> Aim for each unit test method to perform exactly one assertion

This limitation makes no sense.

> Do not print anything out in unit tests

Arbitrary restriciton.

> Capture results using the XML formatter

But why?

tl;dr Thanks, but I stick to my own tried and proven Unit-test guidelines.

