@VisibleForTesting не нужна

В Guava есть аннотация VisibleForTesting. Её описание из кода библиотеки:

Annotates a program element that exists, or is more widely visible than otherwise necessary, only for use in test code.

То есть предполагается, что этой аннотацией мы будем отмечать методы и поля, которые, по идее, должны были быть private, но для тестов были сделаны package-private или public.

По поводу этой аннотации я хотел бы сказать, что она, ИМХО, не нужна.

В случае, когда член нужен тесту из другого пакета, и его поэтому пришлось сделать public, это совсем другая проблема и никакие аннотации тут уже не помогут. Так что рассуждать будем о private полях, которые были сделаны package-private для тестов.

Так вот, нет ничего плохого в package-private членах. Они по-прежнему остаются недоступными для клиентов из других пакетов (это самое главное, потому что их обычно большинство). Для клиентов из того же пакета: сам факт того, что некоторое поле, метод или конструктор имеет ограниченную видимость, должен настораживать разработчика не хуже аннотации. Конечно, наверняка найдётся кто-нибудь, кого это не остановит от доступа к члену или методу не по назначению. Но, ей-богу, оно того не стоит:

До:

@VisibleForTesting
static final double AVOGADROS_NUMBER = 6.02214199e23;
@VisibleForTesting
static final double BOLTZMANN_CONSTANT = 1.3806503e-23;
@VisibleForTesting
static final double ELECTRON_MASS = 9.10938188e-31;

После:

static final double AVOGADROS_NUMBER = 6.02214199e23;
static final double BOLTZMANN_CONSTANT = 1.3806503e-23;
static final double ELECTRON_MASS = 9.10938188e-31;

И ещё. Описание к аннотации намекает, что некоторые члены могут быть не только раскрыты, но и созданы исключительно для тестирования. Так вот, я не Кент Бек, конечно, но вроде бы:

  1. продакшн код не должен испытывать никаких структурных изменений, сделанных исключительно для тестирования;

  2. если некоторый класс ну никак не протестировать без введения туда новых членов, то скорее всего беда с качеством кода. За подробностями можно обратиться к «Чистому коду».

А ещё я иногда вижу такое:

/* package */ class SomeClass implements SomeInterface  {

Для кого этот комментарий вообще?

Несогласные призываются в комменты.

4 thoughts on “@VisibleForTesting не нужна

  1. должен настораживать разработчика не хуже аннотации
    Как говорят питонисты, explicit is better than implicit.

    Вводить новую сущность и фабрику для нее, чтобы протестировать какой-нибудь мини-парсер внутри анализатора логов – кмк, гораздо больший overkill.

    если некоторый класс ну никак не протестировать без введения туда новых членов
    Попробуй надежно протестировать класс, который запускает фоновый поток или зависит от System.currentTimeMillis.
    В первом случае иногда удается отнаследоваться от тестируемого класса и добавить туда локи (привет любителям ставить final в произвольных местах).
    Во втором я запилил класс-обертку и заинжектил его, но не горжусь этим.

    1. Как говорят питонисты, explicit is better than implicit.
      Ну так вот /* package */ и получается. Где-то всё-таки должна быть грань между “слишком явно” и “слишком неявно”, и я бы тут предпочёл лишний раз подумать, чем лишний раз прочитать каких-нибудь аннотаций или комментариев.

      Попробуй надежно протестировать класс, который запускает фоновый поток или зависит от System.currentTimeMillis.
      Если класс работает в несколько потоков, то его можно разложить на методы так, чтобы их можно было протестировать, не задумываясь о потоках. 100% покрытия так не получишь. Наверное. С проверкой потоковой модели можно обратиться к JCIP, я уверен, что там найдётся решение.
      Заинжектить Clock или его аналог через package-private конструктор — хорошее решение второй проблемы. А что, есть более нормальный способ протестировать что-то, зависящее от времени?

      Впрочем, пост был не о тестировании, поэтому я своё мнение совсем не навязываю.

  2. Видимость чего-то только ради тестирования настораживает. Это наводит на мысль, что класс выполняет больше одной задачи. Вероятно, кусок кода стоит вынести в другой package-private класс и тестировать его отдельно. Мне никогда не приходилось сталкиваться с ситуацией, где такая штука могла бы пригодиться, а тесты я пишу много и часто.

    Кстати, похожая велосипедная аннотация присутствует в backa – BroaderVisibilityForTest :)

    1. Каждый юнит-тест должен проверять только один небольшой аспект работы класса, а не весь класс целиком. Для этого нужно больше видимости, чем обычно. Если бы мы говорили о функциональном тестировании, то все верно.

Leave a Reply

Your email address will not be published. Required fields are marked *