What is the best practice for checking exceptions in JUnit?

Question

I am trying my hand at writing test cases. From what I have read, my tests should fail from the start and I should strive to make tests pass. However, I find myself writing tests checking boundaries and the exceptions they should cause:

@Test(expected=NegativeArraySizeException.class)
public void testWorldMapIntInt() {
    WorldMap w = new WorldMap(-1, -1);
}

@Test(expected=IndexOutOfBoundsException.class)
public void testGetnIntnInt() {
    WorldMap w = new WorldMap(10,10);
    Object o = w.get(-1, -1);
}

However, this test passes by default because Java will throw the exception anyway. Is there a better way to handle these kinds of expected exceptions, possibly a way that fails by default-- forcing me to strive to handle these cases?

Well, the constructor of WorldMap creates an array of those dimensions, and trying to declare an array with negative indexes will throw an NegativeArraySizeException, and accessing negative indexes will throw an IndexOutOfBoundsException. So what I am wondering is, how can we write good tests for valid input without making tests which pass from the get-go. — , Sep 01 '11 at 15:38
Easy - you write this test before implementing the WorldMap constructor! — Tom Anderson, Sep 01 '11 at 15:41
More generally, i think it's sometimes okay to write tests that pass first time. If you're writing tests to drive the implementation, as you usually do, then of course they must fail initially. But if you then decide to go back and pin down the behaviour in corner cases, it's okay if they pass straight away. If you find yourself doing this a lot, it's a bad sign, but the red-green rule is not an iron one. — Tom Anderson, Sep 01 '11 at 15:42
I agree, but the test would begin to pass as soon as you wrote the first couple lines of code creating the array. It feels like I don't gain much helpful information from that. I always thought testing was to give a lot of information/feedback about your code. — , Sep 01 '11 at 15:44
@Google - that's only because this is quite an "easy" test to pass. When you don't have a constructor, the test fails. If you write the simplest possible constructor, the test fails. It's only once you start doing actual array-/index-related things in your `WorldMap` constructor that the test starts to pass. And it *should* pass, because at that point your implementation fulfils this particular part of the specification. You don't need to have **every** test fail right up until the last moment. — Andrzej Doyle, Sep 01 '11 at 15:48

score 3 · Accepted Answer · answered Sep 01 '11 at 15:37

3

I agree that the style you present is not so good. The problem is that it doesn't check where in the method the exception is thrown, so it's possible to get false negatives.

We usually write tests for exceptions like this:

public void testWorldMapIntInt() {
    try {
        WorldMap w = new WorldMap(-1, -1);
        Assert.fail("should have thrown IndexOutOfBoundsException");
    }
    catch (IndexOutOfBoundsException e) {}
}

answered Sep 01 '11 at 15:37

Tom Anderson

46,189
17
92
133

Thanks, this is certainly a better way to pin point the exception. I hadn't thought about pin pointing where they are coming from-- that helps get more information from the test~ – Sep 01 '11 at 15:46
Except that it goes against the more conventional `@Test(expected=...)` and also clutters up your test unnecessarily. I don't see why this is an improvement. If I saw this in a test, I'd change it to the other way. – Ryan Stewart Sep 01 '11 at 16:32
This way also let's you assert on the e.getMessage() to make sure that it says what you want it to. – TofuBeer Sep 01 '11 at 16:49

Ashkan Aryan · Answer 2 · 2011-09-02T08:05:26.050

2

Expected behaviour for WorldMap is to throw an exception if (-1, -1) passed into it
Initially it doesn't do that, so your test will fail as it does not see expected exception.
You implement the code for WorldMap correctly, including throwing exception when (-1, -1) passed in.
You rerun your test, it passes.

Sound like good TDD to me!

edited Sep 02 '11 at 08:05

answered Sep 01 '11 at 15:39

Ashkan Aryan

3,504
4
30
44

score 1 · Answer 3 · answered Sep 01 '11 at 15:35

That seems like a fair test to write. WorldMap is no standard Java class. Presumably it's your own class. Therefore the test wouldn't be passing if you hadn't already written some code. This test will force you to throw (or propagate) an appropriate exception from your class. That sounds like a good test to me, which you should write before implementing the behavior.

score 1 · Answer 4 · answered Sep 01 '11 at 15:36

1

I personally look for mistakes like that in the WorldMap constructor and throw an IllegalArgumentException, that way you can provide a better error message, such as what the value passed in was and what the expected range is.

As for having that test fail by default, I cannot think of a reasonable way of doing that if you are going to have it actually do something (if you are writing the tests first then it should fail because the constructor won't have any code).

answered Sep 01 '11 at 15:36

TofuBeer

60,850
18
118
163

I really like the idea of testing arguments. I hadn't thought of that and I think I will go with this route-- it feels like a majority of my test cases could be solved by simply testing arguments, and the cases will fail until I implement argument checking and exception throwing. I like it~ – Sep 01 '11 at 15:48

score 1 · Answer 5 · answered Sep 01 '11 at 16:43

Agree with accepted answer, try-fail-catch idiom, although ugly and cluttering the test, is much better than @Test(expcted=...) as it might report false positives.

A while back I implemented very simple JUnit rule to deal with exception testing in both safe and readable manner:

public class DefaultFooServiceTest {

    @UnderTest
    private FooService fooService = new DefaultFooService();

    @Rule
    public ExceptionAssert exception = new ExceptionAssert();

    @Test
    public void shouldThrowNpeWhenNullName() throws Exception {
        //given
        String name = null;

        //when
        fooService.echo(name);

        //then
        exception.expect(NullPointerException.class);
    }

    @Test
    public void shouldThrowIllegalArgumentWhenNameJohn() throws Exception {
        //given
        String name = "John";

        //when
        fooService.echo(name);

        //then
        exception.expect(IllegalArgumentException.class)
                .expectMessage("Name: 'John' is not allowed");
    }
}

See blog post and source.

Thanks-- I will give this a shot tonight. I see what you mean by avoiding false positives by coding the expected exception within a specific manner. Nice! :) — , Sep 02 '11 at 02:26

What is the best practice for checking exceptions in JUnit?

5 Answers5

Linked