feat: check formatting, linting and tests in CI
Every push runs ruff, prettier, pytest and svelte-check; a tagged release runs the blocking half again before it publishes an image. eslint and svelte-check report without failing, since 87 and 30 findings predate the workflow.
This commit is contained in:
+39
-11
@@ -144,20 +144,48 @@ made there:
|
||||
Dockerfiles' `--mount=type=cache` blocks do nothing across ephemeral runners either way, and a
|
||||
cold build of both images is a few minutes.
|
||||
|
||||
## Gating: what the release job should and should not run
|
||||
## Gating: formatting, linting, types and tests
|
||||
|
||||
Do **not** put `pytest`, `pnpm check` or `pnpm lint` in front of the push. Per `TODO.md` and
|
||||
`frontend/AGENTS.md` those are all currently red: `ruff check src/` reports 114 errors,
|
||||
`pnpm check` has a documented baseline of 30 errors, `pnpm lint` 131. Wiring them into the release
|
||||
path means the first tag fails for reasons that have nothing to do with the release, and the
|
||||
predictable response is to disable the gate. Cleaning those up is worthwhile and is its own task —
|
||||
`TODO.md` already tracks it as "Nothing gates formatting, linting or types".
|
||||
`ci.yml` runs these on every push and pull request, and `release.yml`'s `quality` job runs the
|
||||
blocking half again on the tagged commit, so a release cannot publish an image whose tests fail.
|
||||
|
||||
Also skip `pytest` here specifically: it needs Docker for `pytest-databases`, which means
|
||||
docker-in-docker-in-docker on the runner, for a suite that tests code paths the image build does not
|
||||
affect.
|
||||
My first draft of this document argued for keeping all of it out of the release path, on the
|
||||
grounds that everything was red. Measuring rather than trusting `TODO.md` showed that was mostly
|
||||
wrong:
|
||||
|
||||
What *is* worth gating on is that the images actually start, which is the failure mode a release
|
||||
| Check | Before | After the sweep | Gate |
|
||||
| --- | --- | --- | --- |
|
||||
| `pytest tests/` | **411 passed** in 3 min | unchanged | blocking |
|
||||
| `ruff format --check src/` | 27 of 66 files | clean | blocking |
|
||||
| `ruff check src/` | 110 errors | clean | blocking |
|
||||
| `prettier --check .` | 48 files | clean | blocking |
|
||||
| `eslint .` | 1804 → **87 real** | 87 | non-blocking |
|
||||
| `pnpm check` | 30 errors | 30 | non-blocking |
|
||||
|
||||
The test suite was green the whole time, so gating on it costs nothing. `ruff check`'s 110 were
|
||||
104 unused imports, and **all 64 that survived `--fix` were in `__init__.py`** — deliberate
|
||||
re-exports, so the fix is `per-file-ignores` in `pyproject.toml`, not deleting them. Of eslint's
|
||||
1804, **1717 were the vendored pdf.js under `static/`**, which the config never ignored the way it
|
||||
ignores `src/lib/vendor/`; adding it leaves 87 real ones.
|
||||
|
||||
Two things the sweep turned up that are worth remembering:
|
||||
|
||||
- **ruff's suggested fix for `E712` would have broken the query.** `services/filters/book.py` had
|
||||
`m.BookProgress.completed == True`, and ruff proposes `if m.BookProgress.completed:` — Python
|
||||
truthiness on a SQLAlchemy Column, which does not generate SQL at all. The correct idiom is
|
||||
`.is_(True)`, which the adjacent line already used. Never run `ruff check --fix --unsafe-fixes`
|
||||
over query-building code without reading every hunk.
|
||||
- **Prettier reformatted the generated `schema.d.ts`**, which was 3109 of the sweep's 3946 changed
|
||||
lines. `openapi-typescript` writes its own style, so that file would have churned by thousands of
|
||||
lines on every regeneration — and then failed the very gate being added. It is in
|
||||
`.prettierignore` now.
|
||||
|
||||
`eslint` and `pnpm check` run with `continue-on-error: true`. That is an honest weak gate: it
|
||||
reports without failing, so the counts stay visible and cannot grow silently unnoticed, but nobody
|
||||
is blocked by 117 pre-existing problems they did not create. Drop the line from each step as its
|
||||
count reaches zero.
|
||||
|
||||
The other thing worth gating on is that the images actually start, which is the failure mode a release
|
||||
introduces and which nothing else catches. That is the `smoke` job in the same workflow: it copies
|
||||
`.env.prod-example`, pins `CHITAI_VERSION` to the tag, `docker compose pull`s the images that were
|
||||
just pushed and brings the stack up with `--wait`, then curls both healthchecks.
|
||||
|
||||
Reference in New Issue
Block a user