Stop mocking me

Unit testing can be hard. And it gets harder when you have to test code with with external dependencies. The way you deal with this will depend on whether you are a classical or mockist programmer. Most people don’t exist at the extremes, but instead are somewhere along the line. This isn’t a bad thing. Being pragmatic in your decisions shows that you’re a mature engineer.

Dealing with legacy code is also a thorny problem that most of us will encounter. I don’t believe that anyone can avoid this in their career. In a recent blog post, Federico Tomassetti describes his approach to this problem. He talks about a minor change that he made, and the testing he wrote to support it. The problem is, the change he needed to make is in a private static method – dealsToDisplay. And as he found, writing unit tests for that wasn’t straightforward.

What Federico goes on to describe is his process of writing unit tests around dealsToDisplay. He exposes his method using PowerMock and reflection to manipulate the code at runtime. What he comes up with is pretty cool, and it does the job. In fact, it’s a pretty neat solution to the problem that faced him. It means he touches the minimal amount of code, producing his fix with tests.

My only reservation is should Federico have had to go to these lengths to test his changes? Also, in doing so, was he ignoring the underlying problem?

What follows is a my approach to fixing the issue and refactoring the code. I’m not saying that what Federico did was right or wrong. In fact I’m pretty certain he did the right thing as he had full visibility of the context. It’s a nice exercise to look at the problem and discuss the alternatives.

I’ve been in situations where all you want to do is get in, fix the code and get back out again. The last thing you want to do is get stuck in the weeds. But as you apply this approach over and over again, the weeds will get higher and thicker. Sometimes you need to start attacking the root.

Back to Federico’s problem. Let’s think about what that private static method means. The method is only visible to its host class. This means that anything calling this method also lives in the same class. Knowing this, my immediate thought here wouldn’t be of how to invoke the method in a test.

First up, I would have looked at the callers of dealsToDisplay. My thoughts would be to wrap those in tests covering my change. Depending on how spiky the code is, this might be just as difficult. I also might struggle to isolate and test my changes.

This might not be possible, or there may be too many different callers to cover. If this were the case, I would need to take a different approach.

From what I can infer from the tests and name of method, dealsToDisplay is acting as a filter. In my mind, this is enough for me to promote this to a new abstraction. In this case, my choice might be to create a new class called DealFilter. This class would have the single method, ‘dealsToDisplay’ containing the existing code.

class DealFilter {
  Map<UUID, Set<String>> dealsToDisplay(/*Params..*/) {
    // ...
  }
}

Next up, I’d change the original class to include a private static instance of this class.

private static final DealFilter DEAL_FILTER = new DealFilter();

All original callers of ‘dealsToDisplay’ would now call the method from the deal filter.

DEAL_FILTER.dealsToDisplay();

I understand that it looks like we’re introducing abstractions here to test our code. I also know that this can be a dangerous path to tread. David Heinemeier Hansson has spoken at length about Test Induced Design Damage. One could argue that we’ve just damaged our code. Federico recognises this risk too. He speaks of the problems that can come from introducing abstractions, complete with interfaces.

Programming to interfaces is generally a good guideline to follow. But, it’s not a hard and fast rule. Sometimes, the extra layer of indirection that comes from interfaces isn’t required. You aren’t going to replace it, and YAGNI. It’s also no longer necessary for mocking in tests. Mockito (along with other good mocking frameworks) can mock classes.

In the example, Federico also mocks out some dependencies that dealsToDisplay calls. He is liberal with his mocking, and mocks out URLEncoder (although I’m not sure if JDK version). This is something Writing a unit test does not have to mean complete isolation. It’s about being pragmatic. I’m happy for URLEncoder to do it’s thing when my tests are running. I’m happy for Logger to log. Mocking these entities doesn’t provide any more value for me. They don’t slow down the tests, and I don’t need to control their responses.

The concept of a “unit” is on a sliding scale. Some people may tell you different, but you can still write unit tests depending on collaborators. If you’re always taking a by-the-book approach to this then you might be making life more difficult for yourself.

And one final thought, I’ve added the use of PowerMock to the list of “code smells“. If I ever find myself reaching for it, I take a step back and look for the underlying problems.

Of course, I’m making all these statements without knowing the code in question. I’m showing no respect to the context. So, with this in mind, you should just ignore everything you’ve just read.

The future for manual testing

Is being a manual tester a dead-end career as @jsonmez says in his video response for Simple Programmer?

I’m a developer and I’ll admit that I’ve looked down on testers in the past. It’s not something I’m proud of. I was wrong.

The argument that you need to be a developer to progress with a career in technology is harming our industry. Videos and comments like the ones from John are not helping to get rid of the stigma associated with testing.

There has been a drive to remove all testers from modern development teams. After all, we now have some great test automation frameworks, and developers who can write code. So why would we need to run manual testing? I have to admit, I’ve been part of this school of thinking in the past.

My previous company laid off all our testers apart from those that could write code. These testers became automation testers. At first this made sense, yet it became clear that this wasn’t the right thing to do. Removing the manual testing harmed the quality. Our automated tests were covering the things we knew, meaning that we were confident on these features. It came at the cost of not exploring the product. We missed a lot of the “what if” testing.

Exploration is a key part to testing a product. So much so, that James Bach and Michael Bolton have gone a long way to make the distinction between testing and checking. We should classify the automated testing I mention above as automated checking. When we do, it becomes clear that we’re missing a large part of the picture.

I think, and I hope, that checking is what John is talking about. One can argue that the manual execution of checks, release after release assigning a Pass/Fail could is a dying role. It’s probable that you’ll be able to replace a lot of manual checks with automated checks.

Testing is so much more than that. My current team writes unit and acceptance tests, which are in fact checks on the functionality we’re building. They’re pretty much all automated and they’re written by our developers. This frees our testers up to do what they do best. The testers can go deep into the product, run experiments and make new discoveries about the things we’re building. I find that allowing testers to do this adds more value to the team.

In some of the comments on his Simple Programmer post, John refers to the view in the corporate world between developers and testers. He also comments that developers tend to earn more, as testers reach a glass ceiling. Unfortunately, this will be the case if the organisation shares John’s view that all testing results in a Pass or Fail from a check.