There's one thing that I don't like about the production code that I have written in Thingy regarding the CloseAction, and that is that I can change it so that it's not working and still get a green test. To prove it, I change the production code so that the CloseAction is executed immediately instead of when the OnViewClosed event is raised.
public class Thingy : ApplicationContext { // ... public void ShowView() { CloseAction( this ); // Execute immediately View.OnViewClosed += () => { }; // Do nothing here View.ShowView(); } }The tests are green, but when running the application it hangs on close, just as I feared. So I write another test where I do not call close on the view and verify that it hasn't been closed.
public class SongsFixture { // ... [Test] public void Application_does_not_close_before_songs_window_closes() { var view = new NonClosingSongsViewFake(); var closeActionHasBeenExecuted = false; var thingy = new Thingy(view, ac => { closeActionHasBeenExecuted = true; }); thingy.ShowView(); Assert.That(closeActionHasBeenExecuted, Is.False); } } public class NonClosingSongsViewFake : ISongsView { public event Action OnViewClosed; public void ShowView() { } }I create another fake implementation of the ISongsView interface, one that does not close the view. I use it in the new test to verify that the CloseAction is not called upon when the view is never closed.
I compile and get an error:
How annoying. Never raising an event raises a compile warning, and I have configured my project to treat warnings as errors, hence the compile error.
Compile warnings can, in some cases, give you hints about bugs, and in other cases it can hint you about usage of bad programming practices. Therefore, it's a good practice to take care of the warnings and fix them. The best way to enforce this is to enable the project setting to treat warnings as errors.
In this case it is a hint about a bad programming practice. I'm implementing an interface that has an event on it that I'm not interested in raising. By implementing the interface, I tell users of my class that they can expect a certain behavior, a behavior that I happen not to provide. Bad, indeed.
However, not raising the event is what the test is all about. I want to test the behavior when the event is never raised. And this is something that happens once in a while in test code, that you have to break some rules in order to be able to perform the test.
I don't want to change the treat warning as error-setting, so as a work-around I resort to the ugly fix of using #pragmas to temporarily disable the warning:
public class NonClosingSongsViewFake : ISongsView {#pragma warning disable 67 // The event is never used public event Action OnViewClosed;#pragma warning restore 67 // ... }Now the code compiles, so I run the test with the modified code where I call CloseAction() directly. It is red, just as expected:
What I just did is something called triangulation. This is something that many TDD proponents think that you should always do. They say that when you write the production code you should write the simplest solution that makes the test pass. This does not mean the simplest thing that might work in the final solution, but actually the simplest thing that make the tests pass.
The typical example is that, if you test a method that should add two numbers and you pass in the parameters 3 and 4, you should not return the sum of the parameters, but the constant 7. This forces you to write another test (triangulate) with other parameters where the sum is not 7.
Triangulation is a very handy tool to have with you, both as a way of writing tests, but also as a thinking tool. It helps you write tests that will not only cover the happy paths, but that might also protect you from future bugs. Had I written the simplest possible production code that could possibly work to start with, I would have discovered this flaw much earlier. After all, it was changing the code into something simpler that triggered the bug.
I revert the changes to CloseAction that I made in Thingy resulting in green tests again, so I check in. And now it's time to do some refactorings.