Test any code, even the code you are not proud of | Toucan Toco

A pragmatic approach to unit testing in legacy code (code that people think isn’t testable and that nobody wants to touch anymore). Using simple mocks and design patterns, in javascript.

Once upon a time…

Have you ever heard something like:

“It’s ok, we don’t test that code.”

“Are you sure you want to touch that code? You might break something.”

Sounds familiar no? Usually, it even refers to the same code.

Testing in “good” code is really easy. Just follow the documentation! By “good” code, I mean:

• Code that everyone in your team understands (readability)
• Code that everyone in your team loves to contribute to, and they do it very fast (maintainability)
• Code that everyone in your team agrees on (best practices, conventions)
• If you use a library / framework:
  • the library / framework is recent (or at least it is still maintained)
  • it is quite common (today: VueJS, React, Angular…), or at least uses common concepts so that you can on-board new devs on it easily
  • the code follows the common patterns and practices of this library / framework

Luckily for us we have all these libraries/frameworks that (supposedly) make our life very easy. But let’s remember again, have you ever heard something like:

“Use framework/tool XXXX you will see, it is so awesome, it’s going to work perfectly!! You’re going to love it!

And then you’re like

Ok XXXX seems great! I’m going to start using it all over the place, wohooo!

Some weeks later

Oh I have to do this feature very fast, let’s just do this small hack, by using this dark feature of the framework. It’s ok, temporarily. (…)

We’re so late! We will write tests later.

Over the weeks, people have extracted this code into an utility and is used in 10% of your 100 000 lines of code app.

Let’s upgrade XXXX! (…) Oh my god, what have we done? This is going to take months!

So I bet that all your code is not “good” code, and that you have to deal with legacy code (as most developers have to). If you don’t think you have legacy code, I am pretty sure you are going to change your mind in a couple of months.

That is because most code have an expiration date, more or less in the future. No one can escape that. We are humans, we are limited, we have to take difficult decisions. We have to think about the long term in a very fast changing environment. And sometimes, we make small mistakes. Overtime, these mistakes accumulate to make a mess. But I think it is OK. We have to accept that the code we write today is the legacy code of tomorrow.

But we can still improve this code. Even if your code has expired for so long that’s it’s become a rotten spaghetti monster. The one that smells so bad that you are really afraid of it, you avoid touching any of it, and you dream of trashing it once and for all.

So let’s start to “Make your code great again”!

How do you do that? There are already well known community-approved solutions:

• Tests (and related methodologies such as TDD)
• Thorough code reviews
• Pair programing, mob programming or other kind of group work
• Domain Driven Design
• “Slack time” to refactor
• Zero bug policy
• …

This article focuses on unit tests. I think tests are one of the most difficult thing to do with legacy code. But it’s one of the most important things to do. Sadly, it’s also one of the things that is often put aside, especially in front end development.

Tests allows you to:

• prove that your code works as expected for your end user
• explain your code to the next developer that will have to maintain it
• have better technical designs by forcing you to isolate logical units (so you can test them)
• write exhaustive functional specifications of your component (made explicit by tests titles, like “it should do this”)

I will talk more about why we should test in another article.

We will see that there are a lot of choices that we will have to do while writing tests. But one thing should help us make those choices: we should be as pragmatic as possible in our test strategy.

We want to spend only the time we can afford to on those tests: not a few seconds, nor a few months. Just the time needed to write tests in our new code so that the following can be achieved :

• the tests prove that what’s important for the end user will work
• the tests explain the logic behind your code for the next developer that will have to change it

We also want to test only your new code. We don’t want to take months to refactor the entire existing codebase and write unit tests for all of it. We’re taking a pragmatic incremental approach: every new logic should be tested. We don’t want to test libraries or tools either: for exemple, we trust that the browser will display a <div> correctly.

Of course you can spend more time if you can afford it. You will then refactor more, test more existing code. That will lead to: better designs, exhaustive functional specifications… But be careful; you also want to deliver functionality to you end users. Think about what’s best for them and for your team.

So let’s start to write tests! Every section below contains a pragmatic approach to test in legacy code; at the end of each section, a recap sums up the pros and cons of the approach.

Use the framework!

If the code you want to contribute to uses a framework that supports testing, just follow the documentation of this framework! But I think that if you are reading this, you have to deal with a tricky codebase that have old frameworks or old code that is hardly testable.

