16

First, apologies for the title, I couldn't think of the easiest way to explain it!

I have a method that I want to write unit tests for. I'm going to keep it fairly generic as I don't want to discuss the implementation of the method, just the testing of it. The method is:

public void HandleItem(item a)
{         
     CreateNewItem();
     UpdateStatusOnPreviousItem();
     SetNextRunDate();
}

So this class has one public method that then calls some private methods to perform the logic.

So when writing the unit test I want to check all three things have been done. As they are all called in the same run I thought that I could do it as one test:

public void GivenItem_WhenRun_Thenxxxxx
{
     HandleItem(item);
     // Assert item has been created
     // Assert status has been set on the previous item
     // Assert run date has been set
}

But I thought I could also write it as three separate tests:

public void GivenItem_WhenRun_ThenItemIsCreated()
{
    HandleItem(item);
}

public void GivenItem_WhenRun_ThenStatusIsUpdatedOnPreviousItem()
{
   HandleItem(item);
}

public void GivenItem_WhenRun_ThenRunDateIsSet()
{
     HandleItem(item);
}

So to me this seems nicer as it's essentially listing requirements, but then all three are related and do require exactly the same work performed on the tested method, so am running the same code 3 times.

Is there a recommended approach to take with this?

Thanks

29

There is a subtle difference between both approaches. In the first case, when the first Assert fails, the other two are not run any more. In the second case, all three tests are always run, even if one fails. Depending on the nature of the tested functionality, this might fit or not fit well to your case:

  • if it makes sense to run the three asserts independently from another, because when one fails, the other two might still not fail, then the second approach has the advantage you get the full test results for all 3 tests in one run. This can be beneficial if you have noteable build times, since it gives you a chance to fix up to 3 errors at once before doing the next build.

  • if, however, a failure of the first test will always imply the other two tests will also fail, then it is probably better to use the first approach (since it does not make much sense to run a test if you already know beforehand it will fail).

  • 2
    +1, good point. It didn't occur to me that build times can also be a bottleneck. – Kilian Foth Apr 24 '17 at 12:06
  • 1
    @KilianFoth: You're not working in C++ often enough :( – Matthieu M. Apr 24 '17 at 19:29
  • 1
    @MatthieuM.: to be fair, the question is tagged "C#" – Doc Brown Apr 24 '17 at 19:31
11

Short answer: It's much more important that your tests cover all functionality than how they do it.

Longer answer: If you still want to choose among these largely equivalent solutions, you can use ancillary criteria for what is best. For instance,

  • readibility: if the method does a lot of things that aren't closely related, a combined test might be hard to understand. However, the method itself might also be hard to understand, so maybe you should refactor the method rather than the test!)
  • efficiency: if executing the method takes a long time, that might be a weak reason for combining all three checks to save time
  • efficiency2: if running the setup code of your framework takes a long time, that could also be a weak reason for avoiding multiple test methods. (However, if this is really a problem, yuo should probably fix or change your test set-up - regression tests lose much of their value if you can't run them lightning-fast.)
2

Use one method call with multiple asserts. Here's why:

When you test HandleItem(a), you are testing that the method has brought the item into the correct state. Instead of "one assert per test", think "one logical concept per test".

Question: If a CreateNewItem fails, but the other two methods succeed, does this mean that HandleItem completed successfully? I'm guessing not.

With multiple asserts (with appropriate messages) you'll know exactly what has failed. You typically test a method multiple times for multiple inputs or input states, not to avoid multiple asserts.

IMO, these questions are typically a sign of something else. It's a sign that HandleItem isn't really something you can "unit test" since it seems to just delegate to other methods. When you're simply validating that HandleItem calls other methods correctly, it becomes more of an integration test candidate (in which case you'd still have 3 asserts).

You might want to consider making the other 3 methods public and testing them independently. Or even extracting them to another class.

0

Use the 2nd approach. With your 1st approach, if the test fails you won't know why right away, because it could be one of the 3 functions that failed. With the second approach you'll know right away where the problem occurred. You can put duplicate code inside your Test Setup function.

-1

IMHO you should test the three parts of this method separately so you know more specificly where things are going wrong when they do while avoiding going over the same part of your code twice.

-2

I don't think there is a strong case for writing separate test methods for your use case. If you want to get the results from all three variable conditions, you can test all three and print their failures by concatenating them in a string and asserting whether the string is still empty once you've completed the test. By keeping them all in the same method, your conditions and failure messages document the expected post-condition of the method in one place, rather than splitting it into three methods that might get separated later.

This does mean that your single test will have more code, but if you have so many post-conditions that that is a problem, you probably want to refactor your test methods to test individual methods inside HandleItem anyway.

protected by gnat Apr 25 '17 at 18:40

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.