29

In another question, it was revealed that one of the pains with TDD is keeping the testing suite in sync with the codebase during and after refactoring.

Now, I'm a big fan of refactoring. I'm not going to give it up to do TDD. But I've also experienced the problems of tests written in such a way that minor refactoring leads to lots of test failures.

How do you avoid breaking tests when refactoring?

  • Do you write the tests 'better'? If so, what should you look for?
  • Do you avoid certain types of refactoring?
  • Are there test-refactoring tools?

Edit: I wrote a new question that asked what I meant to ask (but kept this one as an interesting variant).

  • 7
    I would have thought that, with TDD, your first step in refactoring is to write a test that fails and then refactor the code to make it work. – Matt Ellen Sep 21 '10 at 12:55
  • Can't your IDE figure out how to refactor the tests too? – user1249 Aug 15 '11 at 6:19
  • @Thorbjørn Ravn Andersen, yes, and I wrote a new question that asked what I meant to ask (but kept this one as an interesting variant; see azheglov's answer, which is essentially what you say) – Alex Feinman Aug 15 '11 at 13:16
  • Considered adding thar Info to this question? – user1249 Aug 15 '11 at 13:47
35

What you're trying to do is not really refactoring. With refactoring, by definition, you don't change what your software does, you change how it does it.

Start with all green tests (all pass), then make modifications "under the hood" (e.g. move a method from a derived class to base, extract a method, or encapsulate a Composite with a Builder, etc.). Your tests should still pass.

What you're describing seems to be not refactoring, but a redesign, which also augments the functionality of your software under test. TDD and refactoring (as I tried to define it here) are not in conflict. You can still refactor (green-green) and apply TDD (red-green) to develope the "delta" functionality.

  • 7
    Same code X copied 15 places. Customized in each place. You make it a common library and parameterize the X or use strategy pattern to allow for these differences. I guarantee the unit tests for X will fail. Clients of X will fail because the public interface changes slightly. Redesign or refactor? I call it refactor but either way it breaks all sorts of things. The bottom line is you can't refactor unless you know exactly how it all fits together. Then fixing the tests is tedious but ultimately trivial. – Kevin May 18 '11 at 0:35
  • 3
    If tests needs constant adjustment, it's probably a hint of having too detailed tests. For instance, suppose that a piece of code needs to trigger events A, B and C under certain circumstances, in no particular order. The old code does it in order ABC and tests expect the events in that order. If the refactored code spits out events in order ACB it still works according to the spec but the test will fail. – otto Aug 15 '11 at 12:38
  • 3
    @Kevin: I believe what you describe is a redesign, because the public interface changes. Fowler's definition of refactoring (" altering [the code's] internal structure without changing its external behaviour") is quite clear about that. – azheglov Aug 18 '11 at 19:35
  • 3
    @azheglov: maybe but in my experience if the implementation is bad so is the interface – Kevin Aug 18 '11 at 20:04
  • 2
    A perfectly valid and clear question ends up in a “meaning of the word” discussion. Who cares how you call it, let’s have that discussion somewhere else. In the mean time this answer is completely omitting any real answer but still having the most upvotes by far. I get why people refer to TDD as a religion. – Dirk Boer Feb 14 at 11:59
21

One of the benefits of having unit tests is so you can confidently refactor.

If the refactoring does not change the public interface then you leave the unit tests as is and ensure after refactoring they all pass.

If the refactoring does change the public interface then the tests should be rewritten first. Refactor until the new tests pass.

I would never avoid any refactoring because it breaks the tests. Writing unit tests can be a pain in a butt but its worth the pain in the long run.

7

Contrary to the other answers, it is important to note that some ways of testing can become fragile when the system under test (SUT) is refactored, if the test is whitebox.

If I'm using a mocking framework that verifies the order of the methods called on the mocks (when the order is irrelevant because the calls are side-effect free); then if my code is cleaner with those method calls in a different order and I refactor, then my test will break. In general, mocks can introduce fragility to tests.