If you want to create a component or add much functionality to an existing component, you can just refactor directly into a “new” framework. And it’s feasible, using the Adapter design pattern (also called wrapper).

As an example, I recently updated the design of the login page in the Toucan Toco front end. Because the login page is one of the first thing you have to do when you make a front end, the code was an old AngularJS mess. I re-wrote the component with TDD in less than one week, in VueJS, using an Adapter.

The adapter between vueJS and AngularJS looks like this:

import Vue from 'vue';
import Login from './'; // The vue component

angular.module('users')
.directive('loginNg', (authenticationManager) => {
    return {
      restrict: 'E',
      template: '<div class="login__vue-placeholder"></div>',
      link: (scope, element) => {
        const loginWrapperVm = new Vue({
          el: element.find('.login__vue-placeholder')[0], // 1. Insertion in the DOM
          name: 'login-wrapper',
          provide: {
            authenticationManager // 3. Adaptation of an old angular service
          },
          components: { Login },
          render(h) {
            h('login');
          }
        });
        scope.$on('$destroy', () => loginWrapperVm.$destroy()); // 2. Adaptation of 'destroy' lifecycle.
      }
    }
  }
);

We will talk about our approach to incremental refactoring from AngularJS to VueJS in another article (coming up soon).

This adapter has the role to adapt the AngularJS API to the VueJS API. We can see three interesting adaptations:

1. The insertion in the DOM
2. The destroy lifecycle
3. The authenticationManager service

The authenticationManager service is particularly interesting. I wanted to change the Login page was designed, not the way that all the login and authentication works.

This AngularJS service deals with many aspects of authentication in the app. My first thought was to refactor it as well. But it is a very complex class (400 lines of code) with a lot of state hidden inside. It is also used widely in different parts of the legacy code: I would have had to refactor that as well. So refactoring would have been very difficult, and instead of a week, it would have taken a month.

But injection comes to our rescue! When you want to partially refactor a component without changing all the code that depends on this component, you can just inject the “old” objects that have to be called from your refactored code. For our Login VueJS component, it goes like this:

const Login = {

  inject: ['authenticationManager'],

  // Some login component code ...

  methods: {
      
    // Some other methods ... 
    
    performLogin(username, password) {
      API.auth.login(username, password) // HTTP call to the API
        .then(data => {
          this.authenticationManager.onLoginSuccessful(username, data); // Call to the legacy angular service dealing with authentication
        })
    }
  }
}

Many frameworks support injection, or similar mechanisms. Worst case scenario, you can pass your legacy APIs as object parameters of your component. You could even export a function that creates or returns the current instance of your legacy JS service (see the factory section).

Anyway, with that we have brand-new code using our “modern” framework and we’re happy. We know how to test it. But how do we test the Adapter? and the injection? Sadly, you can’t. You can only test that the injected object is called correctly in the new code:

describe('Login', () =>

  beforeEach(function() {
    this.mockAuthenticationManager = { onLoginSuccessful: sinon.spy() }
    this.wrapper = shallowMount(Login, {
      provide: {
        authenticationManager: this.mockAuthenticationManager 
      }
    });
        
    context('when the user inputs his username and his password, and then clicks login', function() {
      beforeEach(function() {
        // ... Set the username, password and clicks login ...
      });

      it('should notify the authentication manager of the successful login', function() {
        // Assert that the legacy API have been called 
        this.mockAuthenticationManager.onLoginSuccessful.should.have.been.calledWith('USERNAME', 'LOGIN_RESPONSE_DATA'); 
      });
    });
  })
);

But it’s OK it’s just a few lines of code without any business rules (see my article on “What to test?”). It’s mostly boiler plate framework-code that we don’t really care about. If it breaks, you will notice it pretty fast (the login page will not display). Also, it could be tested via multi-component tests or end-to-end tests.

Most of the times you have to take pragmatic choices, and put things into perspective. The code is in better shape than before now (it follows conventions), and we have increased test coverage. But it is true that it is not perfect and we introduced a new piece of code (the adapter) that is not tested, and does not follow the best practices of AngularJS nor VueJS. Also, it required a bit of work; that you might not have the time to do.

Conclusion

To sum up, the adapter:

