Один из 12 654 наших тестов сам написал, что нашёл боевой баг — но ему разрешили иногда падать

В кабинете продавца Авито, который мы делаем для клиентов, у восстановленного из архива объявления есть кнопка «Восстановить». Иногда — редко и без видимой закономерности — повторный клик по ней (человек не видит, что первый уже ушёл в работу, и жмёт снова) вместо тихого «уже восстановлено» отдавал ошибку 500. Классическая гонка: два почти одновременных запроса бьются за одну и ту же строку в базе, и кому-то должно достаться молчание, а не крах.

Нашли мы это не по жалобе клиента, а через собственную ревизию. 26 октября Роман поставил задачу на всю компанию: один стандарт тестов для девяти наших продуктов вместо разных правил в каждом — где-то их почти не было, где-то документ на полторы тысячи строк жил как журнал PR, а не как справочник. За несколько дней 53 пачки ИИ-ревьюеров прошли по 12 654 тестам и спросили про каждый одно и то же: что именно он проверяет и проверяет ли вообще что-нибудь. Итог: 86% тестов оставить как есть, 403 удалить (дубли и мёртвый код), 463 объединить, 922 переписать — и отдельно нашлись 90 тестов, которые не проверяли ничего: тест-близнец боевого кода, который сравнивает функцию с самой собой и поэтому не может покраснеть никогда.

Пока переписывали тесты вокруг восстановления объявлений, нашли экземпляр интереснее фальшивок. Тест на гонку двух кликов «Восстановить» был написан заранее и честно воспроизводил IntegrityError — то есть автор теста реальный дефект уже когда-то поймал за руку. Но вместо немедленной починки тест пометили служебным флагом «ожидаемо падает» с комментарием прямо в коде: «найден боевой дефект, не чинится в тестовом PR». После этого CI исправно зеленел каждый прогон — этот флаг и означает «мы знаем, что здесь иногда падает, не считай это новой проблемой». Решение отложить было разумным в моменте: гонка редкая, воспроизводится нестабильно по таймингу, а тот PR был совсем про другое. Проблема в том, что «починим позже» без даты в календаре имеет свойство не наступать — и для любой команды это нормальный риск, если нет периодической ревизии всех таких «потом».

Причина — на уровне одной строки SQL. Вставка восстановленного объявления делала `INSERT ... ON CONFLICT ON CONSTRAINT ux_ads_tenant_fid DO NOTHING` — «если такая запись уже есть по вот этому конкретному уникальному индексу, тихо не вставляй». Но у таблицы объявлений есть второй уникальный индекс, по avito_id, который в арбитр конфликта не входил. При точном совпадении времени двух кликов проигравший запрос иногда попадал не в тот индекс, который умеет молчать, а в тот, который не умеет, — Postgres кидал IntegrityError, а наружу это уходило как 500. Починка — убрать явный арбитр: `ON CONFLICT DO NOTHING` без указания конкретного индекса реагирует на любой уникальный конфликт, а проигравший после этого сам перечитывает строку победителя, сначала по своему ключу, а если не нашёл — по avito_id. Тест снят с «ожидаемо падает», гонка прогоняется в CI десятками повторов и теперь зеленеет по-настоящему, а не по разрешению.

В той же ревизии всплыли ещё два похожих случая, и оба тоже тихо жили за зелёным CI. Ротация объявлений с проблемой на самом оффере — товар смотрят, но не пишут, — почему-то переписывала заголовок вместо описания и цены: функция, которая решает, что переписывать, отдавала одни названия причин, а шаблоны рерайта знали другие, и при несовпадении код всегда откатывался на правку заголовка по умолчанию. И объявление без указанного в тексте города роняло исключением всю проверку перед рерайтом — вместо того, чтобы просто сказать «город не указан, не требуем его в тексте» и пойти дальше. Оба чинятся по одному и тому же принципу: привести список причин к тому единственному месту в коде, где перечисление уже было правильным.

Из всего этого у нас остался один рабочий вывод, который теперь спрашиваем отдельно на каждой большой ревизии тестов: зелёный тест отвечает на вопрос «не нарушил ли я то, что уже решил проверять», а не на вопрос «работает ли код». Флаг «ожидаемо падает», пропуск теста или заглушка вместо реального вызова — это разрешение не проверять что-то здесь и сейчас, и у каждого такого разрешения должна быть дата пересмотра, а не комментарий «починим позже» без неё. Если в тестах лежит хотя бы один такой флаг старше квартала — там, скорее всего, уже готовый список дефектов, который никто не открывал с того дня, когда его туда положили.