Ассерты — одна из само собой разумеющихся вещей, поэтому все тут, казалось бы, специалисты. Кто-то пользуется встроенными в Java или в Junit, кто-то пользуется продвинутыми библиотеками, кто-то сооружает собственную.
Но попробуем подойти к этому более серьезно. Как, на самом деле, правильно?
Во-первых, ассерты дублируют спецификацию. Если в спецификации указан shouldReturnValidOrder, то и проверять нужно именно это, а не assertNotNull.
Во-вторых, ассерты должно быть легко читать. Т.е., мгновенно, сопоставляя со спецификацией.
Если ассертов слишком много (на мой крайний вкус много это больше одного, но поставим ограничение хотя бы в пять-семь), то тест перестает быть понимаемым. По хорошему, у каждого теста должна быть только одна причина упасть.
Ассерты могут быть направлены на: примитивное поле, объект, коллекцию, редко функцию или thunk.
Чем сложнее объект, тем сложнее и разнообразнее будут ассерты, и тем сложнее их будет читать и находить ошибки.
Положительные утверждения читаются лучше двойных отрицательных.
Стандартные методы читаются лучше кастомных хелперов. (Кто знает, кто и где тестировал библиотеку хелперов). И ассерты должны находиться в теле тестового метода, а не где-то в глубине тестовых хелперов (сонар справедливо ругается на отсутствие ассертов).
Ассерты о полях понятнее, чем ассерты об объектах и уж тем более о коллекциях.
В случае вложенных полей имеет смысл тестировать полностью.
Не
assertNotNull(order.getCustomer().getName)
а
assertNotNull(order)
assertNotNull(order.getCustomer())
assertNotNull(order.getCustomer().getName())
Ассерты не просто тупо проверяют то, что возвращается из метода, а влияют на это. И если в наших силах это поменять, то нужно поменять.
Относительно сложно делать утверждения о коллекциях. Содержат ли они нули, отсортированы ли они, как именно отсортированы, как именно проверяется равенство элементов и т.д. — все это делает коллекции трудными объектами для ассертов, особенно когда в методе присутствует и логика относящаяся к элементам коллекций.
Поэтому метод типа:
List<Order> getOrderList(List<OrderDao> orderDaoList){
return orderDaoList.stream().map(orderDao=>order.name(orderDao.getName).build()).collect(Collectors.toList())
}
легко разбить на два, один трансформер orderDao=>order и протестировать его отдельно, а второй будет тестировать маппинг коллекций на абстрактном маппере, и мы сможем его простестировать на заглушке.
List<Order> getOrderList(List<OrderDao> orderDaoList, OrderDaoToOrderMapper mapper){
return orderDaoList.stream().map(mapper::map).collect(Collectors.toList())
}
С другой стороны, коллекции хорошо поддаются типизации и расширению, т.е. мы относительно легко можем сделать коллекцию собственного типа со всеми проверенными свойствами, ко- и контравариантностью и тп. Поэтому вместо generic List мы могли бы сделать свой собственный OrderList, или OrderSet, или OrderSortedSet, и чем специфичнее, тем лучше. И тесты станут проще.
Делать утверждения о функциях не намного сложнее, чем об объектах, и они в некоторых языках хорошо типизируются, поэтому можно разве что посоветовать лучшую типизацию, т.е. вместо Func <Order,Order> возвращать какой-нибудь OrderComparator.
Комментарии (11)
vintage
11.11.2019 13:43assertNotNull(order) assertNotNull(order.getCustomer()) assertNotNull(order.getCustomer().getName())
Ну вот что даёт эта копипаста? И как оно соотновится с вашим же предложением "проверять нужно именно это, а не assertNotNull"?
rudnevr Автор
11.11.2019 15:29В данном случае копипаста показывает, что именно упало — и что мы на это рассчитывали,. Потому что если выпадет нульпойнтер до ассерта, то неясно какой именно в Order или order.getCustomer, и неясно, пропустили мы его случайно или осознанно.
Пойнт про valid order состоит в том, что надо проверять валидность ордера, а не просто нульпойнтер, коль скоро мы это задекларировали. Как именно валидность определена — это зависит от имплементации, главное не пренебрегать ассертами. Многие в погоне за покрытием ставят минимальный ассерт и бегут дальше.
vintage
11.11.2019 15:43неясно какой именно в Order или order.getCustomer
Всё ясно:
неясно, пропустили мы его случайно или осознанно
Да какая разница? Есть ошибка — её надо исправить.
rudnevr Автор
11.11.2019 16:37В джаве будет просто нульпойнтер на этой строчке. Разница в том, что намерения пишущего должны быть ясны. Может, там не должно быть в принципе нулевого кастомера. Может быть, нужно фиксить не код, а фабрику.
vintage
11.11.2019 17:10В джаве будет просто нульпойнтер на этой строчке.
Значит в java следует везде избегать цепочек вызовов, иначе не понятно, где именно возникла ошибка. Тесты тут ни при чём.
Разница в том, что намерения пишущего должны быть ясны.
Они и так предельно ясны — нулевого кастомера быть не должно.
rudnevr Автор
11.11.2019 18:33Возможно, что нулевого кастомера не должно быть благодаря чему-то ещё, а не нашему коду, например. Т. е. возможно, что все нуль чеки произошли в другом слое, и наш сетап должен это отражать. А наш тест может благополучно проверять только то, что делает наш тестируемый юнит, а не все вообще потенциальные нули.
vintage
11.11.2019 18:40Так ему и не надо проверять все нули в любом случае.
ApeCoder
12.11.2019 08:29Тут я согласен с vintage единственная проблема — менее читаемое сообщение об ошибке. Будет просто NPE, но это общая проблема для всего кода на Java.
А еще есть Kotlin, который заставить в синтаксисе выразить отношение к Null в конкретном месте.
rudnevr Автор
13.11.2019 07:32есть и котлин и ломбок, и статический анализ, но они касаются имплементации, а тут чисто описание сценария, которое должно быть отделено от имплементации. Мой пример — невнятный в данном случае — был против недостаточного (когда тестируют только конечные проперти) и избыточного (когда тестируют все приехавшее из метода). Я подумаю, как расписать полнее.
vintage
Вот вы говорите "как правильно", но почему именно так — не говорите. Это секрет или вы не знаете?
Гляньте на мои аргументы что ли: https://habr.com/ru/post/353080/
rudnevr Автор
У вас там так много аргументов, что я не знаю, какой вы имеете в виду. Я, кажется, с основными согласен. Неявные ассерты плохо, один ассерт на тест — согласен, но нерелизуемо пока, про given /when не согласен.