Повторяющиеся имена тестов в одной области действия приводят к тому, что некоторые тесты пропускаются:

# file with 3 tests (with duplicate names)
def test_a():
    pass
def test_a():
    pass
def test_a():
    pass

Тесты были написаны. Давайте запустим их:

$ pytest
collecting ... 
 tests/test_helpers.py::test_a ✓ 100% ██████████
Results (0.85s):
       1 passed

Было написано три теста, но в файле был найден только один тест, поэтому был запущен только один тест. Причиной этого является основная особенность Python (и, вероятно, всех языков программирования): когда функция, класс или переменная определяются несколько раз в одной и той же области видимости, последнее определение перезаписывает все предыдущие. Это верно для всех назначений имен в целом. Вот диаграмма различных областей, в которых может находиться тест:

Тесты могут иметь одно и то же имя, если они находятся в разных областях, но одно и то же имя в глобальной области или одно и то же имя в одной и той же области класса приведет к тому, что более поздние тесты переопределяют более ранние. Средства запуска тестов, такие как pytest или встроенный пакет unittest, могут обнаруживать и запускать только те тесты, которые определены в тестовых файлах, поэтому, если файл содержит повторяющееся имя теста, более ранние тесты будут недоступны. test runner, поэтому тест не запустится.

Мы в Code Review Doctor автоматически просканировали кодовую базу 420 общедоступных репозиториев Python с открытым исходным кодом на GitHub, чтобы обнаружить тесты, которые были пропущены из-за дублирующихся имен в пределах своей области. Мы проверили функциональные тесты в стиле pytest или тесты классов в стиле TestCase (забавный факт: хотя у Pytest много сторонников, 28% разработчиков Python все еще используют тесты unittest TestCase) . Мы обнаружили, что в 5% из 420 проверенных репозиториев по крайней мере один тест не выполнялся из-за этой проблемы. Рекорд по наибольшему дублированию для данного имени теста в той же области действия относится к test_service_mgr_runit, который использовался трижды. Это означает, что два теста были пропущены. Где-то есть разработчик, который крепко спит, ошибочно полагая, что написанный им тест демонстрирует, что код работает так, как ожидалось!

Вот суть затронутых строк, которую мы нашли. Мы автоматически создали простые запросы на внесение исправлений. Вот запросы на слияние, которые мы открыли для более интересных крупных проектов:

Это огромное количество затронутых кодовых баз (1 из 20 в образце) демонстрирует, что эта проблема не просто теоретическая, а реальная проблема, затрагивающая реальные кодовые базы — некоторые из них хорошо зарекомендовали себя с огромным количеством пользователей, отличным стандартом кодирования и сильный жизненный цикл разработки программного обеспечения. Полный список по этой теме смотрите здесь.

Воздействие и причина

Примечательно, что CI-сборки начали давать сбои примерно в 25% пулл-реквестов после того, как пропущенные тесты были переименованы. Это не обязательно означает, что код неисправен, но может указывать на то, что логика теста неверна. Тем не менее, неправильный тест — это все равно плохо, поскольку разработчик часто читает тест, чтобы понять ожидаемое поведение кода. Разработчик, читающий тест, вероятно, не заметит, что тест не выполнялся, и, следовательно, может получить плохие ожидания относительно поведения продукта.

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

Линтеры, такие как pylint или flake8, должны обнаруживать эту проблему, но даже это может привести к сбою, если:

  • Конфигурация линтера случайно неправильно настроена на игнорирование определенных файлов, отключение определенных проверок.
  • Линтер настроен на отключение «надоедливых» сообщений, не осознавая всех последствий отключения проверки.
  • Линтер настроен на отключение «раздражающих» сообщений lint специально для тестов (в конце концов, это просто тесты…)
  • CI может быть случайно неправильно сконфигурирован и не запустить линтер.
  • Линтер может просто не быть настроен. Конечно, это лучшая практика, но ее можно просто не использовать.

Кроме того, люди вряд ли обнаружат такие проблемы вручную во время проверки кода, поскольку это класс ошибок, которые люди могут легко пропустить, и человек, вероятно, полагается на линтер. При наличии достаточного количества времени процесс с участием людей всегда будет иметь человеческую ошибку, и ее следует учитывать, а не бороться с ней. Это проявляется в том факте, что мы вообще делаем обзор кода: если мы делаем обзор кода, потому что ожидаем человеческой ошибки, когда код был написан (разработан) человеком, то, конечно же, следует ожидать человеческой ошибки, когда код читается (просматривается) человеком. Любой процесс, основанный на нулевой человеческой ошибке, всегда будет терпеть неудачу. Вот почему линтинг так полезен, а неправильная конфигурация линтинга так проблематична.

Автоматическое исправление

Процесс сканирования пакетов кодовых баз является очень полезной частью жизненного цикла разработки программного обеспечения Code Review Doctor, поскольку он действует как ворота качества для обнаружения ложных срабатываний в наших правилах сканирования кода. Когда мы пишем новые правила сканера кода, мы подвергаем их стресс-тестированию, запуская их на сотнях кодовых баз, а затем проверяем, какие проблемы были обнаружены. В этом случае мы столкнулись с 420 кодовыми базами. Затем мы вручную выявляем ложные срабатывания и фиксированные пограничные случаи, прежде чем они смогут раздражать наших пользователей, которые полагаются на Code Review Doctor для просмотра своих запросов на включение GitHub. Результатом этого процесса является то, что мы можем внести свой вклад в проекты с открытым исходным кодом, открывая PR, устраняющие обнаруженные нами проблемы.

Простое исправление, которое Code Review Doctor автоматически предлагает при обнаружении повторяющихся имен тестов, заключается в добавлении суффикса _two к повторяющимся тестам. Более описательное имя было бы лучше, но назвать его сложно, и придумать хорошее имя следует оставить разработчику, который знает контекст теста. Сказав это, плохо названный тест, который выполняется, лучше, чем тест, который не выполняется. Более того, как только сопровождающие кодовой базы узнают о наличии проблемы, они могут выбрать лучшее имя, если захотят, или просто принять предложенный суффикс, поскольку он решает насущную проблему, связанную с невыполнением теста. Имея это в виду, эту проблему можно предотвратить, используя нашу проверку запросов на вытягивание GitHub, которая предложит переименовать тест, если возникнет проблема:

Вы также можете проверить, есть ли эта проблема в вашей кодовой базе, просканировав ее безопасным способом бесплатно на CodeReview.doctor.