Sometimes just writing tests is not enough
There's a pattern for organizing the internal code of tests called Arrange Act Assert (AAA). The structure is:
1. Arrange all necessary preconditions and inputs.
2. Act on the object or method under test.
3. Assert that the expected results have occurred.
— Arrange Act Assert definition from wiki.c2.com
Let's say we want to test that a component. The implementation can be something like this, assuming that we are using a Test-Driven approach and a component-based architecture:
https://jsfiddle.net/qegb2xan/10/
The modal will be open by default because this test is just to be sure we create something and append to the DOM. Now let's create the test that will drive us to create the functionality of closing it:
https://jsfiddle.net/wotyLkLw/6/
This failing test forces us to create the functionality of closing the modal. Let's build it using an .active class that uses the display to show/hide the content (see the compiledComponent function):
https://jsfiddle.net/e6bvt39b/7/
But we can only open the modal when an event happens, it should not open by default to the user. Let's edit the existing test so that the modal is not open by default (see the changes on the test the modal is NOT open by default):
https://jsfiddle.net/1ytcfx1s/4/
Now here's the problem: at some point when developing the modal, it's very likely that we commit a mistake and remove the button to close the modal from the DOM. That will break the functionality and we would expect at least one test to fail (see the compiledComponent function, the button has been removed):
https://jsfiddle.net/8braft4t/3/
However, no tests have failed.
The reason that happened is because the jQuery API, by default, doesn't throw anything for an element that doesn't exist. It fails silently and does not add the target element to the jQuery collection. Since we're checking the modal is closed and it is closed by default, then the test will never fail, even if the code is broken.
// This doesn't break, it just runs and do nothing$('.modal').find('.close').click();
If we had an API that fails if the element doesn't exist, then there would be no false positive in the tests:
document.querySelector('.modal .close').click();
However, the error for a non-existent element when we use querySelector is not descriptive enough:
Uncaught TypeError: Cannot read property ‘click’ of null
We can make it fail with a clearer message by wrapping the operation in a custom function:
// This wrapper throws if the element is not therefindElement($('.modal'), '.close').click();
Error: Element not found for selector “.close”
https://jsfiddle.net/4cvprc9m/2/
The problem with passing a test when it shouldn't is that the developer won't know which tests to investigate when that specific bug happens. A passing test is very likely to be ignored, mostly if the modal test is more complex and it's in the middle of several other tests.
A false negative is better than a false positive in the context of testing. If a test fails, it's highlighted, if not then it's ignored by the developer because it's assumed to be working.
This is a problem of the same category of when Mocking Can Lead To False Positives. The difference is that, instead of mocking, we're using the incorrect DOM API for testing.
As a formal problem statement, we can say that a group of tests can lead to a false positive result when
- something is negative by default (modal is closed)
- and the test requires an action (close the modal)
- that changes the state of the component we're testing to the initial one (modal is closed)
In this circumstance, we can't guarantee the functionality is really covered. Even if there are tests written for that, changing the component code can make the test still pass.
There are a few options to prevent this from happening:
- In the tests that ensure the modal can be closed, assert that the modal should be open before the act of closing the modal. Unfortunately, this violates AAA because we'll have an assertion between Arrange and Act to check if the modal is open. Here's an example.
- Use an API that throws when the element doesn't exist. This way, if we remove the target element from the modal, the test will fail and we'll know exactly why. As the example shown above.
Just writing tests is not enough to ensure the application works. You need to write them correctly so that false positives like this one don't happen. That means using proper APIs for what you want to achieve.
jQuery alone is not suitable for this category of testing.
And you. Have you ever stomped in something similar? What's your suggestion to prevent this problem from happening when testing the default state of an object or component?
Thanks for reading. If you have some feedback, reach out to me on Twitter, Facebook or Github.