• good to transform a significant piece of legacy code with an old framework into “new” code with a “modern” framework that you can test
• uses an adaptation code between the two frameworks
• uses injection for the pieces of code you don’t want to refactor (yet)
• requires a bit of work
• adds some code (the adapter) that we do not test

(Sort-of) Factory

Let’s talk about the other cool trick that we have talked about in the previous section: the sort-of Factory Pattern.

Factories are very useful to inject mock-objects at run-time for your tests. They also help you modularize your code. For example, let’s say we are in a chart component, and we want to add a new parameter to the chart legend.

const drawMyChart = function(width, height, hasFilters) {
  
  // ... Some existing D3 JS code ...
  
  // This the new parameter that we want to test
  const displayExtraInformation = (width > 800) && (height > 500) && !hasFilters; 

  // Existing legacy code for the legend
  d3.select('.my-chart__legend-container')
    .append('div')
    .text(d => {
        // Not easily testable code (for some reason)
        if(displayExtraInformation && d.length > 3) { 
          return makeExtraInfo(d)
        } else {
          // More complex untestable code that depend on displayExtraInformation...
        }
    });
};

Let’s also say the code in the text callback is not easily testable (for some reason). So it is hard to test independently when displayExtraInformation is true or not. You could export a getter to this boolean if you make it global, but then it won’t be a const anymore and it would degrade the design by putting the variable as a ‘state’ variable. Also, it’s a lot of modifications that does not necessarily clarify the code.

Let’s see if we can separate the legend in a different module (which make sense, it is a different component, functionally):

import createChartLegend from 'charts/legend';

const drawMyChart = function(width, height, hasFilters) {
  
  // ... Some existing D3 JS code ...
  
  // This is the new code that we want to test
  const displayExtraInformation = (width > 800) && (height > 500) && !hasFilters; 
  
  // New function !
  const myChartLegend = createChartLegend(
    // Other parameters..
    displayExtraInformation
  );
  
  // Wiring to the legacy
  d3.select('.my-chart__legend-container').call(myChartLegend);

  // ... other D3 JS code ...
};

and in ‘charts/legend/index.js’:

const createChartLegend = (displayExtraInformation) => (selection) => {
  selection
    .append('div')
    .text(d => {
        // Not easily testable code (for some reason)
        if(displayExtraInformation && d.length > 3) { 
          return makeExtraInfo(d)
        } else {
          // More complex untestable code that depend on displayExtraInformation...
        }
    });
};
export default createChartLegend;

Now we can stub the createChartLegend and assert the call in our tests! Using the import syntax we have the following:

import sinon from 'sinon'
import drawMyChart from 'charts/my-chart'
import * as ChartLegendFactory from 'charts/legend'


describe('MyChart', function() {
    
  beforeEach(function() {
    this.chartLegendFactoryStub = sinon.stub(ChartLegendFactory, 'default');
  });

  afterEach(function() {
    this.chartLegendFactoryStub.restore();
  });
  
  it('should call the chartLegendFactory with displayExtraInformation as true when the chart is large enough', function() {
    drawMyChart(1200, 900);
    this.chartLegendFactoryStub.should.have.been.calledWith(true);
  });
    
});

If we didnt want to separate the chart legend in a seperate module, we would have done the following:

// Only used for testing
const setChartLegendFactory = (factory) => {
  oldFactory = createChartLegend;
  createChartLegend = factory;
  return oldFactory;
}

var createChartLegend = (displayExtraInformation) => (selection) => {
  // Same as before
};
const drawMyChart = function(width, height, hasFilters) {
  // same as before
};

We would call setChartLegendFactory in beforeEach

this.createChartStub = sinon.stub()
this.originalLegendFactory = chart.setChartLegendFactory(this.createChartStub)

And reinitialize it in afterEach, to avoid side-effects:

this.chart.setChartLegendFactory(this.originalLegendFactory)

Note that this adds some code (here, the setChartLegendFactory function) that is only used for test. It is quite common practice to open some functions or attributes only for testing purposes. Some may argue that it degrades the design of your code, but it is sometimes the most efficient way to be able to test something. I would use it as a last resort when you don’t know how to do otherwise. But if you can inject your object using a clean technique that does not impact your production code, then you should do that instead.

