In my presentation "
Clean Code: Homicidal Maniacs Read Code, Too!", I spend quite a bit of time refactoring code (not as much time as I'd like, which is why I put out a supplemental video:
Clean Code: The Refactoring Bits).
Unit tests are a vital part of the code that I show. The unit tests are what make sure that I don't inadvertently change functionality as I refactor the code. Much of the refactoring involves extracting out pieces of code and putting them into their own methods. This makes the code easier to navigate.
I do get questions about my testing technique as I show this code. Here's a question that I got at Nebraska.Code() right after
the presentation:
 |
| Question Time! |
How do you modify your tests after extracting code into the new private methods?
The short answer is:
I don't.
The longer answer is my approach to unit testing:
I test the public members of my code.
Let's take a look at some examples so that I can show this in action, and then I'll talk about why I take this particular approach.
Refactoring Code
Here's the "Initialize" method that we start with:
During the process of making this more readable, I extract out a couple of methods and move some assignments around. What we end up with are a couple of private methods in addition to our public one:
This takes some of the details and "hides" them in private methods. This way, when we first walk up to the "Initialize" method, we can easily decide which parts of the code are important for what we're doing. If we don't care about the dependency injection container bits, then we can skip right over those and go to the "RefreshCatalog" method.
Here's another example; this time we refactor the "RefreshCatalog" method. Here's the original:
And the version with bits extracted:
This makes "RefreshCatalog" much easier to follow. We get a high-level overview of what the method is doing. If we need to look at details, then we can drill into those methods. This is especially appreciated when you walk up to this method for the very first time. (And we have to remember that sometimes we keep walking up to the same method "for the very first time" over and over again -- we get put on other projects and have to come back to this application 6 months later to make some enhancements, and we have to get our bearings again.)
Testing Public Members Only
So why don't my unit tests change as part of this process? Because I'm only testing the public members of my class -- whether public methods, properties, or events.
So if we turn on CodeLens and take a look, we'll see that only the public methods have unit tests:
In the case of "Initialize", we see that there are 19 tests (which is all of them). This is because each of our tests runs the "Initialize" method as part of the setup even if it's not explicitly testing this code.
We see something similar for "RefreshCatalog":
In this case we have 4 tests that call "RefreshCatalog" directly. In 2 of these tests, we check to see if the service is called based on the state of the cache:
If we look at the details of the first test, we see that it does call "RefreshCatalog":
And it also indirectly calls the "IsCacheValid" property and the "RefreshCatalogFromService" method.
I won't go into the other details of this test; it gets a little weird because we're testing an asynchronous service that uses APM (Asynchronous Programming Model) wrapped in a Task which (sort of) changes it to TAP (Task Asynchronous Pattern). So there's a little helper object ("tracker") to make testing easier. This would be a good topic for another day.
[Update: You can see how the tracker works in this article: Tracking Property Changes in Unit Tests.]
Why Not Test Private Members?
So the question is why don't I create tests for the private members? My reasoning is that I like to keep the code as clean as possible.
When I create production objects, I want to modify the code as little as possible for testing. When we want to test private members, we basically have 3 options:
Option 1: Reflection
Our first option to test private members is to use reflection to crack open our class so that we can access the bits of code we're not normally allowed to look at.
I don't really like this option because our tests become *very* complicated very quickly. Reflection is one of those things that is difficult to understand on its own. When we make it a requirement for our unit tests, then we're just asking for trouble.
The end result when we take this approach is that we just don't bother with tests because they are too difficult to create.
Option 2: Change Access Modifiers to Public
Another option is to change the "private" members that we want to test to "public."
I'll just cross this option off immediately. I don't want to mess with making things visible to the outside world when it's not appropriate. Scoping and visibility are huge parts of building good objects, libraries, and APIs. We can't compromise that for testing.
Option 3: Change Access Modifiers to Protected
Another option is to change the "private" members that we want to test to "protected." When we do this, we can then create a wrapper class in our tests. This wrapper class descends from our production class, so it has access to the protected members. It can then supply wrapper methods or properties to access the protected members of the base class.
I don't like this option, either. The main reason for that is that I feel like I'm not testing my production code. Instead of testing my actual class, I'm testing some mutation of that class. I'm not confident that the behavior between the test class and the production class will be identical, so I lose faith in my tests.
What if I Really Need to Test a Private Member?
In the examples that I've shown here, it's pretty easy to say, "I'm okay testing at a higher level." By testing the "RefreshCatalog" method, we end up testing all of the private members indirectly.
But what if one of those private members is so important to my functionality that I really want to test it directly?
For example, if I'm accepting credit cards, I want to do the
Luhn check against them. This will at least make sure that the number itself is potentially valid before checking against a credit card processor.
My answer to this is pretty simple:
If a private method is important enough that I need to test it directly, it's probably important enough to have its own class.
This really takes me to the Single Responsibility Principle and Separation of Concerns. Needing to test a private method directly is a code smell. If I run across that, then it probably means that it's a separate concern that needs its own place in the code.
When I extract that out into its own class (whether an instance class or a set of related methods in a static class), now I can test that class directly.
Of course, we would have to get into a discussion on the proper visibility of the new class and methods. But that's easier to take care of. If I have a protected or internal class that's only available to the code in the same assembly or namespace, I can create a test class that is only responsible for directly calling into this protected class. But I'm not wrapping the class itself, just creating a test class that is capable of calling into the real class.
Many Approaches to Testing
There are many approaches to unit testing. And I will be the last one to say that this is an example that everyone should follow. This particular approach of testing the public members has worked out well for me in the majority of situations that I've run into. It has the added benefit of being fairly resistant to refactoring.
When we create the public members of our classes, these generally remain unchanged. This is because the public interface is how the outside world interacts with our objects. So we try to change these as little as possible.
The private members, however, are subject to change. We can rename private methods, move things to other methods, properties, or classes, and combine similar code into consolidated methods. If we are directly testing the private members, we're less likely to make these types of changes because it would mean making major changes to our tests as well (especially if we're using reflection which would not give us compile-time errors, only run-time errors).
Wrap Up
I encourage people to try different approaches to testing. Each has its advantages and disadvantages. I've managed to dial in an approach that works well for me and the types of applications that I normally build. But that doesn't mean that I don't keep exploring.
I'm still working on TDD. I've had some really good successes with it lately, and I've got another piece of code that needs some help, so I'll be doing some more experimentation this week.
Overall, automated testing gives me confidence in my code. The tests are proof that my code does what I think it does. And I can get immediate feedback by re-running tests whenever I change code (without having to run my application).
So experiment, try different techniques, and come up with a testing approach that works well for you. Ultimately, you will end up with better code.
Happy Coding!