If I am checking the internal state of my SUT by exposing its private or protected members (we could use "friend" in visual basic, or escalate the access level "internal" and use "internalsvisibleto" in c#; in many OO languages, including c# a "test-specific-subclass" could be used) then suddenly the internal state of the class will matter - you may be refactoring the class as a black box, but white box tests will fail. Suppose a single field is reused to mean different things (not good practice!) when the SUT changes state - if we split it into two fields, we may need to rewrite broken tests.

Test-specific-subclasses can also be used to test protected methods - which may mean that a refactor from the point of view of production code is a breaking change from the point of view of test code. Moving a few lines into or out of a protected method may have no production side effects, but break a test.

If I use "test hooks" or any other test-specific or conditional compilation code, it can be hard to ensure that tests don't break because of fragile dependencies on internal logic.

So to prevent tests from becoming coupled to the intimate internal details of the SUT it may help to:

  • Use stubs rather than mocks, where possible. For more info see Fabio Periera's blog on tautological tests, and my blog on tautological tests.
  • If using mocks, avoid verifying the order of methods called, unless it is important.
  • Try to avoid verifying internal state of your SUT - use its external API if possible.
  • Try to avoid test-specific logic in production code
  • Try to avoid using test-specific subclasses.

All of the points above are examples of white-box coupling used in tests. So to completely avoid refactoring breaking tests, use black-box testing of the SUT.

Disclaimer: For the purpose of discussing refactoring here, I am using the word a little more broadly to include changing internal implementation without any visible external effects. Some purists may disagree and refer exclusively to Martin Fowler and Kent Beck's book Refactoring - which describes atomic refactoring operations.

In practice, we tend to take slightly larger non-breaking steps than the atomic operations described there, and in particular changes that leave the production code behaving identically from the outside may not leave tests passing. But I think it is fair to include "substitute algorithm for another algorithm that has identical behaviour" as a refactor, and I think Fowler agrees. Martin Fowler himself says that refactoring may break tests:

When you write a mockist test, you are testing the outbound calls of the SUT to ensure it talks properly to its suppliers. A classic test only cares about the final state - not how that state was derived. Mockist tests are thus more coupled to the implementation of a method. Changing the nature of calls to collaborators usually cause a mockist test to break.

[...]

Coupling to the implementation also interferes with refactoring, since implementation changes are much more likely to break tests than with classic testing.

Fowler - Mocks aren't stubs

  • Fowler literally wrote the book on Refactoring; and the most authoritative book on Unit testing (xUnit Test Patterns by Gerard Meszaros) is in Fowler's "signature" series, so when he says the refactoring can break a test, he's probably right. – perfectionist Jun 20 '16 at 10:57
5

If your tests break when you're refactoring, then you're not, by definition, refactoring, which is "changing the structure of your program without changing the behaviour of your program".

Sometimes you DO need to change the behaviour of your tests. Maybe you need to merge two methods together (say, bind() and listen() on a listening TCP socket class), so you have other parts of your code trying and failing to use the now altered API. But that's not refactoring!

  • What if he just changes the name of a method tested by the tests? The tests will fail unless you rename them in the tests too. Here he isn't changing the behavior of the program. – Oscar Mederos Feb 1 '11 at 7:26
  • 2
    In which case his tests are also being refactored. You need to be careful though: first you rename the method, then you run your test. It should fail for the right reasons (it can't compile (C#), you get a MessageNotUnderstood exception (Smalltalk), nothing seems to happen (Objective-C's null-eating pattern)). Then you change your test, knowing that you haven't accidentally introduced any bug. "If your tests break" means "if your tests break after you've finished the refactoring", in other words. Try keep the change chunks small! – Frank Shearar Feb 1 '11 at 8:31
  • 1
    Unit Tests are inherently coupled to the structure of the code. For example, Fowler has many in refactoring.com/catalog that would affect unit tests (e.g., hide method, inline method, replace error code with exception, and so on). – Kristian H Jul 17 '18 at 14:57
  • false. Merging two methods together is obviously a refactoring which has official names (e.g. inline method refactoring fits the definition) and it will break tests of a method that is inlined - some of the test cases now should be rewritten / tested by other means. I don't have to change behavior of a program in order to break unit tests, all I need to do is to restructure internals that have unit tests coupled with them. As long as behavior of a program not changed this still fits the definition of refactoring. – KolA Mar 26 at 1:35
  • I wrote the above assuming well-written tests: if you're testing your implementation - if the structure of the test mirrors the internals of the code under test, sure. In which case, test the contract of the unit, not the implementation. – Frank Shearar Mar 26 at 16:01
