Статьи

Не полагайтесь только на юнит-тесты

Когда вы строите сложную систему, одного лишь тестирования компонентов в отдельности недостаточно. Это важно, но недостаточно. Представьте себе автомобильный завод, который производит и импортирует детали самого высокого качества, но после сборки автомобиль так и не заводит двигатель. Если ваш набор тестов состоит только из модульных тестов, вы никогда не можете быть уверены, что система в целом работает. Давайте приведем надуманный пример:

01
02
03
04
05
06
07
08
09
10
11
12
public class UserDao {
  
    public List<User> findRecentUsers() {
        try {
            return //run some query
        } catch(EmptyResultDataAccessException ignored) {
            return null;
        }
    }
  
    //...
}

Я надеюсь, что вы уже заметили анти-паттерн в блоке catch (и я не имею в виду игнорирование исключения, похоже, это и ожидалось). Будучи хорошим гражданином, мы решаем закрепить возвращает пустую коллекцию вместо null :

01
02
03
04
05
06
07
08
09
10
11
12
public class UserDao {
  
    public List<User> findRecentUsers() {
        try {
            return //run some query
        } catch(EmptyResultDataAccessException ignored) {
            return Collections.emptyList();
        }
    }
  
    //...
}

Исправление настолько простое, что мы почти забываем о запуске модульных тестов, но на случай, если мы выполним их и обнаружим, что первый тестовый пример потерпел неудачу:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
public class UserDaoTest {
  
    private UserDao userDao;
  
    @Before
    public void setUp() throws Exception {
        userDao = new UserDao();
    }
  
    @Test
    public void shouldReturnNullWhenNoRecentUsers() throws Exception {
        //given
  
        //when
        final List<User> result = userDao.findRecentUsers();
  
        //then
        assertThat(result).isNull();
    }
  
    @Test
    public void shouldReturnOneRecentUser() throws Exception {
        //given
        final User lastUser = new User();
        userDao.storeLoginEvent(lastUser);
  
        //when
        final List<User> result = userDao.findRecentUsers();
  
        //then
        assertThat(result).containsExactly(lastUser);
    }
  
    @Test
    public void shouldReturnTwoRecentUsers() throws Exception {
        //given
        final User lastUser = new User();
        final User oneButLastUser = new User();
        userDao.storeLoginEvent(oneButLastUser);
        userDao.storeLoginEvent(lastUser);
  
        //when
        final List<User> result = userDao.findRecentUsers();
  
        //then
        assertThat(result).containsExactly(lastUser, oneButLastUser);
    }
  
}

По-видимому, не только код был поврежден (возвращая null вместо пустой коллекции, null ноль), но был тест, проверяющий это поддельное поведение. Я почти уверен, что тест был написан после реализации и должен был как-то соответствовать реальности. Никто бы никогда не написал такой тест без предварительного знания особенностей реализации. Таким образом, мы исправляем тест и с радостью ждем зеленой сборки CI — которая в итоге наступит. Дни спустя наше приложение разрывается с NullPointerException на производстве. Это ломается в месте, которое полностью проверено модулем:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
public class StatService {
  
    private final UserDao userDao;
  
    public StatService(UserDao userDao) {
        this.userDao = userDao;
    }
  
    public void welcomeMostRecentUser() {
        final List<User> recentUsers = userDao.findRecentUsers();
        if (recentUsers != null) {
            welcome(recentUsers.get(0));
        }
    }
  
    private void welcome(User user) {
        //...
    }
}

Мы удивлены, потому что этот класс полностью покрыт модульными тестами (шаг проверки пропущен для ясности):

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
@RunWith(MockitoJUnitRunner.class)
public class WelcomeServiceTest {
  
    @Mock
    private UserDao userDaoMock;
    private WelcomeService welcomeService;
  
    @Before
    public void setup() {
        welcomeService = new WelcomeService(userDaoMock);
    }
  
    @Test
    public void shouldNotSendWelcomeMessageIfNoRecentUsers() throws Exception {
        //given
        given(userDaoMock.findRecentUsers()).willReturn(null);
  
        //when
        welcomeService.welcomeMostRecentUser();
  
        //then
        //verify no message sent
    }
  
    @Test
    public void shouldSendWelcomeMessageToMostRecentUser() throws Exception {
        //given
        given(userDaoMock.findRecentUsers()).willReturn(asList(new User()));
  
        //when
        welcomeService.welcomeMostRecentUser();
  
        //then
        //verify user welcomed
    }
  
    //...
  
}

Вы видите, где проблема? Мы изменили контракт класса UserDao пока он «выглядит» одинаково на поверхности. Исправляя сломанные тесты, мы предполагали, что это все еще работает. Однако WelcomeService прежнему полагался на старое поведение UserDao , которое возвращало либо UserDao , либо список, по крайней мере, с одним элементом. Это поведение было записано с использованием фреймворк-фреймворка, так что мы смогли разделить тестирование WelcomeService . Другими словами, нам не удалось убедиться, что эти два компонента все еще работают друг с другом, мы только протестировали их в одиночку. Возвращаясь к метафоре нашего автомобиля — все части все еще сочетаются друг с другом (один и тот же контракт), но одна из них ведет себя не так, как раньше. Итак, что же на самом деле пошло не так? Здесь есть как минимум четыре проблемы, и если бы какая-то из них была смягчена, ничего бы этого не случилось.

Прежде всего, автору UserDao не удалось распознать, что возвращение null при пустом списке кажется гораздо более интуитивным. Напрашивается вопрос: есть ли значительная разница между null и пустым набором? Если да, может быть, вы пытаетесь «закодировать» слишком много информации в одном возвращаемом значении? Если нет, то зачем усложнять жизнь своим пользователям API? Перебор пустой коллекции не требует дополнительных усилий; перебор коллекции, которая может быть null требует одного дополнительного условия. Автор WelcomeService потерпел неудачу, предполагая, что null означает пустую коллекцию. Он должен работать вокруг уродливого API, а не полагаться на него. В этом случае он мог бы использовать CollectionUtils.isNotEmpty() и быть немного более оборонительным:

1
if (CollectionUtils.isNotEmpty(recentUsers)) {

Для более комплексного решения он мог бы также рассмотреть возможность украшения UserDao и замены null пустой коллекцией. Или даже с помощью AOP для глобального исправления таких API во всем приложении. И кстати, это относится и к String s. В 99% случаев нет никакой «деловой» разницы между null , пустой строкой и строкой с несколькими пробелами. Используйте StringUtils.isBlank() или аналогичный по умолчанию, если вы действительно не хотите различать их.

Наконец человек, «фиксирующий» UserDao не смог увидеть общую картину. Едва фиксируя юнит-тесты недостаточно. Когда вы изменяете поведение класса, не меняя API (это особенно важно для динамических языков), скорее всего, вы упустите места, где этот API использовался, теряя контекст. Но самой большой ошибкой было отсутствие тестов компонентов / системы . Если бы у нас просто был тестовый пример, выполняющий и WelcomeService и UserDao работающие вместе, эта ошибка была бы обнаружена. Недостаточно 100% покрытия кода. Вы проверяете каждую часть головоломки, но никогда не смотрите на готовую картинку. Проведите хотя бы несколько больших тестов на дым. В противном случае у вас больше не будет этой удивительной уверенности в том, что когда тесты зеленого цвета, код будет полезен.

Справка: не полагайтесь только на юнит-тесты нашего партнера JCG Томаша Нуркевича в блоге NoBlogDefFound .