From d16e537a55091d78353a9ac1caa9ba2c2b4451b0 Mon Sep 17 00:00:00 2001 From: sirjmann92 Date: Fri, 18 Sep 2026 08:22:34 -0500 Subject: [PATCH 1/3] ci: run tests and the frontend build on pull requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Run Linters workflow is the only required check, and it ran neither the test suite nor a build. tests/ existed but executed nowhere in CI, and the frontend was built solely inside the Docker image — so a broken frontend dependency surfaced only after merge, when the publish workflow ran, and a backend regression surfaced only when a job hit it. That left mypy as the sole guard over a weekly stream of automated dependency bumps. Pin drift announces itself loudly; this class of failure does not. Both additions are cheap: 35 tests in ~1s, the build in ~2s. The runtime dependencies they need are already installed, since requirements-lint.txt starts with `-r requirements.txt`; only pytest itself was missing. Because docker-publish.yml invokes this workflow via workflow_call before building, tests and build now gate releases as well as pull requests. Also drops both halves of the pre-commit.ci integration, neither of which has ever done anything: - the `ci:` block is read only by the hosted pre-commit.ci app, which is not installed on this repository - pre-commit-ci/lite-action cannot push, because the repository's default workflow token is read-only Commit e653f0d — a formatting fix applied by hand — is what the autofix was supposed to have handled. Config that looks functional but is not is worse than no config; autofix can be added deliberately later if wanted. --- .github/workflows/linters.yml | 13 +++++++++++-- .pre-commit-config.yaml | 9 --------- AGENTS.md | 18 +++++++++++++----- requirements-lint.txt | 1 + 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index a9365e2..ac8613a 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -66,5 +66,14 @@ jobs: - name: Run mypy run: mypy "./backend/" --install-types --non-interactive --config-file pyproject.toml - - uses: pre-commit-ci/lite-action@v1.1.0 - if: always() + # tests/test_workers.py is a manual script that takes a real media file + # argument, not a pytest suite -- it errors under plain collection. + - name: Run backend tests + run: pytest tests/ --ignore=tests/test_workers.py + + # The frontend is built into the image at docker build time, so a broken + # frontend dependency used to surface only after merge, when the publish + # workflow ran. Building here moves that to the PR. + - name: Build frontend + working-directory: frontend + run: npm run build diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5cf90be..d5285f8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -66,12 +66,3 @@ repos: language: system types_or: [ts, tsx, javascript, jsx, json, svelte] pass_filenames: false - -ci: - autofix_commit_msg: | - [pre-commit.ci] auto fixes from pre-commit hooks - autofix_prs: true - autoupdate_commit_msg: '[pre-commit.ci] pre-commit autoupdate' - autoupdate_schedule: weekly - skip: [svelte-check] - submodules: false diff --git a/AGENTS.md b/AGENTS.md index 5b694da..4cf39d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,12 +18,22 @@ docker compose up -d --build # Tests (local venv) source .venv/bin/activate -pytest tests/ +pytest tests/ --ignore=tests/test_workers.py # Lint — run what CI runs, NOT a --files subset pre-commit run --all-files ``` +### What CI actually runs + +The **Run Linters** workflow is the required check, and it is more than its +name suggests — `pre-commit run --all-files`, then `mypy ./backend/`, then the +test suite, then `npm run build`. A clean pre-commit alone does not mean CI is +green. + +`docker-publish.yml` invokes that same workflow via `workflow_call` before it +builds an image, so these four gate releases as well as pull requests. + ### Linting rules that matter - **Always `pre-commit run --all-files` before committing.** `--files ` @@ -31,11 +41,9 @@ pre-commit run --all-files `ruff format` failure through to a red `main`. - A formatting hook that **modifies** a file fails the run *by design*, even though the fix itself succeeded. Re-stage and re-run; it is not a real error. -- CI runs **`mypy ./backend/`** as a **separate step**, so a clean pre-commit - alone does not mean CI is green. - `tests/test_workers.py` is a manual script requiring a real media file - argument, not a pytest suite — it errors under plain collection. Run - `pytest tests/ --ignore=tests/test_workers.py`. + argument, not a pytest suite — it errors under plain collection. Always pass + `--ignore=tests/test_workers.py`, as CI does. ### Biome's version lives in exactly one place diff --git a/requirements-lint.txt b/requirements-lint.txt index e60bdbe..cb5b617 100644 --- a/requirements-lint.txt +++ b/requirements-lint.txt @@ -1,6 +1,7 @@ -r requirements.txt mypy pre-commit +pytest ruff types-PyYAML types-requests From 04ff7ee0184ef1f26fb0d8411b95616db0d9911d Mon Sep 17 00:00:00 2001 From: sirjmann92 Date: Fri, 18 Sep 2026 08:24:43 -0500 Subject: [PATCH 2/3] ci: declare pytest-timeout, which pyproject's addopts requires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first CI run of the new test step failed with pytest exit code 4 — a usage error, not a test failure. pyproject's addopts passes --timeout=20, which the pytest-timeout plugin provides, and that plugin was declared nowhere: not in requirements-lint.txt, not in the dev extras. It happened to be present in a local venv, so `pytest tests/` worked for whoever had installed it by hand and for nobody else. Adding tests to CI surfaced this on its first run, which is the argument for having them there. Declared in both places that claim to describe the test environment, and verified in a venv built only from requirements-lint.txt: 35 passed. --- pyproject.toml | 1 + requirements-lint.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 8ab8e04..54615af 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ dependencies = [ [project.optional-dependencies] dev = [ "pytest>=7.0", + "pytest-timeout", "mypy>=1.0", "pre-commit", "ruff>=0.8.0", diff --git a/requirements-lint.txt b/requirements-lint.txt index cb5b617..2e3e83c 100644 --- a/requirements-lint.txt +++ b/requirements-lint.txt @@ -2,6 +2,7 @@ mypy pre-commit pytest +pytest-timeout ruff types-PyYAML types-requests From c61aaea46605c762734275bac330a50c2f0a0095 Mon Sep 17 00:00:00 2001 From: sirjmann92 Date: Fri, 18 Sep 2026 08:26:33 -0500 Subject: [PATCH 3/3] ci: install FFmpeg so the integration tests can run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test step's second failure was two real assertion failures, not a usage error: tests/test_container_extension.py and the subtitle cleanup test invoke real ffprobe and ffmpeg against fixtures in tests/fixtures/ rather than mocking the subprocess. GitHub's runner has neither binary, so both failed with a bare ENOENT. Installing FFmpeg is the honest fix — this project is an FFmpeg wrapper, and those two are among the most valuable tests in the suite precisely because they exercise the real thing. Stubbing them out to get a green check would remove the coverage that justifies running tests in CI. Adds roughly 20-30s to the job. AGENTS.md now records the requirement, since a contributor without FFmpeg sees the same two failures locally. --- .github/workflows/linters.yml | 8 ++++++++ AGENTS.md | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index ac8613a..1272113 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -66,6 +66,14 @@ jobs: - name: Run mypy run: mypy "./backend/" --install-types --non-interactive --config-file pyproject.toml + # Some tests are genuine integration tests: they run real ffmpeg and + # ffprobe against tiny fixtures in tests/fixtures/ rather than mocking + # the subprocess. Without the binaries they fail with a bare ENOENT. + - name: Install FFmpeg + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends ffmpeg + # tests/test_workers.py is a manual script that takes a real media file # argument, not a pytest suite -- it errors under plain collection. - name: Run backend tests diff --git a/AGENTS.md b/AGENTS.md index 4cf39d7..29901c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,6 +44,11 @@ builds an image, so these four gate releases as well as pull requests. - `tests/test_workers.py` is a manual script requiring a real media file argument, not a pytest suite — it errors under plain collection. Always pass `--ignore=tests/test_workers.py`, as CI does. +- **The suite needs `ffmpeg` and `ffprobe` on `PATH`.** Some tests are real + integration tests that run the binaries against fixtures in + `tests/fixtures/` rather than mocking the subprocess; without them you get + two failures and a bare `No such file or directory: 'ffprobe'`. CI installs + FFmpeg for exactly this reason. ### Biome's version lives in exactly one place