Симптомы кода приводящего к плохим юнит тестам:
Часто нам приходится писать java код которому необходимо предпринимать различные действия в зависимости от текущего времени:
Я искренне надеюсь что Вы сознательный пограммист, следовательно не менее часто Вам приходится писать юнит тесты к такому коду, которые тестируют пошел ли код по определенной ветке или нет в зависимости от того истек ли определенный интервал времени.
Рассмотрим простой пример кода, который написан таким образом что затрудняет своё тестирование. Ниже приведен класс представляющий собой некий аларм тобишь сигнал о том, что в приложении что-то не так, после того как зажегся аларм остаётся взведенным в течении сконфигурированного периода времени, а потом затухает:
public class TemporaryAlarm {
private final AtomicLong lastUpdateTime = new AtomicLong(0L);
private final long expirationTimeInMillis;
public TemporaryAlarm(long expirationTimeMillis) {
this.expirationTimeInMillis = expirationTimeMillis;
}
public void turnOn() {
lastUpdateTime.set(System.currentTimeMillis());
}
public boolean isTurnedOn() {
return System.currentTimeMillis() - lastUpdateTime.get() < expirationTimeInMillis;
}
}
Аларм в конструкторе конфигурируется временем в течении которого он должен хранить взведенное состояние, в методе turnOn запоминается время взведение тревоги, а метод проверяющий взведен ли аларм isTurnedOn сравнивает время прошедшее с момента последнего взведения с сконфигурированным интервалом. Код осуществлющий валидацию параметров конструктра намеренно опущен.
Вот код юнит теста покрывающего все ветки:
public class TemporaryAlarmTest {
@Test
public void testInitStateShouldBeTurnedOff() {
TemporaryAlarm alarm = new TemporaryAlarm(1000);
Assert.assertFalse(alarm.isTurnedOn());
}
@Test
public void testStateShouldBeTurnedOnDuringExpirationTimeout() {
TemporaryAlarm alarm = new TemporaryAlarm(1_000);
alarm.turnOn();
Assert.assertTrue(alarm.isTurnedOn());
Thread.sleep(50);
Assert.assertTrue(alarm.isTurnedOn());
}
@Test
public void testStateShouldBeTurnedOffAfterExpirationTimeout() {
TemporaryAlarm alarm = new TemporaryAlarm(1000);
alarm.turnOn();
Assert.assertTrue(alarm.isTurnedOn());
Thread.sleep(TimeUnit.SECONDS.toMillis(2));
Assert.assertFalse(alarm.isTurnedOn());
}
}
На первый взгляд кажется, что с этим тестом всё хорошо, он стабильно проходит и инструменты анализа coverage показывают, что он покрывает все ветки в тестируемом классе, мы фиксируем код теста и класса в общем репозитарии, сборка на continuous integration сервере проходит и мы благополучно забываем про этот класс и тест к нему.
Но забыть на долго этот код у нас увы не получится. Спустя некторое время с continuous integration сервера приходит письмо о том, что сборка упала, мы проходим по ссылке с отчетом и видим что упал testStateShouldBeTurnedOnDuringExpirationTimeout, смотрим код класса и теста не поменял ли там кто что-нибудь - да нет ничто не менялось, тогда в непонимании чешем репу, запускаем сборку заново, она проходит и мы с чувством выполненного долга и твердой уверенностью, что опять в этом вашем JUnit что-то набыдлокодили забываем про этот тест.
Но увы опять не надолго, к нам приходит коллега которого мы недолюбливаем из-за того, что ему вечно спокойно не сидится и всегда хочется чего-то странного. На этот раз он решил что maven сборка проходит слишком долго а именно 4 минуты, он решил задействать возможности по паралельной сборке, которые на его машине сокращают врем билда до одной минуты, но при запуске билда в паралельном режиме он обнаружил что опять же тест testStateShouldBeTurnedOnDuringExpirationTimeout то проходит, то нет.
Что же не так с этим тестом? Мы настроили время жизни аларма в одну секунду, взвели его, отправили поток в сон всего лишь на 50 милисекунд, следовательно, когда поток выходит из сна остается еще 950 милисекундный запас, и выйдя из сна поток должен увидеть флаг взведенным, однако тесты иногда показывают что это не так и флаг выключен. Всё дело в том, что поток выходит из сна, не сразу когда истечет время переданное в метод sleep, всё сильно зависит от текущей загруки и процессора и наличия паралельно выполняющихся потоков желающих откусить свой кусок пирога от ресурсов процессора, и может случится так, что пройдёт и более одной секунды прежде чем планировщик операционной системы поставит поток обратно на выполнение. Этим объясняется и случайные подения на continuous integration сервере, когда сервер не сильно загружен то тест успешно проходит, но может случиться так что на сервере будет запущенно паралельно сразу много сборок и тогда при достижении определенного порога нагрузки на процессор этот тест может упасть. То же самое и про падения на машинах разработчиков, когда я запускаю maven сборку в паралельном режиме, у меня даже движения курсора мыши по экрану медленно прорисовывается, соответсвенно тест опять работает нестабильно.
Нестабильность это не единственная проблема, с такими рода тестами. Всегда когда мы используеем Thread.sleep() в тестах мы тем самым увеличиваем вермя сборки на то время котрое потоки проводят в этих самых слипах. Это не так страшно когда таких тестов несколько штук, однако в продукте истрия которого длится не один год, таких тестов может накопится сотни, а то и тысячи, а почем зря потраченное время может исчисляться минутами. Это всё напросно потраченные деньги компании, ведь когда разработчик запускает сборку на локальной машине, прежде чем зафиксировать код в общий репозитарий, ему достаточно сложно играть в многорукого шиву и паралельно выполнять еще какую-то полезную работу, большинстве случаев мы тупо залипаем в конслоль и ждём. Заканчиваются такого рода истории как правило тем, что разработчики из-за долгих сборок вообще перестают их выполнять локально на своих машинах или делают это через раз, или локально собирают только то, что по их мнению могло сломаться после внесенных изменений.
Анализируем проблему:
И так наш тест: первое нестабилен, второе увеличивает время сборки. Для обретения стабильности нам нужно заставить метод System.currentTimeMillis() возвращать то время которое нам нужно, а не то которе сейчас реально на машинных часах, а если мы сможем настраивать поведение currentTimeMillis как нам нужно, то автоматически мы избавимся от пробемы со слипами, когда нам потребуется получить момент времени на 100 милисекунд позже текущего, вместо того чтобы реально отправлять поток в сон на 100 милесекунд, мы можем просто указать методу currentTimeMillis сразу вернуть нужное нам значение времени.
Всё бы хорошо, но метод System.currentTimeMillis()является статическим, и с помощью таких библиотек как Mockito или EasyMock стандартно входящих в инструментарий любого Java разработчика настроить его поведение на нужное не удастся, так как обычные библиотеки для мокирования подделывать поведения статических методов не умеют. Но не всё потерянно, если решать проблему в лоб и винить во всём только тест, то для мокирования можно взять более тяжелую артилерию как библиотеку PowerMock,
и легко добиться нужного нам результата. Однако я не буду приводить здесь код теста написанного с помощью PowerMock, и вовсе не потому, что считаю тесты написанные с помощью PowerMock более медленными чем обычные из-за того что под каждый тест они создают отдельный экземпляр класслоадера, или из-за того что считаю их нестабильными и рендомно отправляющими JVM в креш, а перспективы вообще запустить такие тесты на JDK отличающихся от OpenJDK например на J9 от IBM весьма туманны. Вовсе нет, есть более резонные причины в нашем конкретном случае не использовать PowerMock, и давайте пофилософствуем о них:
В написанном нами коде мы получаем текущее время через статику, однако при этом ни одному здравомыслящему Java программисту не прийдёт в голову к примеру через статические методы делать запросы в базу данных. Для запросов к БД мы всегда выделяем отдельный Data Access Object(или его собрат Repository) с нестатическими методами такими как записать, изменить, найти по критерию и.т.д. То есть остальной код приложения не имеет статических завязок на базу данных, код в приложении зависит от полноценного объекта и подложив в тест специально настренный mock вместо реального DAO мы можем протестировать бизнесс логику приложения без привязки к БД. Теперь вернемся ко времени, значение текущего время по отношению к нашей программе является точно таким же внешним ни коем образом неконтроллируемым фактором как и какая-то строка в таблице во внешней СУБД, так почему же мы в одних случаях позволяем работать с внешними по отношению к приложению понятиями через статику а в других случаях нет? Мне кажется ответ достаточно прост - мало кто об этом задумывается, вообще число разработчиков которые при написании кода рассуждают, философствуют и задают задают себе вопрос а не фигню ли я делаю, достаточно мало, а текущий дизайн стандартной библиотеки Java просто подталкивает считывать текущее время посредством статических методов, поскольку ничего другого банально в стандартной библиотеке нет!!!
Итог анализа проблемы следующий: виной плохих тестов является вовсе не непосредственно код самих тестов, как может показаться на первый взгляд, всему причина, то что сам продакшен код работает с внешними ресурсами способом, который человеческим образом невозможно замокать. Каков стол таков и стул каков код таков и тест.
Исправляем проблему:
Решение достаточно тривиально. Если стандартная бибилиотека Java не предоставляет на никаких способов без примения статических методов получать текущее время, так давайте создадим класс-обертку, который будет позволять это делать:
public interface Clock {
long currentTimeMillis();
long nanoTime();
static Clock getDefaultClock() {
return DEFAULT;
}
Clock DEFAULT = new Clock() {
@Override
public long currentTimeMillis() {
return System.currentTimeMillis();
}
@Override
public long nanoTime() {
return System.nanoTime();
}
};
}
Как видим получившийся класс Clock является интерфейсом, во все места в коде где идёт работа со временем и это нужно протестировать, теперь можно инжектить реализации этого интерфейса, вместо того чтобы, запрашивать время напрямую у системы. Экземпляр дефолтной имплементации запомнен в константу и его можно повсеместно использовать, чтобы не порождать зря одинаковые объекты. Создавать моки можно легко как с использованием таких библиотек как Mockito или EasyMock так и без них.
Теперь исходный класс TemporaryAlarm преобразуется в следующий:
public class TemporaryAlarm {
private final AtomicLong lastUpdateTime = new AtomicLong(0L);
private final long expirationTimeInMillis;
private final Clock clock;
public TemporaryAlarm(long expirationTimeMillis) {
this(expirationTimeMillis, Clock.getDefaultClock());
}
// visible for testing
TemporaryAlarm(long expirationTimeMillis, Clock clock) {
this.expirationTimeInMillis = expirationTimeMillis;
this.clock = clock;
}
public void turnOn() {
lastUpdateTime.set(clock.currentTimeMillis());
}
public boolean isTurnedOn() {
return clock.currentTimeMillis() - lastUpdateTime.get() < expirationTimeInMillis;
}
}
Как видно код не сильно то и изменился. Для удобства был создан еще один конструктор, который помимо таймаута действия аларма, еще как параметр принимает экзэмпляр объекта часы. Так нам врятли где захочется менять способ получения текущего времения кроме как в тестах, то с этого констуктора убран модификатор public ибо в продакшене он нам не нужен.
Теперь посмотрим как изменился тест:
public class TemporaryAlarmTest {
Clock clock = Mockito.mock(Clock.class);
@Test
public void testInitStateShouldBeTurnedOff() {
TemporaryAlarm alarm = new TemporaryAlarm(1000);
Assert.assertFalse(alarm.isTurnedOn());
}
@Test
public void testStateShouldBeTurnedOnDuringExpirationTimeout() {
TemporaryAlarm alarm = new TemporaryAlarm(1_000, clock);
when(clock.currentTimeMillis()).thenReturn(100_000_000_000L, 100_000_000_001L, 100_000_000_050L);
alarm.turnOn();
Assert.assertTrue(alarm.isTurnedOn());
Assert.assertTrue(alarm.isTurnedOn());
}
@Test
public void testStateShouldBeTurnedOffAfterExpirationTimeout() {
TemporaryAlarm alarm = new TemporaryAlarm(1000, clock);
when(clock.currentTimeMillis()).thenReturn(100_000_000_000L, 100_000_000_001L, 100_000_002_000L);
alarm.turnOn();
Assert.assertTrue(alarm.isTurnedOn());
Assert.assertFalse(alarm.isTurnedOn());
}
}
В тестах теперь вместо того, чтобы отправлять поток в сон, мы настраиваем серию значений которую должна возвращать заглушка для часов. И всё - мы победили как непредсказуемость результата так и долгое выполнение тестов, без использования грубых хаков в стиле PowerMock.
Теперь по поводу хаков: если Вы мучаетесь вопросом, а не велосипед ли здесь написан, и решает ли такие проблемы таким же способом еще хоть кто-то в обозреваемой галактике, то будте уверенны что Вы не одиноки. Java обычно используется в больших проектах с множеством зафисимостей, Вы можете в своем проекте просто в IDE запустить поиск по классу Clock, у меня нашлись следующие примеры:
- org.apache.http.impl.client.Clock
- com.codahale.metrics.Clock
Эти классы предназначенны ровно для того, для чего мы создавали свой. Так что как видно, что проблема эффективного тестирования кода зависящего от времени таки мучает широкую общественность. Так же во многих библиотеках абстракции для времени могут называться по разному, например в Guava обертка над временем называется Ticker.
О java 8 замолвите слово:
Утверждая, что стандартная бибилиотека Java подталкивает писать трудно тестируемый код, нужно конечно же указывать, а про какую именно версию java идёт речь. В java 8 API для работы с датами и временем был серьезно доработан давайте-ка посмотрим, что получилось и не решена в восьмерке проблема абстракции над получением текущего времени. Класс который нам нужен достаточно быстро обнаруживается в новом пакете java.time, он неожиданно также называется Clock, и он решает нашу проблему с написанием легко тестируемого кода, но к сожалению лишь частично. Почему всего лишь частично и в каких случаях он нам не подойдёт(или не совсем подойдёт) будет написанно в самом конце статьи. Пока же порадуемся, что мы в своем проекте перешли на Java 8, и перепишем наш код на использование java.time.Clock, я подразумеваю что джавадоки к классу Вы осилите самостоятельно:
import java.time.Clock;
import java.util.concurrent.atomic.AtomicLong;
public class TemporaryAlarm {
private final AtomicLong lastUpdateTime = new AtomicLong(0L);
private final long expirationTimeInMillis;
private final Clock clock;
public TemporaryAlarm(long expirationTimeMillis) {
this(expirationTimeMillis, Clock.systemUTC());
}
// visible for testing
TemporaryAlarm(long expirationTimeMillis, Clock clock) {
this.expirationTimeInMillis = expirationTimeMillis;
this.clock = clock;
}
public void turnOn() {
lastUpdateTime.set(clock.millis());
}
public boolean isTurnedOn() {
return clock.millis() - lastUpdateTime.get() < expirationTimeInMillis;
}
}
Как видно продакшен код практически не изменился. Не сильно изменился и соответсвующий юнит тест, всего то вместо одного класса мокаем другой:
import java.time.Clock;
import static org.mockito.Mockito.*;
public class TemporaryAlarmTest {
Clock clock = Mockito.mock(Clock.class);
@Test
public void testInitStateShouldBeTurnedOff() {
TemporaryAlarm alarm = new TemporaryAlarm(1000);
Assert.assertFalse(alarm.isTurnedOn());
}
@Test
public void testStateShouldBeTurnedOnDuringExpirationTimeout() {
TemporaryAlarm alarm = new TemporaryAlarm(1_000, clock);
when(clock.millis()).thenReturn(100_000_000_000L, 100_000_000_001L, 100_000_000_050L);
alarm.turnOn();
Assert.assertTrue(alarm.isTurnedOn());
Assert.assertTrue(alarm.isTurnedOn());
}
@Test
public void testStateShouldBeTurnedOffAfterExpirationTimeout() {
TemporaryAlarm alarm = new TemporaryAlarm(1000, clock);
when(clock.millis()).thenReturn(100_000_000_000L, 100_000_000_001L, 100_000_002_000L);
alarm.turnOn();
Assert.assertTrue(alarm.isTurnedOn());
Assert.assertFalse(alarm.isTurnedOn());
}
}
Однако что же не решено по обозначенной в начале посте проблеме в восьмерке?
- Ну что ж, давайте бросим ложку дёгтя в цистерну бочку мёда восьмерки. Если вы посмотрите обертку для времени которую мы написали своим собственными руками, то помимо метода получения текущего времени в милисекундах, также есть метод получения времени в наносекундах. К сожалению в классе Clock из стандартной библиотеке Java 8 есть метод получение времени в милисекундах millis(), но отcутствуют метод для получения времени в наносекундах. Это в принципе даже понятно почему, пакет java.time он больше про работу на уровне Wall-clock time - то есть со временем которое имеет значение для человеческих человеков, а наносекунды человекам нужны достаточно редко, а если кому-то и потребуются, то сначала следует разобраться, что это за человеки такие. Вот к примеру в TemporaryAlarm наносекунды были не нужны, могу поспорить, что в 99% кода, который Вы пишите наносекунды тоже не нужны, так что Clock из java time в большинстве, то что Вам нужно для написания тестируемого кода, ну это конечно при условии, что Вы можете себе позволить писать на Java 8, многие на восьмерку еще не перешли и такой роскоши себе позволить не могут.
Однако если Вы пишите специфический инфраструктурный код, например код связанный со сбором телеметрии, профилированием, бенчмаркингом и прочими прелестями при написании которых нужно точно знать сколько вешать в граммах, то без кастомной обертки над временем, Вы никак не обойдётесь если захотите написать к нему хорошие юнит тесты. Правда наверно обертку не всегда нужно писать самим и копипастить таскать за сосбой из проекта в проект, к примеру в библиотеке Guava, которая претендует быт расширением стандартной библиотеки java и как правило так же есть в зависимостях у любого Java проекта, уже есть готовая абстракция над получением времения в наносекундах которая называется Ticker.
Постскриптум:
Ну и напоследок. Юнит тесты это конечно же хорошо, но не стоит забывать и про полноценное интеграционное тестирование вашего кода работающего со временем, не поленитесь написать хоть чуть-чуть полноценных тестов работающих по принципу черного ящика, иначе с вашим кодом который юнит тестами покрыт на 146%, может приключиться такая история:
Из-за разницы в календарях (Россия – Юлианский календарь, Англия –
Григорианский календарь) сборная России в 1908 году опоздала на
Олимпиаду на 12 дней.
