airbnb
[testing] Add section: "testing is documentation"
It is great to suggest people to write tests, it is really a good idea, but I think that it is not enough.
From my experience working with other coders I have discovered that they consider hard to write tests. Not because it is difficult (you just use the test API like any other library), but because they think that tests are pointless and a waste of time.
I have done a lot of reviews of PR, including those containing tests, and I have discovered very bad testing practices which usually occurs when people are forced to write tests (usually big companies and some test obsessed people). The tests resulting of these practices are almost useless (usually testing code copies production code) and lacks of many of the expected properties of a test.
But: program testing can be a very effective way to show the presence of bugs, but is hopelessly inadequate for showing their absence. — Edsger W. Dijkstra 1972
So tests objective should not be the only one of creating a system bugs free. It is an impossible task from the point of view of that test can effectively achieve, but there are many other objectives which are very valuables.
Main tests objective should be to make legacy code disappear.
I consider legacy code, not just old unsupported code, but any code difficult to change because we may break our system. And as I far as I know it happens in two different scenarios:
- We want to change some code, but we cannot guarantee that we will break some functionality because we need also to refactor the test, or there is not test,
- We want to change or add some code, but we cannot guarantee to break any functionality because we do not know which are all supported functionalities of our system,
Both points can be summarized in good maintainability and good documentation, and we can achieve both with well written tests.
And here comes the good news, there is a simple trick to achieve these two properties: "testing is documentation".
When coders forgot about it, coders usually focus in testing other points. They being obsessed in test function by function all possible cases, all input combinations, all ranges, they try to get the 100% code coverage regardless its usefulness. But all this effort just creates a rigid codes and poorly covered. I think that you tried to make people think better about it when you said: "Be cautious about stubs and mocks", but I it is not enough.
An example of 100% code coverage and almost useless testing code:
describe('addListener', () => {
it('should add a callback to the queue', () => {
dispatcher.addListener(cb);
expect(dispatcher.queue).toContain(cb);
});
});
describe('deliver', () => {
it('should invoke queue callbacks with the received argument', () => {
dispatcher.queue.push(cb);
dispatcher.deliver('message');
expect(cb).toHaveBeenCalledWith('message');
});
});
These tests achieve 100% of code coverage, even if you suppress expectations in both tests, or replace them by checking anything else, like they both methods return undefined.
If you look carefully these tests tells almost nothing about what to do with the dispatcher, it even exposes some data structures that meant to be internal ( we could implement them with: https://github.com/airbnb/javascript#naming--leading-underscore ). It does not matter how many other test you add (what happens if the queue is empty, if there are many callbacks, if there is no message, ...), you will have hard time trying understand what dispatcher and it will be almost impossible to refactor it without break all tests first (refactor usually implies changing internal representations).
Now check this other test:
it('should deliver messages to listeners', () => {
dispatcher.addListener(cb);
dispatcher.deliver('message');
expect(cb).toHaveBeenCalledWith('message')
});
It is simple, easy to understand, does not use internal representation, and it has 100% of code coverage.
It should be our objective, and if you look carefully, it is a wonderful documentation. You understand involved steps and see the whole picture. If you want can do copypaste quickly, and build new code from here. And of course, you can refactor the dispatcher code as much as you want without changing a simple line of a test as long as you do not break the dispatcher api compatibility.
Because all exposed here that is the reason why I suggest the addition this section, just to help to create better and more effective testing code. It will mitigate the frustration of many programmers that do not know how to start writing a test and it will give them a direction to follow.
With these few indications I hope that coders will feel good with their own test and give them some deeper meaning that helps them to be proud of their work.
Note: I was tempted to add something about AAA (Arrange Act Assert) or GWT (Give When Then), and force people to split tests that test something more than one thing, but I think that as far as they understand that each test case is an example of usage for the documentation it should not be necessary.
I want to review comments, give me some days to review them. (although any comment is welcome meanwhile)
