Monday, June 8, 2015

Unit Testing: Setup Methods or Not?

Developer events are a great place to talk to other developers about technologies and techniques that are on your mind. This past week at Denver Dev Day was no different. In particular, I had a chat with David Batten (Pluralsight & Twitter) and Brian Lagunas (Blog & Twitter) about unit testing.

This was especially timely for me because I've been reading the 2nd edition of Roy Osherove's book The Art of Unit Testing (Amazon). This is an excellent read, and I'll post a review in the next few days. There were a couple of topics that came up while reading, and I've been thinking about them a bit more. One of them has to do with Setup methods in unit tests.

We were talking about various unit testing frameworks (NUnit vs xUnit.NET) and different techniques. David mentioned that he moved away from using Setup methods in unit tests because there were other ways of doing that work that keeps the code more apparent. This is exactly what Osherove also says in The Art of Unit Testing. The idea is that we use factory methods instead of creating objects in the Setup for the tests.

I've found myself using both over the years. I've been thinking about this a bit more due to the book, so let's take a look at why we may want to use factory methods instead of setup methods.

A Look at a Setup Method
We'll start by taking a look at a set of unit tests that uses a Setup method. This is taken from my presentation "DI Why: Getting a Grip on Dependency Injection". I won't explain all of the code (you can check out the PDF Walkthrough in the session materials for that).

In this code, we have unit tests for a view model in our application. The class is called "MainWindowViewModel", so I have a test class called "MainWindowViewModelTest" (a bit of a mouthful).

These tests have a Setup method:


The primary purpose of this is to initialize a stub object that we can use in our tests. This stub represents the repository that we need. Rather than using the real object, we use Moq to create an in-memory object that behaves the way we want for testing. In this case, if someone calls the "GetPeople" method, it will return the hard-coded "people" collection that we have here. (And yes, using "DateTime.Parse" with a string like we have here is not good because it depends on the current culture having "MM/dd/yyyy" as the date format. I'm re-doing this demo code (which will fix this), but it's not posted on my site yet.)

Notice the class-level field "IPersonRepository _repository". This holds our stub object that we use later in the tests.

We're using MSTest in this example. When we mark the method with the [TestInitialize] attribute, this method is run before every test.

Here's what one of our tests looks like:


This test uses the "_repository" field as a parameter for our MainWindowViewModel constructor (an example of constructor injection). Since "_repository" is initialized in the Setup method, we have a valid value that we can use for this test.

The rest of the test makes sure that our "RefreshPeopleCommand" behaves as expected. In this case, it should populate the "People" property with our fake data.

Here's another test for the ClearPeopleCommand:


This is similar to the previous test. Again, feel free to check out the session materials for more details.

What's So Bad About This?
The problem with this code is that we need to go somewhere else to get more information about this test.

We are using the "_repository" field, but just by looking at the test, it's hard to tell exactly what that variable is (what type is it?) or how it's initialized.

This actually isn't quite as bad as the problem described by Osherove (and we'll look at that in just a bit). To see how things can be more readable, let's change our Setup method to a factory that's responsible for creating our stub.

A Look at a Factory Method
Instead of using a Setup method and a class-level field, we'll use a factory method that returns the type that we're looking for.


When we look at this method, it looks almost exactly like the "Setup" method from above. The difference is that instead of setting a class-level field, we return the "IPersonRepository" stub.

Notice that this method does not have the [TestInitialize] attribute. This means that it is *not* run automatically, so we need to call it in our tests.

Here's a refactored test:


We have a new line of code in this test. The first line of code calls the "GetRepositoryStub" method that we just created and assigns it to a local variable that is then used in our test.

The other test will have a similar change:


What's So Good About This?
The advantage of using the factory method is that all of the code is directly run in the test methods. There is now no question as to what "repository" is. We can easily see that it is an "IPersonRepository" without having to scroll to the top of the class. In addition, we can see that we are initializing it with the "GetRepositoryStub" method. This gives us a clue that we're creating a stub object that we can use in our tests.

Note: I'm currently examining how I name the variables used in my unit tests based on Osherove's book and my experiences. I'll have to get back around to this in a future article.

Observations
Readability of unit tests is extremely important. We need to think about how these tests are used. They aren't just used when we're writing the code, they are also used as we continue to develop the code. And they are also used when we go back to fix bugs or add features.

Debugging
When a test fails, we want to make things as easy as possible for the developer to figure out what's going on. The first step is to make sure we have good names on our tests. If we have good names for our tests (that describe what the test is doing and what is expected), then we often don't have a need to look at the test code itself.

But when we do look at the test code, we want things to be as obvious as possible. The developer trying to debug the code may not be the same developer who wrote the test. So we want to make things as direct and non-tricky as possible.

