views:

264

answers:

4

What do you think is cleanest way of doing multiple asserts on a result? In the past I've put them all the same test but this is starting to feel a little dirty, I've just been playing with another idea using setup.

[TestFixture]
public class GridControllerTests
{
    protected readonly string RequestedViewId = "A1";

    protected GridViewModel Result { get; set;}

    [TestFixtureSetUp]
    public void Get_UsingStaticSettings_Assign()
    {
        var dataRepository = new XmlRepository("test.xml");

        var settingsRepository = new StaticViewSettingsRepository();

        var controller = new GridController(dataRepository, settingsRepository);

        this.Result = controller.Get(RequestedViewId);

    }

    [Test]
    public void Get_UsingStaticSettings_NotNull()
    {
        Assert.That(this.Result,Is.Not.Null);
    }

    [Test]
    public void Get_UsingStaticSettings_HasData()
    {
        Assert.That(this.Result.Data,Is.Not.Null);
        Assert.That(this.Result.Data.Count,Is.GreaterThan(0));
    }

    [Test]
    public void Get_UsingStaticSettings_IdMatches()
    {
        Assert.That(this.Result.State.ViewId,Is.EqualTo(RequestedViewId));
    }

    [Test]
    public void Get_UsingStaticSettings_FirstTimePageIsOne()
    {
        Assert.That(this.Result.State.CurrentPage, Is.EqualTo(1));
    }
}
+2  A: 

What you need to keep to is the pattern of Arrange, Act, Assert (and then end the test). In your case all the arrangement is in the TestFixtureSetUp, as is the action being tested. I would re-arrange this a bit, it may become unwieldy when you have more tests. As Dockers notes, heavy test set-ups should be avoided, they can become problems - they are "one size fits all" across all tests in the class and so can become heavier than most of the tests need.

If you're tempted to carry on to another follow-on action and then more asserts, put this in a separate test.

I have no issue with putting multiple asserts in the same test, so long as they contribute to testing the same thing (i.e are part of the same "Logical Assertion"). In this case, any number of asserts on the contents of this.Result.Data would be OK by me - they would all inspect the same result value. Your Get_UsingStaticSettings_HasData does this very clearly. It's best to use a unique failure message on each assert so that it's easier to tell which assert failed.

Alternately, you could wrap up the related asserts in a single method. this is useful for the usual DRY reasons if you use it more than once, but otherwise I don't see that it's a big difference.

In Summary
* Do one action per test
* After the action, use as many related asserts as you need to test one thing
* End the test there.

Anthony
+9  A: 

Having multiple assertions in the same test can lead to Assertion Roulette, so this is something of which you should always be careful.

However, Assertion Roulette is mostly a problem when the assertions are unrelated. If they are conceptually closely related, many assertions can often be viewed as a single Logical Assertions.

In many cases you can get the best of both worlds by explicitly encapsulating such a Logical Assertion in a a custom type or method.

Mark Seemann
"Logical Assertions" is a good word for it. It is interesting that the Assertion Roulette example is essentially an example of "how to go wrong by not doing Arrange-Act-Assert". Do the concepts differ?
Anthony
@Anthony: The concepts differ, although they are closely related. You are much less likely to feel the pain of Assertion Roulette if you follow AAA (or Four Phase Test as xUnit Test Patterns calls it), but I would still be vary of having completely unrelated Assertions in the same Assert block.
Mark Seemann
The idea of Assertion Roulette can be minimized in NUnit by specifying a comment in the Assert statement. Instead of doing "Assert.That(condition)" use "Assert.That(condition, failureMessage)" where 'failureMessage' is info on what the Assert was checking. This will let you know exactly which Assert within a unit test failed.
Pedro
A: 

You can use Oapt - An NUnit Addin for Running One Assert Per Test:

[TestFixture]
public class GridControllerTests
{
  [TestCase, ForEachAssert]
  public void Get_UsingStaticSettings_Assign()
  {
      var dataRepository = new XmlRepository("test.xml");
      var settingsRepository = new StaticViewSettingsRepository();
      var controller = new GridController(dataRepository, settingsRepository);

      var result = controller.Get("A1");

      AssertOne.From(
        () => Assert.That(this.Result,Is.Not.Null),
        () => Assert.That(this.Result.Data,Is.Not.Null),
        () => Assert.That(this.Result.Data.Count,Is.GreaterThan(0)),
        () => Assert.That(this.Result.State.ViewId,Is.EqualTo(RequestedViewId)),
        () => Assert.That(this.Result.State.CurrentPage, Is.EqualTo(1)));
  }
}

This will create 5 different test cases, one for each assert.

Omer Rauchwerger
A: 

I tend to put assertions on their own only if they're valuable on their own. If I want an assertion on its own, I make a custom assertion:

AssertThatMyObjectMatches(field1, field2, field3, field4, myObject);

Sometimes, though, I like to have more than one example in a test. I do this when half of the behaviour has no value without the other half.

Assert.True(list.IsEmpty());

list.Add(new Thing());
Assert.False(list.IsEmpty());

Others, including much of the Ruby community, have a different opinion about this. It's primarily driven by Dave Astels' blog post, here:

http://www.artima.com/weblogs/viewpost.jsp?thread=35578

I find the "One assertion per test" method very useful for things like validation, where each small aspect is valuable. Otherwise I don't worry about it so much.

Whatever works for you and your team is probably the right way. I tend to do whatever seems simple and easy to change to be the right thing later, when I have a better idea of what the right thing is. I also put lots of unit-level Given / When / Then comments in more complex examples, and split the class if it becomes too complex to understand.

The reason for writing tests this way isn't so that you can catch things which break. It's to help people understand the code and change it without breaking things in the first place.

Lunivore