little cubes

5 JavaScript Testing Mistakes to Avoid

Zach Posten - 2022 Apr 26

In order of importance

5 JavaScript Testing Mistakes to Avoid

Mistake #1: Excessive mocking

I've heard it said before that mocks are a code smell, but I disagree with that. Mocks are great, in moderation. It makes a lot of sense to mock out things like network calls or responses from a 3rd party library. Sometimes it also makes sense to mock your own modules in order to isolate the unit you're trying to test.

But when a particular test starts requiring you to mock out multiple other modules, or when the amount of code dedicated to mocking rivals or exceeds the amount of code actually dedicated to testing in that file, something has gone wrong. Those tests have now become much, much harder to maintain than they otherwise would be.

To fix this you either need to restructure your code to make it more testable, or write end-to-end tests to cover this module because it's not suitable for unit tests.

Mistake #2: Using enzyme

Enzyme is dead.

Even before Enzyme died, React Testing Library was already well on its way to becoming the community standard (it is supported out of the box with Create React App) because unlike enzyme, Testing Library's API encourages you to avoid including component implementation details in your tests.

Mistake #3: Snapshot testing entire components

Snapshot tests are very alluring because they give you a lot of output while requiring you to write very little code. Jest says that:

But unfortunately, that sense of security is a lie.

First and foremost, jest is wrong to say that snapshots "make sure your UI does not change"; what they actually do is let you know when your markup changes. And it's not necessarily problematic that the markup of your component changed. There are an infinite number of changes that I can make to your markup without changing your UI at all. You know how else I can determine if your markup is going to change? By actually reading the source code.

The other biggest problem with snapshots is that in real world applications they end up changing very frequently and quite dramatically. The diffs of snapshot files end up being so long that the people reviewing your code are 90% of the time going to completely ignore them, removing 100% of their value. And even when people do take the time to attempt to read your massive snapshot diff, what are they supposed to be looking for? It is an exercise in futility.

Mistake #4: Writing tests in TypeScript

TypeScript is great. Every single project that I create professionally or personally (this very website included) is written in TypeScript. However, writing your tests in TypeScript provides little to no value.

In fact, more often than not your TypeScript test files end up having to define custom types of their own or do a bunch of funky typecasting in order to tell the TypeScript compiler to calm down and accept my fake data. Doing this makes the tests more difficult to maintain, harder to read, and simply creates cruft that does not add any value and has no reason to be there.

This is a minor point, but TypeScript tests also usually require more work to setup because they have to be compiled, and you always have to tell typescript to add all the global functions that your tests reference. It's not that these things are difficult, they're just more setup to do that again...adds no value to your project.

Mistake #5: Having a describe() that wraps the entire test module

If you've ever worked with me you already know that I really hate repeating myself. I try quite hard to make my code as DRY as reasonably possible. So when I see duplication for the sake of duplication, I need to put a stop to it immediately.

Here's an example:

1 2 3 4 5 6 // get-link.test.js describe('get link handler', () => { it('should do this', () => {}) it('should do that', () => {}) })

What possible purpose could that describe() serve? When the test is run, this is the output

1 2 3 4 PASS get-link.test.ts get link handler ✓ should do this (4 ms) ✓ should do that (4 ms)

The exact same information is repeated on lines 1 and 2! For Pete's sake, just remove the pointless describe().

The only defense of this tactic that I've heard is that it makes the code consistent if you later add a second describe() in the file. But it would not make sense for get-link.test.js to have any tests in it that didn't test "get link"; so the only way it could have another describe() would be inside of the root one, in which case you can STILL remove the root one. 🙃