You can also use the factory pattern to inject old legacy code into a new piece of code as we talked about in the Adapter section.

Conclusion

The (sort-of) factory pattern is:

• good for testing small bits of “new” code calling the legacy code
• gives you the ability to inject objects or functions at run-time
• usually, this means mocking the legacy code
• also useful to inject legacy services into “new” code

Functional programing

Functional programing is not a design pattern itself, but a paradigm we can follow to help us test our “new” code. We can see it as a programing style: make everything stateless functions. How can you achieve that? Simple:

1. Identify a block of code with business logic in it (a bunch of if, elses, and calculus). If the block is not clear, try to group the lines of code that seem independent.
2. Identify the inputs and outputs of this block of code. Typically, input variables are variables used at the right of = and output are new variables declared at the left of =, and used later in the code.
3. For the state-dependent variables, the same rule applies.
4. Declare the outputs clearly at the end of your block of code.
5. Create a separate function where parameters of this function are the inputs and the function returns an object containing all the outputs.
6. Put the block of code into this new function; rename the variables if you have to.
7. Call this function where the block of code was.

Got it? Let’s try that in practice. So our example is a D3 chart, and we want to change some part of the layout of this chart. Sadly, this chart is in legacy code, and not tested. We have an existing function scope.update; that does all the layout, and that is very long

// ... Some chart stuff ...
update = function() {
    
  drawVariation();
  drawSparklines();
  
  // ... some very long layout and draw logic
  
}
// ...

So we add our new layout code, not refactored (yet):

// ... Some chart code...
update = function(inputData) {
  let shouldDisplayVariation, estimatedVariationWidth;
  if (scope.chartOptions.variation) {
    estimatedVariationWidth = estimateValueWidth(inputData) + scope.config.variationSignWidth;
    shouldDisplayVariation = scope.width > estimatedVariationWidth;
  }

  let shouldDisplaySparklines;
  if (scope.chartOptions.sparklines) {
    shouldDisplaySparklines = scope.width > scope.config.sparklineWidth;
  }
  
  if (shouldDisplayVariation) {
    drawVariation(estimatedVariationWidth);
  }

  if (shouldDisplaySparklines) {  
    drawSparklines();
  }
  // ... some very long layout and draw logic un angular JS
}
// ...

Using the methodology I talked about before, we can clearly identify a block of code that looks functional: the beginning of the code. It is pure math and assignments.

The inputs:

• the inputData array
• the chartOptions object
• the config object
• the width number

And the outputs:

• the boolean shouldDisplaySparklines
• the boolean shouldDisplayVariation
• the estimatedVariationWidth number

With that in mind, let’s create the prototype of our function:

const layout = function(inputData, width, chartOptions, config) {

  return {
    shouldDisplaySparklines: false,
    shouldDisplayVariation: false,
    estimatedVariationWidth: 0
  };
};

We named it layout after seeing the inputs and outputs: it has clearly something to do with layout.

So now, copy paste our functional code block, renaming the variables a bit:

const layout = function(inputData, width, chartOptions, config) {
    
  let shouldDisplayVariation, estimatedVariationWidth, shouldDisplaySparklines;
  if (chartOptions.variation) {
    estimatedVariationWidth = estimateValueWidth(inputData) + config.variationSignWidth;
    shouldDisplayVariation = width > estimatedVariationWidth;
  }

  if (chartOptions.sparklines) {
    shouldDisplaySparklines = width > config.sparklineWidth;
  }
  return {
    shouldDisplaySparklines,
    shouldDisplayVariation,
    estimatedVariationWidth
  };
};

This function is pure: it has no side effects (assuming estimateValueWidth is a pure function as well). For the same inputs, it will always return the same outputs. So we improved the reliability of our code. Also the next developer that will see this will understand better what it is about, and what is the business rule behind it.

Then, we can use this function inside the “old” code:

update = function(inputData) {
  const {
    shouldDisplaySparklines,
    shouldDisplayVariation,
    estimatedVariationWidth
  } = layout(inputData, scope.width, scope.chartOptions, scope.config);
  
  if (shouldDisplayVariation) {
    drawVariation(estimatedVariationWidth);
  }

  if (shouldDisplaySparklines) {  
    drawSparklines();
  }
  // ... some very long layout and draw logic
}
// ...

