How to prevent 'over-testing' in a test case? (C#/nUnit)

Question

I'm working on some test cases at the moment, and I'm regularly finding that I'm ending up with multiple asserts in each case. For example (over-simplified and comments stripped for brevity):

[Test]
public void TestNamePropertyCorrectlySetOnInstantiation()
{
  MyClass myInstance = new MyClass("Test name");
  Assert.AreEqual("Test Name", myInstance.Name);
}

This looks acceptable in principle, but the point of the test is to verify that when the the class is instantiated with a given name, the Name property is set correctly, but it fails if anything goes wrong on instantiation, before it even gets to the assertion.

I refactored it like this:

[Test]
public void TestNamePropertyCorrectlySetOnInstantiation()
{
  MyClass myInstance;
  string namePropertyValue;

  Assert.DoesNotThrow(() => myInstance = new MyClass("Test name"));
  Assert.DoesNotThrow(() => namePropertyValue = myInstance.Name);
  Assert.AreEqual("Test Name", namePropertyValue);
}

but of course, now I'm actually testing three things here; In this test, I'm not interested in testing whether or not the instance of MyClass was successfully instantiated, or that the Name property was read successfully, these are tested in another case. But how can I test the last assertion without asserting the other two first, given that it's not possible to even do the test if the first two fail?

Answer 1

Just have other tests which check that an exception is thrown if you initialize it in an invalid way. The first form is fine at that point, IMO.

Personally I'd avoid getting hung up on the dogma of "one assert per test". Try to test one logical path through the code, to as fine a granularity as makes practical sense.

Answer 2

In your particular example you don't need to assert that something doesn't throw if its part of your test execution. This aspect is already tested in your first test, which is way more readable. If the constructor or the property getter throw an exception NUnit will fail the test with an appropriate error message. TBH I am not sure what is the idea behind Assert.DoesNotThrow(), because if you omit it you get pretty much the same result - but definitely you shouldn't use it as part of the normal test execution. The whole point of having exceptions as part of the language is that you don't need to check for errors after every single line of code.

Answer 3

Grzenio is correct in this. The first, simplest example test will fail if an exception is thrown - no need to explicitly test for that.

You should, however, test that an exception is thrown when invalid data is passed to it.

[Test]
public void TestInvalidNameThrowsException {
  MyClass myInstance;
  string namePropertyValue;
  Assert.Throws(() => myInstance = new MyClass(null));
  Assert.Throws(() => myInstance = new MyClass("INVALID_NAME_125356232356"));
}

(for example, I don't know C# one bit. But you should get the idea.)

Answer 4

I really don't understand what do you mean by over-testing IMO, over-testing is something like trying to test private methods.

I take my tests to be the documentation of the code. So, if I have more than one assert in a statement, then there is a high chance that I may refactor the method under test into several smaller methods or sometimes I split my test method into several different test methods. Following the rule of one-assert-per-test allows allows you to have sensible test method names, which in turn forms the documentation of your code. The naming convention I follow for test methods is methodName_scenario_expectation (From RoyOsherove's Art of Unit Testing). So, also thinks in terms of documentation of code. Do, you think having an assert will (apart from verifying expectation) help you/some other developer to understand the code better then go ahead and write that assert. But, to reiterate again, then always ensure your have proper test method names.