Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions .github/actions/check/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,6 @@ runs:
run: ./scripts/check-layering.sh
env:
GOLANGCI_LINT: golangci-lint
- name: Test boundary probes
if: inputs.mode != 'db' && inputs.full == 'true'
shell: bash
run: ./scripts/check-test-probes.sh

# ---- tests ----
# With -coverprofile go test also compiles packages without tests (lint
Expand Down Expand Up @@ -142,9 +138,22 @@ runs:
chmod +x "$RUNNER_TEMP/wait-db"
exec_flag=(-exec "$RUNNER_TEMP/wait-db")
fi
go test -race "${exec_flag[@]}" -coverprofile=.cache/coverage.out $PACKAGES # word splitting intended
# testbudget prints plain go test output, the slowest tests, and fails
# a package or test over its time budget (AGENTS.md "Fast tests").
set -o pipefail
go test -race -json "${exec_flag[@]}" -coverprofile=.cache/coverage.out $PACKAGES | # word splitting intended
go run ./scripts/testbudget
TESTED="$PACKAGES" ./scripts/check-coverage.sh .cache/coverage.out

# After the tests: the coverage probes plant their uncovered statements in
# the run's profile (this job tested every probed package on a full run).
- name: Test boundary probes
if: inputs.mode != 'db' && inputs.full == 'true'
shell: bash
run: ./scripts/check-test-probes.sh
env:
PROFILE: .cache/coverage.out

