Make lint and test phases of the Dockerfile (closes #96) #112

Merged
clawbot merged 1 commits from issue-96-dockerfile-phases into next2 2026-09-23 07:18:46 +02:00
Collaborator

Re-vendors the lint and test setup from the template, for #96.

  • The Dockerfile has a lint phase (eslint ., then prettier --check .) and a test phase (vitest, 90s timeout, verbose rerun on failure). Its last stage copies a file from each phase and runs make build as before, so the image cannot be built unless both pass.
  • script/lint, script/test and script/docker are byte-identical to the template. script/cibuild is the template's image build with --no-cache. CHECK_EPOCH, LINT_EPOCH and their guards are gone.
  • REPO_POLICIES.md is re-copied from prompts/REPO_POLICIES.md, and the README describes the new layout.

Deleted:

  • Dockerfile.lint, replaced by the lint phase.
  • test/packaging/lint-docker.test.ts: it checked Dockerfile.lint, the epoch guards and the old script/cibuild order, all of which are gone.
  • test/packaging/lint-once.test.ts: it counted prettier runs, and prettier now runs only in the lint phase. build-context.test.ts drops its Dockerfile.lint case.

Where quak differs from the template:

  • script/cibuild does not run script/bootstrap or script/check first: the issue asks for lint and the tests to run once each in CI, and the image build runs both.
  • script/check does not call script/fmt-check, because the lint phase already runs prettier.
  • script/precommit still runs lint only, so the red-phase test commit the README requires can land.
  • The test phase runs the suite as the image's non-root node user, not root. As root, the two tests of a destination directory that is not writable (downloadFile and downloadThumbnail) fail, because root ignores directory permissions.

Model: opus-5-5

Re-vendors the lint and test setup from the template, for https://git.eeqj.de/sneak/quak/issues/96. - The `Dockerfile` has a `lint` phase (`eslint .`, then `prettier --check .`) and a `test` phase (vitest, 90s timeout, verbose rerun on failure). Its last stage copies a file from each phase and runs `make build` as before, so the image cannot be built unless both pass. - `script/lint`, `script/test` and `script/docker` are byte-identical to the template. `script/cibuild` is the template's image build with `--no-cache`. `CHECK_EPOCH`, `LINT_EPOCH` and their guards are gone. - `REPO_POLICIES.md` is re-copied from `prompts/REPO_POLICIES.md`, and the README describes the new layout. Deleted: - `Dockerfile.lint`, replaced by the `lint` phase. - `test/packaging/lint-docker.test.ts`: it checked `Dockerfile.lint`, the epoch guards and the old `script/cibuild` order, all of which are gone. - `test/packaging/lint-once.test.ts`: it counted prettier runs, and prettier now runs only in the `lint` phase. `build-context.test.ts` drops its `Dockerfile.lint` case. Where quak differs from the template: - `script/cibuild` does not run `script/bootstrap` or `script/check` first: the issue asks for lint and the tests to run once each in CI, and the image build runs both. - `script/check` does not call `script/fmt-check`, because the `lint` phase already runs prettier. - `script/precommit` still runs lint only, so the red-phase test commit the README requires can land. - The `test` phase runs the suite as the image's non-root `node` user, not root. As root, the two tests of a destination directory that is not writable (`downloadFile` and `downloadThumbnail`) fail, because root ignores directory permissions. Model: opus-5-5
clawbot self-assigned this 2026-09-23 05:38:58 +02:00
clawbot added the needs-review label 2026-09-23 05:39:04 +02:00
Author
Collaborator

FAIL on a92e492 rebased onto next2 bf3b20d (TODO.md-only conflict, resolved locally).

  1. Two tests now run nowhere. test/download/download.test.ts:1014 skips "fails without creating anything when the destination directory is not writable" when the user is root. That covers both entry points, so two tests. Before this change, make check ran the suite on the host as a normal user, so they ran. Now the only test run is the test phase at Dockerfile:24-36, which runs as root, so the tests are skipped in make check and in CI alike. Acceptable: the test phase runs the suite as a non-root user, with that difference from the template named in the PR body. Or the test is changed so it runs as root. Either way, make check has to execute it.
  2. Stale comment. test/download/download.test.ts:226 still says the suite runs under "the 30-second cap in script/test". script/test no longer has a cap. The limit is now the 90-second timeout in the test phase of the Dockerfile. Acceptable: the comment names the current limit and where it is set.

Model: opus-5-5

FAIL on `a92e492` rebased onto `next2` `bf3b20d` (TODO.md-only conflict, resolved locally). 1. **Two tests now run nowhere.** `test/download/download.test.ts:1014` skips "fails without creating anything when the destination directory is not writable" when the user is root. That covers both entry points, so two tests. Before this change, `make check` ran the suite on the host as a normal user, so they ran. Now the only test run is the `test` phase at `Dockerfile:24-36`, which runs as root, so the tests are skipped in `make check` and in CI alike. Acceptable: the `test` phase runs the suite as a non-root user, with that difference from the template named in the PR body. Or the test is changed so it runs as root. Either way, `make check` has to execute it. 2. **Stale comment.** `test/download/download.test.ts:226` still says the suite runs under "the 30-second cap in `script/test`". `script/test` no longer has a cap. The limit is now the 90-second `timeout` in the `test` phase of the `Dockerfile`. Acceptable: the comment names the current limit and where it is set. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-23 05:54:16 +02:00
clawbot force-pushed issue-96-dockerfile-phases from a92e4929f8 to 1e69159700 2026-09-23 06:02:15 +02:00 Compare
Author
Collaborator

Rework:

  1. The test phase of the Dockerfile now runs the suite as the node user, so make check runs both not-writable-destination tests (no test is skipped). The PR body names this difference from the template.
  2. The comment at test/download/download.test.ts:226 now names the 90-second timeout in the test phase of the Dockerfile.
  3. Rebased onto next2 (390401a); TODO.md resolved by hand, all entries kept, newest first.

Model: opus-5-5

Rework: 1. The `test` phase of the `Dockerfile` now runs the suite as the `node` user, so `make check` runs both not-writable-destination tests (no test is skipped). The PR body names this difference from the template. 2. The comment at `test/download/download.test.ts:226` now names the 90-second `timeout` in the `test` phase of the `Dockerfile`. 3. Rebased onto `next2` (`390401a`); `TODO.md` resolved by hand, all entries kept, newest first. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-23 06:02:21 +02:00
Author
Collaborator

FAIL on 1e69159 rebased onto next2 c19943a (TODO.md-only conflict, resolved locally).

  1. Stale comment. test/crypto/kdf.test.ts:32 still says the cheap parameters keep the suite "under the 30-second budget". This change removed the 30-second cap from script/test. The limit is now the 90-second timeout in the test phase of the Dockerfile. Acceptable: the comment names the current limit and where it is set, as test/download/download.test.ts:226 now does.
  2. PR body too long. It is about 310 words, over the roughly 250-word limit. The closing "Verification" paragraph describes how the change was checked and is not needed by a reader. Acceptable: drop that paragraph and keep the lists of what was deleted and where quak differs from the template.

Non-blocking: nothing guards the non-root test phase. If USER node is dropped, the two not-writable-destination tests at test/download/download.test.ts:1041 are skipped again and make check stays green. Removing the skipIf would make them fail as root instead of skipping.

Model: opus-5-5

FAIL on `1e69159` rebased onto `next2` `c19943a` (TODO.md-only conflict, resolved locally). 1. **Stale comment.** `test/crypto/kdf.test.ts:32` still says the cheap parameters keep the suite "under the 30-second budget". This change removed the 30-second cap from `script/test`. The limit is now the 90-second `timeout` in the `test` phase of the `Dockerfile`. Acceptable: the comment names the current limit and where it is set, as `test/download/download.test.ts:226` now does. 2. **PR body too long.** It is about 310 words, over the roughly 250-word limit. The closing "Verification" paragraph describes how the change was checked and is not needed by a reader. Acceptable: drop that paragraph and keep the lists of what was deleted and where quak differs from the template. Non-blocking: nothing guards the non-root `test` phase. If `USER node` is dropped, the two not-writable-destination tests at `test/download/download.test.ts:1041` are skipped again and `make check` stays green. Removing the `skipIf` would make them fail as root instead of skipping. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-23 06:26:19 +02:00
clawbot force-pushed issue-96-dockerfile-phases from 1e69159700 to 80f691413a 2026-09-23 06:50:30 +02:00 Compare
Author
Collaborator

Rework on 80f6914, rebased onto next2 cd05a45 (TODO.md resolved by hand, all entries kept, newest first); make check green.

  1. test/crypto/kdf.test.ts now names the 90-second timeout in the test phase of the Dockerfile.
  2. PR body: Verification paragraph dropped.
  3. The skipIf on the not-writable-destination test is gone, so as root it fails instead of skipping; the Dockerfile comment and the PR body say "fail" instead of "skipped".

Model: opus-5-5

Rework on `80f6914`, rebased onto `next2` `cd05a45` (`TODO.md` resolved by hand, all entries kept, newest first); `make check` green. 1. `test/crypto/kdf.test.ts` now names the 90-second `timeout` in the `test` phase of the `Dockerfile`. 2. PR body: Verification paragraph dropped. 3. The `skipIf` on the not-writable-destination test is gone, so as root it fails instead of skipping; the `Dockerfile` comment and the PR body say "fail" instead of "skipped". Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-23 06:50:41 +02:00
Author
Collaborator

PASS on 80f6914 rebased onto next2 cda57ee (TODO.md-only conflict, resolved locally).

Model: opus-5-5

PASS on `80f6914` rebased onto `next2` `cda57ee` (TODO.md-only conflict, resolved locally). Model: opus-5-5
clawbot added 1 commit 2026-09-23 07:15:49 +02:00
Follows the template: Dockerfile.lint is gone; the Dockerfile has a lint
phase (eslint, prettier --check .) and a test phase (vitest, run as the
node user, which the not-writable-directory tests need), and its
last stage compiles and depends on both. script/lint and script/test
build one phase each with --no-cache; script/docker and script/cibuild
pass --no-cache, so CHECK_EPOCH and LINT_EPOCH are removed.
script/cibuild is the single image build, so CI runs lint and the tests
once each. The tests that checked the old layout are deleted,
REPO_POLICIES.md is re-copied and the README describes the new layout.

Model: opus-5-5
clawbot force-pushed issue-96-dockerfile-phases from 80f691413a to c1eb4b8e46 2026-09-23 07:15:49 +02:00 Compare
clawbot merged commit cb61582ae6 into next2 2026-09-23 07:18:46 +02:00
clawbot deleted branch issue-96-dockerfile-phases 2026-09-23 07:18:46 +02:00
Sign in to join this conversation.