На днях я буду делать внутренний доклад, на котором расскажу нашим разработчикам про неприятные ошибки, которые могут возникнуть при написании юнит тестов. Самые неприятные с моей точки зрения ошибки — когда тесты проходят, но при этом делают это настолько некорректно, что лучше бы не проходили. И я решил поделиться примерами таких ошибок со всеми. Наверняка ещё что-нибудь подскажете из этой области. Примеры написаны для Node.JS и Mocha, но в целом эти ошибки справедливы и для любой другой экосистемы.
Чтобы было интереснее, часть из них оформлена в виде проблемного кода и спойлера, открыв который, вы увидите, в чём была проблема. Так что рекомендую сначала смотреть на код, находить в нём ошибку, а затем открывать спойлер. Решения проблем указано не будет — предлагаю самим подумать над ним. Просто потому, что я ленивый. Порядок списка не имеет глубокого смысла — просто это очерёдность, в которой я вспоминал про всякие реальные проблемы, которые доводили нас до кровавых слёз. Наверняка многие вещи покажется вам очевидными — но даже опытные разработчики могут случайно написать такой код.
Итак, поехали.
Как ни странно, до сих пор многие считают, что написание тестов замедляет скорость разработки. Конечно, это очевидно, что больше времени приходится тратить на то, чтобы писать тесты, и чтобы писать код, который можно тестировать. Но на отладку и регрессы после этого придётся тратить в разы больше времени…
Если у вас есть тесты, которые вы не запускаете, или запускаете от раза к разу — то это всё равно что отсутствие тестов. И это даже хуже — у вас есть устаревающий код тестов и ложное чувство безопасности. Тесты как минимум должны запускаться в CI процессах при пуше кода в ветку. А лучше — локально перед пушем. Тогда разработчику не придётся через несколько дней возвращаться к билду, который, как оказалось, не прошёл.
Если вы ещё не знаете, что такое покрытие в тестах, то самое время во прямо сейчас пойти и прочитать. Хотя бы википедию. Иначе велики шансы того, что ваш тест будет проверять от силы 10% от того кода, который, как вы думаете, он проверяет. Рано или поздно вы на это обязательно наступите. Конечно, даже 100% покрытие кода никак не гарантирует его полную корректность — но это гораздо лучше, чем отсутствие покрытия так как покажет вам гораздо больше потенциальных ошибок. Недаром в последних версиях Node.JS даже появились встроенные инструменты для того, чтобы его считать. Вообще, тема покрытия глубокая и крайне холиварная, но не буду в неё слишком углубляться — хочется сказать по понемножку о многом.
Очень часто видел, как прямо в тесте лежали огромные JSON файлы, и даже XML. Думаю, очевидно, почему это не стоит делать — становится больно это смотреть, редактировать, и любая IDE не скажет вам за такое спасибо. Если у вас есть большие тестовые данные — выносите их вне файла теста.
Гораздо проще запускать тесты с живой базой и сотальными сервисами, и гонять тесты на них.
Но рано или поздно это аукнется — тесты по удалению данных выполнятся на продуктовой базе, начнут падать из-за неработающего партнёрского сервиса, или в вашем CI просто не будет базы, на которой можно их прогнать. В общем, пункт достаточно холиварный, но как правило — если вы можете эмулировать внешние сервисы, то лучше это делать.
Вот такая вот подборка вышла. Все эти тесты отлично проходят проверки, но они broken by design. Добавляйте свои варианты в комментарии, или в репозиторий, который я сделал для коллекционирования таких ошибок.
Чтобы было интереснее, часть из них оформлена в виде проблемного кода и спойлера, открыв который, вы увидите, в чём была проблема. Так что рекомендую сначала смотреть на код, находить в нём ошибку, а затем открывать спойлер. Решения проблем указано не будет — предлагаю самим подумать над ним. Просто потому, что я ленивый. Порядок списка не имеет глубокого смысла — просто это очерёдность, в которой я вспоминал про всякие реальные проблемы, которые доводили нас до кровавых слёз. Наверняка многие вещи покажется вам очевидными — но даже опытные разработчики могут случайно написать такой код.
Итак, поехали.
0. Отсутствие тестов
Как ни странно, до сих пор многие считают, что написание тестов замедляет скорость разработки. Конечно, это очевидно, что больше времени приходится тратить на то, чтобы писать тесты, и чтобы писать код, который можно тестировать. Но на отладку и регрессы после этого придётся тратить в разы больше времени…
1. Отсутствие запуска тестов
Если у вас есть тесты, которые вы не запускаете, или запускаете от раза к разу — то это всё равно что отсутствие тестов. И это даже хуже — у вас есть устаревающий код тестов и ложное чувство безопасности. Тесты как минимум должны запускаться в CI процессах при пуше кода в ветку. А лучше — локально перед пушем. Тогда разработчику не придётся через несколько дней возвращаться к билду, который, как оказалось, не прошёл.
2. Отсутствие покрытия
Если вы ещё не знаете, что такое покрытие в тестах, то самое время во прямо сейчас пойти и прочитать. Хотя бы википедию. Иначе велики шансы того, что ваш тест будет проверять от силы 10% от того кода, который, как вы думаете, он проверяет. Рано или поздно вы на это обязательно наступите. Конечно, даже 100% покрытие кода никак не гарантирует его полную корректность — но это гораздо лучше, чем отсутствие покрытия так как покажет вам гораздо больше потенциальных ошибок. Недаром в последних версиях Node.JS даже появились встроенные инструменты для того, чтобы его считать. Вообще, тема покрытия глубокая и крайне холиварная, но не буду в неё слишком углубляться — хочется сказать по понемножку о многом.
3.
const {assert} = require('chai');
const Promise = require('bluebird');
const sinon = require('sinon');
class MightyLibrary {
static someLongFunction() {
return Promise.resolve(1); // just imagine a really complex and long function here
}
}
async function doItQuickOrFail()
{
let res;
try {
res = await MightyLibrary.someLongFunction().timeout(1000);
}
catch (err)
{
if (err instanceof Promise.TimeoutError)
{
return false;
}
throw err;
}
return res;
}
describe('using Timeouts', ()=>{
it('should return false if waited too much', async ()=>{
// stub function to emulate looong work
sinon.stub(MightyLibrary, 'someLongFunction').callsFake(()=>Promise.delay(10000).then(()=>true));
const res = await doItQuickOrFail();
assert.equal(res, false);
});
});Что тут не так
Таймауты в юнит тестах.
Здесь хотели проверить, что установка таймаутов на долгую операцию действительно работает. В целом это и так имеет мало смысла — не стоит проверять стандартные библиотеки — но так же такой код приводит к другой проблеме — увеличению выполнения времени прохождения тестов на секунду. Казалось бы, это не так много… Но помножьте эту секунду на количество аналогичных тестов, на количество разработчиков, на количеств запусков в день… И вы поймёте, что из-за таких таймаутов вы можете терять впустую много часов работы еженедельно, если не ежедневно.
4.
const fs = require('fs');
const testData = JSON.parse(fs.readFileSync('./testData.json', 'utf8'));
describe('some block', ()=>{
it('should do something', ()=>{
someTest(testData);
})
})Что тут не так
Загрузка тестовых данных вне блоков теста.
На первый взгляд кажется, что всё равно, где читать тестовые данные — в блоке describe, it или в самом модуле. На второй тоже. Но представьте, что у вас сотни тестов, и во многих из них используются тяжёлые данные. Если вы их грузите вне теста, то это приведёт к тому, что все тестовые данные будут оставаться в памяти до конца выполнения тестов, и запуск со временем будет потреблять всё больше и больше оперативной памяти — пока не окажется, что тесты больше вообще не запускаются на стандартных рабочих машинах.
5.
const {assert} = require('chai');
const sinon = require('sinon');
class Dog
{
// eslint-disable-next-line class-methods-use-this
say()
{
return 'Wow';
}
}
describe('stubsEverywhere', ()=>{
before(()=>{
sinon.stub(Dog.prototype, 'say').callsFake(()=>{
return 'meow';
});
});
it('should say meow', ()=>{
const dog = new Dog();
assert.equal(dog.say(), 'meow', 'dog should say "meow!"');
});
});
Что тут не так
Код фактически заменён стабами.
Наверняка вы сразу увидели эту смешную ошибку. В реальном коде это, конечно, не настолько очевидно — но я видел код, который был обвешан стабами настолько, что вообще ничего не тестировал.
6.
const sinon = require('sinon');
const {assert} = require('chai');
class Widget {
fetch()
{}
loadData()
{
this.fetch();
}
}
if (!sinon.sandbox || !sinon.sandbox.stub) {
sinon.sandbox = sinon.createSandbox();
}
describe('My widget', () => {
it('is awesome', () => {
const widget = new Widget();
widget.fetch = sinon.sandbox.stub().returns({ one: 1, two: 2 });
widget.loadData();
assert.isTrue(widget.fetch.called);
});
});
Что тут не так
Зависимость между тестами.
С первого взгляда понятно, что здесь забыли написать
afterEach(() => { sinon.sandbox.restore(); });
Но проблема не только в этом, а в том, что для всех тестов используется один и тот же sandbox. И очень легко таким образом смутировать среду выполнения тестов таким образом, что они начнут зависеть друг от друга. После этого тесты начнут выполняться только в определённом порядке, и вообще непонятно что тестировать.
К счастью, sinon.sandbox в какой-то момент был объявлен устаревшим и выпилен, так что с такой проблемой вы можете столкнуться только на легаси проекте — но есть огромное множество других способов смутировать среду выполнения тестов таким образом, что потом будет мучительно больно расследовать, какой код виновен в некорректном поведении. На хабре, кстати, недавно был пост про какой-то шаблон вроде «Ice Factory» — это не панацея, но иногда помогает в таких случаях.
7. Огромные тестовые данные в файле теста
Очень часто видел, как прямо в тесте лежали огромные JSON файлы, и даже XML. Думаю, очевидно, почему это не стоит делать — становится больно это смотреть, редактировать, и любая IDE не скажет вам за такое спасибо. Если у вас есть большие тестовые данные — выносите их вне файла теста.
8.
const {assert} = require('chai');
const crypto = require('crypto');
describe('extraTests', ()=>{
it('should generate unique bytes', ()=>{
const arr = [];
for (let i = 0; i < 1000; i++)
{
const value = crypto.randomBytes(256);
arr.push(value);
}
const unique = arr.filter((el, index)=>arr.indexOf(el) === index);
assert.equal(arr.length, unique.length, 'Data is not random enough!');
});
});Что тут не так
Лишние тесты.
В данном случае разработчик был очень обеспокоен тем, что его уникальные идентификаторы будут уникальными, поэтому написал на это проверку. В целом понятное стремление — но лучше прочитать документацию или прогнать такой тест несколько раз, не добавляя его в проект. Запуск его в каждом билде не имеет никакого смысла.
Ну и завязка на случайные значения в тесте — сама по себе является отличным способом выстрелить себе в ногу, сделав нестабильный тест на пустом месте.
9. Отсутствие моков
Гораздо проще запускать тесты с живой базой и сотальными сервисами, и гонять тесты на них.
Но рано или поздно это аукнется — тесты по удалению данных выполнятся на продуктовой базе, начнут падать из-за неработающего партнёрского сервиса, или в вашем CI просто не будет базы, на которой можно их прогнать. В общем, пункт достаточно холиварный, но как правило — если вы можете эмулировать внешние сервисы, то лучше это делать.
11.
const {assert} = require('chai');
class CustomError extends Error
{
}
function mytestFunction()
{
throw new CustomError('important message');
}
describe('badCompare', ()=>{
it('should throw only my custom errors', ()=>{
let errorHappened = false;
try {
mytestFunction();
}
catch (err)
{
errorHappened = true;
assert.isTrue(err instanceof CustomError);
}
assert.isTrue(errorHappened);
});
});Что тут не так
Усложнённая отладка ошибок.
Всё неплохо, но есть одна проблема — если тест вдруг упал, то вы увидите ошибку вида
1) badCompare
should throw only my custom errors:
AssertionError: expected false to be true
+ expected - actual
-false
+true
at Context.it (test/011_badCompare/test.js:23:14)
Дальше, чтобы понять, что за ошибка собственно случилась — вам придётся переписывать тест. Так что в случае неожиданной ошибки — постарайтесь, чтобы тест рассказал о ней, а не только сам факт того, что она произошла.
12.
const {assert} = require('chai');
function someVeryBigFunc1()
{
return 1; // imagine a tonn of code here
}
function someVeryBigFunc2()
{
return 2; // imagine a tonn of code here
}
describe('all Before Tests', ()=>{
let res1;
let res2;
before(async ()=>{
res1 = await someVeryBigFunc1();
res2 = await someVeryBigFunc2();
});
it('should return 1', ()=>{
assert.equal(res1, 1);
});
it('should return 2', ()=>{
assert.equal(res2, 2);
});
});
Что тут не так
Всё в блоке before.
Казалось бы, классный подход сделать все операции в блоке `before`, и таким образом оставить внутри `it` только проверки.
На самом деле нет.
Потому что в этом случае возникает каша, в которой нельзя ни понять время реального выполнения тестов, ни причину падения, ни то, что относится к одному тесту, а что к другому.
Так что вся работа теста (кроме стандартных инициализаций) должна выполняться внутри самого теста.
13.
const {assert} = require('chai');
const moment = require('moment');
function someDateBasedFunction(date)
{
if (moment().isAfter(date))
{
return 0;
}
return 1;
}
describe('useFutureDate', ()=>{
it('should return 0 for passed date', ()=>{
const pastDate = moment('2010-01-01');
assert.equal(someDateBasedFunction(pastDate), 0);
});
it('should return 1 for future date', ()=>{
const itWillAlwaysBeInFuture = moment('2030-01-01');
assert.equal(someDateBasedFunction(itWillAlwaysBeInFuture), 1);
});
});
Что тут не так
Завязка на даты.
Тоже казалось бы очевидная ошибка — но тоже периодически возникает у уставших разработчиков, которые уже считают, что завтра никогда не наступит. И билд который отлично собирался вчера, внезапно падает сегодня.
Помните, что любая дата наступит рано или поздно — так что или используйте эмуляцию времени штуками вроде `sinon.fakeTimers`, или хотя бы ставьте отдалённые даты вроде 2050 года — пускай голова болит у ваших потомков...
14.
describe('dynamicRequires', ()=>{
it('should return english locale', ()=>{
// HACK :
// Some people mutate locale in tests to chinese so I will require moment here
// eslint-disable-next-line global-require
const moment = require('moment');
const someDate = moment('2010-01-01').format('MMMM');
assert.equal(someDate, 'January');
});
});Что тут не так
Динамическая подгрузка модулей.
Если у вас стоит Eslint, то вы наверняка уже запретили динамические зависимости. Или нет.
Часто вижу, что разработчики стараются подгружать библиотеки или различные модули прямо внутри тестов. При этом они в целом знают, как работает `require` — но предпочитают иллюзию того, что им будто бы дадут чистый модуль, который никто пока что не смутировал.
Такое предположение опасно тем, что загрузка дополнительных модулей во время тестов происходит медленнее, и опять же приводит к большему количеству неопределённого поведения.
15.
function someComplexFunc()
{
// Imagine a piece of really strange code here
return 1;
}
describe('cryptic', ()=>{
it('success', ()=>{
const result = someComplexFunc();
assert.equal(result, 1);
});
it('should not fail', ()=>{
const result = someComplexFunc();
assert.equal(result, 1);
});
it('is right', ()=>{
const result = someComplexFunc();
assert.equal(result, 1);
});
it('makes no difference for solar system', ()=>{
const result = someComplexFunc();
assert.equal(result, 1);
});
});Что тут не так
Непонятные названия тестов.
Наверное, вы устали от очевидны вещей, да? Но всё равно придётся о ней сказать потому что многие не утруждаются написанием понятных названий для тестов — и в результате понять, что делает тот или иной тест, можно только после долгих исследований.
16.
const {assert} = require('chai');
const Promise = require('bluebird');
function someTomeoutingFunction()
{
throw new Promise.TimeoutError();
}
describe('no Error check', ()=>{
it('should throw error', async ()=>{
let timedOut = false;
try {
await someTomeoutingFunction();
}
catch (err)
{
timedOut = true;
}
assert.equal(timedOut, true);
});
});Что тут не так
Отсутствие проверки выброшенной ошибки.
Часто нужно проверить, что в каком-то случае функция выкидывает ошибку. Но всегда нужно проверять, те ли это дроиды, которых мы ищем — поскольку внезапно может оказаться, что ошибка была выкинута другая, в другом месте и по другим причинам...
17.
function someBadFunc()
{
throw new Error('I am just wrong!');
}
describe.skip('skipped test', ()=>{
it('should be fine', ()=>{
someBadFunc();
});
});Что тут не так
Отключенные тесты.
Конечно, всегда может появиться ситуация, когда код уже много раз протестирован руками, нужно срочно его катить, а тест почему-то не работает. Например, из-за не очевидной завязки на другой тест, о которой я писал ранее. И тест отключают. И это нормально. Не нормально — не поставить мгновенно задачу на то, чтобы включить тест обратно. Если это не сделать, то количество отключенных тестов будет плодиться, а их код — постоянно устаревать. Пока не останется единственный вариант — проявить милосердие и выкинуть все эти тесты нафиг, поскольку быстрее написать их заново, чем разбираться в ошибках.
Вот такая вот подборка вышла. Все эти тесты отлично проходят проверки, но они broken by design. Добавляйте свои варианты в комментарии, или в репозиторий, который я сделал для коллекционирования таких ошибок.
