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.
Table of Contents
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:
- The Interface was not documented well enough, because
toBeFalsyis too broad.
- 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.
Now that we had a test in place, could also ensure our initial value will be more specific and according to the HTML spec (
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.
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.