If Your Refactors Break A Lot of Tests, You May Not Be Testing the Right Thing
Nick Scialli
May 14, 2020
Testing is supposed to validate that your app works. If you do some refactoring and your app still works but your tests are failing, are you really testing the right thing?
I recently ran into this issue myself at work. I spend most of my time there working on a React/Redux/Typescript front-end. I noticed that lower level components had some gnarly conditionals to determine a numbering scheme. There are 10 of these components that each conditionally render based on state, and they have to maintain consecutive numbering. For example, the following table represents an example display state for each component and the numbering scheme:
component | display? | number |
---|---|---|
A | true | 1 |
B | true | 2 |
C | false | |
D | true | 3 |
E | false | |
F | false | |
G | false | |
H | true | 4 |
I | false | |
J | false |
This refactor seemed simple enough—I would create a selector that takes state as an argument and outputs an object with the component name as keys and the numbering as values. Here’s a simplified version that outputs the information above, but the function would obviously have a lot more logic baked in:
const getNumbers = (state) => {
return {
A: 1,
B: 2,
D: 3,
H: 4,
};
};
So if I mapped this selector into my lower level components, I’d always have the correct numbering without having a bunch of redundant logic.
const ComponentA = (props) => {
return (
<>
<h1>{props.number}. Some Title</h1>
<p>Some content</p>
</>
);
};
const mapStateToProps = (state) => ({
number: getNumbers(state).A,
});
export default connect(mapStateToProps)(ComponentA);
This is great! I was excited to run my tests. One of the best things about testing is you can refactor things and be fairly confident you haven’t broken anything because your tests still pass.
Tests Did Not Still Pass
As you may have guessed, my tests did not pass: I had a bunch of failing snapshots generated by Storybook—all of my numbers were now showing as undefined
!
It turns out, I had a bunch of tests at the component level, not connected to a Redux store within the test. This means I was using <ComponentA />
in the test without passing it the number
prop, and therefore the number
was undefined
.
Testing Realistic Things
The unconnected component is never used in production, nor is it a realistic representation of what a user would see. So one takeaway here is that we should test realistic things. Do we really care if these low-level assertions are passing if they don’t have any impact on our users?
Also, there are some great minds that recommend writing mostly integration tests. I definitely understand why, especially when I battle lower-level tests that fail for reasons that wouldn’t actually manifest in front of users.
The Unit Test Conundrum
Why did I even write the unit tests to begin with?
Well, I did it because I enjoy Test-Driven Development (TDD) a lot at the unit level. It would take a lot of “flying in the dark” before an integration or end-to-end test could be assembled to actually make sure things function well together. By incrementally testing units, I feel like I have a much better shot at them working well together.
So if it’s handy to write unit tests while developing but then it’s more worthwhile to have integration and end-to-end tests in the end, what do we do?
I’m definitely going to keep writing my unit tests as I develop; it’s a tool that works well for me in my development process. But, I’ll try to keep them relatively minimal and realistic. In the case of my aforementioned component, I should have at least tested that component in its redux-connected form rather than in total isolation.
I’m also going invest more time in writing integration and end-to-end tests. If I break these tests, that will be more indicative of a real, user-facing issue.
Nick Scialli is a senior UI engineer at Microsoft.