Tests can improve communication and save time (and frustration). Bad tests can do the opposite. In this article, we’ll explore an example from real life of how bad tests are harmful and how good ones convey the right information.

A Tale About a Title

In Vivid, we needed to add a title to our button. It’s not that the button didn’t have one. Vivid is a web components based library, and inside our button element, there’s a hidden native button under the Shadow DOM.

This native button did not get its parent’s title, which was bad for a11y.

So… the task was pretty simple. Get a title from the host (the custom element) and reflect it on the internal button. Just like that:

We will use this example and follow its commit path to understand and learn how to avoid simple testing mistakes.

Lesson #1: Changing the Interface is a Red Flag

The first commit in this Pull Request meddled with the interface. Here’s the change in the test:

You can see that the test changed from toBeFalsy to “be equal to an empty string”. While not much of a change – it is a change. toBeFalsy can be many things (empty string, null, undefined, NaN, 0 and false). An empty string can be many things.

There are two errors in this case:

1. The Interface was not documented well enough, because toBeFalsy is too broad.
2. The interface was changed to '' in the test for some reason.

Here, the plot thickens. These two mistakes are just the appetizers for a loss of 24 hours of development. Let’s get to the main dish.

How Bad Tests Lead to Bad Code

A side story to this one is that while I was trying to help sort out the failing test, I was on the way back from a vacation. I know, excuses… but here’s what happened.

I looked at the test code using my phone and saw that the documented interface implied the title value was supposed to be an empty string.

“Why, of course it fails! The initial value is not an empty string. Just set it as an empty string, and it’ll work.”

So, the change was pretty easy. Just set the value in the constructor, and the test will pass:

This fix worsened the issue we saw in the first mistake. Not only did we change the documentation – we also changed the actual interface. Yes, the tests passed, but was it the right test? And how did such a simple thing cost us 24 hours of work? Let’s get to the second mistake.

Mistake #2: You Shall Not Pass (for the Right Reason)

The interface change was just the first step. There was actual logic to implement, right? Here’s the test for the next implementation:

The first thing that pops up is the test’s description. The typo (missing set after the not) is the most prominent error there, but there’s something else.

The description is written in negative form. In science, you cannot prove that something does not exist. Because software falls into Computer Science we can look at it the same. Don’t tell me what it should not do – tell me what it should do.

The other two things that are even worse than the description lie in the test code itself. Take a minute to see if you find them.

…

…

Okay, the minute has passed. Did you find one or two more errors?

Wrong Reason #1: The test is made on the element and not the internal button

I mentioned at the beginning our missing was to set the title on the internal button. This test doesn’t describe that scenario.

Can you see that the test is done on the element, and not the internal button? When I read the test, I assumed the author wanted the title to appear on the element. That’s the documentation. And that’s what I expect the component to do.

Wrong Reason #2: The expectation is not according to the description

Another thing is that the expectation in this test is to have a title with an empty string. What we want to achieve is completely different – we want to remove the title attribute in such a case.

12 hours passed, and I was back home from my vacation, thinking about how to implement the code to comply with the given spec.

A few Slack messages confirmed my suspicions – the title attribute should be removed when the title is falsy. I did not doubt for a minute that the title should not be set on the element.

I changed the test a bit to make sure we’re on the right path:

Now, the expectation is to actually remove the attribute.

In order for it to pass, I had to change a few things in the component’s template and class. In the class, I had to override the title attribute (our class extends another basic Element class). I also had to set a converter that sets the value to null in the template, if the value is falsy, but leave it as a string if changed from the view itself:

Everything worked as expected. I pushed and expected the praise from the PR author of how I saved her day. Or… not?

Communication is Key

The expected slack message came in a few hours later. The contents of the message were less expected, though:

And so started another slack correspondence. It was a short one, but in the end, that was my take:

The fix, as expected, was very small. I had to do the test both on the element as well as on the internal button (a.k.a. control element):

Now that we had a test in place, could also ensure our initial value will be more specific and according to the HTML spec (null):

This simple fix took us 24 hours (yes, I know I was on vacation half that time, but excuses are for the weak 😉 ). It could have been saved with better tests or better communication skills on my part.

Summary

Tests are meant to fail when you write code that doesn’t do what it’s supposed to do. Tests are also supposed to be a straightforward description of how things work. If the test says a title should be on the element, then these are the instructions for the implementor.

The interface is also guarded by tests. If the test doesn’t guard the interface correctly or tightly enough (as in the toBeFalsy instead of toBeNull example here), then we do not really know what to expect as developers and consumers of the interface. You can read more about the importance of interface testing here.

Good communication skills are important. Not everyone has them. Especially not in remote working environments. If we were coding together in person, this would not have taken so long. Our team works remotely (an international team), and the communication is usually “offline” – meaning there’s usually a delay in response.

Writing the tests correctly, with a clear description and a logic that describes how things should be helps mitigate such communication errors in the team.

Thanks a lot to Rachel B. Tannenbaum, Benjamin Aronov, and Nick Ribal for the kind and thorough review.