Red For The Right Reason: Fail By Assertion, Not By Anything Else
Thomas Weller commented on my Red/Green/Refactor For The Right Reasons post and asked me to explain why I don’t think a throwing a NotImplementedException is a good reason for a test to be red. It’s a good question, one that I questioned for a long time, and something that is worth talking about on it’s own rather than just replying in the original comment stream. To get things started, here is Thomas’ comment:
“A “Throw NotImplementedException();” is never the right reason” for a test to be red !? I’m not sure I’m buying that. I don’t exactly understand the problem with that approach.
Having a test being red because of a NotImplementedException tells me that my test set up is right and the test fails only because the method under test is not yet implemented – and that’s exactly what I want. The effect is the same that you get when hard-coding a return value, only that you don’t depend on a specific value and you get the code for free with R#. - Or, in your words: If the test fails because of a NotImplementedException, it is red for the right reasons (at least in my eyes…).
I use this approach regularly in TDD and I made very good experiences with it – especially because with a NotImplementedException a test will NEVER become green for the wrong reasons, and therefore there will be no danger that it is overlooked in the (sometimes hectic) production process.
This is no religion, of course. But I don’t really get your arguments at this point (while I’m totally d’accord with the rest of your post). Maybe you could clarify a bit more on that…
The Underlying Principle: Fail By Assertion, Not By Anything Else
The underlying principle behind why a NotImplementedException is never the right reason to fail is that it does not let an assertion fail the test. A test must be able to fail because an assertion caused it to fail, and for no other reason. We must prove that the test fails because the assertion did not find the expected values or interactions, or we do not know that the test can fail for the right reasons. Any other reason for failure is an abnormal condition that tells us we have something wrong with our code.
Test For Your Needs, Not The Framework’s Functionality
Throwing a NotImplementedException as the red part of your red/green/refactor is doing nothing more than proving that the .NET framework can throw exceptions and that your testing framework can fail a test when an exception is thrown.
2: public void Test_Foo()
4: var result = Foo();
5: Assert.AreEqual(60, result);
8: public int Foo()
10: throw new NotImplementedException();
This test doesn’t fail for the right reason because it does let the assertion fail the test. It does not show that an abnormal condition exists between our expectations and the implementation other than to say that the method has not yet been implemented. It is only proving that an exception will fail the test, which is not something we need test. The unit tests that the testing framework was written with will prove that a test can fail when an exception is caught.
A Simple State Based Test
In this test, we are asking an object to do some work which results in it’s own state being modified.
2: public void A_State_Test()
4: var myObj = new MyObject();
6: Assert.AreEqual(60, myObj.SomeState);
If the DoSomething method throws a NotImplementedException, we know that we have not yet implemented it but we do not know that the test can fail because the assertion failed. If we don’t let the assert fail because the value of myObj.SomeState is not what we expected, then we can have no confidence that this test will fail for the right reasons.
A Simple Interaction Test
In this test, we are asking an object to do some work and then expecting that it will interact with another object via an interface. The assertion is made against the interaction by checking to see if the correct method was called on the interface.
2: public void An_Interaction_Test()
4: var anotherObject = MockRepository.CreateMock<IAmThatThing>();
5: var myObject = new MyObject(anotherObject);
7: anotherObject.AssertWasCalled(ao => ao.ThatThing());
If the DoSomething method throws a NotImplementedException here, the method called ThatThing() on the IAmThatThing interface may not be called, but we don’t actually know that it was not called. We are not proving that the method is not called and that the assertion can fail the test. Rather, we are only proving that an exception can fail the test.
Complex Test Scenarios
f the object being tested has a private method and let put a NotImplementedException into that private method, we have not proven that the method being called is not yet implemented. In fact, we have no certainty of which method in the object is not implemented until we look at the stack trace that is produced by the exception being thrown. If, on the other hand, the private method is expected to return a specific value under the specific circumstances that were set up by the public method being called from the test then we can return a value that is known to be incorrect and the assertion will fail for the right reason.
If we are dealing with a more complex set of dependencies and interactions then the NotImplementedException becomes more and more of an obstruction to the tests for every assertion that we have. Having an exception will fail the test when we have 5 asserts against interactions means that we have 5 tests that are failing for an invalid reason.
False Positives, False Negatives
A false positive exists when a test passes for the wrong reasons, and a false negative exists when a test fails for the wrong reasons. Thomas said this about false positives in his comment:
because with a NotImplementedException a test will NEVER become green for the wrong reasons, and therefore there will be no danger that it is overlooked in the (sometimes hectic) production process.
He is right about that, no doubt. A NotImplementedException certainly will prevent false positives while it is in place, but there are two problems that I see with this statement:
- You are introducing false negatives, which are just as dangerous as false positives. Since the test is failing because of an exception and not because an assertion found an abnormality, the test failure is a false negative
- You have not prevented false positives, only postponed them. Once the NotImplementedException is removed, the code is open to the possibility of false positives
We must be diligent about preventing false positives and false negatives, both, not just false positives.
When To Test For Exceptions
There are times when it’s important to do both positive and negative testing around exceptions… when your code throws them. This example shows that we expect a process to not throw any exceptions under one condition, and we expect it to throw an exception under another condition.
2: public void Dont_Throw_Exceptions()
4: var myObject = new MyObject();
5: Exception caughtException = null;
10: catch(Exception ex)
12: caughtException = ex;
18: public void Throw_Exceptions()
20: var myObject = new MyObject();
21: Exception caughtException = null;
26: catch(Exception ex)
28: caughtException = ex;
These tests are valid because they will fail is the assertion fails (and yes I know about the various attributes that can be put on the tests to expect/not expect an exception. This is an example of the principle, not the implementation detail)
When To Use NotImplementedException
I’m not saying that you shouldn’t use not implemented exceptions at all, or that I never use them. I actually do use them on occasion, but for a very specific purpose: to remind myself that I have not yet implemented the method in question. I find it very convenient to do this when I’m about to leave my workstation or change my current train of thought to something else for a while. Having the NotImplementedException in place will be a big red flag to tell me that I was not yet done when I was interrupted and went on to do something else. When I do this, though, I am specifically stating that the method is not yet implemented and that I need to re-engage myself in the context of the code and the tests to figure out where I left off and continue. It’s not a sign that my test fails for the right reason, it’s a sign that I left in the middle of something that needs to be completed.
I’d like to say thanks for questioning this. In the process of of thinking about this and writing it up, I had to dig deep into the principles and beliefs that I hold for test-first development. My first draft of this response contained a bunch of extraneous nonsense that had nothing to do with the real issue, then around 75% of the way through this that I had the “aha!” moment and wrote down the underlying principle of fail by assertion, not anything else. From that revelation, I was able to cut the nonsense and unrelated content and solidify my own understanding of why I don’t think a NotImplementedException is red for the right reason.
Thanks, Thomas. Without your questions and comments I may not have had this little revelation.