diff --git a/.build.yml b/.build.yml index 4e6cb5c0c1fc6685e97f81ccb8e78f3254410a92..ca1196a3b7574b1326c11dda5906f43b8b78429b 100644 --- a/.build.yml +++ b/.build.yml @@ -15,6 +15,9 @@ - git - rclone - sassc - minify + # For the database suites, not for the package — see docs/ci.md#packages. + - postgresql + - postgresql-client secrets: # File secret `apk-ci-s3`, installed at ~/.apk-ci.env, containing # APK_CI_S3_ACCESS_KEY / APK_CI_S3_SECRET_KEY for the Garage `repo` bucket. @@ -131,6 +134,37 @@ go mod verify # And the tree is printed because the two lines above are the last thing # that touches it before abuild does. docs/ci.md#cache_restore. git status --porcelain + - postgres: | + # A real Postgres in the VM. Without it 67 tests of db/, service/ and + # cmd/specsrht-migrate/ skip themselves and the build goes green having + # exercised none of the persistence layer — the migration-agreement check + # included. Every flag below is load-bearing: docs/ci.md#postgres. + sudo install -d -o postgres -g postgres /run/postgresql /var/lib/postgresql/data + sudo -u postgres initdb -D /var/lib/postgresql/data + sudo -u postgres pg_ctl -D /var/lib/postgresql/data -l /tmp/pg.log -w start \ + -o "-k /run/postgresql -h 127.0.0.1 \ + -c fsync=off -c full_page_writes=off -c synchronous_commit=off" + sudo -u postgres createuser -s "$(id -un)" + sudo -u postgres createdb -O "$(id -un)" specsrht_test + echo "export SPECSRHT_TEST_PG='postgresql://$(id -un)@127.0.0.1/specsrht_test?sslmode=disable'" \ + >> ~/.buildenv + - test: | + cd "$REPO" + # An empty DSN would skip every Postgres-backed suite and leave the build + # green over untested code — and `options="!check"` in the APKBUILD means + # this task is the only place the suites run at all. It also catches a + # reordering of the two tasks. docs/ci.md#test. + if [ -z "$SPECSRHT_TEST_PG" ]; then + echo "SPECSRHT_TEST_PG is unset: the postgres task did not export it," >&2 + echo "so every database suite would skip and this build would lie." >&2 + exit 1 + fi + test -z "$(gofmt -l .)" || { gofmt -l .; echo "gofmt: files above need formatting" >&2; exit 1; } + go vet ./... + # `make test` and not a bare `go test ./...`: the Makefile is where this + # repository's test command names its -timeout, and a second copy of that + # number here is a second copy to forget. docs/ci.md#test. + make test - build: | cd "$REPO" # -d: makedepends come from `packages:`. The APKBUILD runs `make css` diff --git a/APKBUILD b/APKBUILD index c16e94bf6362ca7ee0aa79592a0f736337fc0d87..5d1b106ec0a660adc8441e8f7238e2b1509462ee 100644 --- a/APKBUILD +++ b/APKBUILD @@ -23,7 +23,13 @@ pkgdesc="Reviewable document storage for humans and agents" url="https://sourcecraft.dev/bigbes/sr-ht-spec" arch="x86_64" license="MIT" -# !check — tests want a live Postgres and a git work area +# !check — the suites are run by the `test` task of .build.yml, against the +# Postgres that manifest brings up, and they run BEFORE this +# package is built. Letting abuild run them again would repeat the +# work with SPECSRHT_TEST_PG unset, i.e. with every database suite +# skipping — 67 tests of db/, service/ and cmd/specsrht-migrate/, +# measured. That task's DSN guard is what makes this line true; a +# pipeline without it packages code nothing tested. # !tracedeps — CGO_ENABLED=0, so the binaries are static options="!check !tracedeps" diff --git a/Makefile b/Makefile index d965152c1e1fcf47972f40602ed420473bad3209..fe64c197ac5a32b2c67c100c35cfc3dfe4dbfa5d 100644 --- a/Makefile +++ b/Makefile @@ -56,8 +56,27 @@ $(MIGRATE_BIN): go build -o $@ ./cmd/$(MIGRATE_BIN) +# TEST_TIMEOUT is the per-package ceiling handed to `go test`, and it is named +# here rather than left to the toolchain because the toolchain's default is ten +# minutes, it is silent about being a default, and what it produces when a +# package walks into it is `panic: test timed out after 10m0s` over a few +# hundred lines of goroutine dump naming whichever test happened to be +# mid-flight — neither a hang nor a deadlock, and a Postgres schema left behind +# because a panic runs no t.Cleanup. +# +# Measured on this tree with go1.26.5 against a local Postgres, the whole suite +# is under two minutes and its slowest package (service/) is 20 s, so twenty +# minutes is not a bound anything is near. It is the family's number, and it is +# here so that the builder — slower, cold-cached, and running the database +# suites this laptop can also run — is nowhere near it either. +TEST_TIMEOUT?=20m + +# PKG is what to run, a variable so `make test PKG=./db/` is the same command +# with the -timeout still on it. +PKG?=./... + test: - go test ./... + go test -timeout $(TEST_TIMEOUT) $(PKG) # CSS pipeline: sassc -> minify -> content-hashed filename. The running service # globs web/static/main.min.*.css at startup, so the hash in the name is the diff --git a/docs/ci.md b/docs/ci.md index 2153b61441ec5ed46f932f0320a77503b5e8befb..faf48795c460bcac2c864b66927dcbbf950a2228 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -7,8 +7,9 @@ particular, and what it produces is a branch with no CI rather than a red build. Rationale therefore lives here, and the manifest carries pointers. The pipeline is one linear job on `alpine/edge`: install the cache helper, -assemble the shared SCSS, stamp a version, restore the Go caches, package with -`abuild`, publish the apk to `repo.bigb.es/alpine/v3.22/bigbes`, save the caches. +assemble the shared SCSS, stamp a version, restore the Go caches, start a +Postgres, test, package with `abuild`, publish the apk to +`repo.bigb.es/alpine/v3.22/bigbes`, save the caches. It is triggered by a push to the **sourcehut** side. A push to sourcecraft cannot reach builds.sr.ht; the gitsync mirror is what puts the commit on @@ -16,6 +17,10 @@ `git.srht.bigb.es`, and that push is what submits the job. So the mirror is on the critical path for the package repository, not merely an offsite copy. ## packages + +`postgresql` and `postgresql-client` are for the test suite, not for the +package. The apk declares no runtime dependency on Postgres — the daemon talks +to whatever `connection-string` names, which in production is another host. `sassc` and `minify` are the stylesheet pipeline (`make css`), and they are build-time only: the compiled CSS is embedded into the binary by `//go:embed`, @@ -168,6 +173,54 @@ they are what `go.sum` is. This is not theoretical either — it is what the sibling bench.sr.ht build #359 failed on, at its `check-version` gate, with `M go.sum` named as the culprit. + +## postgres + +A real Postgres in the VM, initialised from scratch each build. + +It is what turns this repository's 93 test files, across 16 packages, from files +that compile on the builder into tests that ran there. `db/db_test.go`, +`service/fixture_test.go` and `cmd/specsrht-migrate/main_test.go` all gate their +integration cases on `SPECSRHT_TEST_PG`, and without it **67 tests skip** — +measured on this tree by counting `--- SKIP` with and without the variable. The +persistence layer, the GraphQL/service layer over it and the migration runner +are all in that set, the migration-agreement check included: the one that +catches `schema.sql` and `migrations/` drifting apart, which is a defect a fresh +install and an upgraded instance disagree about and nothing else notices. + +`fsync=off`, `full_page_writes=off` and `synchronous_commit=off` are safe here +and only here: the database lives for the length of one build and its durability +guarantees protect nothing. + +`createuser -s "$(id -un)"` makes the build user a superuser and the DSN carries +no password, because the tests create a scratch schema per test +(`specsrht_test_`), apply `schema.sql` into it and drop it afterwards. +One database therefore serves the whole suite, and no `CREATE DATABASE` right is +needed. + +## test + +The guard on an empty `SPECSRHT_TEST_PG` exists because the failure it prevents +is silent. If the `postgres` task did not export the DSN — or if someone +reordered the two tasks — every database suite skips with a friendly message, +`go test` exits 0, and the build is green over untested code. An explicit +refusal is the difference between a broken pipeline and a lying one. + +It carries more weight here than in the siblings: `options="!check"` in the +`APKBUILD` means abuild runs no tests at all, so this task is the *only* place +the suites run. + +`gofmt` and `go vet` run here too, before the tests, so a formatting regression +fails the build rather than waiting for someone to notice in review. `gofmt` +needs the `test -z "$(gofmt -l .)"` spelling because `gofmt -l` reports the +files it would change and still exits 0. + +The suite is invoked as `make test` and not as a bare `go test ./...`: the +Makefile is where this repository's `-timeout` is named (`TEST_TIMEOUT?=20m`), +and a second copy of that number here is a second copy to forget. The default +`go test` timeout is ten minutes, it is silent about being a default, and what +it produces on a slow builder is a goroutine dump rather than a failure anyone +can read. ## build