- uses: actions/cache/save@v6
if: inputs.save == 'true' && inputs.packages != '' && inputs.mode == 'all'
with:
Expand Down
64 changes: 64 additions & 0 deletions .github/workflows/bench-devloop.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Dev-loop timings on a CI-shaped runner (`make bench-devloop` with cold and
# warm caches): build, generate, lint per linter, probes, tests per package
# with and without -race, and an A/B of the CI test run against a base ref on
# the same machine (separate runs land on different machines; their noise
# hides changes of tens of percent). The report is the run's summary page.
# Run it by hand (Actions > Bench dev loop), or push a branch to bench/<name>
# to compare it with main.
name: Bench dev loop

on:
workflow_dispatch:
inputs:
base:
description: Ref to compare against (A/B of the test run on the same machine)
default: origin/main
rounds:
description: Alternating A/B rounds (the median is compared)
default: '3'
push:
branches: ['bench/**']

permissions:
contents: read

concurrency:
group: bench-${{ github.ref }}
cancel-in-progress: true

jobs:
bench:
runs-on: ubuntu-latest
timeout-minutes: 60
env:
TEST_DATABASE_URL: postgres://postgres:postgres@127.0.0.1:5432/postgres?sslmode=disable
BENCH_BASE: ${{ inputs.base || 'origin/main' }}
BENCH_ROUNDS: ${{ inputs.rounds || '3' }}
steps:
# Same Postgres as CI's db shard.
- name: Start Postgres
run: |
docker run -d --name pg -p 5432:5432 -e POSTGRES_PASSWORD=postgres \
--tmpfs /var/lib/postgresql postgres:18-alpine \
-c fsync=off -c synchronous_commit=off -c full_page_writes=off -c max_connections=200
- uses: actions/checkout@v7
with:
fetch-depth: 0 # the A/B base ref
# Read-only: the module cache CI saves. Build and lint caches start
# empty on purpose; the bench measures cold and warm itself.
- uses: actions/cache/restore@v6
with:
path: ~/go/pkg/mod
key: v7-gomod-${{ runner.os }}-${{ hashFiles('go.sum') }}
restore-keys: v7-gomod-${{ runner.os }}-
- name: Go toolchain and modules
run: |
go version
go mod download
go mod download -modfile=tools/go.mod
- name: Wait for Postgres
run: |
for _ in $(seq 1 120); do docker exec pg pg_isready -q -h 127.0.0.1 -U postgres && exit 0; sleep 0.5; done
docker logs pg; exit 1
- name: Bench
run: make bench-devloop BENCH_COLD=1
13 changes: 13 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ linters:
- misspell
- nilerr
- noctx
- paralleltest # fast tests: every test calls t.Parallel() (AGENTS.md "Fast tests")
- rowserrcheck
- sqlclosecheck
- tparallel
- unconvert
- usestdlibvars
- wastedassign
Expand All @@ -32,6 +34,14 @@ linters:
- pattern: ^auth\.Auth\.Users$
pkg: ^github\.com/jayleaton/ai-patterns-go/packages/auth$
msg: user administration (ban, roles, any user's sessions) is for apps/admin only
# Tests only (see exclusions): a test that sleeps is slow and flaky.
- pattern: ^time\.Sleep$
msg: tests never sleep; wait on a channel or condition, or use testing/synctest for a fake clock (AGENTS.md "Fast tests")
# Top-level tests must call t.Parallel(); a test that cannot (shared
# global state) says why with //nolint:paralleltest // <reason>.
# Subtests are parallel where they are slow; tparallel keeps it consistent.
paralleltest:
ignore-missing-subtests: true
# Layering: packages/database -> services -> transports (api, ui), for
# apps/web and apps/admin alike. See AGENTS.md "Architecture".
depguard:
Expand Down Expand Up @@ -303,6 +313,9 @@ linters:
- path: (apps/admin|packages/auth)/
linters: [forbidigo]
text: is for apps/admin only
- path-except: _test\.go
linters: [forbidigo]
text: tests never sleep

formatters:
enable:
Expand Down
15 changes: 14 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ The most common defect is a change that works on the path you tested and is miss
- CI layout: jobs bill rounded up to whole minutes, so CI is ONE job (`ci`): affected-only generators, gofmt, golangci-lint and race tests (all packages on pushes to main, the merge queue and manual runs), plus the mobile app and `deploy/` module checks when those change. Docs-only PRs post `ci-ok` from that job with nothing to check; drafts start no jobs. Sharding is an opt-in fallback only: label a PR `ci:fanout` (takes effect on its next push) or run the workflow manually with `fanout`, which splits into `lint + test (db)` and `test (other)` for faster wall time at more billed minutes. Timeouts are ~2x measured averages (see the workflow comments). Shared steps live in `.github/actions/check`. Avoid adding jobs: fold new checks into the existing one. A push to main whose tree is identical to a PR merge commit that already passed CI is not tested again: `ci-ok` is posted from that result (`scripts/ci-reuse.sh`; PR results record the tested tree in their `ci-ok` description). Caches: modules keyed by go.sum; one build/lint cache keyed by commit, saved by main pushes that run and by a scheduled run every 4 hours (skipped when main has not moved), restored by prefix elsewhere.
- Run the narrowest thing first: `go test ./apps/web/internal/services/...`.
- Behaviour changes ship with tests at the boundaries below, and only there.
- A test that needs a sleep or timeout to pass is wrong.
- A test that needs a sleep or timeout to pass is wrong. See "Fast tests".
- Test memory: `auth.New` hashes passwords with cheap argon2 settings inside test binaries (`testing.Testing()`), so suites that sign users up stay small under `-race`; only `TestPasswordHashDefaults` exercises production cost. A full cold `make test` peaks around 2 GB, almost all of it compiling, so don't lower `-p`. If a test or compile dies with `signal: killed`, check the cgroup's `memory.events`: `oom_kill 0` means something outside the repo sent the kill (another job on a shared machine), not a memory limit.
- User-visible UI changes get one pass in a real browser, over the LAN or Tailscale URL when origins matter.

Expand All @@ -91,6 +91,19 @@ The allowlist in `scripts/check-test-boundaries.sh` names the files that have no

Mocks are generated, never written: `//go:generate go tool mockgen -source=<file>.go -destination=mocks/<file>.go -package=mocks` next to the interfaces, `make generate`, commit the output (CI fails on a stale mock). `mockgen` is a `tool` in the root `go.mod` so it always matches the `go.uber.org/mock` version the mocks import.

## Fast tests

The suite runs on every change it can affect, under `-race` on 2-vCPU CI runners, so its speed is a feature that has to survive growth. Lint and `make test` enforce these; `make lint-tests` proves the budgets trip.

- **Parallel.** Every test starts with `t.Parallel()` (`paralleltest`). Slow table tests make their cases parallel subtests too; `tparallel` keeps a test and its subtests consistent. Build expensive fixtures once per group and share them with the parallel subtests (the crawls build one handler per state and method, then run each route as a subtest): CI has 2 vCPUs, so a split that doubles CPU time gains nothing, and a long subtest trips the budget. Judge CPU cost with `GOMAXPROCS=2`; on a big machine idle parallel workers inflate it. Never share a gomock controller, `httptest.Server` or recorder across parallel subtests; build them per case. A test that must touch process-wide state stays serial with `//nolint:paralleltest // <why>`; prefer removing the global instead (the crawl tests used to share `crawlMethod`).
- **No sleeps.** `time.Sleep` in a test fails lint (forbidigo). Wait on a channel, a condition or `synctest.Wait`; for intervals, expiry and backoff use `testing/synctest` (its fake clock: `packages/secrets` `advance`) or an injected clock (`auth`'s `testClock`).
- **One database per test, cloned.** Repository tests use `dbtest.New(t, template)`: a `CREATE DATABASE … TEMPLATE` clone of a database migrated once per server, so tests never migrate and never depend on each other's rows. `packages/auth` does the same in its `TestMain` (`newDatabase`); tests on its shared database use unique data (`uniqueEmail`).
- **Cheap crypto.** `auth.New` uses cheap argon2 inside test binaries; only `TestPasswordHashDefaults` pays production cost.
- **Table tests.** One test with a case table, not a copy per case.
- **Do expensive checks once.** Work that depends only on immutable input (a static asset's body in the crawl) is checked once per distinct input (`checkBody`), not once per request.
- **Budgets.** `make test` and CI pipe `go test -json` through `scripts/testbudget`: plain `go test` output, then the slowest tests, then a failure if a package's test binary runs over 60 s or a single test (a leaf test or subtest) over 10 s, measured on CI under `-race` (about twice today's slowest, so a doubling fails). Over budget means split or speed up the test. Raise a budget (constants in `scripts/testbudget`) only with the reason in the commit.
- **Measure, don't guess.** `make bench-devloop` prints the timing tables (build, generate, lint per linter, probes, tests per package with and without `-race`); `BENCH_COLD=1` adds cold caches, `BENCH_PKGS` narrows the packages, and `BENCH_BASE=<ref>` adds an A/B of the CI test run against that ref on the same machine (medians of alternating rounds). Separate CI runs land on different machines and vary by tens of percent, so compare a perf change with the A/B: push it to a `bench/<name>` branch (the "Bench dev loop" workflow compares it with main) and delete the branch afterwards.

## Admin app

`apps/admin` is optional and copies the web app's layers exactly: `packages/database/adminrepo` (sqlc) and adapters over `packages/auth` → `apps/admin/internal/services` → `apps/admin/internal/api` (`/api/admin/v1`, not mounted by `cmd/admin`: the UI calls the services in-process) and `apps/admin/ui`, wired in `apps/admin/internal/server` and `apps/admin/cmd/admin` (`admin serve | migrate | invite <email>`). Config, routes and validators are its own (`apps/admin/internal/{config,validate}`).
Expand Down
24 changes: 19 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,23 @@ lint-layering: ## Prove forbidden imports (code and tests) fail lint
./scripts/check-layering.sh

.PHONY: lint-tests
lint-tests: ## Prove misplaced test files and coverage drops fail
lint-tests: ## Prove misplaced test files, coverage drops and slow tests fail (PROFILE=<coverprofile> reuses a test run)
./scripts/check-test-probes.sh

# go test -json through scripts/testbudget: plain go test output, the slowest
# tests, and a failure when a package or test is over its time budget.
GOTEST := set -o pipefail; $(GO) test -race -json -coverprofile=.cache/coverage.out ./... | $(GO) run ./scripts/testbudget

.PHONY: test
test: ## go test -race against this checkout's throwaway Postgres (or TEST_DATABASE_URL), then coverage floors
test: ## go test -race against this checkout's throwaway Postgres (or TEST_DATABASE_URL), then time budgets and coverage floors
@mkdir -p .cache
ifdef TEST_DATABASE_URL
$(GO) test -race -coverprofile=.cache/coverage.out ./...
bash -c '$(GOTEST)'
else
@$(MAKE) --no-print-directory test-db-up
@port=$$($(COMPOSE) port postgres-test 5432 | sed 's/.*://'); \
echo "test db: 127.0.0.1:$$port (project $(COMPOSE_PROJECT_NAME))"; \
TEST_DATABASE_URL="postgres://postgres:postgres@127.0.0.1:$$port/postgres?sslmode=disable" $(GO) test -race -coverprofile=.cache/coverage.out ./...
TEST_DATABASE_URL="postgres://postgres:postgres@127.0.0.1:$$port/postgres?sslmode=disable" bash -c '$(GOTEST)'
endif
./scripts/check-coverage.sh .cache/coverage.out

Expand All @@ -92,13 +96,23 @@ ci: ## Run the same checks as CI
$(MAKE) lint test
@# Alone: their temporary probe files would leak into the other checks under make -j.
$(MAKE) lint-layering
$(MAKE) lint-tests
$(MAKE) lint-tests PROFILE=.cache/coverage.out # the probes reuse make test's profile
$(MAKE) deploy-check

.PHONY: affected
affected: ## Show what CI checks for this branch (packages affected vs origin/main)
./scripts/affected.sh

.PHONY: bench-devloop
bench-devloop: ## Print dev-loop timings: build, generate, lint, probes, tests per package (BENCH_COLD=1: also cold caches)
ifdef TEST_DATABASE_URL
./scripts/bench-devloop.sh
else
@$(MAKE) --no-print-directory test-db-up
@port=$$($(COMPOSE) port postgres-test 5432 | sed 's/.*://'); \
TEST_DATABASE_URL="postgres://postgres:postgres@127.0.0.1:$$port/postgres?sslmode=disable" ./scripts/bench-devloop.sh
endif

.PHONY: migrate
migrate: ## Run auth then app migrations against DATABASE_URL
$(GO) run ./apps/web/cmd/server migrate
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ Each checkout gets its own compose project and database ports, derived from its
| | |
|---|---|
| `make dev` | run the server with `.env` |
| `make test` | `go test -race ./...` against this checkout's throwaway tmpfs Postgres (or `TEST_DATABASE_URL`), then per-boundary coverage floors |
| `make test` | `go test -race ./...` against this checkout's throwaway tmpfs Postgres (or `TEST_DATABASE_URL`), then per-package and per-test time budgets and per-boundary coverage floors |
| `make lint` | gofmt + golangci-lint (layering and test-import rules) + test file locations |
| `make lint-layering` / `make lint-tests` | prove the layering, test-location and coverage rules still reject planted violations |
| `make lint-layering` / `make lint-tests` | prove the layering, test-location, coverage and test-time rules still reject planted violations |
| `make ci` | everything CI runs |
| `make generate` | sqlc, mocks (`go generate`) and templ code generation |
| `make css` | rebuild both apps' `app.css` (pinned Tailwind standalone CLI) |
| `make bench-devloop` | timing tables for the dev loop: build per binary (with size and cold start), generators, lint per linter, probes, tests per package with and without `-race`; `BENCH_COLD=1` adds cold caches, `BENCH_BASE=<ref>` an A/B of the test run against that ref on the same machine |
| `make build` / `make docker` | static binary / production image |
| `make db-up` / `make db-down` | start / stop the local databases |
| `make env` | print this checkout's compose project, ports and `DATABASE_URL` |
Expand Down
1 change: 1 addition & 0 deletions apps/admin/internal/api/admins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ var adminRoutes = map[string]route{
// TestCreateAdminInviteBody covers every CreateAdminInvite rule: the address
// is normalised and the name defaults to its local part.
func TestCreateAdminInviteBody(t *testing.T) {
t.Parallel()
long := strings.Repeat("x", 244) + "@example.com" // 256 characters
for _, email := range []string{"", " ", "not an email", "Ada <ada@example.com>", "ada@", "@example.com", long} {
wantInvalid(t, "POST", r.AdminInvites, `{"email":"`+email+`","name":"Ada"}`, map[string]string{"email": emailRule})
Expand Down
2 changes: 2 additions & 0 deletions apps/admin/internal/api/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ var auditRoutes = map[string]route{
// TestListAuditLogQuery: filters are trimmed, the page size defaults, and the
// bounds are inclusive.
func TestListAuditLogQuery(t *testing.T) {
t.Parallel()
cases := []struct {
q url.Values
want validate.ListAuditLog
Expand All @@ -62,6 +63,7 @@ func TestListAuditLogQuery(t *testing.T) {

// TestListAuditLogInvalid covers every ListAuditLog rule.
func TestListAuditLogInvalid(t *testing.T) {
t.Parallel()
limit := map[string]string{"limit": "must be 1-100"}
cases := []struct {
q string
Expand Down
Loading
Loading