Revise Tests While Refactoring? It Depends

Some refactorings (e.g., Extract Method or Extract Class) introduce new elements. Should you revise the tests to take these into account? The classic consultant’s answer applies: It depends. The obvious next question is “Depends on what?”, and we’ll explore some of those tradeoffs.

Example: Extract Class

Let’s take the Extract Class refactoring. It looks like this:

A complex class gets split into a simpler class and a helper class.

Choices in Testing

Our default testing situation looks like this:

Keep the original tests to cover newly created entities

We have whatever tests drove Class into existence. Since the Extract Method refactoring doesn’t change the interface to Class, the test will be unchanged. Whatever paths it tested before will be tested now, either in Class or Helper.

The question we have to address is: “Is the original test good enough? Or should we move some of those tests to another test class?”

Add new tests for new entities, simplifying existing tests

If you do split out the new tests, your original tests will almost certainly get simpler: some of your tests required setup of Class to test the Helper aspects. You can instead only set up what Helper needs.

It’s not a case of “every method and every class needs to be tested directly”, but rather, “Can I trust this entity to behave as expected?”

Leaning on the Original Tests

When might you lean on the original tests?

  • You’re exploring designs, not committed to a particular breakdown: If the design is likely to shift, the cost of splitting out tests is relatively high (since we’ll be splitting or merging them again).
  • The extracted class is too simple: For example, if it’s a “data bag” with just a field or two and setter/getter, this is probably not a stable point for the design. The usage is so simple, the original tests are likely sufficient.
  • You’re in a rush: I don’t like this one, but it does happen. You don’t want to pay today, and so you keep the original tests, knowing that there’s no other usage (and no other usage pattern) of the helper so it suffices.
  • You don’t see a benefit: It doesn’t look like the overall tests will get simpler or more clear if you split them.

A Caution on Usage Patterns

When you extract a method or class and make it public, you create a hidden danger.

Before you extract the helper, the to-be-extracted code has a certain pattern of use. For example method x() is always called before method y(), and it sets up data critical for y(). If you extract the code so that both x and y are public, there’s nothing to tell a new client that this constraint exists.

Such usage patterns create unexpected opportunities for defects. From a design point of view, I don’t like these dependencies in an interface, but you’re starting from where you are.

Adding New Tests

Why might you decide a new test is warranted?

  • You want to test a private method and/or want to increase observability: Extracting the class gives you more direct access to inner mechanisms, which you may need. Remember, not every class can be verified through its public interface. I recently described a moderately complex cache – if it’s working right, you’ll see a performance difference, but not a functional difference. Proper testing required more direct access to the cache. (See the References.)
  • The entities change at a different rate of speed: It will be easier to change the faster-changing class if it has more restricted tests.
  • You want to reuse the helper’s code elsewhere: You may have different access patterns in the other clients; the original test probably only tests the original access pattern. This pushes you to new tests covering all reasonable uses.
  • You want to decouple the two classes more: For example, suppose the helper you’ve extracted is really acting like a strategy, and we may want to plug in alternative strategies. This can simplify the original tests significantly if they no longer care how the job is done.

A Range of Options

“Take it or leave it” aren’t the only options; there are several choices, including:

  1. Leave tests as is: test only through the original class.
  2. Extract tests for the helper: simplify the original tests, and ensure the new class is tested directly.
  3. Leave as is, but add new tests to the helper whenever you modify it: New helper behaviors get tests, while the original behavior is tested by the original tests.
  4. Leave as is for now, but use any modification of the helper as a trigger to backfill its tests: New patterns of use of the helper require new tests.

I default to creating new tests because then you can assume that your objects are tested and working. When you extract a class (or other entity), you understand both classes, so it’s a good time to separate tests as well as classes.

Summary

When you refactor to create a new entity, you face a decision about whether to modify tests as well. Some reasons you might not: exploring designs, judging it’s not worth it, or in a rush (accepting any increase in technical debt). Some reasons you might create a new test: testing a private method or behaviors that require more observability, managing things that change at a different rate of speed, using the entity in multiple places (with different access patterns), and explicitly decoupling entities so they don’t rely on each other as much.

My leaning is to improve tests for new entities, but it is a tradeoff decided by particular circumstances.

References

Programming: Using What’s Hidden“, by Bill Wake. Retrieved 2021-06-08.