- проверки больше не зовут ffprobe и не меняют рабочий каталог процесса; добавлены случаи на отказ чтения метаданных и на отсутствие поля audio - заведена спека intake на приём по HTTP, ADR о подставных адаптерах, запись в журнал ревью о проверке, которая не могла упасть - go test снят из объявленных долгов CLAUDE.md, послабление errcheck для _test.go в .golangci.yml убрано
11 KiB
Context
Тесты приёма записи по HTTP лежат в репозитории с коммита 87d8b05 и не проходили
ни разу. Отказов четыре, и причина у них одна: тест поднимает приём вместе с
настоящим источником метаданных — внешней программой ffprobe, — а
скармливает ему либо файл, которого нет, либо строку «test audio content».
ffprobe такой вход отвергает, приём отвечает отказом, тест ждал успеха.
Отсюда следствие дороже самих тестов: красный go test в проекте объявлен долгом,
и настоящий отказ приёма от него неотличим.
Второе, что мешает: каталог хранения приём берёт из строки data/files,
относительной к рабочему каталогу процесса. Чтобы файлы не улетали в репозиторий,
каждый тест зовёт os.Chdir — то есть меняет состояние всего процесса.
Параллельно такие тесты гонять нельзя, а errcheck на непроверенных os.Chdir
молчит только потому, что весь _test.go вынесен в исключения линтера.
Goals / Non-Goals
Goals:
- проверка приёма судит наш код, а не способность
ffprobeразобрать вход; - отказ чтения метаданных проверен отдельным случаем: сегодня эту ветку не проверяет ничто;
- проверки проходят на чистом клоне без подготовки файлов руками и без
установленного
ffprobe; - проверки не трогают состояние процесса и не мешают друг другу.
Non-Goals:
- поведение приёма не меняется — ни коды ответов, ни имена полей, ни раскладка файлов на диске. Задача про то, чем поведение проверяется;
- код
internal/serviceиinternal/controller/httpне правится: подстановка туда уже заведена, и пользоваться ей — вопрос теста, а не сервиса; - проверка настоящего
ffprobeна настоящей записи здесь не заводится — см. «Risks / Trade-offs».
Decisions
Подставной источник метаданных вместо настоящего ffprobe
Приём берёт длительность через интерфейс contract.AudioMetaViewer, а
реализацию получает снаружи при сборке. Тест подставляет свою: она отдаёт
заданную длительность и, когда тесту нужен отказ, — заданную ошибку. Одним
типом закрываются оба случая, и ветка отказа впервые становится проверяемой.
Рассмотрено и отвергнуто:
- положить настоящую запись в
testdata. Отвергнуто дважды:.gitignoreстрокой*.m4aеё не пустит, аCLAUDE.mdпрямо говорит, чтоtestdataв проекте нет и тесты создают нужное во временном каталоге. Снимать запрет ради теста — менять правило проекта под удобство одного файла; - порождать запись
ffmpegпрямо в тесте. Отвергнуто: проверка приёма начинает требовать установленныхffmpegиffprobe, а критерий приёмки требует обратного — прогона сffprobe, убранным изPATH; - проверять приём сквозь настоящий
ffprobe. Отвергнуто: это проверка внешней программы, а не нашего кода. Она найдёт отказffprobeи не найдёт ошибку в приёме — ровно наоборот тому, зачем эти тесты писались.
Каталог хранения задаётся снаружи, а не рабочим каталогом процесса
Каталог тесту даёт t.TempDir(), и он же уезжает в сборку сервиса. os.Chdir
уходит целиком. Человек увидит разницу в двух местах: тесты можно гонять
параллельно, и упавший тест больше не оставляет процесс в чужом каталоге, ломая
следующие за ним.
Рассмотрено и отвергнуто: оставить os.Chdir, но восстанавливать каталог
надёжнее. Отвергнуто — надёжного способа нет: рабочий каталог у процесса один
на все горутины, и любой параллельный тест увидит чужой.
Запрос собирается из байтов, а не из файла на диске
Сегодня вспомогательная функция открывает файл по пути, поэтому каждому случаю нужен файл в текущем каталоге. Она начинает принимать имя и содержимое — диск из подготовки запроса уходит, а имя файла (то, чем проверяется выбор расширения) задаётся прямо, без создания одноимённого файла.
Внешних программ в этом тесте не остаётся ни одной
Конвертер в сборке теста тоже настоящий, хотя проверки его не зовут: приём конвертацию не делает. Он заменяется подставным вместе с источником метаданных. Смысл не в экономии: после этого «тест не зависит от внешних программ» проверяется взглядом на список импортов, а не рассуждением о том, какие ветки кода отработают.
Послабление линтера для тестов снимается
Исключение errcheck на _test.go в .golangci.yml заведено под непроверенные
os.Chdir. Их не остаётся — исключение снимается вместе с ними. Держать
послабление после того, как ушла его причина, значит оставить весь будущий тестовый
код без проверки возвращаемых ошибок и не помнить почему.
Risks / Trade-offs
- Настоящий
ffprobeтеперь не проверяется ничем. До этой правки его звал тест приёма — правда, звал так, что тот всегда отказывал, то есть проверял отказ и выдавал его за успех. Формально покрытие не теряется: проверялось и раньше ничего. Но дыра называется прямо — уinternal/adapter/metaviewer/ffmpegсвоего теста нет, и разбор выводаffprobeне проверен. → Смягчение: находка уходит в урожай отдельной задачей; здесь она не чинится, потому что требует записи в репозитории либо отдельного вида проверок, а это своё решение. - Снятое послабление линтера действует на весь будущий тестовый код. → Смягчение: это и есть цель. Если оно окажется тяжёлым, его вернут осознанно и с причиной в самом файле, а не по наследству.
- Норма про имя отправителя накрывает хранилище, но не журнал. Ревью дизайна нашло, что приём пишет имя файла пользователя в журнал — это нарушение инварианта «содержимое записи остаётся приватным», объявленного критическим и необратимым, и идёт оно по общему пути, то есть и для записей из Telegram. Решением человека на чекпоинте правка вынесена отдельной задачей: она про поведение сервиса, а не про проверки. → Смягчение: находка уходит в урожай. Спека при этом нормирует только хранилище и о журнале молчит намеренно — писать норму, которой код заведомо не следует, значит завести спеку, которая врёт с первого дня.
- Отказ чтения метаданных оставляет файл на диске. Соседняя ветка отказа (не удалось завести учётную запись файла) файл убирает, эта — нет, как и отказ записи на диск. Ни задачи, ни записи в учёте под такой файл нет, и сопоставить его не с чем. Решением человека на чекпоинте вынесено отдельной задачей: правка трогает три ветки отказа и меняет поведение на диске. → Смягчение: находка уходит в урожай; сценарий отказа в спеке нормирует только то, что проверяется здесь, — код ответа и незаведённую задачу.
- Спека
intakeописывает только приём по HTTP. Приём из Telegram остаётся в коде и без требований. → Смягчение: назван в самой спеке. Требование, написанное без проверки, было бы предположением, а не нормой.