Главная » Хабрахабр » Неприятные ошибки при написании юнит тестов

Неприятные ошибки при написании юнит тестов

На днях я буду делать внутренний доклад, на котором расскажу нашим разработчикам про неприятные ошибки, которые могут возникнуть при написании юнит тестов. Самые неприятные с моей точки зрения ошибки — когда тесты проходят, но при этом делают это настолько некорректно, что лучше бы не проходили. И я решил поделиться примерами таких ошибок со всеми. Наверняка ещё что-нибудь подскажете из этой области. Примеры написаны для Node.JS и Mocha, но в целом эти ошибки справедливы и для любой другой экосистемы.

Так что рекомендую сначала смотреть на код, находить в нём ошибку, а затем открывать спойлер. Чтобы было интереснее, часть из них оформлена в виде проблемного кода и спойлера, открыв который, вы увидите, в чём была проблема. Просто потому, что я ленивый. Решения проблем указано не будет — предлагаю самим подумать над ним. Наверняка многие вещи покажется вам очевидными — но даже опытные разработчики могут случайно написать такой код. Порядок списка не имеет глубокого смысла — просто это очерёдность, в которой я вспоминал про всякие реальные проблемы, которые доводили нас до кровавых слёз.

Итак, поехали.

0. Отсутствие тестов

Как ни странно, до сих пор многие считают, что написание тестов замедляет скорость разработки. Конечно, это очевидно, что больше времени приходится тратить на то, чтобы писать тесты, и чтобы писать код, который можно тестировать. Но на отладку и регрессы после этого придётся тратить в разы больше времени…

1. Отсутствие запуска тестов

Если у вас есть тесты, которые вы не запускаете, или запускаете от раза к разу — то это всё равно что отсутствие тестов. И это даже хуже — у вас есть устаревающий код тестов и ложное чувство безопасности. Тесты как минимум должны запускаться в CI процессах при пуше кода в ветку. А лучше — локально перед пушем. Тогда разработчику не придётся через несколько дней возвращаться к билду, который, как оказалось, не прошёл.

2. Отсутствие покрытия

Если вы ещё не знаете, что такое покрытие в тестах, то самое время во прямо сейчас пойти и прочитать. Хотя бы википедию. Иначе велики шансы того, что ваш тест будет проверять от силы 10% от того кода, который, как вы думаете, он проверяет. Рано или поздно вы на это обязательно наступите. Конечно, даже 100% покрытие кода никак не гарантирует его полную корректность — но это гораздо лучше, чем отсутствие покрытия так как покажет вам гораздо больше потенциальных ошибок. Недаром в последних версиях Node.JS даже появились встроенные инструменты для того, чтобы его считать. Вообще, тема покрытия глубокая и крайне холиварная, но не буду в неё слишком углубляться — хочется сказать по понемножку о многом.

3.

const = require('chai');
const Promise = require('bluebird'); function someLongFunction()
{ return Promise.delay(10000);
} describe('using Timeout', ()=>{ it('should cancel after 1 second', async ()=>{ let timedOut = false; try { await someLongFunction().timeout(1000); } catch (err) { if (err instanceof Promise.TimeoutError) { timedOut = true; } else { throw err; } } assert.equal(timedOut, true); });
});

Что тут не так

Таймауты в юнит тестах.

В целом это и так имеет мало смысла — не стоит проверять стандартные библиотеки — но так же такой код приводит к другой проблеме — увеличению выполнения времени прохождения тестов на секунду. Здесь хотели проверить, что установка таймаутов на долгую операцию действительно работает. Казалось бы, это не так много… Но помножьте эту секунду на количество аналогичных тестов, на количество разработчиков, на количеств запусков в день… И вы поймёте, что из-за таких таймаутов вы можете терять впустую много часов работы еженедельно, если не ежедневно.

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. После этого тесты начнут выполняться только в определённом порядке, и вообще непонятно что тестировать.

На хабре, кстати, недавно был пост про какой-то шаблон вроде «Ice Factory» — это не панацея, но иногда помогает в таких случаях. К счастью, sinon.sandbox в какой-то момент был объявлен устаревшим и выпилен, так что с такой проблемой вы можете столкнуться только на легаси проекте — но есть огромное множество других способов смутировать среду выполнения тестов таким образом, что потом будет мучительно больно расследовать, какой код виновен в некорректном поведении.

7. Огромные тестовые данные в файле теста

Думаю, очевидно, почему это не стоит делать — становится больно это смотреть, редактировать, и любая IDE не скажет вам за такое спасибо. Очень часто видел, как прямо в тесте лежали огромные JSON файлы, и даже XML. Если у вас есть большие тестовые данные — выносите их вне файла теста.

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. Вот такая вот подборка вышла. Добавляйте свои варианты в комментарии, или в репозиторий, который я сделал для коллекционирования таких ошибок.


Оставить комментарий

Ваш email нигде не будет показан
Обязательные для заполнения поля помечены *

*

x

Ещё Hi-Tech Интересное!

[Перевод] Интервью с Дэвидом Гобелем

Дэвид любезно согласился дать LEAF очень интересное интервью. Дэвид Гобель – изобретатель, филантроп, футурист и ярый сторонник технологий омоложения; вместе с Обри де Греем он известен как один из основателей Methuselah Foundation и как автор концепции Longevity Escape Velocity (LEV), ...

10 долларов на хостинг: 20 лет назад и сегодня

Всё кругом дорожает, а технологии дешевеют. Когда-то компьютер или мобильный телефон могли позволить себе единицы, сейчас эти устройства есть в каждой российской семье. Цена мегабайта за последние 20 лет упала в несколько тысяч раз. Ещё один пример — хостинг. В ...