Unpleasant errors while writing unit tests

 
3r3-31.
The other day I will be doing an internal report in which I will tell our developers about the unpleasant errors that may occur when writing unit tests. The most unpleasant errors from my point of view are when tests pass, but at the same time they do it so incorrectly that it would be better not pass. And I decided to share examples of such errors with everyone. Surely something else tell me from this area. Examples are written for Node.JS and Mocha, but in general these errors are true for any other ecosystem. 3r3640. 3r33535.  
To make it more interesting, some of them are framed in the form of a problem code and a spoiler, opening which you will see what the problem was. So I recommend first to look at the code, find an error in it, and then open the spoiler. Solutions to the problems will not be indicated - I suggest you think about it yourself. Just because I'm lazy. The order of the list does not make deep sense - it's just the sequence in which I remembered all sorts of real problems that brought us to bloody tears. Surely many things will seem obvious to you - but even experienced developers may accidentally write such code. 3r3640.
3r33535.  
So let's go. 3r3640. 3r33535.  
0. Lack of tests
3r33535.  
Oddly enough, many still believe that writing tests slows development speed. Of course, it’s obvious that you have to spend more time writing tests and writing code that can be tested. But after that, debugging and regressions will have to be spent several times longer 3r3640. 3r33535.  
1. No run tests
3r33535.  
If you have tests that you do not run, or run from time to time - then this is like the absence of tests. And it's even worse - you have an aging test code and a false sense of security. Tests should at least be run in CI processes when pushing code to a branch. And better, locally before we push. Then the developer will not have to return to the build in a few days, which, as it turned out, did not work. 3r3640. 3r33535.  
2. Lack of coverage
3r33535.  
If you still do not know what the coating is in the tests, then it's time to go and read right now. At least 3r333. Wikipedia 3r3639. . Otherwise, chances are that your test will test on the strength of 10% of the code that you think it checks. Sooner or later you will definitely step on it. Of course, even a 100% coverage of the code does not guarantee its complete correctness in any way - but this is much better than no coverage, as it will show you much more potential errors. Not without reason in the latest versions of Node.JS even appeared built-in tools to count it. In general, the topic of coverage is deep and extremely holivar, but I will not go into it too much - I want to say a little about a lot. 3r3640. 3r33535.  
3r3342. 3.
3r33535.  
3r3608. 3r3609. const {assert} = require ('chai');
const Promise = require ('bluebird');
function someLongFunction ()
{
return Promise.delay (10000);
}
describe ('using Timeout', () => {3r3652. it ('should cancel after 1 second', async () => {3r3652. let timedOut = false; 3r3652. try {3r3652. awwit someLongFunction (). timeout ( 1000);
}
Catch (err)
{
If (err instanceof Promise.TimeoutError)
{3r33652. TimedOut = true;
}
}
}
Assert.equal (timedOut, true);
});
}); 3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
Timeouts in unit tests. 3r3640. 3r33535.  
Here we wanted to check that the installation of timeouts for a long operation really works. In general, this already makes little sense - you should not check the standard libraries - but this code also leads to another problem - an increase in the execution time of the test for a second. It would seem that this is not so much But multiply this second by the number of similar tests, by the number of developers, by the number of launches per day And you will understand that because of such timeouts you can waste many hours of work every week, if not daily. 3r3640.
3r33535.  
4.
3r33535.  
3r3608. 3r3609. const testData = require ('./testData.json');
describe ('some block', () => {
it ('should do something', () => {
someTest (testData);
})
}) 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
Loading test data outside the test blocks. 3r3640. 3r33535.  
At first glance, it seems that it doesn’t matter where the test data is read in the describe, it block or in the module itself. On the second too. But imagine that you have hundreds of tests, and many of them use heavy data. If you load them outside of the test, this will lead to the fact that all test data will remain in memory until the end of the tests, and the launch will eventually consume more and more RAM - until it turns out that the tests no longer run on standard working machines. 3r3640.
3r33535.  
3r3122. 5.
3r33535.  
3r3608. 3r3609. const {assert} = require ('chai');
const sinon = require ('sinon');
class Dog
{
//eslint-disable-next-line class-methods-use-this
say ()
{
return 'Wow';
}
}
describe ('stubsEverywhere', () => {
before (() => {3rr3652. sinon.stub (Dog.prototype, 'say'). callsFake (() => {
return 'meow';
.}); 3r3652.}); 3r3652. It ('should say meow', () => {3r3652. Const dog = new Dog (); 3r3652. Assert.equal (dog.say (), 'meow', 'dog should say "meow!"');
});
});
3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
The code is actually replaced by stubs. 3r3640. 3r33535.  
Surely you immediately saw this ridiculous mistake. In real code, this, of course, is not so obvious - but I saw code that was hung with stubs so much that I didn’t test anything at all. 3r3640.
3r33535.  
6. 3r3605. 3r33535.  
3r3608. 3r3609. const sinon = require ('sinon');
const {assert} = require ('chai');
class Widget {
fetch ()
{
loadData ()
{
this.fetch ();
}
}
if (! sinon.sandbox ||! sinon.sandbox.stub) {3r3652. sinon.sandbox = sinon.createSandbox ();
}
describe ('My widget', () => {3r3365365.
it ('is awesome', () => {
const widget = new Widget (); 3r33652. widget.fetch = sinon.sandbox.stub ( ) .returns ({one: ? two: 2});
widget.loadData ();
assert.isTrue (widget.fetch.called);
});
}); 3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
Dependence between tests. 3r3640. 3r33535.  
At first glance it is clear that they forgot to write here 3r33640. 3r33535.  
3r3608. 3r3609. afterEach (() => {
sinon.sandbox.restore ();
}); 3r3619. 3r3620. 3r33535.  
But the problem is not only this, but that the same sandbox is used for all tests. And it’s very easy to confuse the test run environment in such a way that they begin to depend on each other. After that, the tests will be performed only in a certain order, and it is generally not clear what to test. 3r3640. 3r33535.  
Fortunately, sinon.sandbox was at some point declared obsolete and drunk, so you can run into such a problem only on a legacy project - but there are many other ways to disrupt the test run environment in such a way that it will be painfully painful to investigate later what code is guilty of incorrect behavior. On Habré, by the way, recently there was a post about some kind of template like "Ice Factory" - this is not a panacea, but sometimes it helps in such cases. 3r3640.
3r33535.  
7. Huge test data in the 3r3605 test file. 3r33535.  
Very often I saw how huge JSON files, and even XML, lay right in the test. I think, obviously, why it is not worth doing - it becomes painful to watch it, edit it, and any IDE will not tell you for this thanks. If you have large test data, take it out of the test file. 3r3640. 3r33535.  
8.
3r33535.  
3r3608. 3r3609. const {assert} = require ('chai');
const crypto = require ('crypto');
describe ('extraTests', () => {3r333365. it ('should generate unique bytes', () => {
const arr =[];
for (let i = 0; i 3r33232. {
const value = crypto.randomBytes (256);
arr.push (value);
}
const unique = arr.filter ((el, index) => arr.indexOf (el) === index); 3r333365. Assert.equal (arr.length, unique.length, 'Data is not random enough!');
});
}); 3r3r262. 3r31919.
 
3r32424. What is wrong here 3r3363625.
Superfluous tests. 3r3640. 3r33535.  
In this case, the developer was very concerned that his unique identifiers would be unique, so I wrote a check for this. In general, a clear desire - but it is better to read the documentation or drive such a test several times without adding it to the project. Running it in every build makes no sense. 3r3640. 3r33535.  
Well, the tie-in for random values ​​in the test - in itself is a great way to shoot yourself in the foot, making an unstable test from scratch. 3r3640.
3r33535.  
9. Lack of mokov 3r3605. 3r33535.  
It is much easier to run tests with a live base and sotalnogo services, and drive tests for them. 3r33535.  
But sooner or later it will come around - data deletion tests will be performed on the product base, they will start to fall due to a non-functioning partner service, or your CI simply will not have a base on which to drive them away. In general, the item is quite holivarny, but as a rule - if you can emulate external services, then it is better to do it. 3r3640. 3r33535.  
11.
3r33535.  
3r3608. 3r3609. const {assert} = require ('chai');
class CustomError extends Error
{
}
function mytestFunction ()
{
throw new CustomError ('important message');
}
describe ('badCompare', () => {3r3652. it ('should throw only my custom errors', () => {3r3652. let errorHappened = false; 3r3652. try {3r3652. mytestFunction ();
}
.catch (err) 3r3652. {3r?652. errorHappened = true; 3r3652. assert.isTrue (err instanceof CustomError);
} 3r363365. assert.isTrue (errorHappened);
}; 3r3rr3r3r.32. 3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
Advanced debugging errors. 3r3640. 3r33535.  
Everything is not bad, but there is one problem - if the test suddenly drops, then you will see an error of the form 3r33640. 3r33535.  
3r3608. 3r33333. 1) badCompare
should throw only my custom errors:
Assertionerror: expected to be true
+ expected - actual
-false
+ true
at Context.it (test /011_badCompare /test.js: 23: 14) 3r3620. 3r33535.  
Further, to understand what the error actually happened - you have to rewrite the test. So in case of an unexpected error - try to have the test tell about it, and not just the fact that it occurred. 3r3640.
3r33535.  
3r33333. 12. 3r3605. 3r33535.  
3r3608. 3r3609. const {assert} = require ('chai');
function someVeryBigFunc1 () 3r3365365. {
return 1; //imagine a tonn of code here
}
function someVeryBigFunc2 () 3r3365365. {
return 2; //imagine a tonn of code here
}
describe ('all Before Tests', () => {3r3652. let res1; 3r3652. let res2; 3r3652. before (async () => {
res1 = await someVeryBigFunc1 ();
res2 = await someVeryBigF22 Res (= await someVeryBigFunc1 (); 3r3652. res2 = await someVeryBigFunc1 (); 3r3652. res2; 3r3652. 3 ; 3r3652.}); 3r3652. It ('should return 1', () => {3r33652. Assert.equal (res? 1);
}); 3r3652. It ('should return 2', () = > {
Assert.equal (res? 2); 3r3652.}); 3r3652.});
3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
Everything in the before block. 3r3640. 3r33535.  
It would seem a cool approach to do all the operations in block 3r3503. before , and thus leave inside it just checkki 3r33535.  
Not really. 3r33535.  
Because in this case there is porridge, in which one cannot understand the time of the actual execution of the tests, nor the cause of the fall, nor what relates to one test, and what belongs to another. 3r33535.  
So all the work of the test (except for standard initializations) should be performed inside the test itself. 3r3640. 3r33535.  
3r33420. 13. 3r33535.  
3r3608. 3r3609. const {assert} = require ('chai');
const moment = require ('moment');
function someDateBasedFunction (date)
{
if (moment (). isAfter (date))
{
return 0;
}
return 1;
}
describe ('useFutureDate', () => {3r3652. it ('should return 0 for passed date', () => {3r33652. const pastDate = moment ('2010-01-01'); 3r3652. assert.equal (someDateBasedFunction (pastDate), 0);
});
it ('should return 1 for future date', () => {
const itWillAlwaysBeInFuture = moment ('2030-01-01');
assert.equal (someDateBasedFunction (itWillAlwaysBeInFuture), 1);
};
}); 3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.

The plot for the date. 3r3640. 3r33535.  

It would also seem like an obvious mistake - but it also occasionally occurs among tired developers who already believe that tomorrow will never come. And the build that was going well yesterday, suddenly falls today. 3r3640. 3r33535.  

Remember that any date will come sooner or later - so either use time emulation with things like 3r3503. sinon.fakeTimers , or at least place distant dates like the year 2050 - let your descendants have a headache 3r33640.

3r33535.  
3r33471. 14. 3r33535.  
3r3608. 3r3609. describe ('dynamicRequires', () => {3r3652. it ('should return english locale', () => {3r33652.3r3652. ////HACK: 3r???. //I will require now here
//eslint-disable-next-line global-require
const moment = require ('moment');
const someDate = moment ('2010-01-01'). format (' MMMM ');
Assert.equal (someDate,' January ');
});
}); 3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.

Dynamic loading of modules. 3r3640. 3r33535.  

If you have Eslint, then you probably already have banned dynamic dependencies. Or not. 3r33535.  
I often see that developers are trying to load libraries or various modules directly inside the tests. However, they generally know how 3r3503 works. require - but they prefer the illusion that they are supposed to be given a clean module that no one has yet embarrassed. 3r33535.  
Such an assumption is dangerous in that the loading of additional modules during tests is slower, and again leads to more undefined behavior. 3r3640.
3r33535.  
3r33512. 15. 3r3605. 3r33535.  
3r3608. 3r3609. function someComplexFunc ()
{
//Imagine a piece of really strange code here
return 1;
}
describe ('cryptic', () => {
it ('success', () => {
const result = someComplexFunc ();
assert.equal (result, 1); 3r33652.});
. it ('should not fail', () => {3r3652. const result = someComplexFunc ();
assert.equal (result, 1);
});
it ('is right', () => {
Const result = someComplexFunc (); 3r33652. Assert.equal (result, 1);
});
It ('makes no difference for solar system', () => {
Const result = someComplexFunc ();
assert.equal (result, 1);
});
}); 3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
Unclear test names. 3r3640. 3r33535.  
You must be tired of the obvious things, right? But you still have to talk about it because many people don’t bother to write clear names for tests - and as a result, you can understand what a test does only after long research. 3r3640.
3r33535.  
3r3-3559. 16. 3r3605. 3r33535.  
3r3608. 3r3609. const {assert} = require ('chai');
const Promise = require ('bluebird');
function someTomeoutingFunction ()
{
throw new Promise.TimeoutError ();
}
describe ('no Error check', () => {3r333365. it ('should throw error', async () => {3r?656. let timedOut = false; 3r3652. try {3r33652. await someTomeoutingFunction ();
}
Catch (err) 3r?652. {
TimedOut = true;
}
Assert.equal (timedOut, true); 3r3652.});
});
3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
Lack of verification of thrown error. 3r3640. 3r33535.  
Often you need to check that in some cases the function throws an error. But it is always necessary to check whether these are the droids we are looking for - because suddenly it may turn out that the error was thrown out by another, in a different place and for other reasons 3r3640.
3r33535.  
17. 3r3605. 3r33535.  
3r3608. 3r3609. function someBadFunc ()
{
throw new Error ('I am just wrong!');
}
describe.skip ('skipped test', () => {
it ('should be fine', () => {3r33652. someBadFunc ();
}); 3r3652.}); 3r3619. 3r3620. 3r33535.  
3r32424. What is wrong here 3r3363625.
Disabled tests. 3r3640. 3r33535.  
Of course, there may always be a situation where the code has already been tested many times by hand, you need to roll it urgently, but for some reason the test does not work. For example, because of the not obvious link to another test, which I wrote about earlier. And the test is disabled. And that's fine. Not normal - do not instantly set the task to turn the test back on. If this is not done, then the number of disabled tests will multiply, and their code will constantly become obsolete. Until the only option left is to show mercy and throw out all these tests nafig, because it is faster to write them again than to understand the mistakes. 3r3640.
3r33535.  
Here is such a selection came out. All of these tests are well tested, but they are broken by design. Add your options in the comments, or in 3r3638. 3r3639 repository. which I did to collect such mistakes. 3r3640.
3r3645. ! function (e) {function t (t, n) {if (! (n in e)) {for (var r, a = e.document, i = a.scripts, o = i.length; o-- ;) if (-1! == i[o].src.indexOf (t)) {r = i[o]; break} if (! r) {r = a.createElement ("script"), r.type = "text /jаvascript", r.async =! ? r.defer =! ? r.src = t, r.charset = "UTF-8"; var d = function () {var e = a.getElementsByTagName ("script")[0]; e. ): d ()}}} t ("//mediator.mail.ru/script/2820404/"""_mediator") () ();

+ 0 -

Add comment