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.
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:
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:
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:
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 :
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.
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:
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.
To sum up, the adapter:
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.
The (sort-of) factory pattern is:
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:
if, elses, and calculus). If the block is not clear, try to group the lines of code that seem independent.= and output are new variables declared at the left of =, and used later in the code.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:
inputData arraychartOptions objectconfig objectwidth numberAnd the outputs:
shouldDisplaySparklinesshouldDisplayVariationestimatedVariationWidth numberWith 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:
Using a functional programming approach is:
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
Using decorator is:
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.
Using a proxy object:
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:
Tests should be part of your journey to make your bug fix or your feature:
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: