From cb61582ae66fc587035d3b54a738e0dbbaff8459 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 23 Sep 2026 07:18:46 +0200 Subject: [PATCH] Make lint and test phases of the Dockerfile (closes #96) 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 --- .dockerignore | 2 +- Dockerfile | 75 +++- Dockerfile.lint | 35 -- README.md | 106 +++--- REPO_POLICIES.md | 345 ++++++++++++++---- TODO.md | 7 + script/check | 18 +- script/cibuild | 30 +- script/docker | 16 +- script/lint | 27 +- script/precommit | 14 +- script/test | 26 +- test/crypto/kdf.test.ts | 8 +- test/download/download.test.ts | 49 ++- test/packaging/build-context.test.ts | 33 +- test/packaging/entrypoints.test.ts | 6 +- test/packaging/lint-docker.test.ts | 184 ---------- test/packaging/lint-once.test.ts | 503 --------------------------- 18 files changed, 480 insertions(+), 1004 deletions(-) delete mode 100644 Dockerfile.lint delete mode 100644 test/packaging/lint-docker.test.ts delete mode 100644 test/packaging/lint-once.test.ts diff --git a/.dockerignore b/.dockerignore index 4beaf77..e2a5e34 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,6 +1,6 @@ # Mirrors .gitignore, with one deliberate exception: .gitignore itself stays # in the build context, because prettier 3 reads it as a default ignore file -# and dropping it would change what `make fmt-check` sees inside the image. +# and dropping it would change what the lint phase's prettier check sees. # VCS .git diff --git a/Dockerfile b/Dockerfile index a194ffd..144308a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,28 +1,69 @@ -# Test and build image: the suite, then the compile. +# Lint phase. The linters are invoked directly rather than through `make +# lint` or `script/lint`, which are themselves a docker build and would +# recurse into a daemon that does not exist in a build step. # -# Linting deliberately does not happen here. `script/lint` is a build of -# Dockerfile.lint, and `script/check` calls `script/lint`, so running -# `make check` in this image would mean running `docker build` inside a -# container. Lint runs exactly once, in Dockerfile.lint; script/cibuild -# builds that first and this second. # node 22.22.0 on Alpine 3.23.3 (node:22-alpine), 2026-08-09 -FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS check +FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS lint + WORKDIR /app COPY script/ script/ COPY package.json yarn.lock ./ RUN script/bootstrap + COPY . . -# CHECK_EPOCH is a cache buster: without it Docker serves the test layer from -# cache on an unchanged tree, the suite never executes, and the build still -# exits 0. The guard makes an absent argument a hard failure — an unset ARG -# is the empty string, which is a perfectly stable cache key, so a plain -# `docker build .` would otherwise still get the false green. Fail closed. -ARG CHECK_EPOCH -RUN [ -n "$CHECK_EPOCH" ] || exit 1 -RUN make test +RUN yarn run eslint . +RUN yarn run prettier --check . + +# Test phase, same shape and for the same reason. The suite runs without +# verbose output first and is rerun verbosely only if it fails; the timeout +# catches a hung test. +# +# node 22.22.0 on Alpine 3.23.3 (node:22-alpine), 2026-08-09 +FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS test + +WORKDIR /app + +COPY script/ script/ +COPY package.json yarn.lock ./ +RUN script/bootstrap + +COPY . . + +# Unlike the template, the suite runs as the image's non-root `node` user: +# root ignores directory permissions, so the tests of a destination that is +# not writable would otherwise fail. vitest writes into /app. +RUN chown -R node:node /app +USER node + +RUN timeout 90 yarn run vitest run --reporter=dot || \ + { echo "--- Rerunning with verbose for details ---"; \ + timeout 90 yarn run vitest run --reporter=verbose; exit 1; } + +# Build stage, and the last stage: a plain `docker build .` names no target +# and so builds this one. Nothing is wanted from the two phases above; the +# copies are what make BuildKit build them first, so this image cannot be +# produced unless lint and test passed. A stage appended after this one +# would drop all three out of a plain build. +# +# node 22.22.0 on Alpine 3.23.3 (node:22-alpine), 2026-08-09 +FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 + +WORKDIR /app + +COPY --from=lint /app/package.json /dev/null +COPY --from=test /app/package.json /dev/null + +COPY script/ script/ +COPY package.json yarn.lock ./ +RUN script/bootstrap + +COPY . . + +# The version is computed on the host and passed in, because +# .dockerignore excludes .git. +ARG VERSION=dev +LABEL org.opencontainers.image.version="${VERSION}" -ARG CHECK_EPOCH -RUN [ -n "$CHECK_EPOCH" ] || exit 1 RUN make build diff --git a/Dockerfile.lint b/Dockerfile.lint deleted file mode 100644 index 0ee2a9a..0000000 --- a/Dockerfile.lint +++ /dev/null @@ -1,35 +0,0 @@ -# Lint image: every lint run happens here, and nowhere else. The repo is -# COPYed into a digest-pinned image and the linters run as build steps, so a -# successful build IS a clean lint. `script/lint` does nothing but build this -# file, which also works where the docker daemon is remote and bind mounts are -# impossible. Nothing that runs inside a container may call `script/lint`: -# that is why Dockerfile no longer runs `make check`. -# node 22.22.0 on Alpine 3.23.3 (node:22-alpine), 2026-08-09 -FROM node@sha256:e4bf2a82ad0a4037d28035ae71529873c069b13eb0455466ae0bc13363826e34 AS lint -WORKDIR /app - -# Manifests before sources, so the dependency install layer stays cached -# until package.json or yarn.lock changes. script/bootstrap ends in -# `yarn install --frozen-lockfile`; the lint steps below are deliberately -# not cached. -COPY script/ script/ -COPY package.json yarn.lock ./ -RUN script/bootstrap - -COPY . . - -# LINT_EPOCH is a cache buster, with the same fail-closed contract as -# CHECK_EPOCH in Dockerfile. No lint cache is wanted: on an unchanged tree -# Docker serves the linter layers in well under a second, having linted -# nothing, and the build still exits 0. The guard makes an absent argument a -# hard failure — an unset ARG is the empty string, which is a perfectly -# stable cache key, so a plain `docker build -f Dockerfile.lint .` would -# otherwise get exactly that false green. Every layer below this one is a -# child of the guard, so a fresh epoch forces all of them to execute. -ARG LINT_EPOCH -RUN [ -n "$LINT_EPOCH" ] || exit 1 - -# The linters are invoked directly rather than through `make lint`, because -# `make lint` is the build of this file. -RUN yarn run eslint . -RUN yarn run prettier --check . diff --git a/README.md b/README.md index aa2727c..e2b5237 100644 --- a/README.md +++ b/README.md @@ -97,20 +97,18 @@ alpine. We provide: - `script/build` — compile the TypeScript sources into `dist/`, then verify that the entrypoints `package.json` declares (`main`, `types`, `bin`) are among the files the compiler wrote, and make the CLI executable (our own extension) -- `script/test` — run the test suite (vitest, hard-capped at 30s where `timeout` - is available, verbose rerun on failure) -- `script/lint` — run eslint and a prettier check, by building - `Dockerfile.lint`; requires docker (see Linting below) +- `script/test` — run the test suite, by building the `test` phase of the + `Dockerfile` (vitest, 90s timeout, verbose rerun on failure); requires docker +- `script/lint` — run eslint and a prettier check, by building the `lint` phase + of the `Dockerfile`; requires docker (see Linting and testing below) - `script/fmt` — format all files with prettier (writes) - `script/fmt-check` — check formatting on the host (read-only); standalone, and not called by `script/check` or `script/precommit`, because `script/lint` - already checks formatting in the container (see Linting below) + already checks formatting in the container - `script/check` — run all checks: `test`, `lint` (our own extension) -- `script/docker` — build the test and build image, tagged via - `script/projectname` -- `script/cibuild` — cd to the repo root and build both images (what CI runs): - `script/lint` first, then the `Dockerfile` image, which runs `make test` and - `make build` +- `script/docker` — build the image, tagged via `script/projectname` +- `script/cibuild` — build the image (what CI runs); its last stage depends on + the `lint` and `test` phases, so this one build lints, tests and compiles - `script/precommit` — run by the git pre-commit hook (our own extension); runs `script/lint`, which checks both lint and formatting, but deliberately not the tests, so the TDD red-phase commit can land @@ -119,50 +117,32 @@ alpine. We provide: `make hooks` installs the pre-commit hook that runs `script/precommit`. -### Linting +### Linting and testing -Linting runs in a container, one way, everywhere. `script/lint` builds -`Dockerfile.lint`, which copies the repo into a digest-pinned node image and -runs eslint and prettier as build steps, so a successful build is a clean lint. -There is no host lint path: docker is required to lint, and that also works -where the docker daemon is remote and bind mounts are impossible. +Linting and testing are phases of the `Dockerfile`. The `lint` phase copies the +repo into a digest-pinned node image and runs eslint and `prettier --check .`; +the `test` phase does the same with the suite. `script/lint` and `script/test` +each build one phase with `docker build --no-cache --target `. There is +no host lint or test path: docker is required, and that also works where the +docker daemon is remote and bind mounts are impossible. -The formatting check is part of that, not a step beside it. `script/check` and -`script/precommit` therefore call `script/lint` and stop; neither calls -`script/fmt-check` as well, which would run prettier a second time over the same -tree for the same verdict — and the weaker of the two, since the host's prettier -is whatever the working tree has installed. So `make check` and the pre-commit -hook both still fail on a badly formatted tree, and prettier runs exactly once -in each. `test/packaging/lint-once.test.ts` asserts that count by walking the -invocation graph, so a second pass cannot creep back in unnoticed. +The last stage of the `Dockerfile` compiles the package, and it copies a file +from each phase, so it cannot be built unless lint and the tests pass. That is +why `script/cibuild` is a single `docker build`: it runs lint and the tests once +each and then compiles. +Every `docker build` in `script/` passes `--no-cache`. On an unchanged tree +Docker would otherwise serve the lint and test steps from cache, nothing would +run, and the build would still exit 0. + +The formatting check is part of the `lint` phase, not a step beside it, so +`script/check` and `script/precommit` do not call `script/fmt-check` as well; +that would run prettier a second time over the same tree for the same verdict. `script/fmt-check` remains as a standalone entrypoint for asking the formatting -question on its own, without docker and without the rest of lint. Its verdict -cannot drift from the container's: prettier is pinned to an exact version, -installed from `yarn.lock` under `--frozen-lockfile` in both places, and reads -`.gitignore` as its default ignore file — which is why `.dockerignore` -deliberately keeps `.gitignore` in the build context. - -Lint happens in exactly one place, which constrains the rest of the build. -`script/check` calls `script/lint`, so `make check` cannot run inside a -container without asking for docker inside docker. The image built from -`Dockerfile` therefore runs `make test` and `make build` and does not lint; -`script/cibuild` builds `Dockerfile.lint` first and that image second, so CI -gets both verdicts. - -### Build epochs - -`script/lint` passes `--build-arg LINT_EPOCH="$(date +%s)"`, and `script/docker` -and `script/cibuild` pass `--build-arg CHECK_EPOCH="$(date +%s)"`. Both -Dockerfiles refuse to build without their argument. This is deliberate: on an -unchanged tree Docker would otherwise serve the linter and test layers from -cache, so nothing would run and the build would still exit 0 — a lint build over -an untouched tree returns success in well under a second, having linted nothing. -A changing epoch invalidates every layer below the guard on every invocation -while leaving the dependency layers above them cached, and the missing-argument -guard means a bare `docker build .` fails loudly instead of quietly reporting a -green it did not earn: an unset build argument is the empty string, which is a -perfectly stable cache key. +question on the host. Its verdict matches the container's: prettier is pinned to +an exact version, installed from `yarn.lock` under `--frozen-lockfile` in both +places, and reads `.gitignore` as its default ignore file — which is why +`.dockerignore` keeps `.gitignore` in the build context. ## Rationale @@ -197,10 +177,9 @@ All work on quak is test-driven. No exceptions. 3. Subsequent commits add the implementation and any refactors needed to make the tests pass. 4. A feature branch can only be merged into `main` when `make check` is green. - `main` is always green. CI runs `script/cibuild`, which lints via - `Dockerfile.lint` and then runs `make test` and `make build` in the - `Dockerfile` image, so neither a red branch nor one that does not compile can - pass CI. + `main` is always green. CI runs `script/cibuild`, which builds the + `Dockerfile`: its `lint` and `test` phases, then the compile, so neither a + red branch nor one that does not compile can pass CI. 5. Tests are the canonical API documentation for this library. Every test file is commented thoroughly enough that a reader who has never seen quak can learn how to use it from the tests alone. Comments explain why a behavior @@ -217,7 +196,7 @@ All work on quak is test-driven. No exceptions. runs `script/lint` — eslint and the prettier check, in the container — but not the tests, and so not the full `make check`. This is deliberate so the TDD red-phase commit (failing tests, no implementation yet) can land. The - suite runs as part of the image build, which is what CI executes via + `test` phase is part of the image build, which is what CI executes via `script/cibuild`, so a red branch still cannot reach `main`. ## Design @@ -245,8 +224,7 @@ quak/ quak.ts CLI entrypoint (commander.js) test/ unit + integration tests (vitest) Makefile - Dockerfile test suite and compile - Dockerfile.lint eslint and prettier, as build steps + Dockerfile lint phase, test phase, compile package.json tsconfig.json ``` @@ -773,13 +751,13 @@ documents: commented thoroughly. `main` is always green. - **Required checks before every commit:** `make lint` must pass — that is - eslint plus the prettier check, and it builds `Dockerfile.lint`, so it needs - docker. The pre-commit hook enforces exactly that. `make check` (which also - runs the tests) must pass before merging to `main`. `make fmt-check` is - available for a host-side formatting check on its own, but it is not a - separate requirement: `make lint` already covers it, and running both would - check formatting twice. Never invoke eslint or prettier directly; linting runs - in the container only. + eslint plus the prettier check, and it builds the `lint` phase of the + `Dockerfile`, so it needs docker. The pre-commit hook enforces exactly that. + `make check` (which also runs the tests) must pass before merging to `main`. + `make fmt-check` is available for a host-side formatting check on its own, but + it is not a separate requirement: `make lint` already covers it, and running + both would check formatting twice. Never invoke eslint or prettier directly; + linting runs in the container only. - **Formatting:** prettier with 4-space indents and `proseWrap: always` for markdown. Use `make fmt` to format. Use `yarn` not `npm`. diff --git a/REPO_POLICIES.md b/REPO_POLICIES.md index bc2f161..2256291 100644 --- a/REPO_POLICIES.md +++ b/REPO_POLICIES.md @@ -1,6 +1,6 @@ --- title: Repository Policies -last_modified: 2026-07-06 +last_modified: 2026-09-08 --- This document covers repository structure, tooling, and workflow standards. Code @@ -60,17 +60,28 @@ style conventions are in separate documents: prerequisite since nvm requires bash. yarn is then pinned via `corepack prepare yarn@ --activate`. Never install "latest" or "lts"; always exact versions. `script/cibuild` runs the CI build: it changes to the - repo root and runs `docker build .`; the Gitea workflow calls it. Four further - scripts are our own extensions to the standard: `script/check` runs - `script/test`, `script/lint`, and `script/fmt-check`; `script/precommit` is - what the git pre-commit hook runs, and it calls `script/check`; - `script/install-precommit` installs the git pre-commit hook (the `make hooks` - target shims to it); and `script/projectname` (literally that filename) simply - outputs the project's name. Scripts that need the name call - `script/projectname` — e.g. `script/docker` assembles its image tag from it — - so those scripts stay byte-identical across all repos. Repo-type-specific - pre-commit extras (e.g. `go mod tidy` verification in Go repos) belong in - `script/precommit`, not in the hook itself. Model scripts are at + repo root, runs `script/bootstrap`, runs `script/check`, and builds the image + with the version; the Gitea workflow calls it. **`script/cibuild` runs + `script/bootstrap` first**, because the workflow checks out the repo and runs + nothing else, while `script/fmt-check` runs the formatter on the host: on a + pristine checkout with nothing installed the run dies there, after the + containerised gates have passed. **The bootstrap alone is not enough**: + `script/bootstrap` installs node and yarn under nvm and leaves neither on the + `PATH` of the shell that called it, so a bare `yarn` still exits 127. The host + entrypoints that need yarn — `script/fmt` and `script/fmt-check` — therefore + source nvm for the pinned node version before invoking it, exactly as + `script/bootstrap`'s own install step does. A runner carrying nothing but + docker and git then gets through `script/check`. Four further scripts are our + own extensions to the standard: `script/check` runs `script/test`, + `script/lint` and `script/fmt-check`; `script/precommit` is what the git + pre-commit hook runs, and it calls `script/check`; `script/install-precommit` + installs the git pre-commit hook (the `make hooks` target shims to it); and + `script/projectname` (literally that filename) simply outputs the project's + name. Scripts that need the name call `script/projectname` — e.g. + `script/docker` assembles its image tag from it — so those scripts stay + byte-identical across all repos. Repo-type-specific pre-commit extras (e.g. + `go mod tidy` verification in Go repos) belong in `script/precommit`, not in + the hook itself. Model scripts are at `https://git.eeqj.de/sneak/prompts/raw/branch/main/script/`. The README must document the provided scripts in an **Entrypoints** section (see the README requirements below). @@ -89,87 +100,140 @@ style conventions are in separate documents: contributor should be able to understand the entire development workflow by reading the Makefile. -- Every repo should have a `Dockerfile`. All Dockerfiles must run `make check` - as a build step so the build fails if the branch is not green. For non-server - repos, the Dockerfile should bring up a development environment and run - `make check`. For server repos, `make check` should run as an early build - stage before the final image is assembled. Dockerfiles install development - prerequisites by running `script/bootstrap` rather than duplicating installs - inline; COPY `script/` and the dependency manifests (`package.json` + - `yarn.lock`, `go.mod` + `go.sum`, etc.) before running it so the bootstrap - layer stays cached until dependencies change. +- Every repo should have a `Dockerfile`, and it carries the repo's gates: a + `lint` phase and a `test` phase, with the final stage depending on both so the + image cannot be built unless they pass. For non-server repos the final stage + brings up a development environment; for server repos it is the runtime image. + Dockerfiles install development prerequisites by running `script/bootstrap` + rather than duplicating installs inline; COPY `script/` and the dependency + manifests (`package.json` + `yarn.lock`, `go.mod` + `go.sum`, etc.) before + running it. -- **Dockerfiles must use a separate lint stage for fail-fast feedback.** Go - repos use a multistage build where linting runs in an independent stage based - on the `golangci/golangci-lint` image (pinned by hash). This stage runs - `make fmt-check` and `make lint` before the full build begins. The build stage - then declares an explicit dependency on the lint stage via - `COPY --from=lint /src/go.sum /dev/null`, which forces BuildKit to complete - linting before proceeding to compilation and tests. This ensures lint failures - surface in seconds rather than minutes, without blocking on dependency - download or compilation in the build stage. +- **Linting and testing run in Docker, as phases of the `Dockerfile`.** There is + no separate lint file. `script/lint` and `script/test` each build one phase + and nothing else: - The standard pattern for a Go repo Dockerfile is: + ```sh + docker build --no-cache --target lint -t "$(script/projectname)-lint" . + docker build --no-cache --target test -t "$(script/projectname)-test" . + ``` + + **A stage that is not the last one in the file is built only when the final + stage's chain depends on it, or when `--target` names it.** That is why the + two gates are always invoked by name here, and why the final stage carries a + `COPY --from=` of a harmless file from each of them: without that edge a + plain `docker build .` builds the last stage alone and exits 0 having linted + and tested nothing. + + **Every `docker build` in `script/` is tagged**, here and in + `script/cibuild` and `script/docker`. An untagged build leaves a dangling + image behind on every invocation, on every developer host and every CI + runner; a tagged one replaces the previous image. + + Inside a phase the tool is invoked directly — `golangci-lint`, `go test`, + `eslint`, `prettier` — never through `make lint` or `script/test`, which are + themselves a `docker build` and would recurse into a daemon that does not + exist in a build step. Formatting is the exception and stays on the host: + `script/fmt` writes the working tree, and `script/fmt-check` is its + read-only twin. + + **No lint verdict may come from a host invocation of the linter.** On a + shared host golangci-lint reads a result cache keyed on file content rather + than location, so a second checkout of the same content is served the first + one's findings, and a host-global lock in `$TMPDIR` makes concurrent runs + exit non-zero with `parallel golangci-lint is running` — a status a caller + cannot tell from real findings. Both have produced wrong verdicts in this + org, in both directions. A container has its own cache, its own `TMPDIR` and + a digest-pinned binary, so neither is reachable. + +- **Any build that runs checks is built with `--no-cache`.** Docker invalidates + a `COPY` layer only when the copied content changes, so on an unchanged tree + the check `RUN` is served from cache, nothing executes, and the build still + exits 0. Every `docker build` in `script/` therefore passes `--no-cache`: + `script/lint`, `script/test`, `script/cibuild` and `script/docker` are the + four, and there is no fifth — `script/check` runs the two gate phases and + `script/fmt-check`, and builds no image of its own. A bare `docker build .` is + not evidence that anything ran: a sub-second build reporting success is a + cache hit, not a result. Never invalidate by pruning — `docker builder prune` + and friends destroy a build cache shared with every other build on the host. + +- **The gate phases are separate stages, and the build stage depends on both.** + The lint phase is based on the `golangci/golangci-lint` image (pinned by + hash), so lint failures surface in seconds rather than after a full compile, + and the test phase is based on the Go image. The canonical Go repo + `Dockerfile`: ```dockerfile - # Lint stage — fast feedback on formatting and lint issues + # Lint phase # golangci/golangci-lint:v2.x.x, YYYY-MM-DD FROM golangci/golangci-lint@sha256:... AS lint WORKDIR /src COPY go.mod go.sum ./ RUN go mod download COPY . . - RUN make fmt-check - RUN make lint + RUN golangci-lint run --config .golangci.yml ./... - # Build stage + # Test phase # golang:1.x-alpine, YYYY-MM-DD + FROM golang@sha256:... AS test + WORKDIR /src + COPY go.mod go.sum ./ + RUN go mod download + COPY . . + RUN go test -timeout 90s -race -cover ./... || \ + { echo "--- Rerunning with -v for details ---"; \ + go test -timeout 90s -race -v ./...; exit 1; } + + # Build stage. Nothing is wanted from either phase above; the copies + # are what make BuildKit build them first, so this stage cannot run + # unless lint and test passed. + # golang:1.x-alpine, YYYY-MM-DD FROM golang@sha256:... AS builder + COPY --from=lint /src/go.sum /dev/null + COPY --from=test /src/go.sum /dev/null WORKDIR /src - - # Force BuildKit to run the lint stage before proceeding - COPY --from=lint /src/go.sum /dev/null - COPY go.mod go.sum ./ RUN go mod download COPY . . - RUN make test ARG VERSION=dev RUN CGO_ENABLED=0 go build -trimpath \ -ldflags="-s -w -X main.Version=${VERSION}" \ -o /app ./cmd/app/ - # Runtime stage + # Runtime stage, and the last one FROM alpine@sha256:... COPY --from=builder /app /usr/local/bin/app ENTRYPOINT ["app"] ``` Key points: - - The lint stage uses the `golangci/golangci-lint` image directly (it - includes both Go and the linter), so there is no need to install the - linter separately. - - `COPY --from=lint /src/go.sum /dev/null` is a no-op file copy that creates - a stage dependency. BuildKit runs stages in parallel by default; without - this line, the build stage would not wait for lint to finish and a lint - failure might not fail the overall build. + - The lint phase uses the `golangci/golangci-lint` image directly (it has + both Go and the linter), so nothing needs installing. + - `COPY --from= /src/go.sum /dev/null` is a no-op copy whose only + purpose is the ordering edge. BuildKit runs stages in parallel by default, + and a stage nothing depends on is not built at all, so without these two + lines a red gate would not fail the build. + - Keep the runtime stage last, and if you add a stage after it, give it the + same two copies. A plain `docker build .` builds the last stage's chain + and nothing else. - If the project uses `//go:embed` directives that reference build artifacts - (e.g. a web frontend compiled in a separate stage), the lint stage must + (e.g. a web frontend compiled in a separate stage), the lint phase must create placeholder files so the embed directives resolve. Example: `RUN mkdir -p web/dist && touch web/dist/index.html web/dist/style.css`. - The lint stage should not depend on the actual build output — it exists to - fail fast. - If the project requires CGO or system libraries for linting (e.g. - `vips-dev`), install them in the lint stage with `apk add`. - - The build stage runs `make test` after compilation setup. Tests run in the - build stage, not the lint stage, because they may require compiled - artifacts or heavier dependencies. + `vips-dev`), install them in the lint phase with `apk add`. + - `ARG VERSION=dev` is declared in the stage that compiles and supplied by + `script/docker` and `script/cibuild`; no stage may call `git describe`. - Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that - runs `script/cibuild` (which runs `docker build .`) on push. Since the - Dockerfile already runs `make check`, a successful build implies all checks - pass. + runs `script/cibuild` on push, and checks out the repo as its only other step. + That script bootstraps, runs the gate phases, and then builds the image, so a + successful run means every check passed; a bare `docker build .` does not + carry the same guarantee, because its gate phases may come from the cache. The + image build is uncached and so runs the gate phases a second time. That is the + price of the rule above, and it is worth paying: the image that ships is built + from a run of its own gates rather than from a cache entry. - Use platform-standard formatters: `black` for Python, `prettier` for JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with @@ -189,14 +253,21 @@ style conventions are in separate documents: module under test to verify it compiles/parses. There is no excuse for `make test` to be a no-op. -- `make test` must complete in under 20 seconds. Add a 30-second timeout in the - Makefile. +- `make test` must complete in under 60 seconds. That is the hard cap, and a + suite that exceeds it fails. Under 20 seconds is the target. A suite between + 20 and 60 seconds is still green, but the overage must be filed as an + improvement bug against that repo. Add a 90-second timeout to the test + invocation (`go test -timeout 90s`). The backstop deliberately sits above the + hard cap so that it catches a genuinely hung test rather than a merely slow + one. -- **`make test` should use the conditional verbose rerun pattern.** Run tests - without `-v` (verbose) first. If tests fail, automatically rerun with `-v` to - show full output. This keeps CI logs and `docker build` output clean on - success (just package/suite summaries) while providing full diagnostic detail - on failure (every test case, every assertion). The general shell pattern: +- **The test command should use the conditional verbose rerun pattern.** Run + tests without `-v` (verbose) first. If tests fail, automatically rerun with + `-v` to show full output. This keeps CI logs and `docker build` output clean + on success (just package/suite summaries) while providing full diagnostic + detail on failure (every test case, every assertion). The command lives in the + `test` phase of the `Dockerfile`, since `script/test` builds that phase; the + Makefile form below is the same pattern for any repo-local invocation: ```makefile test: @@ -209,11 +280,24 @@ style conventions are in separate documents: ```makefile test: - @go test -timeout 30s -race -cover ./... || \ + @go test -count=1 -timeout 90s -race -cover ./... || \ { echo "--- Rerunning with -v for details ---"; \ - go test -timeout 30s -race -v ./...; exit 1; } + go test -count=1 -timeout 90s -race -v ./...; exit 1; } ``` + `-count=1` is required on both invocations: it defeats Go's test _result_ + cache, so the target cannot report a pass it did not earn, and the rerun + reproduces a failure instead of replaying it. It leaves the build cache + alone, so it costs the runtime of the suite and no recompilation. + + Note that this is a second, independent cache, stacked below the Docker + layer cache that [issue #26](https://git.eeqj.de/sneak/prompts/issues/26) + addresses. `CHECK_EPOCH` guarantees the `RUN make test` _step_ re-executes; + it does not guarantee `go test` inside that step does any work, because the + `GOCACHE` baked into earlier image layers survives into the re-executed + step. They are two separate defects requiring two separate fixes, and a fix + for one must not be recorded as covering the other. + Python example: ```makefile @@ -239,10 +323,83 @@ style conventions are in separate documents: must be in `.gitignore`. No exceptions. - `.gitignore` should be comprehensive from the start: OS files (`.DS_Store`), - editor files (`.swp`, `*~`), language build artifacts, and `node_modules/`. - Fetch the standard `.gitignore` from - `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when setting up - a new repo. + editor files (`.swp`, `*~`), in-repo agent scratch directories (`.claude/`), + language build artifacts, and `node_modules/`. Fetch the standard `.gitignore` + from `https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitignore` when + setting up a new repo. These patterns are written to `.gitignore`'s own + semantics, in which an unanchored pattern already matches at every depth; they + are not a `.dockerignore` and must not be transplanted into one unmodified. + +- **`.dockerignore` does not use `.gitignore` semantics, and copying patterns + across unmodified leaves secrets in the build context.** Docker matches with + `moby/patternmatcher`: `filepath.Match` semantics plus a `**` extension, so + `*` does not cross `/` and a pattern without a leading `**/` is anchored at + the build-context root. A `.dockerignore` listing `.env`, `*.pem` and `*.key` + therefore excludes only the copies at the repository root, while `config/.env` + and `certs/server.key` still reach the context and can land in an image layer + — which is more dangerous than a short file with no secret patterns at all, + because it reads as solved and stops anyone looking. Give every + depth-independent pattern the `**/` prefix and leave only genuinely + root-anchored entries unprefixed: `.git`, and the repo's own host-built + binary, written `/myapp` and never `**/myapp`, which would also match + `cmd/myapp/` and delete the package directory from the context. Matching is + case-sensitive, and an ALL-CAPS twin per pattern still misses `Server.Key`, so + secret names use character ranges — `**/*.[kK][eE][yY]`, `**/*.[pP][eE][mM]`, + and likewise for `.envrc` and the extensionless SSH keys. Where such a pattern + also catches something the build needs, re-include it with a negation + (`!docs/example.env`); deleting the pattern reopens the exposure for every + other file it covers. Fetch the standard `.dockerignore` from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.dockerignore` and extend + it with the repo's own artifacts. + +- **In-repo agent scratch belongs in both files, written to each file's own + semantics.** `.claude/` holds one worktree per in-flight agent — an entire + additional checkout of the repo — so under `COPY . .` the build context + inflates by a multiple of the repo and another session's unreviewed work can + be copied into an image layer. In `.gitignore` the entry is `.claude/`, + unanchored. In `.dockerignore` it is `.claude`, anchored and with **no** `**/` + prefix, because the prefixed form would also delete any nested directory of + that name from the build. Anchoring carries a known gap that the canonical + `.dockerignore` states in its own comment, since consuming repos receive the + file and not the tracker: the directory is created in the agent's working + directory, so a repo running agents in subdirectories still ships + `services/api/.claude/` and must add its own anchored entry there. + +- **Excluding `.git` means `git describe` cannot run inside any build stage, and + it fails quietly there.** In a build stage there is no repository, so + `git describe` writes nothing to stdout, `-X main.Version=` comes out empty, + the binary reports no version at all, and the build still exits 0. Compute the + version on the host and thread it in as a build arg. `script/docker` and + `script/cibuild` do this, byte-identically across repos: + + ```sh + # Own line: a failing command substitution inside an argument does not + # trip `set -e`, so the inline form degrades to an empty constant. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ + -t "$(script/projectname)" . + ``` + + `--always` makes an untagged repo yield an abbreviated commit hash rather + than failing, and the `[ -n "$version" ]` line is the single place the + fallback is applied — a live check that fires on a build from an export with + no `.git` and on a repository with no commits yet. Do not fold it into the + substitution as `|| echo unknown`, which makes the guard unreachable. The + Dockerfile's side is `ARG VERSION=dev` in the stage that compiles, declared + there because `ARG` is stage-scoped; passing `VERSION` to a repo whose + Dockerfile declares no such `ARG` is ignored and costs nothing, which is why + the scripts stay byte-identical. One consequence for CI: the standard + checkout action clones shallow and fetches no tags, so a repo that embeds a + tag-derived version must set `fetch-depth: 0` on its checkout step. + +- **Verify `.dockerignore` by enumerating the image, not by reading the + patterns.** Plant files at the root _and_ at least two directories deep, build + a probe image that does `COPY . .`, and list what actually landed + (`docker run --rm --entrypoint find IMAGE /app`). The `transferring context` + size is not a substitute: a nested secret is a few bytes, and BuildKit + transfers only the delta from the previous build. - **No build artifacts in version control.** Code-derived data (compiled bundles, minified output, generated assets) must never be committed to the @@ -258,9 +415,45 @@ style conventions are in separate documents: - Make all changes on a feature branch. You can do whatever you want on a feature branch. -- `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only - manually by the user. Fetch from - `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. +- `.golangci.yml` is standardized. The vendored copy in a consuming repo must + _NEVER_ be modified by an agent: fetch it from + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml` and keep it + byte-identical, so that no repo can quietly loosen its own linting. Linter + configuration changes are made to the canonical copy in the `prompts` repo and + reach consuming repos by re-vendoring; an agent may open a PR against + canonical, which only the user merges. One list is exempt from byte-identity, + because it cannot be written once for every repo: the `deny` list of the + `test-support` depguard rule, where a repo names its own test-support packages + by full import path. A repo adds entries there and changes nothing else, and a + re-vendor carries its entries forward. The canonical golangci-lint version is + v2.12.2 (released 2026-05-06), pinned as the digest of the lint phase's base + image + (`golangci/golangci-lint@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240`, + which reports `2.12.2 built with go1.26.2 from c0d3ddc9`). That digest is the + only pin, since no repo installs golangci-lint on the host: bumping the + version means changing it and nothing else. + +- **`script/bootstrap` installs a pinned tool by comparing versions, never by + testing presence.** An `if ! command -v ; then install; fi` guard tests + `PATH` only, so on an already-provisioned machine the pin is inert and a + version bump is a silent no-op — while the Dockerfile, installing into a clean + image, gets the pinned version, so a local `make check` and `make docker` can + disagree about what the tool even is. The canonical form: + - compares the installed version against the pin over the **whole** version + token; a parser that stops at the first `-` reports `2.12.2` for a host + running `2.12.2-rc1` and skips the install; + - treats absent, non-zero, empty or unrecognised `--version` output as a + mismatch, so the failure direction is a redundant install and never a + skipped one; + - after installing, re-resolves the binary the way callers do — `hash -r`, + then through `PATH`, not through the directory the installer wrote to — + and fails naming the resolved path, since an install that a shadowing + binary hides succeeds while changing nothing any caller sees; + - is actually called, and prints the version on both success paths: a + function defined and never invoked has the same exit status and the same + empty output as one that worked. + + Keep it POSIX sh: no arrays, no `[[`, no `grep -P`. - When pinning images or packages by hash, add a comment above the reference with the version and date (YYYY-MM-DD). @@ -379,7 +572,9 @@ style conventions are in separate documents: language-specific config). Everything else goes in a subdirectory. Canonical subdirectory names: - `bin/` — executable scripts and tools - - `cmd/` — Go command entrypoints + - `cmd/` — Go command entrypoints; thin only: one `main.go` per binary whose + body is a single call into `internal/` or `pkg/`, no project logic in + `cmd/` - `configs/` — configuration templates and examples - `deploy/` — deployment manifests (k8s, compose, terraform) - `docs/` — documentation and markdown (README.md stays in root) diff --git a/TODO.md b/TODO.md index 61c505f..69c9eb8 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,13 @@ Tag v1.0.0. # Completed Steps +- 2026-09-23: Re-vendored the lint and test setup from the template (issue 96). + Linting and testing are the `lint` and `test` phases of the `Dockerfile`; + `script/lint` and `script/test` each build one with `--no-cache`, and the last + stage compiles and depends on both, so `script/cibuild` is one build. + `Dockerfile.lint`, `CHECK_EPOCH`, `LINT_EPOCH` and the tests that checked them + are gone; `REPO_POLICIES.md` is re-copied. + - 2026-09-23: Stopped `helper fix-missing-thumbnails` retrying files the server always refuses (issue 109). Both thumbnail helpers skip a file another account owns without fetching it. The fixer skips a file whose recorded thumbnail size diff --git a/script/check b/script/check index 2f37914..1607369 100755 --- a/script/check +++ b/script/check @@ -1,19 +1,11 @@ #!/bin/sh # script/check: run all checks (test, lint). Our own extension to -# scripts-to-rule-them-all. Must not modify any files. +# scripts-to-rule-them-all. Both are Docker phases. Must not modify any +# files. # -# The formatting check is part of lint, not a step of its own: -# script/lint builds Dockerfile.lint, which runs eslint AND -# `prettier --check .` as build steps. Calling script/fmt-check here as -# well would run prettier a second time over the same tree for the same -# verdict — the weaker of the two, since the host toolchain is whatever -# the working tree happens to have installed while the container's is -# digest-pinned. script/fmt-check remains a standalone entrypoint for -# asking the formatting question by itself. -# -# script/lint builds Dockerfile.lint, so this script requires docker and -# must never be run from inside a container: that is why the Dockerfile -# image runs script/test and script/build rather than this. +# script/fmt-check is not called here, unlike the template: the lint +# phase already runs `prettier --check .`, so calling it would run +# prettier a second time over the same tree for the same verdict. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" diff --git a/script/cibuild b/script/cibuild index dd1f7b5..38f5706 100755 --- a/script/cibuild +++ b/script/cibuild @@ -1,15 +1,11 @@ #!/bin/sh -# script/cibuild: run the CI build, which is both images in a defined order. -# -# First script/lint, which builds Dockerfile.lint and is the one and only -# place linting happens — it goes first so a lint failure is reported before -# the slower suite runs. Then the Dockerfile image, which runs script/test -# and script/build. CHECK_EPOCH and LINT_EPOCH differ on every invocation, so -# neither the linters nor the suite can be served from Docker's cache: a -# green build here means the checks ran now, not that a previous run was -# remembered. The layers below the epochs (bootstrap, yarn install) are -# unaffected and stay cached. A build that omits the arguments fails by -# design. +# script/cibuild: run the CI build. The image's last stage depends on the +# lint and test phases, so this one build runs eslint, prettier and the +# suite once each and then compiles. Unlike the template it does not run +# script/check first, which would run lint and the tests a second time. +# --no-cache for the same reason as script/docker: the gate phases the +# final stage depends on are RUN steps, and a cached one is a check that +# did not run. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" @@ -17,8 +13,16 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - "$SCRIPT_DIR/lint" - docker build --build-arg CHECK_EPOCH="$(date +%s)" . + # Own line: a failing command substitution inside an argument does + # not trip `set -e`, so the inline form degrades silently to an + # empty constant. VERSION is computed here because .dockerignore + # excludes .git, so `git describe` in a build stage yields an empty + # version without failing. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ + -t "$("$SCRIPT_DIR/projectname")" . } main "$@" diff --git a/script/docker b/script/docker index c691f2c..c4688e8 100755 --- a/script/docker +++ b/script/docker @@ -1,10 +1,8 @@ #!/bin/sh # script/docker: build the Docker image tagged with the project name. # Identical in all repos; the tag comes from script/projectname. -# CHECK_EPOCH is passed for the same reason script/cibuild passes it: the -# Dockerfile refuses to build without it, so that no path to an image can -# quietly serve the test and build layers from cache. This builds the test -# and build image only; linting is a separate image, built by script/lint. +# --no-cache because the gate phases the final stage depends on are RUN +# steps, and a cached one is a check that did not run. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" @@ -12,7 +10,15 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - docker build --build-arg CHECK_EPOCH="$(date +%s)" \ + # Own line: a failing command substitution inside an argument does + # not trip `set -e`, so the inline form degrades silently to an + # empty constant. VERSION is computed here because .dockerignore + # excludes .git, so `git describe` in a build stage yields an empty + # version without failing. + version="$(git describe --tags --always --dirty 2>/dev/null || true)" + [ -n "$version" ] || version="unknown" + docker build --no-cache \ + --build-arg VERSION="$version" \ -t "$("$SCRIPT_DIR/projectname")" . } diff --git a/script/lint b/script/lint index 4a602f2..2d8b075 100755 --- a/script/lint +++ b/script/lint @@ -1,24 +1,23 @@ #!/bin/sh -# script/lint: run the linters. eslint and prettier are never run against -# the working tree from here: linting runs via docker only, one way, -# everywhere — script/lint builds Dockerfile.lint, which COPYs the repo into -# the pinned node image and runs the linters as build steps. That works even -# when the docker daemon is remote and bind mounts are impossible. +# script/lint: run the linter. Linting is a phase of the Dockerfile and +# this builds that phase alone; the linter is never installed or run on +# a developer host, where a shared result cache and a host-global lock +# make its answer untrustworthy. # -# LINT_EPOCH is passed on every invocation because no lint cache is wanted: -# on an unchanged tree Docker would otherwise serve the linter layers, having -# linted nothing, and still exit 0. Dockerfile.lint refuses to build without -# the argument, so no path to a lint result can quietly come from cache. -# -# Nothing that runs inside a container may call this script; see the header -# of Dockerfile. +# The phase is not the last stage in the file, so it is built only when +# --target names it. --no-cache because a cached lint layer is a lint +# that did not run. The tag makes each build replace the previous image +# instead of leaving a dangling one behind. set -eu -ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - docker build --build-arg LINT_EPOCH="$(date +%s)" -f Dockerfile.lint . + docker build --no-cache \ + --target lint \ + -t "$("$SCRIPT_DIR/projectname")-lint" . } main "$@" diff --git a/script/precommit b/script/precommit index 489e72d..e87c5c8 100755 --- a/script/precommit +++ b/script/precommit @@ -4,17 +4,9 @@ # # Runs lint but deliberately NOT the tests, so the TDD red-phase commit # (failing tests, no implementation yet) can land. CI runs -# script/cibuild, which builds both images and so catches any branch -# that ships red. -# -# The formatting check is still enforced here, because script/lint is a -# build of Dockerfile.lint and that runs `prettier --check .` as a build -# step: a badly formatted tree fails this hook, and therefore the -# commit. Calling script/fmt-check as well would only run prettier a -# second time over the same tree for the same verdict. -# -# script/lint is a docker build (Dockerfile.lint); docker is required to -# commit, which is the point of linting one way, everywhere. +# script/cibuild, whose image build includes the test phase, and so +# catches any branch that ships red. The lint phase includes the +# prettier check, so a badly formatted tree still fails the commit. set -eu SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" diff --git a/script/test b/script/test index d4cec4b..cd239f2 100755 --- a/script/test +++ b/script/test @@ -1,25 +1,19 @@ #!/bin/sh -# script/test: run the test suite. Uses `timeout` (GNU coreutils) when -# available so the run is hard-capped at 30s; on macOS without -# coreutils the cap is skipped. +# script/test: run the test suite. Testing is a phase of the Dockerfile +# and this builds that phase alone, on the same terms as script/lint: +# --target because a phase that is not the last stage is built only when +# named, --no-cache because a cached test layer is a test that did not +# run, and a tag so each build replaces the previous image. set -eu -ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" - -rerun_verbose() { - echo "--- Rerunning with verbose for details ---" - yarn run vitest run --reporter=verbose - exit 1 -} +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" +ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)" main() { cd "$ROOT" - TIMEOUT="$(command -v timeout 2>/dev/null || command -v gtimeout 2>/dev/null || true)" - if [ -n "$TIMEOUT" ]; then - "$TIMEOUT" 30s yarn run vitest run --reporter=dot || rerun_verbose - else - yarn run vitest run --reporter=dot || rerun_verbose - fi + docker build --no-cache \ + --target test \ + -t "$("$SCRIPT_DIR/projectname")-test" . } main "$@" diff --git a/test/crypto/kdf.test.ts b/test/crypto/kdf.test.ts index 670d80e..d252653 100644 --- a/test/crypto/kdf.test.ts +++ b/test/crypto/kdf.test.ts @@ -29,10 +29,10 @@ describe("crypto.deriveKEK (Argon2id)", () => { }); /** - * Cheap parameters used so the test suite stays under the 30-second - * budget. The real production parameters Ente uses are larger - * (memLimit up to 1 GiB, opsLimit 3-16). The algorithm is the same - * regardless of parameters. + * Cheap parameters used so the test suite stays under the 90-second + * `timeout` in the `test` phase of the `Dockerfile`. The real production + * parameters Ente uses are larger (memLimit up to 1 GiB, opsLimit 3-16). + * The algorithm is the same regardless of parameters. */ const TEST_OPS = 2; const TEST_MEM = 64 * 1024 * 1024; // 64 MiB diff --git a/test/download/download.test.ts b/test/download/download.test.ts index 4e7e03e..81110d5 100644 --- a/test/download/download.test.ts +++ b/test/download/download.test.ts @@ -248,7 +248,8 @@ afterAll(() => { * `sodium.randombytes_buf` goes through the wasm wrapper a byte at a time and * costs roughly 20 seconds for the 4 MiB chunk below — about two hundred * times what it costs to encrypt the same buffer, and on its own enough to - * push `make test` past the 30-second cap in `script/test`. This loop fills + * push `make test` past the 90-second `timeout` in the `test` phase of the + * `Dockerfile`. This loop fills * 4 MiB in a few milliseconds. */ const patternBytes = (length: number, seed: number): Uint8Array => { @@ -1034,32 +1035,28 @@ describe.each(entryPoints)( expect(readdirSync(dir)).toEqual([]); }); - // Root ignores directory permissions, so this cannot fail as root - // (the Docker test image runs as root). - it.skipIf(process.getuid?.() === 0)( - "fails without creating anything when the destination directory is not writable", - async () => { - const key = - sodium.crypto_secretstream_xchacha20poly1305_keygen(); - const { header, ciphertext } = encryptFileBody( - patternBytes(64, 34), - key, - ); - const { api, file } = fixtureFor(key, header, ciphertext); - const dir = freshDir(); - const outPath = join(dir, "never.bin"); - chmodSync(dir, 0o500); - try { - await expect( - download(api, file, outPath), - ).rejects.toMatchObject({ code: "EACCES" }); - } finally { - chmodSync(dir, 0o700); - } + // Root ignores directory permissions, so this fails when run as root. + // The `test` phase of the `Dockerfile` runs as the `node` user. + it("fails without creating anything when the destination directory is not writable", async () => { + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const { header, ciphertext } = encryptFileBody( + patternBytes(64, 34), + key, + ); + const { api, file } = fixtureFor(key, header, ciphertext); + const dir = freshDir(); + const outPath = join(dir, "never.bin"); + chmodSync(dir, 0o500); + try { + await expect( + download(api, file, outPath), + ).rejects.toMatchObject({ code: "EACCES" }); + } finally { + chmodSync(dir, 0o700); + } - expect(readdirSync(dir)).toEqual([]); - }, - ); + expect(readdirSync(dir)).toEqual([]); + }); }, ); diff --git a/test/packaging/build-context.test.ts b/test/packaging/build-context.test.ts index 056fb57..d6c24e4 100644 --- a/test/packaging/build-context.test.ts +++ b/test/packaging/build-context.test.ts @@ -2,13 +2,13 @@ // failures are silent. // // Excluding too little: a worktree left under `.claude/` is copied into the -// image, vitest globs its `test/` tree as well as the real one, and the -// containerised `make check` runs the whole suite twice over while reporting -// success. A compiled `bin/quak` is ~100 MB of context nobody needs. +// image, vitest globs its `test/` tree as well as the real one, and the test +// phase runs the whole suite twice over while reporting success. A compiled +// `bin/quak` is ~100 MB of context nobody needs. // // Excluding too much: Prettier 3 reads `.gitignore` as a default ignore file, -// so dropping it from the context silently changes which files -// `make fmt-check` looks at inside the image compared to the host. +// so dropping it from the context silently changes which files the lint +// phase's prettier check looks at compared to `make fmt-check` on the host. // // Neither shows up as a build failure, so they are asserted here. import { describe, expect, it } from "vitest"; @@ -47,18 +47,13 @@ describe(".dockerignore", () => { expect(dockerignore).not.toContain(".gitignore"); }); - // Both images are built from this same context, and the lint image runs - // eslint and prettier across it. BuildKit lets a `.dockerignore` - // shadow the root one for a single build; such a file would silently give - // the lint build a different, unreviewed context — and eslint's flat config - // does not ignore dot-directories, so a stray `.claude/` worktree would be - // linted. - it.each(["Dockerfile", "Dockerfile.lint"])( - "is not shadowed by a per-Dockerfile ignore file for %s", - (name) => { - expect(existsSync(join(repoRoot, `${name}.dockerignore`))).toBe( - false, - ); - }, - ); + // BuildKit lets a `Dockerfile.dockerignore` shadow the root one; such a + // file would silently give the build a different, unreviewed context — + // and eslint's flat config does not ignore dot-directories, so a stray + // `.claude/` worktree would be linted. + it("is not shadowed by a Dockerfile.dockerignore", () => { + expect(existsSync(join(repoRoot, "Dockerfile.dockerignore"))).toBe( + false, + ); + }); }); diff --git a/test/packaging/entrypoints.test.ts b/test/packaging/entrypoints.test.ts index 58149c5..a9b5426 100644 --- a/test/packaging/entrypoints.test.ts +++ b/test/packaging/entrypoints.test.ts @@ -1,9 +1,7 @@ // The package manifest promises three files that only exist after a build: // `main`, `types`, and the `quak` binary. Nothing in the test suite used to -// look at them, and `make check` runs the suite and the lint container but -// never the build, so `tsconfig.json` and `package.json` were free to drift -// apart. (The formatting check is part of the lint container, not a step of -// its own; `test/packaging/lint-once.test.ts` is what holds that shape.) They +// look at them, and `make check` runs the test and lint phases but never the +// build, so `tsconfig.json` and `package.json` were free to drift apart. They // did: `rootDir` was `./src` while `include` also pulled in `bin/**/*`, which // is TS6059, and no build had succeeded for as long as that was true. // diff --git a/test/packaging/lint-docker.test.ts b/test/packaging/lint-docker.test.ts deleted file mode 100644 index fa561f2..0000000 --- a/test/packaging/lint-docker.test.ts +++ /dev/null @@ -1,184 +0,0 @@ -// Linting runs in Docker, one way, everywhere: `script/lint` builds -// `Dockerfile.lint`, which COPYs the repo into a digest-pinned image and runs -// eslint and prettier as build steps, so a successful build IS a clean lint. -// -// Three things can quietly undo that, and none of them shows up as a build -// failure, which is why they are asserted here: -// -// 1. Recursion. `script/check` calls `script/lint`, and `script/lint` is now a -// `docker build`. Anything that runs `make check` inside a container is -// therefore asking for Docker inside Docker, and CI breaks. The image built -// from `Dockerfile` runs the suite and the compile only; lint happens once, -// in `Dockerfile.lint`. -// 2. Cache. A lint build over an unchanged tree returns success in well under a -// second having linted nothing. The `LINT_EPOCH` guard is what forces the -// linter layers to execute, and it has to fail closed: an unset build -// argument is the empty string, which is a perfectly stable cache key, so an -// invocation that omits it must be rejected rather than served a cached -// green. -// 3. A host lint path surviving alongside the container one, which would let a -// lint result come from an unpinned local toolchain. -import { describe, expect, it } from "vitest"; -import { readFileSync } from "node:fs"; -import { fileURLToPath } from "node:url"; -import { join } from "node:path"; - -const repoRoot = fileURLToPath(new URL("../../", import.meta.url)); - -const read = (name: string): string => - readFileSync(join(repoRoot, name), "utf-8"); - -// The executable lines of a shell script or Dockerfile: comments carry the -// reasoning and frequently name the very commands these tests forbid, so they -// would otherwise trigger every assertion below. -const instructions = (name: string): string[] => - read(name) - .split("\n") - .map((line) => line.trim()) - .filter((line) => line !== "" && !line.startsWith("#")); - -const lintScript = instructions("script/lint"); -const dockerfileLint = instructions("Dockerfile.lint"); -const dockerfile = instructions("Dockerfile"); -const cibuild = instructions("script/cibuild"); - -const has = (lines: string[], pattern: RegExp): boolean => - lines.some((line) => pattern.test(line)); - -describe("script/lint", () => { - it("lints by building Dockerfile.lint", () => { - expect(has(lintScript, /docker build .*-f Dockerfile\.lint/)).toBe( - true, - ); - }); - - // The whole point of the ruling: no invocation of a linter against the - // working tree survives, so a lint verdict can only come from the pinned - // image. - it("runs no linter on the host", () => { - expect(has(lintScript, /eslint|prettier/)).toBe(false); - }); - - // Without a fresh epoch the build is served from cache in under a second, - // having linted nothing, and still exits 0. - it("passes a fresh LINT_EPOCH on every run", () => { - expect( - has(lintScript, /--build-arg LINT_EPOCH="\$\(date \+%s\)"/), - ).toBe(true); - }); -}); - -describe("Dockerfile.lint", () => { - // Tag references are server-mutable, so they are remote code execution. - it("pins its base image by digest", () => { - expect(has(dockerfileLint, /^FROM \S+@sha256:[0-9a-f]{64}/)).toBe(true); - }); - - it("runs eslint as a build step", () => { - expect(has(dockerfileLint, /^RUN .*eslint \./)).toBe(true); - }); - - it("runs prettier as a build step", () => { - expect(has(dockerfileLint, /^RUN .*prettier --check \./)).toBe(true); - }); - - // An unset ARG is the empty string, and an empty string is a perfectly - // stable cache key. Rejecting it is what stops a bare - // `docker build -f Dockerfile.lint .` from reporting a green it did not - // earn. - it("refuses to build without LINT_EPOCH", () => { - expect(has(dockerfileLint, /^ARG LINT_EPOCH$/)).toBe(true); - expect( - has(dockerfileLint, /^RUN \[ -n "\$LINT_EPOCH" \] \|\| exit 1$/), - ).toBe(true); - }); - - // The guard only forces execution of the layers below it, so both linters - // have to sit after it. Layer order is the mechanism, not a style choice. - it("puts both linters below the epoch guard", () => { - const guard = dockerfileLint.findIndex((line) => - /^RUN \[ -n "\$LINT_EPOCH" \]/.test(line), - ); - const linters = dockerfileLint - .map((line, index) => ({ line, index })) - .filter(({ line }) => /^RUN .*(eslint|prettier)/.test(line)); - - expect(linters.length).toBeGreaterThan(0); - for (const { line, index } of linters) { - expect( - index, - `${line} must run below the LINT_EPOCH guard`, - ).toBeGreaterThan(guard); - } - }); - - // Dependency installation is the slow layer and has nothing to do with the - // sources, so it caches separately: manifests first, sources afterwards. - it("copies the manifests before the sources", () => { - const manifests = dockerfileLint.findIndex((line) => - /^COPY package\.json yarn\.lock/.test(line), - ); - const sources = dockerfileLint.findIndex((line) => - /^COPY \. \.$/.test(line), - ); - - expect(manifests).toBeGreaterThanOrEqual(0); - expect(sources).toBeGreaterThan(manifests); - }); - - // script/lint is a docker build; a lint step that shelled out to it would - // recurse. - it("does not call script/lint or make lint", () => { - expect(has(dockerfileLint, /make lint|script\/lint/)).toBe(false); - }); -}); - -describe("Dockerfile", () => { - // `make check` runs script/lint, which is a docker build, so an image that - // ran it would need a Docker daemon inside the container. - it("does not run make check, make lint or script/lint", () => { - expect( - has(dockerfile, /make check|make lint|script\/(check|lint)/), - ).toBe(false); - }); - - // The replaced lint stage took a `COPY --from=lint` dependency to order - // itself before the check stage. Dockerfile.lint is that stage now, and - // two definitions of how to lint is one too many. - it("has no lint stage", () => { - expect(has(dockerfile, /AS lint\b|--from=lint\b/)).toBe(false); - }); - - it("still runs the suite and the build under the epoch guard", () => { - expect(has(dockerfile, /^RUN make test$/)).toBe(true); - expect(has(dockerfile, /^RUN make build$/)).toBe(true); - expect( - has(dockerfile, /^RUN \[ -n "\$CHECK_EPOCH" \] \|\| exit 1$/), - ).toBe(true); - }); -}); - -describe("script/cibuild", () => { - // CI has to get both verdicts. Lint goes first so the fast failure is - // reported before the suite runs. - it("builds the lint image before the test and build image", () => { - const lint = cibuild.findIndex((line) => /\/lint"/.test(line)); - const check = cibuild.findIndex((line) => - /docker build .*CHECK_EPOCH/.test(line), - ); - - expect(lint).toBeGreaterThanOrEqual(0); - expect(check).toBeGreaterThan(lint); - }); -}); - -describe("package.json", () => { - // `yarn lint` was a second, unpinned way to get a lint verdict, from - // whatever eslint the working tree happened to have installed. - it("exposes no host lint script", () => { - const pkg = JSON.parse(read("package.json")) as { - scripts: Record; - }; - expect(pkg.scripts.lint).toBeUndefined(); - }); -}); diff --git a/test/packaging/lint-once.test.ts b/test/packaging/lint-once.test.ts deleted file mode 100644 index 16fbad1..0000000 --- a/test/packaging/lint-once.test.ts +++ /dev/null @@ -1,503 +0,0 @@ -// `make check` used to run `prettier --check .` twice: once inside the lint -// container (`script/lint` builds `Dockerfile.lint`, which runs eslint and -// prettier as build steps) and once again on the host, because `script/check` -// also called `script/fmt-check`. Two passes, one verdict, and the host one is -// the weaker of the two — its prettier is whatever the working tree happens to -// have installed, while the container's is digest-pinned and installed under -// `--frozen-lockfile`. -// -// The fix was to delete the host call from `script/check` and `script/precommit`. -// Nothing about that fix is self-enforcing: anyone can wire `script/fmt-check` -// back in, or add a prettier step to a Dockerfile, and every build stays green -// while quietly doing the work twice again. So the count is asserted here -// rather than promised in a comment. -// -// The assertion is a static walk of the invocation graph, not a string match -// against one file. Starting from an entrypoint, it follows every edge the repo -// actually uses to reach another command — `run:` steps in the CI workflow, -// `"$SCRIPT_DIR/"` and `script/` into other scripts, `make ` -// through the Makefile shims, `yarn run ` through the `package.json` -// scripts, and `docker build -f ` into that Dockerfile's `RUN` steps — and -// counts the prettier invocations it finds. A prettier call added anywhere in -// that graph is therefore caught, wherever it is added. -// -// Two entrypoints are walked, because they cover different graphs: `make check` -// is what a developer runs, and `.gitea/workflows/check.yml` is what CI runs. -// The CI walk starts at the workflow file rather than at a hand-picked script, -// so "the path CI executes" is read out of the repo instead of assumed; it -// reaches `script/cibuild`, and through it the `Dockerfile` image that `make -// check` never touches. Walking only `make check` is how a duplicate prettier -// pass in `Dockerfile` stayed invisible. -// -// Undercounting is the failure mode that would make this test worthless. Three -// things guard against it: the walk is asserted to have reached the nodes that -// matter, an unresolvable or empty node is a thrown error rather than a quiet -// zero, and prettier is counted per occurrence rather than per line, so two -// invocations chained with `&&` cannot read as one. -import { describe, expect, it } from "vitest"; -import { readFileSync } from "node:fs"; -import { fileURLToPath } from "node:url"; -import { join } from "node:path"; - -const repoRoot = fileURLToPath(new URL("../../", import.meta.url)); - -const read = (name: string): string => - readFileSync(join(repoRoot, name), "utf-8"); - -// A backslash at end of line continues the command; the resolver has to see the -// whole invocation, since the interesting flags (`-f Dockerfile.lint`) can sit -// on the continuation. -const joinContinuations = (text: string): string[] => { - const joined: string[] = []; - for (const raw of text.split("\n")) { - const line = raw.trim(); - const previous = joined[joined.length - 1]; - if (previous !== undefined && previous.endsWith("\\")) { - joined[joined.length - 1] = - `${previous.slice(0, -1).trim()} ${line}`; - } else { - joined.push(line); - } - } - return joined; -}; - -// Comments are stripped everywhere. The headers of these scripts explain the -// duplication this test exists to prevent, and therefore name `prettier` and -// `script/fmt-check` repeatedly; counting them would make the test assert the -// prose instead of the behaviour. -const executable = (text: string): string[] => - joinContinuations(text).filter( - (line) => line !== "" && !line.startsWith("#"), - ); - -// Every occurrence, not "does this line mention prettier": a line that reads -// `yarn run prettier --check . && yarn run prettier --check src` is two passes -// over the same tree, which is exactly the bug this file exists to catch, and -// counting it as one would hide it. `.prettierrc` and `.prettierignore` are not -// invocations and do not match, because `\b` requires a non-word character -// after the name. -const countPrettier = (line: string): number => - (line.match(/\bprettier\b/g) ?? []).length; - -// Makefile targets are thin shims (`check:` / tab / `@script/check`), so a -// `make ` edge has to resolve through them to keep "per `make check`" -// meaning what it says. Recipe lines are the tab-indented ones. -const makeRecipes = (): Map => { - const recipes = new Map(); - let current: string | null = null; - for (const raw of read("Makefile").split("\n")) { - if (raw.startsWith("\t")) { - if (current !== null) { - recipes.get(current)?.push(raw.trim().replace(/^[@-]+/, "")); - } - continue; - } - const target = /^([a-z][a-z-]*)\s*:(?!=)/.exec(raw); - current = target === null ? null : target[1]; - if (current !== null && !recipes.has(current)) { - recipes.set(current, []); - } - } - return recipes; -}; - -const recipes = makeRecipes(); - -const packageScripts = (): Record => { - const pkg = JSON.parse(read("package.json")) as { - scripts?: Record; - }; - return pkg.scripts ?? {}; -}; - -const scripts = packageScripts(); - -// Node keys: `script/`, `docker:`, `make:`, -// `yarn:`, `workflow:`. -const resolve = (node: string): string[] => { - if (node.startsWith("script/")) return executable(read(node)); - if (node.startsWith("docker:")) { - return executable(read(node.slice("docker:".length))) - .filter((line) => line.startsWith("RUN ")) - .map((line) => line.slice("RUN ".length)); - } - // The `run:` steps of a workflow, in file order. `uses:` steps are actions, - // not commands, and have no edges into this repo's graph. A `run: |` block - // would resolve to the bare `|`, which reaches nothing and therefore fails - // the count rather than passing quietly. - if (node.startsWith("workflow:")) { - return executable(read(node.slice("workflow:".length))) - .filter((line) => /^-?\s*run:\s*\S/.test(line)) - .map((line) => line.replace(/^-?\s*run:\s*/, "")); - } - if (node.startsWith("make:")) { - const target = node.slice("make:".length); - const recipe = recipes.get(target); - // A renamed or deleted target must be a loud failure: silently walking - // an empty recipe would report zero prettier invocations, which reads - // like the tidiest possible result. - if (recipe === undefined) { - throw new Error(`no such Makefile target: ${target}`); - } - return recipe; - } - if (node.startsWith("yarn:")) { - const name = node.slice("yarn:".length); - const script = scripts[name]; - if (script === undefined) { - throw new Error(`no such package.json script: ${name}`); - } - return [script]; - } - throw new Error(`unresolvable node: ${node}`); -}; - -// Same reasoning as the missing-target error, applied to every node kind: a -// node that resolves to no commands contributes zero prettier invocations and -// zero edges, which is indistinguishable from a clean result. Fail instead. -const commandsOf = (node: string): string[] => { - const commands = resolve(node); - if (commands.length === 0) { - throw new Error(`node resolved to no commands: ${node}`); - } - return commands; -}; - -const edgesOf = (line: string): string[] => { - const edges: string[] = []; - - // `"$SCRIPT_DIR/lint"`, `"$ROOT/script/lint"` and a bare `script/lint` are - // all the same edge. - for (const match of line.matchAll( - /(?:\$SCRIPT_DIR|\$\{SCRIPT_DIR\}|script)\/([a-z][a-z-]*)/g, - )) { - edges.push(`script/${match[1]}`); - } - - // Only real targets: `pkg_install gnumake make make make` in - // script/bootstrap is a package name, not an invocation of this Makefile. - for (const match of line.matchAll(/\bmake\s+([a-z][a-z-]*)/g)) { - if (recipes.has(match[1] ?? "")) edges.push(`make:${match[1]}`); - } - - // Same rule for yarn: `yarn run prettier` is the linter itself (counted, - // not followed), `yarn run fmt-check` would be a package.json script that - // runs it indirectly. - for (const match of line.matchAll(/\byarn(?:\s+run)?\s+([a-z][a-z-]*)/g)) { - if ((match[1] ?? "") in scripts) edges.push(`yarn:${match[1]}`); - } - - // The container lint pass lives behind a `docker build`; without following - // it the count would miss the one invocation that is supposed to survive. - if (/\bdocker\s+build\b/.test(line)) { - const file = /\s-f\s+(\S+)/.exec(line); - edges.push(`docker:${file === null ? "Dockerfile" : file[1]}`); - } - - return edges; -}; - -interface Walk { - prettier: number; - reached: Set; -} - -// Repeated invocations must count repeatedly — running the same script twice is -// exactly the bug — so nodes are not deduplicated. The path stack is only there -// to turn a cycle into a loud failure instead of a hang. -// -// Counting and edge-following both happen for every line: a line that invokes -// prettier can also invoke something else, and skipping the edges of counted -// lines silently truncated the graph. -const walk = (node: string, path: string[] = [], into?: Walk): Walk => { - const result = into ?? { prettier: 0, reached: new Set() }; - if (path.includes(node)) { - throw new Error(`invocation cycle: ${[...path, node].join(" -> ")}`); - } - result.reached.add(node); - - for (const line of commandsOf(node)) { - result.prettier += countPrettier(line); - for (const edge of edgesOf(line)) { - walk(edge, [...path, node], result); - } - } - return result; -}; - -describe("prettier runs exactly once per make check", () => { - const check = walk("make:check"); - - // The headline assertion, and the one the issue is about. - it("invokes prettier once for the whole of make check", () => { - expect(check.prettier).toBe(1); - }); - - // Guards against the count being 1 (or 0) because the walk never got - // anywhere. `make check` has to reach the suite, the lint script, and the - // Dockerfile whose build IS the lint verdict. - it.each(["script/check", "script/test", "script/lint", "Dockerfile.lint"])( - "reaches %s while counting", - (node) => { - const key = node.startsWith("script/") ? node : `docker:${node}`; - expect([...check.reached]).toContain(key); - }, - ); - - // The one that survives is the container's, not the host's: that is the - // authoritative verdict, since a successful Dockerfile.lint build is what - // CI treats as proof of a clean tree. - it("keeps the surviving invocation inside the lint container", () => { - expect(walk("docker:Dockerfile.lint").prettier).toBe(1); - }); - - it("does not reach the host formatting check from make check", () => { - expect([...check.reached]).not.toContain("script/fmt-check"); - }); -}); - -describe("prettier runs exactly once per CI build", () => { - // Rooted at the workflow file, so this is the graph CI executes rather than - // the graph someone believed CI executes. `make check` cannot stand in for - // it: CI runs script/cibuild, which builds Dockerfile as well as - // Dockerfile.lint, and nothing under `make check` ever reads Dockerfile. - const ci = walk("workflow:.gitea/workflows/check.yml"); - - it("invokes prettier once for the whole CI build", () => { - expect(ci.prettier).toBe(1); - }); - - // script/cibuild is here because the workflow is asserted to run it; - // Dockerfile is here because it is the half of the CI graph that the - // `make check` walk cannot see. - it.each([ - "script/cibuild", - "script/lint", - "docker:Dockerfile.lint", - "docker:Dockerfile", - ])("reaches %s while counting", (node) => { - expect([...ci.reached]).toContain(node); - }); - - // The test and build image must not lint: linting is Dockerfile.lint's job, - // and a prettier step added here would be a second pass over the same tree - // for the same verdict — on the one path where it matters most. - it("keeps prettier out of the test and build image", () => { - expect(walk("docker:Dockerfile").prettier).toBe(0); - }); -}); - -describe("the standalone entrypoints still do what their names say", () => { - // REPO_POLICIES.md requires both `make lint` and `make fmt-check` to exist - // and mean something. Dropping fmt-check from script/check must not turn it - // into a target nobody can use, and must not leave `make check` passing - // because both halves became no-ops. - it("still checks formatting under make fmt-check", () => { - expect(walk("make:fmt-check").prettier).toBe(1); - }); - - it("still checks formatting under make lint", () => { - expect(walk("make:lint").prettier).toBe(1); - }); -}); - -describe("script/precommit", () => { - // Same duplication as script/check, same fix. The hook still catches a - // badly formatted tree before the commit lands, because script/lint is the - // container prettier run — that is the whole reason the host call could go. - it("checks formatting exactly once", () => { - expect(walk("script/precommit").prettier).toBe(1); - }); - - it("gets that check from the lint container", () => { - expect([...walk("script/precommit").reached]).toContain( - "docker:Dockerfile.lint", - ); - }); -}); - -// script/bootstrap installs the dependencies, and it has two install sites: one -// for the case where yarn has to be reached through nvm, and one for the case -// where yarn is already on PATH. A substring check against the whole file -// cannot tell them apart, so it reports the first and says nothing about the -// second — which is the one the containers take, because the pinned node image -// ships yarn. Both are resolved separately here. -const installBranches = (): { withoutYarn: string[]; withYarn: string[] } => { - const lines = executable(read("script/bootstrap")); - const open = lines.findIndex((line) => - /^install_js_deps\s*\(\)/.test(line), - ); - if (open === -1) { - throw new Error("script/bootstrap: no install_js_deps function"); - } - const close = lines.indexOf("}", open); - const body = lines.slice(open + 1, close === -1 ? undefined : close); - const guard = body.findIndex((line) => - /^if\b.*\bmissing yarn\b/.test(line), - ); - const otherwise = body.indexOf("else", guard); - const end = body.indexOf("fi", otherwise); - if (guard === -1 || otherwise === -1 || end === -1) { - throw new Error( - "script/bootstrap: install_js_deps is not the expected " + - "if missing yarn / else / fi shape", - ); - } - return { - withoutYarn: body.slice(guard + 1, otherwise), - withYarn: body.slice(otherwise + 1, end), - }; -}; - -// Every `yarn install` in the given lines, with its flags, so an unpinned -// install cannot hide next to a pinned one. -const yarnInstalls = (lines: string[]): string[] => - lines.flatMap((line) => - [...line.matchAll(/\byarn install\b[^"'&|;]*/g)].map((match) => - match[0].trim(), - ), - ); - -describe("host and container prettier cannot disagree", () => { - // With the host pass gone from `make check`, `make fmt-check` is the only - // host-side formatting check left, and the container is the gate. The two - // must keep producing the same verdict on the same tree, or a developer - // running `make fmt-check` gets a green that CI then rejects. - // - // Three things make them agree, and all three are load-bearing: - it("pins the same prettier for both", () => { - const pkg = JSON.parse(read("package.json")) as { - devDependencies: Record; - }; - // An exact version, not a range: `^3.8.1` would let the container and - // the host resolve different builds with different formatting. - expect(pkg.devDependencies.prettier).toMatch(/^\d+\.\d+\.\d+$/); - }); - - it("installs from the lockfile on the branch the container takes", () => { - // Both images are FROM a node image, which ships yarn, so `missing - // yarn` is false and this is the branch that runs in the container. - const installs = yarnInstalls(installBranches().withYarn); - expect(installs).not.toHaveLength(0); - for (const install of installs) { - expect(install).toContain("--frozen-lockfile"); - } - }); - - it("installs from the lockfile on the nvm branch too", () => { - // Not the container's branch, but it is the one a developer without - // yarn on PATH gets, and their prettier has to match the container's. - const installs = yarnInstalls(installBranches().withoutYarn); - expect(installs).not.toHaveLength(0); - for (const install of installs) { - expect(install).toContain("--frozen-lockfile"); - } - }); - - it("runs script/bootstrap inside the lint container", () => { - // Without this the lockfile assertions above would be about a script - // the container never executes. - expect([...walk("docker:Dockerfile.lint").reached]).toContain( - "script/bootstrap", - ); - }); - - it("keeps .gitignore in the build context", () => { - // Prettier 3 reads .gitignore as a default ignore file, so excluding it - // from the context would change which files the container checks. - const dockerignore = read(".dockerignore") - .split("\n") - .map((line) => line.trim()); - expect(dockerignore).not.toContain(".gitignore"); - }); -}); - -describe("the walk cannot pass vacuously", () => { - // An earlier draft of this file computed a Makefile target as - // `node.slice("make:")` — a string where a number belongs, which coerces to - // NaN and made every target resolve to nothing. The count went to zero and - // an assertion of "not twice" would have been satisfied by a walk that had - // read nothing at all. Every way of reaching nothing is therefore an - // error here, and the ways are tested rather than assumed. - it("reports zero for a subgraph that does not run prettier", () => { - expect(walk("make:clean").prettier).toBe(0); - }); - - it("refuses a Makefile target that does not exist", () => { - expect(() => walk("make:no-such-target")).toThrow( - /no such Makefile target/, - ); - }); - - it("refuses a package.json script that does not exist", () => { - expect(() => walk("yarn:no-such-script")).toThrow( - /no such package.json script/, - ); - }); - - it("refuses a script that does not exist", () => { - expect(() => walk("script/no-such-script")).toThrow(/ENOENT/); - }); - - it("refuses a node that resolves to no commands", () => { - // .dockerignore has no RUN steps, standing in for a Dockerfile whose - // steps a restructure moved somewhere the resolver cannot see. - expect(() => walk("docker:.dockerignore")).toThrow( - /resolved to no commands/, - ); - }); - - it("refuses a node kind it does not understand", () => { - expect(() => walk("nonsense")).toThrow(/unresolvable node/); - }); - - it("refuses to walk in circles", () => { - expect(() => walk("make:check", ["script/check"])).toThrow( - /invocation cycle/, - ); - }); -}); - -describe("the resolver reads what the shell would run", () => { - // Counting per line is how `yarn run prettier --check . && yarn run - // prettier --check src` read as a single invocation. - it("counts every prettier invocation on a line", () => { - expect( - countPrettier( - "yarn run prettier --check . && yarn run prettier --check src", - ), - ).toBe(2); - }); - - it("does not count the config files as invocations", () => { - expect(countPrettier("COPY .prettierrc .prettierignore ./")).toBe(0); - }); - - // The counting `continue` also dropped every edge that shared a line with a - // prettier call, so a whole subtree could be hidden behind one `&&`. - it("still follows the edges of a line that invokes prettier", () => { - expect( - edgesOf('yarn run prettier --check . && "$SCRIPT_DIR/lint"'), - ).toContain("script/lint"); - }); - - it("resolves every spelling of a script call to one node", () => { - expect( - edgesOf('"$SCRIPT_DIR/lint" "${SCRIPT_DIR}/test" script/fmt'), - ).toEqual(["script/lint", "script/test", "script/fmt"]); - }); - - it("follows a bare docker build to Dockerfile and -f to its file", () => { - expect(edgesOf("docker build .")).toContain("docker:Dockerfile"); - expect(edgesOf("docker build -f Dockerfile.lint .")).toContain( - "docker:Dockerfile.lint", - ); - }); - - it("reads the run steps of the CI workflow and not its uses steps", () => { - expect(commandsOf("workflow:.gitea/workflows/check.yml")).toEqual([ - "script/cibuild", - ]); - }); -});