Writing New Tests
When we're adding new features, we want to create new tests for those features. That means we'll often look at the tests that are already there so that we can (hopefully) create tests in the same style. If the tests are direct and obvious, it will be much easier for us to write new tests without having to go look for any "magic" that is going on in the Setup method.

Setup & Teardown Still Have Uses
As mentioned in a previous article (Beware of "Never" & "Always"), I examine advice from other developers to see if it fits my particular scenario. In this case, I ended up looking at how I've used Setup and Teardown methods (Teardown methods run automatically *after* each test has run). In doing so, I've found that there are a few scenarios where I would still use a Setup or Teardown method.

In a previous article (Mocking Current Time with a Simple Time Provider), I had a static property that needed to be reset before each test would run. I accomplished this with a Setup method:


This sets the static "TimeProvider" property to a valid default value. The reason we need to do this is because static objects are shared across tests, so if a test overrides this value, then it may cause another test to fail if it is not reset.

After thinking about this a bit more, I figured out that it would actually be more appropriate to do this in a Teardown method that runs *after* each test has completed:


This has the effect of resetting the static "TimeProvider" property. Since we've set the property to automatically create the correct default time provider if it is "null", this code works just fine. (In fact, I think it works a little better because now we leave it up to the production code to figure out what the correct default value is).

I still haven't updated this code in the project, but it's been on my mind for a few days. But the main point is that Setup and Teardown methods do still have their uses. In this case, it's a true "reset to default", and it makes sure that we don't have unexpected interactions between our tests.

Err on the Side of Readability
The biggest reason that I've been thinking about this is that I've seen small variations in the examples that Osherove shows in his book and how I've been writing my tests. I had to do a bit of thinking on this because in some of his examples, he shows how things can be a lot worse.

Here's a test based on a worse-case scenario described by Osherove:

Don't Do This!

For this test, I moved *all* initialization up to the Setup method. Notice that the entire "Arrange" section is gone; we are simply using the "vm" object (which is now a class-level field). This makes this test extremely hard to follow, and this is the type of thing that Osherove is trying to avoid (and for good reason).

Why would someone do this? In order to consolidate similar code (i.e. following the DRY principle). All shared code is in the Setup method, but at the cost of readability. By using factory methods, we can still follow DRY while keeping our test code readable and maintainable.

Let's take a look at our test using the factory method one more time:


This code is much easier to follow. We can see that "vm" is a MainWindowViewModel. And we can see that "repository" is a stub object based on the IPersonRepository interface. If we really care about how the stub is created, we can click into that method. But there's no need to look for a Setup method to see if something else is going on.

As another option, we could create a factory method for the view model object. And I've done this in my own projects. This made sense to me if I was initializing objects in a few different ways (in order to test different scenarios); then I could have multiple factories that gave me just the initial state I needed for a particular test. And the factories were shared across tests so I didn't have to keep writing the same initialization code over and over.

Wrap Up
It's great to talk to other developers about their experiences. Most of us have found better ways of doing things as we try different techniques. It's also reassuring to find out that you've been thinking about things in the same way as someone else.

It's really easy to feel alone in the development world. "Am I doing this right?" And we have this problem because "right" varies depending on the situation. I really like it when I talk to other developers who are doing things the same way that I'm doing something. It lets me move forward more confidently.

I also like to hear from developers who are doing things differently from me. This makes me take a look at my techniques and process to see if I can improve them.

In this particular case, I've found that I'm generally on the same path as David, Brian, and Roy Osherove. But I've seen where I vary things a bit as well. This gives me the opportunity to re-evaluate things to see where I can improve my processes.

I'm always looking for better ways of doing things. I'll be experimenting some more to determine which parts of my testing process work well for me and which parts I can tweak to make a bit better. Lots to think about -- all spurred by a small conversation at a developer event.

Happy Coding!

3 comments:

  1. I completely agree. Usually a setup method means splitting your test into several region and in some unit testing framework having a shared state between the tests.
    In the last year I have been using Automocking containers to reduce the overhead of creating the same objects over and over again: http://blog.drorhelper.com/2013/12/on-object-creation-and-unit-tests.html

    ReplyDelete
  2. Good explanation as usual! I like the setup method because I'm really lazy but the factory method is just as simple and the added line in "Arrange" really improves the readability of the test. Thanks also for your contribution at the Denver Dev Day!

    ReplyDelete
  3. I usually use setup in my integration tests to clean database, setup Ninject and other stuff like that but never ever for getting and instance of a mock object. When you put something in the setup method, you glue all your test to have to use all the time this code. Just to prevent having to write 2 or 3 additional lines of code in each test... For sure that I prefer more lines of code in this case yes for the readability but also for the maintainability and flexibility that it gives to me.

    ReplyDelete