views:

134

answers:

7

I've just finished reading Roy Osherove's "The Art of Unit Testing" and I am trying to adhere to the best practices he lays out in the book. One of those best practices is to not use multiple asserts in a test method. The reason for this rule is fairly clear to me, but it makes me wonder...

If I have a method like:

public Foo MakeFoo(int x, int y, int z)
{
     Foo f = new Foo();
     f.X = x;
     f.Y = y;
     f.Z = z;

     return f;
}

Must I really write individual unit tests to assert each separate property of Foo is initialized with the supplied value? Is it really all that uncommon to use multiple asserts in a test method?

FYI: I am using MSTest.

EDIT: Thanks for all the responses. I think I'll wind up going with the multiple asserts. In my situation the behavior being tested is that MakeFoo makes a proper Foo. So asserting that each property is getting the expected value should suffice. However, if there was a condition around setting one of the properties then I'd test each of the individual outcomes separately.

I still don't like it though.... The reason I like the idea of having one assert per test is that you know the exact reason a test failed. If you fix the problem then the test will pass. With multiple asserts you don't have the same guarantee. If you fix the problem alluded to by the failing assertion, there is nothing to stop another assertion later on in the test from failing next. If the asserts were split up, then you'd have known about both failures from the start.

And finally, the reason I'm not just using .Equals() is because in my situation Foo is a LINQ-To-SQL entity which introduces some complications that aren't worth getting into here.

Thank again.

+3  A: 

In situations like this, if I'm trying to be strict about one assert per test, I'll assert equality on the Foo, rather than its components. That drives me to write Foo.equals(), and that's usually a good thing in and of itself. But I'm not always strict about one assert per test. See what feels good to you.

Carl Manaster
+1  A: 

It's common to use multiple asserts in a test, but I don't feel that it's a "best practice."

I would say that yes, you do want to use multiple tests for the above function, exactly so you can see where the problem is. I've recently been working with a fairly large test suite that has some weird failures in it that I have yet to track down, and the problem I'm experiencing is that the test cases each do too much. I've split them up, and now I can disable the specific ones that are failing so that I can get back to them when I have the opportunity.

However, you can still use a fixture to factor out the commonality of your setup and teardown. I can't speak to MSTest, but in UnitTest++ you'd do something like:

class FooFixture
{
public:
   FooFixture() : f(MakeFoo(kX, kY, kZ)) { }

   static const int kX = 1;
   static const int kY = 2;
   static const int kZ = 3;

   Foo f;
};

TEST_FIXTURE(FooFixture, IsXInitializedCorrectly)
{
   CHECK_EQUAL(kX, f.X);
}

// repeat for Y and Z

