1

When we work with legacy code and need to do changes, we first write tests on the current behavior. That way we can implement new changes with confidence. We can even refactor the code.

Legacy code is often bad code, and after some refactoring the code may be simpler, easier to test. Since the refactor has been validated by the tests should we also refactor the tests if we can make them simpler/clearer or keep them as they were written?

  • 5
    refactor code XOR refactor tests never both at the same time – Ewan Aug 26 '20 at 14:33
  • 1
    @Ewan Beware overapplication of your statement. If you rename a method, you obviously need to change that in the test as well. If you intended to focus on changing the purpose of a test, I agree. But syntax can change during refactoring and you can't quite avoid that. – Flater Aug 26 '20 at 20:07
  • 1
    you can avoid it with 2min extra work and you'll know your tests are still good. and remember, knowing is half the battle – Ewan Aug 26 '20 at 22:34
4

Automated tests ARE code, so maintaining this code makes sense, including refactoring tests when appropriate. However:

  • productive code and tests have different quality requirements

    • don't invest time into things that don't matter
  • productive code should have a single source of truth, whereas tests should be largely self-contained

    • duplication isn't necessarily bad

And as Ewan points out, you should never change the code and the tests at the same time. Together, code+tests are a self-testing system. A change in one part is verified by running it together with the other part. Changing both at the same time gives up this safety. This is not always possible in practice (e.g. when changing a cross-cutting concern such as an underlying standard library), but it would be foolish to give up this safety without a very strong need.

Common reasons I've refactored tests, in no particular order: the API being tested had changed, switching to a different testing approach (e.g. scenario-based testing vs property-based testing, API-level tests vs behaviour level tests), changing the testing framework (e.g. to get better failure reports or to use parametrized tests), changing the test organization (e.g. xUnit style suites and cases vs RSpec style describe–it), getting rid of accumulated duplication (e.g. extracting common code to create a fixture), …

2

When we work with legacy code and need to do changes, we first write tests on the current behavior. That way we can implement new changes with confidence. We can even refactor the code.

That may be reflecting sometimes your working process, but in my experience, a way more efficient process is:

  1. you write tests

  2. you refactor to make a change easier

  3. you implement the change

This way, it becomes more apparent that you refactor when there is a real reason for a change, not just because the code "is not clean any more".

Now try to apply the same measures to your tests: you don't refactor your tests because "they are not clean any more". You refactor them when they start hindering you to make easy changes to your existing code.

For example, when you have ten tests all calling the same public method of a class under tests, whilst in your production code that public method is only called in one place, then this is a form of code duplication by tests which may hinder you to change the signature of that public method.

I would usually leave it that way unless you really get the requirement for the latter, or more general: when you notice this code duplication requires you to make the same change to your tests in several places.

0

You might want to begin with refactoring the tests.

The tests capture what the legacy application does; a kind of documentation. Tests tell you input and output, code tells you process. If the tests are bad then tidying them up (depending on how bad they are) will help you understand the code.

It's also a great way to judge whether the tests adds value; a legacy system I worked on had great code coverage but on inspection the tests asserted on pointless things... like ensuring Getters and Setters worked (testing the .NET framework not the application).

Once you got a clean test, you will have a better understanding of the code, and then you can make better decisions on how to refactoring the code.

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.