~bigbes/sr-ht-dolt

fe24d7dd96f01d86cef22f387c82ceaa16af46e4 — Eugene Blikh 9 days ago ec98df3
ci: run the test suites against a real postgres

Not one of this repository's 28 test files had ever run on the builder: the
manifest went from cache_restore straight to abuild, and options="!check"
meant abuild did not run them either. Every apk published so far was built
from code CI compiled and never executed.

A postgres task brings up a database in the VM and exports DOLTSRHT_TEST_PG;
a test task runs make vet and make test. Measured on this tree, db/ has 10
tests and 9 of them gate on that DSN, so without it the whole persistence
layer is skipped and the build is green regardless; no other package needs
it. Hence the guard: an empty DSN fails the task loudly rather than skipping
every database suite. That guard is what makes !check an honest claim about
where the suites ran instead of a licence to ship untested code, and the
APKBUILD now says so.

make vet and make test rather than bare go commands, because the Makefile is
where -tags gms_pure_go and CGO_ENABLED=0 are named — without the tag either
command pulls go-icu-regex in and wants ICU headers the builder lacks.

remoteapi/integration_test.go prefers DOLTSRHT_TEST_PG and only falls back to
docker run when it is empty, so it never reaches for a daemon the builder
does not have. It is moot either way: the file is behind //go:build
integration and go test ./... does not compile it, as with spike.

No gofmt gate: web/beads.go and four test files are gofmt-dirty on master, so
one would be red on arrival.
3 files changed, 123 insertions(+), 6 deletions(-)

M .build.yml
M APKBUILD
M docs/ci.md
M .build.yml => .build.yml +38 -0
@@ 15,6 15,9 @@ packages:
  - 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.


@@ 127,6 130,41 @@ tasks:
      # Neither line above may rewrite go.mod or go.sum — a dirty tree here is a
      # "-dirty" apk, so check rather than trust. docs/ci.md#cache_restore.
      git status --porcelain
  - postgres: |
      # A real Postgres in the VM. Without it db/ skips 9 of its 10 tests and
      # the build goes green over the whole persistence layer — which is where
      # this repository had stood since it was written. 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)" doltsrht_test
      echo "export DOLTSRHT_TEST_PG='postgresql://$(id -un)@127.0.0.1/doltsrht_test?sslmode=disable'" \
        >> ~/.buildenv
  - test: |
      cd "$REPO"
      # An empty DSN would skip every database suite and leave the build green
      # over untested code — and options="!check" in the APKBUILD says this task
      # is where the suites run, so a silent skip here is a package built on a
      # promise nothing kept. It also catches a reordering of the two tasks.
      # docs/ci.md#test.
      if [ -z "$DOLTSRHT_TEST_PG" ]; then
        echo "DOLTSRHT_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
      # `make vet` and `make test` rather than bare go commands: the Makefile is
      # where -tags gms_pure_go and CGO_ENABLED=0 are named, and a second copy
      # of those here is a second copy to forget. docs/ci.md#test.
      make vet
      make test
      # Printed, not gated, for the same reason as cache_restore's: this task
      # runs before abuild, so anything the suites left in the checkout is a
      # "-dirty" apk, and this is where it would show. docs/ci.md#test.
      git status --porcelain
  - build: |
      cd "$REPO"
      # -d: makedepends are already installed via `packages:` above.

M APKBUILD => APKBUILD +8 -1
@@ 24,7 24,14 @@ pkgdesc="Dolt database hosting for a sourcehut instance"
url="https://sourcecraft.dev/bigbes/sr-ht-dolt"
arch="x86_64"
license="MIT"
# !check      — tests want a live Postgres and a chunk store
# !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 DOLTSRHT_TEST_PG unset, i.e. with 9 of db/'s 10 tests
#               skipping. What makes this option honest rather than a licence to
#               ship untested code is that task's DSN guard: an empty
#               DOLTSRHT_TEST_PG fails it loudly instead of skipping the
#               database suites and going green.
# !tracedeps  — CGO_ENABLED=0 with -tags gms_pure_go, so the binaries are
#               static: no ICU, no gozstd, nothing to trace
options="!check !tracedeps"

M docs/ci.md => docs/ci.md +77 -5
@@ 6,8 6,9 @@ submitted at all — the failure is at submission time and reads like nothing in
particular. 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, decide a version, restore caches, package with
`abuild`, publish the apk to `repo.bigb.es/alpine/v3.22/bigbes`, save caches.
assemble the shared SCSS, decide a version, restore caches, start a Postgres,
test, package with `abuild`, publish the apk to
`repo.bigb.es/alpine/v3.22/bigbes`, save 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