And now, we can write unit test for the layout function, as it is a simple JS function.

describe('Score Card Layout', () =>
  it('should display only the values', function() {
    const {
      shouldDisplaySparklines,
      shouldDisplayVariation
    } = layout(INPUT_DATA, WIDTH, CHART_OPTIONS, CONFIG);
    shouldDisplaySparklines.should.be.false;
    shouldDisplayVariation.should.be.false;
  })
  // etc etc... other tests
);

Note that we could go even further and split layout into smaller functions.

So in the end, our “new” code is mostly tested. And we have modularized our code a bit. The next developer will understand better the code via this new function, and also via the documentation provided by the tests.

Of course it is not perfect: first, we have more code that before. But not much more; and the contribution to the big chart code file has lowered, in the end.

Second, what about the drawing functions at the end: drawVariation and drawSparklines? They are not tested, nor in separate modules. It might be difficult to test since we are in a big chart, but we could try!

But then, what’s the point? I mean we have tested our business rules (the most important thing to test). If it is easy to test the rest then, ok let’s go ahead.

In the end, it depends on:

• how much time we can afford to refactor and test this
• how much our tests will prove that the end user will have something working
• how much our tests will help the next developer that will have to work on that code

Conclusion

Using a functional programming approach is:

• good for testing small bits of “new” code inside a big chunk of legacy code
• improves the reliability and the maintainability of our code
• means making functions for complex logic, conditions and arithmetic.
• does not solve the fact that you have to adapt and use the result in the legacy code, so you will have to decide if you want to test that or not.

Decorator

The decorator pattern, for us, is just a fancy way to say that you’re changing methods of an object at run-time. It can be useful to stub internal methods of your new component. For example, let’s take our login component again, but with a different use case: the dirty SSO.

const Login = {

  // Some login component code ...

  methods: {
    // Some other methods...
    onClickOnLoginWithSSOButton() {
      if(this.hasSSOActivated) {
        this.legacyLoginWithSSO()
      }
    },
    legacyLoginWithSSO() {
      // Some very dirty code handling SSO login, that we don't want to test
    }
  }
}

So we want to test that legacyLoginWithSSO is called, but we don’t want the code inside it to be called. In JS we can just do something like:

// Before each test
this.originalLoginWithSSO = this.legacyLoginWithSSO
this.legacyLoginWithSSO = sinon.stub()

// After each test, to avoid side-effects
this.legacyLoginWithSSO = this.originalLoginWithSSO

But thankfully, our frameworks and their testing tools already do that for us. For example, with vue it would look like:

describe('Login', () =>

  beforeEach(function() {
    this.wrapper = shallowMount(Login, {
      propsData: { hasSSOActivated: true },
      methods: {
        legacyLoginWithSSO: sinon.stub()
      }
    });
        
    it('should call legacyLoginWithSSO when the user clicks on login with SSO', function() {
      this.wrapper.vm.onClickOnLoginWithSSOButton();
      this.wrapper.vm.legacyLoginWithSSO.should.have.been.calledOnce
    });
  })
);

In the end, it’s quite simple and we have something tested! You should plan, in the long term, on refactoring the dirty method though.

Again, we are not testing everything here, but we test what’s the most relevant to us: the business rule. So here it is the fact that legacyLoginWithSSO has been called in our context.

Note that the test depend on the legacyLoginWithSSO function, wich is (supposidely) somthing internal to the component. You might not want that as it can be more difficult to change and refactor that. But it’s a compromise, as always

Conclusion

Using decorator is:

• Good for testing “new” code using a small piece of legacy code
• Quite simple
• Should not hide the fact that you should refactor this small piece of legacy code sometime
• Makes our test depend on something internal to the component.

Proxy

How do you do when you need to mock a global object (for example window)? You can use the Proxy pattern.

This pattern introduces a small level of abstraction between the actual global object and your usage of it in the production code. This level of abstraction can be mocked so you can test that the mock has been called.

Let’s say in your production code you need to do something like:

window.location = 'https://toucantoco.com/'

If you have an object like this:

const WindowLocation = {
  getHref() { return window.location.href; },
  
  set(url) {
    return window.location = url;
  }
};
export default WindowLocation;

You can change the production code to

WindowLocation.set('https://toucantoco.com/')

