av
6c04c801c9
- проверки больше не зовут ffprobe и не меняют рабочий каталог процесса; добавлены случаи на отказ чтения метаданных и на отсутствие поля audio - заведена спека intake на приём по HTTP, ADR о подставных адаптерах, запись в журнал ревью о проверке, которая не могла упасть - go test снят из объявленных долгов CLAUDE.md, послабление errcheck для _test.go в .golangci.yml убрано
124 lines
11 KiB
Markdown
124 lines
11 KiB
Markdown
## 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 остаётся
|
||
в коде и без требований. → Смягчение: назван в самой спеке. Требование,
|
||
написанное без проверки, было бы предположением, а не нормой.
|