It's not a ton more typing, especially given cut&paste. Heck, in vim it's just esc y ctrl-{ pp and then minor edits. With this setup, though, you can see if it's just one field failing and then dig in to see why.

dash-tom-bang
multiple asserts have bitten me before, especially after the SUT get complicated
Gutzofter
SUT? Stupid Unit Tests?
dash-tom-bang
LOL, no it's System Under Test.
Igor Zevaka
A: 

I typically find myself having multiple asserts in a method, but they're usually all related to the test at hand. I sometimes chuck in an assert to verify a precondition that I know will break the test because I don't want the test to succeed by accident.

[Test] // NUnit style test.
public void SearchTypeAndInventory() 
{
    var specification = new WidgetSearchSpecification 
                    {
                        WidgetType = Widget.Screw,
                        MinimumInventory = 10
                    }; 
var widgets = WidgetRepository.GetWidgets(specification);
if( !widgets.Any() ) 
    Assert.Inconclusive("No widgets were returned.");
Assert.IsTrue( 
    widgets.All( o => o.WidgetType == Widget.Screw), 
    "Not all returned widgets had correct type");
Assert.IsTrue( 
    widgets.All( o => o.InventoryCount >= 10), 
    "Not all returned widgets had correct inventory count.");

* While I could have combined the Asserts, I find it's more useful to separate what went wrong.


I don't think sticking to a hard rule of 1-assert-per-test is very useful. What's more important is that a test is only testing 1 thing. I've seen many uber-tests that are one enormous method that tests everything about a class. These uber-tests are fragile and hard to maintain.

You should think of your tests as being as important and well-written as the code they are testing. E.g. apply the same refactoring techniques to ensure that a test does one thing and does it well, and isn't a stovepipe that tests everything in sight.

Robert Paulson
A: 

The way I read the books, you're supposed to do "Red, Green, Refactor". In the "Refactor" part, you're supposed to refactor both the code under test and the unit tests.

I see nothing wrong with the following refactoring:

Original

[TestMethod]
public void TestOne()
{
    LetsDoSomeSetup();
    AssertSomething();
}

[TestMethod]
public void TestTwo()
{
    LetsDoSomeSetup(); // Same setup
    AssertSomethingElse();
}

Refactored

[TestMethod]
public void TestOneTwo()
{
    LetsDoSomeSetup();
    AssertSomething();
    AssertSomethingElse();
}

Of course, that assumes that the two asserts are related, and of course relies on the same scenario.

John Saunders
I'm not familiar with the framework you are using, but wouldn't it be better to put the setup in a `setUp` method to be called before each test case (compared with JUnit, *et al.*) and test the two concepts in separate tests?
Johnsyweb
This depends on whether the setup is expensive. If not, then [Setup], or even calling the same method as i did here, would help. But if there are many different test setups, and if it's not convenient to group all tests that have the same setups in a single test class (to be able to share the [Setup]), then you get my situation.
John Saunders
In this case, the refactoring would tend to want to extract class on the setup, into a fixture, and then you still have TestOne() and TestTwo(), they both just share the fixture so the setup is automated.
dash-tom-bang
+4  A: 

It is far more important to test just one concept per unit test.

It may take more than one assertion to test a concept, so don't worry overly about the number of assertions. Of course if you end up with a large number of assertions, you may want to take a step back and think about what you are really testing.

Johnsyweb
What do you do with a concept like, "all the properties of the request are validated"? One test per property? But then, you need to validate for min and max length (or min and max value), null, empty, blank, regex matches, failures of business rules, etc. Wouldn't it be better to have a test that says, "test all properties for validation" rather than a dozen that do the same things, but to different properties?
John Saunders
It is hard to answer this (excellent) question without a specific example. Where I have had multiple similar assertions in multiple test cases, I have extracted these as a single assertion-type method to be re-used.
Johnsyweb
@John Saunders - This is where parameterized tests are helpful. Unfortunately MSTest is lacking those (AFAIK).
whatispunk
+1  A: 

AFAI see, the principle behind the practice was

  • to test one cohesive/logical thing per test ; leading to concise and easy to read tests.
  • to have better test isolation. One test should not fail for 1 of n reasons. If it does, then it is not immediately apparent as to how to fix it ; without first investigating which reason caused it to fail. A single change should not fail a truckload of seemingly unrelated tests.
  • a starter-rule from the rulebook for TDD beginners ; so that they can internalize the process.

the point is not that you need one 'NUNit'/'xUnit' assert per test ; rather you have one logical assert per test.

[Test]
public void MakeFoo_SeedsInstanceWithSpecifiedAttributes()
{
  Assert.AreEqual( new Foo(1,2,3), Foo.MakeFoo(1,2,3), "Objects should have been equal );
}

Here I have one NUnit assert (keeping the assert police happy) : However it is testing three things behind the scenes (in Foo.Equals I'd be checking if all the variables are equal.)
The guideline is to prevent tests like (i.e. an integration test masquerading as a unit test)

[Test]
public void ITestEverything()
{
  // humongous setup - 15 lines
  payroll.ReleaseSalaries();
  // assert that the finance department has received a notification
  // assert employee received an email
  // assert ledgers have been updated
  // statements have been queued to the printqueue
  // etc.. etc..
}

Moral of the story: It is a nice target to aim for. Try and compose all the things you want to test into one logical/cohesive Assert with a good name. e.g. Assert.True(AreFooObjectsEqual(f1,f2). If you find that you're having a tough time naming the cohesive verification/assert - maybe you need to review your test and see if you need to split it.

Gishu
A: 

I’ve actually written an NUnit addin to help with this. Try it out at http://www.rauchy.net/oapt

Omer Rauchwerger
Just noted you are using MSTest.Here's another reason to move to NUnit for ya ;-)
Omer Rauchwerger