@@ 19,7 20,11 @@ the critical path for the package repository, not merely an offsite copy.
`sassc` and `minify` build the stylesheet; `curl` installs `cacher`; `rclone`
publishes the apk; `abuild`, `go` and `git` are the build itself.

There is no `postgresql` here and no compiler toolchain beyond `go`: this
`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.

There is no compiler toolchain beyond `go`: this
package is built `CGO_ENABLED=0` with `-tags gms_pure_go` (see the APKBUILD and
the Makefile), which is what keeps `libicu` and `gozstd` off the builder
entirely. A default cgo build of this tree fails on missing ICU headers and


@@ 189,6 194,71 @@ largest in the family, so the gap here is the widest; it is not zero anywhere.
The same failure was seen for real on a sibling: bench build #359 failed its
version gate with `M go.sum` as the only thing in the way.

## postgres

A real Postgres in the VM, initialised from scratch each build.

Until this task existed, **not one of this repository's 28 test files had ever
run on the builder** — the manifest went straight from `cache_restore` to
`abuild`, and `options="!check"` in the `APKBUILD` meant abuild did not run them
either. Every apk this pipeline has published was built from code CI had
compiled and never executed.

What the database needs to be there for, measured on this tree: `db/` has 10
tests and 9 of them gate themselves on `DOLTSRHT_TEST_PG` (`db/db_test.go`'s
`newTestStore`), so without a DSN the entire persistence layer — repository and
key CRUD, the access-control queries, the schema — is skipped and the build is
green regardless. The other packages do not need it and do not skip without it.

The suites do not share state: each creates a scratch schema
`doltsrht_test_<random>`, applies `schema.sql` into it, routes its pool there
with lib/pq's `options=-c search_path=…`, and drops it afterwards. So one
database serves the whole suite, and no `CREATE DATABASE` privilege is needed
past the one `createdb` above.

`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. `-k /run/postgresql` is why the task creates that
directory — Alpine's package does not — and `createuser -s "$(id -un)"` is what
lets the build user connect without a password, which is what keeps the DSN in
`~/.buildenv` free of a credential.

### `remoteapi/integration_test.go` does not want Docker

That file's `startPostgres` prefers `DOLTSRHT_TEST_PG` and falls back to
`docker run postgres:16-alpine` only when the variable is empty, so with the DSN
exported it never looks for a Docker daemon the builder does not have.

It is moot in any case: the file is behind `//go:build integration`, so `go test
./...` does not even compile it. Running it would additionally need the `dolt`
CLI, which it skips on. `storage/spike_test.go` is behind `//go:build spike` for
the same kind of reason. Neither tag is set here, deliberately.

## test

`make vet` and `make test`, not bare `go` commands: the Makefile is where
`-tags gms_pure_go` and `CGO_ENABLED=0` are named, and a second copy of those
two here is a second copy to forget. The tags are not optional — a `go vet` or
`go test` without `gms_pure_go` pulls `go-icu-regex` in and wants ICU headers
the builder has never had.

The guard on an empty `DOLTSRHT_TEST_PG` exists because the failure it prevents
is silent. If the `postgres` task did not export the DSN — or if someone
reorders 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 is also
what makes `options="!check"` in the `APKBUILD` an honest claim rather than a
licence: that option says "the suites ran in CI", and this guard is the only
thing that keeps it true.

There is no `gofmt` gate, unlike the tokens sibling. `web/beads.go` and four
test files are gofmt-dirty on master, so a gate would be red on arrival; fixing
them is a separate change and not one to smuggle into a CI wave.

The task runs **before** `build`, so the tree it leaves behind is the tree
`abuild` packages; the closing `git status --porcelain` is where anything the
suites wrote into the checkout would show.

## build

`REPODEST=$HOME/packages abuild -d` builds and stages the apk.


@@ 288,5 358,7 @@ subpackage ships changes bytes on the next build even where nothing else did.
  name changes every commit, which `artifacts:` cannot express (it has no
  globbing).
- **No matrix.** One architecture, one image.
- **No `gofmt` gate.** `web/beads.go` and four test files are gofmt-dirty on
  master; a gate would be red on arrival. `go vet` runs instead.
- **No `gofmt` gate.** See [test](#test).
- **No `integration` or `spike` build tag.** See
  [`remoteapi/integration_test.go` does not want
  Docker](#remoteapiintegration_testgo-does-not-want-docker).