4
+100

I think the trouble with this question, is that different people are taking the word 'refactoring' differently. I think it's best to carefully define a few things you probably mean:

>  Keep the API the same, but change how the API is implemented internally
>  Change the API

As one other person already noted, if you are keeping the API the same, and all your regression tests operate on the public API, you should have no problems. Refactoring should cause no problems at all. Any failed tests EITHER mean your old code had a bug and your test is bad, or your new code has a bug.

But that's pretty obvious. So you PROBABLY mean by refactoring, that you are changing the API.

So let me answer how to approach that!

  • First create a NEW API, that does what you want your NEW API behavior to be. If it happens that this new API has the same name as an OLDER API, then I append the name _NEW to the new API name.

    int DoSomethingInterestingAPI();

becomes:

int DoSomethingInterestingAPI_NEW( int takes_more_arguments );
int DoSomethingInterestingAPI_OLD();
int DoSomethingInterestingAPI() { DoSomethingInterestingAPI_NEW (whatever_default_mimics_the_old_API);

OK - at this stage - all your regression tests sill pass - using the name DoSomethingInterestingAPI ().

NEXT, go through your code and change all calls to DoSomethingInterestingAPI() to the appropriate variant of DoSomethingInterestingAPI_NEW(). This includes updating/rewriting whatever parts of your regression tests need to be changed to use the new API.

NEXT, mark DoSomethingInterestingAPI_OLD () as [[deprecated()]]. Keep around the deprecated API as long as you like (until you've safely updated all code that might depend on it).

With this approach, any failures in your regression tests simply are bugs in that regression test or identify bugs in your code - exactly as you would want. This staged process of revising an API by explicitly creating _NEW and _OLD versions of the API allows you to have bits of the new and old code coexisting for a while.

  • I like this answer because it makes it obvious that Unit Tests to the SUT is the same as external clients to a published Api. What you prescribe is very similar to SemVer protocol to manage published library/component in order to avoid 'dependency hell'. This is however comes at a cost of time and flexibility, extrapolating this approach to public interface of every micro unit means extrapolating costs too. A more flexible approach is to decouple tests from implementation as much as possible i.e. integration testing or a separate DSL to describe test inputs and outputs – KolA Mar 26 at 1:43
1

I assume your unit tests are of a granularity that I would call "stupid" :) ie, they test the absolute minutiae of each class and function. Step away from the code-generator tools and write tests that apply to a bigger surface, then you can refactor the internals as much as you want, knowing that the interfaces to your applications have not changed, and your tests still work.

If you want to have unit tests that test each and every method, then expect to have to refactor them at the same time.

  • 1
    Most useful answer that actually addresses the question - don't build your test coverage on a shaky foundation of internal trivia, or expect it to constantly fall apart - yet most downvoted one because TDD prescribes to do exactly the opposite. This is what you get for pointing out inconvenient truth about an over-hyped approach. – KolA Mar 26 at 4:06
1

keeping the testing suite in sync with the codebase during and after refactoring

What makes it difficult is coupling. Any tests come with some degree of coupling to implementation details but unit tests (regardless whether it's TDD or not) are especially bad at it because they interfere with internals: more unit tests equals more code coupled to units i.e. methods signatures / any other public interface of units - at very least.

"Units" by definition are low-level implementation details, interface of units can and should change/split/merge and otherwise mutate as system evolves. Abundance of unit tests can actually hinder this evolution more than it helps.

How to avoid breaking tests when refactoring? Avoid coupling. In practice it means avoid as much unit tests as possible and prefer higher level / integration tests more agnostic of implementation details. Remember though that there is no silver bullet, tests still have to couple to something at some level but ideally it should be interface that is explicitly versioned using Semantic Versioning i.e. usually at the published api/application level (you don't want to do SemVer for every single unit in your solution).

0

Your tests are too tightly coupled to the implementation and not the requirement.

consider writing your tests with comments like this:

//given something
...test code...
//and something else
...test code...
//when something happens
...test code...
//then the state should be...
...test code...

this way you can't refactor the meaning out of tests.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.