Should I test that methods don't throw exceptions?

Question

I'm making my first baby steps with unit testing and have written (among others) these two methods:

    [TestCase]
    public void InsertionSortedSet_AddValues_NoException()
    {
        var test = new InsertionSortedSet<int>();

        test.Add(5);
        test.Add(2);
        test.Add(7);
        test.Add(4);
        test.Add(9);
    }

    [TestCase]
    public void InsertionSortedSet_AddValues_CorrectCount()
    {
        var test = new InsertionSortedSet<int>();

        test.Add(5);
        test.Add(2);
        test.Add(7);
        test.Add(4);
        test.Add(9);

        Assert.IsTrue(test.Count == 5);
    }

Is the NoException method really needed? If an exception is going to be thrown it'll be thrown in the CorrectCount method too.

I'm leaning towards keep it as 2 test cases (maybe refactor the repeated code as another method) because a test should only test for a single thing, but maybe my interpretation is wrong.

I think you would need at least `Assert.IsTrue(true)` at the end of the _NoException test. And then you can see better that it is not needed. — H H, Jan 09 '12 at 12:50
I would rather test that a method did throw an exception if that was expected for some input. — Svish, Jan 09 '12 at 15:57
I think Dijkstra said it best: "Program testing can be used to show the presence of bugs, but never to show their absence!" — Daniel Pryden, Jan 09 '12 at 20:16
The accepted answer to this question is such good guidance, this question deserves some upvote love as well. — Tetsujin no Oni, Jan 13 '12 at 18:21

k.m · Accepted Answer · 2012-01-13T18:05:21.867

To put it in the most simple words, IMO testing what method does not do might be very slippery, as you can come up with more and more scenarios when you think about it. Going other way around tho, asserting that your code does stuff you intended it to do is pretty much purpose of unit testing.

There are two simple questions which usually help me spotting suspicious test and dealing with figuring out whether test makes any sense:

what part of desired functionality is test exercising?
what simple change can I make in tested class to break test?

Note that it's extremely easy to deal with those questions having second test (_CorrectCount) in mind. We haven't really seen Add method code, but we can most likely produce decent guess what could be changed to break that test. Tested functionality is even more obvious. Answers are intuitive and appear fast (which is good!).

Now let's try to answer those questions for the first test (_NoException). It immediately raises new questions (Is working code an actual functionality? Isn't it obvious? Isn't that implied? Isn't that what we always strive for? Why there is no assertion at the end? How can I make it fail?). For the second question it's even worse - breaking that test would probably require explicitly throwing exception... which we all agree is not the way to go.

Conclusion

Is simple. Second test is perfect example of well-written unit test. It's short, it tests single thing, it can be easily figured out. First test is not. Even tho it is just as short and (what seems to be) simple, it introduces new questions (while it really should answer already stated ones - Does Add actually add? Yes.) - and as a result brings unnecessary complexity.

Accepted this answer, as it exactly answers the question, raising a good point about unit testing too. — George Duckett, Jan 09 '12 at 13:27
This is a great answer - I did however almost like it more when it was short, sweet and straight to the point — kastermester, Jan 14 '12 at 21:54

score 6 · Answer 2 · answered Jan 09 '12 at 12:42

6

It makes much more sense to make a method that tests that the method under test throws exceptions when you expect it to. The first test you have asserts no behavior that the second test doesn't already cover.

answered Jan 09 '12 at 12:42

kastermester

3,058
6
28
44

So in your view it's not worth including a test that checks something runs with no exceptions, when that will run as part of another test? – George Duckett Jan 09 '12 at 12:46
@Kaster: Exactly. The second Testcase already checks for "no exception". And probably does it better. – H H Jan 09 '12 at 12:48
I think it's worth to test that there are no exceptions in rare cases, only in regression tests. When, for example, tested parts are not working under some conditions together. – Dmitry Polyanitsa Jan 09 '12 at 12:49
1

Like Henk (and the OP) says: The second test will fail in case any exceptions are thrown - why would you wanna duplicate your code with just removing the assert? It violates DRY in a big way... – kastermester Jan 09 '12 at 22:23

score 3 · Answer 3 · answered Jan 09 '12 at 12:40

3

I always test for both working and non-working cases. This way, you validate that the code works, returning the correct results, as well as handles errors in an expected manner.

answered Jan 09 '12 at 12:40

AlG

14,697
4
41
54

1

I understand what you're saying, and I check for invalid arguments etc. separately, but I'm not sure your answer applies in this case because in the first method I'm not checking results, I'm checking that the method runs ok. It's this check that I'm debating removing. – George Duckett Jan 09 '12 at 12:48
Depends on what your goal for the tests are. When I do my TDD I have tests for "expected" and "unexpected" behavior. I'd modify your first test to have assertions that show the data was inserted as expected - otherwise, why is it there? – AlG Jan 09 '12 at 21:16

score 1 · Answer 4 · answered Jan 09 '12 at 12:42

1

Imo, if you're sure on InsertionSortedSet list working (I'm not sure where it comes from), I would skip testing InsertionSortedSet_AddValues_NoException as you intend to do, if it so necessary.

For sure it's better to test as much as possible.

answered Jan 09 '12 at 12:42

Tigran

61,654
8
86
123

score 1 · Answer 5 · answered Jan 09 '12 at 12:43

There is no 100% correct answer here.

On the one hand you are right, a single test should test for a single thing.
This is specially true in cases where one of the tests might change in the future, and then you will not be able know for sure if you are checking for the other test to pass.

On the other hand, you are creating redundancy, as both test actually check for the same thing.
This redundancy is bad only in cases where your tests take too much time to run, but since (as it seems) you have only a few tests, this should not be a problem.

score 1 · Answer 6 · answered Jan 09 '12 at 12:46

There is no assertion nor expected exception in the first test, I think it should be refactored.

IMO, one test should check for the incorrect behavior (expecting an error to be thrown) and the other for the correct behavior (no exception, good value returned).

Should I test that methods don't throw exceptions?

6 Answers6

Conclusion

Linked