You can then stub it in your tests and assert that it has been called:

// In before each
describe('Redirection', () =>
  beforeEach(function() {
    this.setWindowLocationHrefStub = sinon.stub(WindowLocation, 'set');
    this.wrapper = shallowMount(ToucanComponent);
  });
  afterEach(function() {
    this.setWindowLocationHrefStub.restore();
    wrapper.destroy();
  });
  it('should redirect when the user clicks on the button', function() {
    this.wrapper.find('.toucan__button').trigger('click');
    this.setWindowLocationHrefStub.should.have.been.calledWith('https://toucantoco.com/');
  })

Same goes with getters, if in your production code you need to do something like:

if(window.location.href === 'https://toucantoco.com/') {
  this.showPopup("Welcome to toucan toco!")
}

You can add more methods to stub to this Proxy:

const WindowLocation = {
  getHref() { return window.location.href; },
  
  set(url) {
    return window.location = url;
  }
};
export default WindowLocation;

and the production code becomes:

if(WindowLocation.getHref() === 'https://toucantoco.com/') {
  this.showPopup("Welcome to toucan toco!")
}

This simple trick allows you to test any global object that you can’t mock easily. It introduces this little Proxy object that only delegates to the actual object. Note that the Proxy should not have too much intelligence, and be as small as possible. Be careful that your proxy does not transform into an hidden utility function; that is not tested.

Conclusion

Using a proxy object:

• is good for testing global objects,
• introduces a level of indirection between your code and the actual object,
• should not be seen and used as an utility

Are you ready?

So now, you don’t have any excuses not to write test, even in your old rotten spaghetti :)

There are a lot of other design patterns, programming paradigms or methods out there that you can also use. I just read books and articles, and also experimented a bit myself.

While you write your tests, be as pragmatic as possible. Don’t spend too much or too little time writing your tests. Tests should not be a pain that you address at the end of your coding session. It shouldn’t be something you skip because it looks like a wall that you cannot climb.

Tests should be something nice you do during development, while you ask yourself design questions like:

• “Should I refactor this?”
• “What are the inputs? and the outputs?”
• “What is the expected behaviour?”
• “How can I make the code better?” …

Tests should be part of your journey to make your bug fix or your feature:

• If you write a fix (even a hotfix), how can prove that the code won’t have bugs again? How can you document your fix for the next developer? It is sure that someone missed something tricky in that particular case.
• If you write a feature, how can you prove that it works? How can you make sure that it is the right design? How can you make sure that the next developer will understand it? How can you document it?

You can answer all of these questions with tests, and methodologies such as TDD; even in legacy code.

I did not talk about CSS. I don’t know today how we would do to test such complex logics in CSS (it is possible to do quite complex things with CSS). I know we can use design patterns such as BEM or SMACCS or … But I don’t know if we can test it, or at least test the logic behind it. In Toucan Toco, we try to have very simple BEM CSS, and don’t add too much conditions and inheritance in our CSS. We assert on the classes of the DOM set by the JS code. It is not ideal, but we’re happy with it for now.

I also did not talk about other kind of tests: multi-component, service, e2e. Similar patterns can be used to do this kind of tests in legacy code. Keep in mind the Pragmatical Test Pyramid: test what makes sense to be tested, careful with the cost.

To finish, design patterns make a great toolbox that can be used for many other problems that you might encounter as a developer. I have used it many times in my career and even though they seem a bit Object-Oriented, you can still apply some concepts to javascript (and other languages). They come from the fact that we are not the first ones to have this kind of problems in programming, and many before us have solved this problems; maybe with other languages. But still, we can benefit a lot from this knowledge.

It is true that some of the methodologies explained here seem quite difficult to apply; but refactoring reflexes in legacy code is not easy but it comes with training and experience. I encourage you to do some programming Katas or Dojos, to train yourself at this craftsmanship, experiment and be better and better. When you master this, you can then mentor Juniors in your company and make sure that your code will be maintainable in the very-long run.

Sources:

• “Test-Driven Development by Example” - Kent Beck
• “Working Effectively with Legacy Code” - Michael C. Feathers
• “The Clean Coder” - Robert C. Martin
• Refactoring - Martin Fowler
• Martin Fowler’s blog: https://martinfowler.com
• Wikipedia’s design pattern page.