CODEWITHSUNDEEP
c#unit-testingnunit
George Duckett
George Duckett

Reputation: 32428

Should I test that methods don't throw exceptions?

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.

Upvotes: 18

Views: 1188

Answers (6)

k.m
k.m

Reputation: 31454

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.

Upvotes: 18

ken2k
ken2k

Reputation: 48985

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).

Upvotes: 1

Svarog
Svarog

Reputation: 2208

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.

Upvotes: 1

kastermester
kastermester

Reputation: 3064

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.

Upvotes: 6

Tigran
Tigran

Reputation: 62246

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.

Upvotes: 1

AlG
AlG

Reputation: 15157

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.

Upvotes: 4

Related Questions