Inspired by the increased readability I got with the previous refactoring, I decide to continue improving the test methods.
Let's have a look at the first test I wrote.
[Test] public void Songs_window_is_displayed_when_the_application_starts() { var view = new SongsViewFake(v => v.ClickWindowCloseButton()); var thingy = new Thingy(view, ac => { }); thingy.ShowView(); Assert.That(view.HasBeenShown, Is.True); }Had I seen this test method for the first time, I think it would confuse me. All I see is implementation details. Most of the code is about wiring up dependencies for some class called Thingy. Let's see if I can improve some of it.
I've previously said that what might be the most important thing with code (test and production) is that it is easy to read. This, however, is, in my experience, generally more difficult to achieve with test code than with production code. After all, in each test we have to do so many things: setup of code that the test depends on, setup of the initial state, perform some action and make some kind of assertion.
You'd typically like to have the initial state, the action performed and the assertion stated as clearly and briefly as possible. The dependencies and other implementation details should be hidden.
A popular trick to make code readable is to create internal Domain Specific Languages (DSL). A DSL is a programming language developed to solve exactly the problem at hand. When the DSL is internal it means that it is implemented as an extension of another language (in this case C#). It is internal to that language, so you could say that it is like an API written in that language.
Using a DSL I would like to describe the flow just like the actor that the test mimics would describe it. In this case I mimic the end user. I figure the end user would see the steps performed as something like this:
- Start application.
- In the songs window that opens:
- Do whatever I want to perform
- Close the window
One way to express this in code would be like this:
Application .Start(songsView => { // arbitrary code songsView.ClickCloseButton(); });Here, Application is a property that returns an instance of some class that has a Start() method on it. The class acts as a representation of the application, so I will call it ApplicationFake. When the Start() method executes it opens the songs window, which in this case is represented by the SongsViewFake. What happens when the window is shown is described using the ShowAction delegate that I used previously.
The parts that I would like to move into the Application property or the Start() method is the creation of SongsViewFake and Thingy, and the call to ShowView(). I have a look at what these three lines look like in the three tests:
// Songs_window_is_displayed_when_the_application_starts() var view = new SongsViewFake(view => view.ClickWindowCloseButton()); var thingy = new Thingy(view, ac => { }); thingy.ShowView(); // Application_closes_when_songs_window_closes() var view = new SongsViewFake(view => view.ClickWindowCloseButton()); var thingy = new Thingy(view, ac => { closeActionHasBeenExecuted = true; }); thingy.ShowView(); // Application_does_not_close_before_songs_window_closes() var view = new SongsViewFake(view => { }); var thingy = new Thingy(view, ac => { closeActionHasBeenExecuted = true; }); thingy.ShowView();What differs are the parameters closeAction and showAction, so if I want to use the same method in all tests I have to be able to customize those parameters for the different tests. I start refactoring the first test by extracting these two actions into locals.
[Test] public void Songs_window_is_displayed_when_the_application_starts() { Action<SongsViewFake> showAction = view => view.ClickWindowCloseButton(); Action<ApplicationContext> closeAction = ac => { }; var songsView = new SongsViewFake(showAction); var thingy = new Thingy(songsView, closeAction); thingy.ShowView(); Assert.That(songsView.HasBeenShown, Is.True); }Next I extract the three lines into a method called Temp(), passing the showAction and closeAction as parameters.
[Test] public void Songs_window_is_displayed_when_the_application_starts() { Action<SongsViewFake> showAction = view => view.ClickWindowCloseButton(); Action<ApplicationContext> closeAction = ac => { }; var songsView = Temp(showAction, closeAction); Assert.That(songsView.HasBeenShown, Is.True); } private SongsViewFake Temp(Action<SongsViewFake> showAction, Action<ApplicationContext> closeAction) { var view = new SongsViewFake(showAction); var thingy = new Thingy(view, closeAction); thingy.ShowView(); return view; }I can start using this method from my other tests as well. My plan is to make the content of this Temp() method look like the final result, and when it does, inline it into the three tests.
[Test] public void Application_closes_when_songs_window_closes() { var closeActionHasBeenExecuted = false; Temp(view => view.ClickWindowCloseButton(), ac => { closeActionHasBeenExecuted = true; }); Assert.That(closeActionHasBeenExecuted, Is.True); } [Test] public void Application_does_not_close_before_songs_window_closes() { var closeActionHasBeenExecuted = false; Temp(view => { }, ac => { closeActionHasBeenExecuted = true; }); Assert.That(closeActionHasBeenExecuted, Is.False); }The tests are still green. Already now, the tests are starting to look better, although the parameters are somewhat cryptic.
Next, I start writing the code as I would like the test to look like, inside the Temp() method.
private SongsViewFake Temp(Action<SongsViewFake> showAction, Action<ApplicationContext> closeAction) { return new ApplicationFake() .WithCloseAction(closeAction) .Start(showAction); // ... }I create the ApplicationFake class and the methods one by one, manually moving most of the code from the Temp() method into the Start() method.
public class ApplicationFake { private Action<ApplicationContext> CloseAction { get; set; } public ApplicationFake WithCloseAction(Action<ApplicationContext> closeAction) { CloseAction = closeAction; return this; } public SongsViewFake Start(Action<SongsViewFake> showAction) { var view = new SongsViewFake(showAction); var thingy = new Thingy(view, CloseAction); thingy.ShowView(); return view; } }This was quite a big refactoring step (it took me almost a minute), but the tests are still green afterwards.
Finally, I move the creation of ApplicationFake into a test Setup() method and store the instance in the property Application.
[TestFixture] public class SongsFixture { private ApplicationFake Application { get; set; } [SetUp] public void Setup() { Application = new ApplicationFake(); } private SongsViewFake Temp(Action<SongsViewFake> showAction, Action<ApplicationContext> closeAction) { return Application .WithCloseAction(closeAction) .Start(showAction); } // ... }Now I can inline the Temp() method into the three tests.
[TestFixture] public class SongsFixture { // ... [Test] public void Songs_window_is_displayed_when_the_application_starts() { var songsView = Application .WithCloseAction(ac => { }) .Start(view => { view.ClickWindowCloseButton(); }); Assert.That(songsView.HasBeenShown, Is.True); } [Test] public void Application_closes_when_songs_window_closes() { var closeActionHasBeenExecuted = false; Application .WithCloseAction(ac => { closeActionHasBeenExecuted = true; }) .Start(view => { view.ClickWindowCloseButton(); }); Assert.That(closeActionHasBeenExecuted, Is.True); } [Test] public void Application_does_not_close_before_songs_window_closes() { var closeActionHasBeenExecuted = false; .WithCloseAction(ac => { closeActionHasBeenExecuted = true; }) .Start(view => { }); Assert.That(closeActionHasBeenExecuted, Is.False); } }The tests look much better with this syntax. I've managed to hide the Thingy implementation details completely from the tests. It's clear that the tests start the application and perform operations on the view. Also, the assertions are clear, just like before.
I don't like a few things about the first test though. First, it specifies a closeAction, something that is of no importance in this test and therefore only adds noise. I fix this by letting ApplicationFake have an empty CloseAction by default. This way I don't have to specify it in my test.
public class ApplicationFake { public ApplicationFake() { CloseAction = ac => { }; } // ... }Second, the Start() method returns the view so that the test can make assertions on it. If this were the real view, the one I use in production, it would be disposed and handed over to the garbage collector when returning from the Start() method. To create symmetry with the production code I would like to act like the same is true in the tests. I fix this by introducing a local variable.
[Test] public void Songs_window_is_displayed_when_the_application_starts() { var hasBeenShown = false; Application .Start(v => { hasBeenShown = true; v.ClickWindowCloseButton(); }); Assert.That(hasBeenShown, Is.True); }The hasBeenShown variable is really ugly, as is the closeActionHasBeenExecuted variable in the other tests. It's much better than when I started, though. And I will probably get the opportunity to do something about these variables later on.
The tests are green, so I check in the code.