DEV Community

Cover image for Your test is green. Is it green for the right reason?
Etka Ozer
Etka Ozer

Posted on

Your test is green. Is it green for the right reason?

There's a specific kind of bad feeling I want to describe, and I don't think it has a name.

It's not finding a bug. It's realizing that the thing you trusted to catch the bug was never actually looking.

Mine was a test called rejects a token minted for another agent instance. Green. It had been green for weeks. I had shipped features on top of it.

Then I changed something nearby, an unrelated test went red, and I ended up opening both files. That's when I saw it: my test was minting the token without a project id. So the token was undefined. So the connection got rejected — not because the token belonged to somebody else, but because there was no token at all.

It passed. Every run. And it proved absolutely nothing.

Here's the part that stuck with me. A failing test is loud. It blocks your merge, it shows up red, it demands attention. A lying test is quiet. It's on your side. It's part of the number you say out loud in standup. Nothing in my setup was ever going to tell me — not coverage, not CI, not the test count going up every week.

A rejection for the wrong reason looks exactly like a rejection for the right one.

If you have one of these, it's probably a negative test. The ones with "rejects" or "throws" or "denies" or "returns 401" in the name. They're where this hides, because their success condition is that something didn't happen — and nothing happening is the default state of the universe. The test can do nothing at all and still go green.

So here's the thirty-second version you can run today. Open your negative tests and ask one question: if the setup silently failed, would this still pass?

If the answer is yes, you need one line.

const token = await mint({ projectId: OTHER_PROJECT });
expect(token).toBeDefined();   // the line I didn't have
await expect(connect(token)).rejects.toThrow();
Enter fullscreen mode Exit fullscreen mode

That's the whole fix. It took a minute. Finding it took three weeks and pure luck.

I've stopped being impressed by green. The question I ask now is a different one: is this green for the reason I think it is?

Go look at your negative tests. I'd bet money you have at least one.

Top comments (6)

Collapse
 
rondo profile image
Rondo •

Maybe we should record overall process of testing. To check whether the test result is derived as we intended, like you said. Likewise, detailed test cases would be also required.

Collapse
 
etkaozer profile image
Etka Ozer •

Agreed on recording the process. The thing I'd add: a test case being detailed doesn't help if it can pass for the wrong reason.

I had a test asserting two backends returned the same search order. It passed for months. Then I lengthened the documents and it broke.Turns out the two rank differently, and they'd only agreed because my fixtures happened to be short enough that the difference never showed. The test wasn't wrong. It just wasn't testing what I thought.

What actually caught it for me was mutation testing: break the shipped code one line at a time and confirm the suite goes red. A test that stays green when you break the thing it claims to check is a test that was never checking it.

Collapse
 
rondo profile image
Rondo •

I thought making test cases as detailed as possible would resolve this problem. But you're completely right. Detailed test cases alone can't guarantee that we're actually testing what we intend to test, like in your example with the backends. It also makes me see why mutation testing is useful here.

Thread Thread
 
etkaozer profile image
Etka Ozer •

That's exactly where I landed too. Detail and correctness turn out to be different properties.

If you do try it, start narrow. Mutating a whole repo takes hours, mutating just the surface you changed takes minutes and catches nearly as much.

Thread Thread
 
rondo profile image
Rondo •

Thank you for the advice. I'll give it a try😊

Some comments may only be visible to logged-in visitors. Sign in to view all comments.