From 4b3784c0c4867e3e1ffea2f3a63f247b6322a035 Mon Sep 17 00:00:00 2001 From: Jalil Date: Tue, 10 Nov 2026 16:39:09 -0800 Subject: [PATCH] test(server): run the acceptance scenarios on Postgres as well as memory The acceptance scenarios in internal/server ran only on the in-memory store, which production does not use. Postgres was covered by the nine cases of the store conformance suite. The server test harness now takes its store from one place: the memory store by default, and an empty Postgres schema when DIFFERENTLY_TEST_POSTGRES_URL is set. The new postgrestest package creates that schema, migrates it and drops it when the test ends; the conformance suite uses it too, instead of dropping tables between subtests (it missed the secrets table). `make test-postgres` runs both packages against Postgres, one after the other because scheduler leadership is one advisory lock per database, and the CI job that already starts Postgres calls it. With DIFFERENTLY_TEST_POSTGRES_REQUIRED set, a missing database fails the run instead of skipping it. The whole server suite passes on Postgres (three runs in a row, with -race). It exposed no defect. Co-Authored-By: Claude Fable 5.2 Signed-off-by: Jalil --- .github/workflows/ci.yml | 6 +- AGENTS.md | 3 +- CHANGELOG.md | 9 ++ CONTRIBUTING.md | 2 +- Makefile | 13 ++- docs/acceptance-scenarios.md | 2 +- docs/architecture/README.md | 4 +- docs/backlog.md | 9 +- docs/maintaining.md | 4 +- docs/roadmap.md | 2 +- internal/server/server_test.go | 15 ++-- internal/server/store_test.go | 20 +++++ internal/store/postgres/postgres_test.go | 27 ++---- .../postgres/postgrestest/postgrestest.go | 83 +++++++++++++++++++ 14 files changed, 155 insertions(+), 44 deletions(-) create mode 100644 internal/server/store_test.go create mode 100644 internal/store/postgres/postgrestest/postgrestest.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c754cc..ddf1f63 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,8 +45,10 @@ jobs: - name: build, vet and test with -tags experimental_adapters run: make experimental + # The store conformance suite and the server acceptance scenarios, both + # against a real Postgres. A missing database fails the run, never skips it. postgres: - name: postgres conformance + name: postgres conformance and acceptance runs-on: ubuntu-latest services: postgres: @@ -61,7 +63,7 @@ jobs: - uses: actions/setup-go@v5 with: go-version-file: go.mod - - run: go test -count=1 ./internal/store/postgres/ + - run: make test-postgres env: DIFFERENTLY_TEST_POSTGRES_URL: postgres://postgres:test@localhost:5432/postgres?sslmode=disable diff --git a/AGENTS.md b/AGENTS.md index f36e129..f05bab6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,7 +27,8 @@ make verify It runs gofmt, go vet, Go tests with `-race`, the same build, vet and tests with `-tags experimental_adapters` (`make experimental`), the SDK tests and the Helm lint, including the replay of recorded OpenSandbox runtimes. For changes touching -`internal/store/postgres`, also run `make test-postgres`. For `console/`, run +`internal/store` or `internal/server`, also run `make test-postgres` (the +store conformance suite and the server acceptance scenarios on Postgres). For `console/`, run `npm run typecheck && npm run build && npm run build:server` there; for `site/` or any page it publishes from `docs/`, `npm run build` in `site/` (it fails on a broken link). For anything a real diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c98a61..c7c3f69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ All notable changes are recorded here. The format follows ## [Unreleased] — on `pre-release` +### Tests + +- The server acceptance scenarios (`internal/server`) run on Postgres as well + as the memory store: every pull request runs them against Postgres 17 in + the `postgres conformance and acceptance` job, and `make test-postgres` + runs them locally. They used to run only on the memory store, which + production does not use. The first run found no defect. A run that is meant + to use Postgres now fails, instead of skipping, when it has no database. + ## [0.1.0-alpha.3] — 2026-11-10 Third alpha, published as a GitHub pre-release. Not for production. It is diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5fecc1c..37e3600 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -90,7 +90,7 @@ Every commit needs a `Signed-off-by` line (`git commit -s`) certifying the ```bash make build # bin/differently-server, bin/differently make test # Go unit and integration tests with -race -make test-postgres # store conformance suite against a throwaway Postgres container +make test-postgres # store conformance and server acceptance tests against Postgres (a throwaway container, or DIFFERENTLY_TEST_POSTGRES_URL) make sdk-test # TypeScript and Python SDK tests make helm-lint # chart lint and template against ci/ value sets make verify # everything CI runs diff --git a/Makefile b/Makefile index 2cd6bd4..dd31e26 100644 --- a/Makefile +++ b/Makefile @@ -43,13 +43,22 @@ experimental: go vet -tags "$(EXPERIMENTAL_TAG) e2e live" ./... go test -race -count=1 -tags $(EXPERIMENTAL_TAG) ./... -# Runs the store conformance suite against a disposable Postgres container. +# Runs the store conformance suite and the server acceptance scenarios against +# Postgres. With DIFFERENTLY_TEST_POSTGRES_URL set it uses that database (each +# test gets its own schema); otherwise it starts a disposable container. +# The packages run one after another (-p 1): scheduler leadership is a single +# advisory lock per database. A missing database fails the run, never skips it. +PG_TEST := DIFFERENTLY_TEST_POSTGRES_REQUIRED=1 go test -race -count=1 -p 1 ./internal/store/postgres/ ./internal/server/ test-postgres: +ifdef DIFFERENTLY_TEST_POSTGRES_URL + $(PG_TEST) +else @docker rm -f differently-pg-test >/dev/null 2>&1 || true docker run -d --rm --name differently-pg-test -p $(PG_PORT):5432 -e POSTGRES_PASSWORD=test postgres:17-alpine >/dev/null @for i in $$(seq 1 30); do docker exec differently-pg-test pg_isready -U postgres >/dev/null 2>&1 && break; sleep 1; done - DIFFERENTLY_TEST_POSTGRES_URL=postgres://postgres:test@localhost:$(PG_PORT)/postgres?sslmode=disable go test -count=1 ./internal/store/postgres/; \ + DIFFERENTLY_TEST_POSTGRES_URL=postgres://postgres:test@localhost:$(PG_PORT)/postgres?sslmode=disable $(PG_TEST); \ status=$$?; docker rm -f differently-pg-test >/dev/null; exit $$status +endif sdk-test: cd sdk/typescript && npm ci --no-audit --no-fund && npm test diff --git a/docs/acceptance-scenarios.md b/docs/acceptance-scenarios.md index 178f057..36ced8a 100644 --- a/docs/acceptance-scenarios.md +++ b/docs/acceptance-scenarios.md @@ -2,7 +2,7 @@ | | | |---|---| -| Status | 2026-10-28, after the move to OpenSandbox removed the Firecracker node agent. Every test named below exists on this branch. CI runs the Go tests, including the Postgres conformance suite (`internal/store/postgres` `TestConformance`) in its `postgres conformance` job. | +| Status | 2026-10-28, after the move to OpenSandbox removed the Firecracker node agent. Every test named below exists on this branch. CI runs the Go tests on the memory store, and its `postgres conformance and acceptance` job runs the Postgres conformance suite (`internal/store/postgres` `TestConformance`) and every scenario test in `internal/server` again on Postgres 17. | | Evidence level | The server tests run in-process with fake backends. The `opensandbox` adapter is also checked against a real OpenSandbox server (release 1.2.0, Docker runtime, arm64 on Colima) by `TestLiveOpenSandbox`. The [end-to-end suite](../test/e2e/README.md) runs the real control plane (Postgres and `differently-server`) against a real OpenSandbox server with the Docker runtime and passed 10/10 on arm64 Colima on 2026-10-28. Earlier end-to-end results on real Firecracker microVMs tested the removed agent and no longer apply. OpenSandbox's Kubernetes and Fast Sandbox runtimes, cloud KVM, and live Daytona or Freestyle accounts are not validated. | Scenario definitions follow the product brief. Stateful sandbox behaviour and diff --git a/docs/architecture/README.md b/docs/architecture/README.md index b377d0d..bed89af 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -42,8 +42,8 @@ proven in production". | Area | Evidence today | Not yet done | |---|---|---| -| API, auth, quotas, policy, scheduler, reconciler | In-process integration tests against the real server, scheduler and memory store with fake pools (`internal/server/server_test.go`); audit fail-closed and metrics-listener tests (`internal/server/hardening_test.go`) | Multi-replica run against Postgres | -| Postgres store | Shared conformance suite `internal/store/storetest`; the `postgres conformance` CI job runs it against Postgres 17 and passed on `pre-release` on 2026-10-28 | That run predates migration `0002_drop_agents.sql`. Locally the suite runs only when `DIFFERENTLY_TEST_POSTGRES_URL` is set | +| API, auth, quotas, policy, scheduler, reconciler | In-process integration tests against the real server and scheduler with fake pools (`internal/server/server_test.go`), on the memory store and, in the `postgres conformance and acceptance` CI job, on Postgres 17; audit fail-closed and metrics-listener tests (`internal/server/hardening_test.go`) | Multi-replica run against Postgres | +| Postgres store | Shared conformance suite `internal/store/storetest`; the `postgres conformance and acceptance` CI job runs it, and the server acceptance scenarios, against Postgres 17 on every pull request | Locally both run only when `DIFFERENTLY_TEST_POSTGRES_URL` is set (`make test-postgres`) | | `opensandbox` adapter | Fake-server tests that cover the differences found between OpenSandbox's spec and its images (`internal/adapters/opensandbox/opensandbox_test.go`). Real Docker and Kubernetes responses recorded in `testdata/recordings` and replayed in every `go test`. `TestLiveOpenSandbox` (build tag `live`) passes against OpenSandbox release 1.2.0 on the Docker runtime (Colima, arm64) and the Kubernetes runtime (k3s on Azure) | The Fast Sandbox runtime (`runtime: fastsandbox`) is tested only with a fake server, so microVM isolation through Differently is unverified ([backlog E18](../backlog.md#e18-fast-sandbox-spike)) | | End-to-end suite (`test/e2e`) | Postgres, `differently-server` and a real OpenSandbox 1.2.0 server: 14/14 on the Docker runtime (arm64 Colima) and on the Kubernetes runtime (single-node k3s on Azure, x86_64, with a snapshot registry, signed ports and egress policy), 2026-10-29 ([results](../../test/e2e/README.md)) | Fast Sandbox; multi-node clusters and node drains | | Helm chart | `helm lint` and `helm template` pass for every CI value set; installed on single-node k3s on Azure next to OpenSandbox's charts, and the published `v0.1.0-alpha.1` chart installed from public artifacts alone ([single VM](../deploy/single-vm.md)) | Managed EKS, GKE and AKS; multi-node clusters | diff --git a/docs/backlog.md b/docs/backlog.md index 3ec5511..e24e2ed 100644 --- a/docs/backlog.md +++ b/docs/backlog.md @@ -476,10 +476,11 @@ E0.20. **Priority** P1. **Depends on** none. -- **E5.1 Conformance in CI.** The `postgres conformance` job in - `.github/workflows/ci.yml` exists; make it a required check, and make the - test fail instead of skipping when CI sets a flag (a skipped test passes - today). *Acceptance:* removing the database URL in CI turns the job red. +- **E5.1 Conformance in CI.** Partly done: the `postgres conformance and + acceptance` job in `.github/workflows/ci.yml` runs the conformance suite and + the server acceptance scenarios, and fails instead of skipping when the + database URL is missing (`DIFFERENTLY_TEST_POSTGRES_REQUIRED`). Still open: + make it a required check. - **E5.2 POC 6 (failover).** Run and publish; document recommended Postgres settings (TCP keepalives, synchronous replication). *Acceptance:* POC 6 criteria. diff --git a/docs/maintaining.md b/docs/maintaining.md index 1f5d2fc..dcb5c5a 100644 --- a/docs/maintaining.md +++ b/docs/maintaining.md @@ -21,7 +21,7 @@ control plane does with it. | Public API (a route, a field, an error) | `pkg/api`, `internal/server`, `api/openapi.yaml`, the SDKs | `internal/server` tests (the OpenAPI test keeps the router and spec in sync); SDK tests | | Placement, policy, classification | `internal/policy`, `internal/scheduler` | `internal/server/server_test.go` (acceptance scenarios A–L) | | Slots, the resume queue, preemption, lifecycle limits | `internal/scheduler/capacity.go`, `internal/store` (`HoldsPoolSlot`, `ReclaimSlot`), the model in `internal/capsim` | The capacity tests named in [capacity.md](scheduling/capacity.md); store conformance through both stores; rerun `go run ./cmd/differently-capsim` when a strategy changes | -| Quotas, admission, activation | `internal/store` (memory and Postgres) | `internal/store/storetest` through both stores; `make test-postgres` | +| Quotas, admission, activation | `internal/store` (memory and Postgres) | `internal/store/storetest` through both stores, and the acceptance scenarios in `internal/server` on either store; `make test-postgres` | | Reconciliation and recovery | `internal/scheduler` (`Reconcile`, `retry`) | Server tests using the fake backend; the e2e drift test | | A backend's behaviour | `internal/adapters/` | Its fake-server tests; for OpenSandbox also the recordings ([below](#recordings-real-backends-replayed-in-go-test)) | | A new provider kind | The adapter, then every registration list in [the provider catalog's checklist](providers/README.md#add-a-built-in-adapter) | `TestEveryAdapterKindHasATrustRule`, `TestRegisteredKindsMatchConfig`; its fake-server tests, which run only with `-tags experimental_adapters` (`make experimental`) | @@ -53,7 +53,7 @@ the one before; repeating a rung for reassurance adds nothing. |---|---|---| | Package tests | `go test ./internal/adapters/opensandbox/` (or the package you changed) | Every change, while iterating | | CI locally | `make verify` | Before every push: gofmt, vet, tests with `-race`, the same again with the experimental adapters (`make experimental`: build, vet and test with `-tags experimental_adapters`), SDK tests, Helm lint | -| Postgres store | `make test-postgres` (needs Docker) | Changes in `internal/store` | +| Postgres store | `make test-postgres` (Docker, or `DIFFERENTLY_TEST_POSTGRES_URL` for a database you already run): the conformance suite and the server acceptance scenarios | Changes in `internal/store`, `internal/server` or `internal/scheduler` | | Recorded runtimes | part of `go test` (`TestReplayRecordings`) | Any OpenSandbox adapter change: real responses from the Docker and Kubernetes runtimes, no server | | Live adapter | `OPENSANDBOX_URL=… go test -tags live -run TestLiveOpenSandbox ./internal/adapters/opensandbox/`; for the managed providers `DAYTONA_API_KEY=… go test -tags live -run TestLiveDaytona ./internal/adapters/daytona/` and `FREESTYLE_API_KEY=… go test -tags live -run TestLiveFreestyle ./internal/adapters/freestyle/` (a few small sandboxes, all deleted) | A new OpenSandbox call or version; re-recording; any Daytona or Freestyle adapter change | | Real stack, local | `make e2e-prepare e2e-up e2e` | Behaviour across the API, scheduler, store and a real OpenSandbox server | diff --git a/docs/roadmap.md b/docs/roadmap.md index e0a2c7c..bbc9a6a 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -37,7 +37,7 @@ quota and placement policy, runnable on a laptop. | Placement policy engine (filter, rank, fail-closed external) | ✅ | `internal/policy` | | Scheduler, retries, reconciler | ✅ | `internal/scheduler` (no package-level tests; covered by server tests) | | Memory store | ✅ | `internal/store/memory.go` | -| Postgres store, migrations, advisory-lock leadership | 🟡 | Implemented; the conformance suite needs `DIFFERENTLY_TEST_POSTGRES_URL` and runs in the CI `postgres conformance` job; multi-replica failover not yet run ([E5](backlog.md#e5-postgres-in-ci-failover-and-scale)) | +| Postgres store, migrations, advisory-lock leadership | 🟡 | Implemented; the conformance suite and the server acceptance scenarios need `DIFFERENTLY_TEST_POSTGRES_URL` and run in the CI `postgres conformance and acceptance` job; multi-replica failover not yet run ([E5](backlog.md#e5-postgres-in-ci-failover-and-scale)) | | CI and release workflows | 🟡 | `.github/workflows/ci.yml`, `release.yml`; encode the branch model | | Tamper-evident audit chain, verify endpoint, JSON-lines mirror | ✅ | `internal/audit`, `internal/auditlog` | | Dev mode with `devhost` pool | ✅ | `differently-server --dev` | diff --git a/internal/server/server_test.go b/internal/server/server_test.go index dfa7bef..5ad012b 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -36,14 +36,15 @@ type harness struct { srv *httptest.Server api *server.Server sched *scheduler.Scheduler - store *store.Memory + store store.Store // the store itself, under any wrapper a test added fc *fake.Adapter ext *fake.Adapter pools *pools.Manager } -// newHarness runs the real server, scheduler and memory store with two fake -// pools: "fc" (private, 4 CPU) and "burst" (external, unbounded). +// newHarness runs the real server and scheduler on the test store (memory, or +// Postgres when DIFFERENTLY_TEST_POSTGRES_URL is set) with two fake pools: +// "fc" (private, 4 CPU) and "burst" (external, unbounded). func newHarness(t *testing.T, policy api.PlacementPolicy) *harness { t.Helper() return newHarnessWith(t, policy, nil, nil) @@ -61,15 +62,15 @@ func newHarnessWith(t *testing.T, policy api.PlacementPolicy, tweak func(*config tweak(&cfg) } log := slog.New(slog.NewTextHandler(io.Discard, nil)) - mem := store.NewMemory() - var st store.Store = mem + base := newTestStore(t) + st := base if wrap != nil { - st = wrap(mem) + st = wrap(base) } rec, _ := auditlog.New(st, log, "") box, _ := secrets.New(make([]byte, secrets.KeySize)) pm := pools.NewManager(pools.Deps{Store: st, Logger: log, Resolve: server.CredentialResolver(st, box)}) - h := &harness{t: t, store: mem, pools: pm, + h := &harness{t: t, store: base, pools: pm, fc: fake.New(fake.Options{Trust: api.TrustPrivate, TotalCPU: 4, TotalMemoryMiB: 16384}), ext: fake.New(fake.Options{Trust: api.TrustExternal, Features: []string{adapter.FeatureExec, adapter.FeatureFiles, adapter.FeaturePorts}}), } diff --git a/internal/server/store_test.go b/internal/server/store_test.go new file mode 100644 index 0000000..04af373 --- /dev/null +++ b/internal/server/store_test.go @@ -0,0 +1,20 @@ +package server_test + +import ( + "testing" + + "github.com/different-ai/differently/internal/store" + "github.com/different-ai/differently/internal/store/postgres/postgrestest" +) + +// newTestStore returns the store the acceptance scenarios run on: the memory +// store by default, and an empty Postgres schema when +// DIFFERENTLY_TEST_POSTGRES_URL is set, so the same scenarios cover the store +// production uses. +func newTestStore(t *testing.T) store.Store { + t.Helper() + if dsn := postgrestest.DSN(t); dsn != "" { + return postgrestest.Open(t, dsn) + } + return store.NewMemory() +} diff --git a/internal/store/postgres/postgres_test.go b/internal/store/postgres/postgres_test.go index ef7527c..2045fa2 100644 --- a/internal/store/postgres/postgres_test.go +++ b/internal/store/postgres/postgres_test.go @@ -1,34 +1,19 @@ -package postgres +package postgres_test import ( - "context" - "os" "testing" "github.com/different-ai/differently/internal/store" + "github.com/different-ai/differently/internal/store/postgres/postgrestest" "github.com/different-ai/differently/internal/store/storetest" ) // TestConformance runs the shared store suite against a real database when -// DIFFERENTLY_TEST_POSTGRES_URL is set (each subtest gets a fresh schema). +// DIFFERENTLY_TEST_POSTGRES_URL is set (each subtest gets an empty schema). func TestConformance(t *testing.T) { - dsn := os.Getenv("DIFFERENTLY_TEST_POSTGRES_URL") + dsn := postgrestest.DSN(t) if dsn == "" { - t.Skip("set DIFFERENTLY_TEST_POSTGRES_URL to run the postgres conformance suite") + t.Skip("set " + postgrestest.EnvDSN + " to run the postgres conformance suite") } - storetest.Run(t, func(t *testing.T) store.Store { - ctx := context.Background() - s, err := Open(ctx, dsn) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { s.Close() }) - for _, tbl := range []string{"sandboxes", "snapshots", "audit_events", "agents", "api_keys", "projects", "organizations", "pools", "policies", "meta", "schema_migrations"} { - _, _ = s.pool.Exec(ctx, "DROP TABLE IF EXISTS "+tbl+" CASCADE") - } - if err := s.Migrate(ctx); err != nil { - t.Fatal(err) - } - return s - }) + storetest.Run(t, func(t *testing.T) store.Store { return postgrestest.Open(t, dsn) }) } diff --git a/internal/store/postgres/postgrestest/postgrestest.go b/internal/store/postgres/postgrestest/postgrestest.go new file mode 100644 index 0000000..54e3907 --- /dev/null +++ b/internal/store/postgres/postgrestest/postgrestest.go @@ -0,0 +1,83 @@ +// Package postgrestest gives tests an empty Postgres store. +// +// Each store lives in its own schema of the database the DSN names, so tests +// do not see each other's rows and nothing has to be dropped between them. +// Scheduler leadership is an advisory lock on the whole database, though: run +// the packages that use this one after another (go test -p 1), not at once. +package postgrestest + +import ( + "context" + "crypto/rand" + "encoding/hex" + "net/url" + "os" + "testing" + + "github.com/jackc/pgx/v5" + + "github.com/different-ai/differently/internal/store/postgres" +) + +// EnvDSN names the environment variable with the test database's DSN. +const EnvDSN = "DIFFERENTLY_TEST_POSTGRES_URL" + +// EnvRequire, when set, makes a missing DSN a failure instead of a skip, so a +// CI job that is meant to run against Postgres cannot pass without it. +const EnvRequire = "DIFFERENTLY_TEST_POSTGRES_REQUIRED" + +// DSN returns the test database's DSN, or "" when there is none. It fails the +// test when EnvRequire is set and the DSN is missing. +func DSN(t testing.TB) string { + t.Helper() + dsn := os.Getenv(EnvDSN) + if dsn == "" && os.Getenv(EnvRequire) != "" { + t.Fatalf("%s is set but %s is empty: this run must use Postgres", EnvRequire, EnvDSN) + } + return dsn +} + +// Open returns a migrated store in a new, empty schema of the database at +// dsn. The schema is dropped and the store closed when the test ends. +func Open(t testing.TB, dsn string) *postgres.Store { + t.Helper() + ctx := context.Background() + b := make([]byte, 8) + if _, err := rand.Read(b); err != nil { + t.Fatal(err) + } + schema := "t_" + hex.EncodeToString(b) + + admin, err := pgx.Connect(ctx, dsn) + if err != nil { + t.Fatalf("connect to %s: %v", EnvDSN, err) + } + if _, err := admin.Exec(ctx, "CREATE SCHEMA "+schema); err != nil { + t.Fatalf("create schema: %v", err) + } + _ = admin.Close(ctx) + + u, err := url.Parse(dsn) + if err != nil { + t.Fatalf("%s must be a postgres:// URL: %v", EnvDSN, err) + } + q := u.Query() + q.Set("search_path", schema) + u.RawQuery = q.Encode() + + s, err := postgres.Open(ctx, u.String()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _ = s.Close() + if c, err := pgx.Connect(context.Background(), dsn); err == nil { + _, _ = c.Exec(context.Background(), "DROP SCHEMA "+schema+" CASCADE") + _ = c.Close(context.Background()) + } + }) + if err := s.Migrate(ctx); err != nil { + t.Fatalf("migrate: %v", err) + } + return s +}