From 5cd10bf2a9871cfb298f4a46b6a36797e73326c9 Mon Sep 17 00:00:00 2001 From: Bretton Date: Wed, 29 Jul 2026 03:35:03 -0700 Subject: [PATCH] =?UTF-8?q?test(testkit):=20complete=20the=20harness=20?= =?UTF-8?q?=E2=80=94=20PDS=20client,=20cursor-gated=20firehose,=20AppView?= =?UTF-8?q?=20client?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 task 4. pds.go consolidates five drifted PDS-client-factory adapters, both createPDSAccount copies, and the hand-rolled XRPC clients (TID() now wraps indigo's TIDClock — the old generateTID never emitted a valid TID). firehose.go replaces ten broken subscribeToJetstream copies: gorilla marks a websocket corrupt after a read-deadline expiry, so their continue-on-timeout idiom silently capped every ' 30s' wait at ~5s. The testkit subscriber captures its cursor at construction, re-dials on any read error resuming from last time_us with a same-microsecond dedupe set, counts cursor-predating discards, and fails on buffer overflow. appview.go adds typed XRPC status errors (retry-vs-terminal for WaitFor), ConsumerHealth diagnostics, and transient-aware wait adapters. Full-panel reviewed (Codex + 3 specialty streams + security: clean); ~28 fixes applied. testkit: 117 tests, -race -shuffle green. make ci green: 3429 tests, 14 allowlisted skips. Co-Authored-By: Claude Fable 5 --- loop_state.md | 19 +- tests/e2e/user_signup_test.go | 17 +- tests/integration/community_e2e_test.go | 5 +- tests/testkit/appview.go | 548 ++++++++++++++++ tests/testkit/appview_test.go | 546 ++++++++++++++++ tests/testkit/firehose.go | 820 ++++++++++++++++++++++++ tests/testkit/firehose_pin_test.go | 149 +++++ tests/testkit/firehose_test.go | 658 +++++++++++++++++++ tests/testkit/harness_test.go | 31 +- tests/testkit/pds.go | 706 ++++++++++++++++++++ tests/testkit/pds_test.go | 564 ++++++++++++++++ tests/testkit/testkit.go | 208 +++++- tests/testkit/testkit_test.go | 73 ++- 13 files changed, 4319 insertions(+), 25 deletions(-) create mode 100644 tests/testkit/appview.go create mode 100644 tests/testkit/appview_test.go create mode 100644 tests/testkit/firehose.go create mode 100644 tests/testkit/firehose_pin_test.go create mode 100644 tests/testkit/firehose_test.go create mode 100644 tests/testkit/pds.go create mode 100644 tests/testkit/pds_test.go diff --git a/loop_state.md b/loop_state.md index 8efe0f0..2170a98 100644 --- a/loop_state.md +++ b/loop_state.md @@ -40,7 +40,7 @@ Stop the loop when every task is done, or on any blocked task. | 1 | Gate green as imported: run `make ci`, fix what surfaces, record baseline timing + allowlist | 0 ⛩ | S | done | (no diff) | GREEN FIRST RUN: 3307 tests, 3288 pass, 0 fail, 19 skips (all allowlisted, 21 entries → 2 unused are ~conditional), 2.2 min WARM caches. No fixes → no review stream | | 2 | Move public-network tests to tests/live/ (+`live` tag, move-only); flip compose nets `internal: true` + module-cache pre-pull; cold-cache egress-blocked `make ci` green | 0 ⛩ | S | done | (see git log) | COLD egress-blocked GREEN 2:21; warm 1:57. 16 funcs → tests/live (4 files + helpers). Egress block found 4 runtime deps, not 1: Turnstile URL hardcoded (→ WithSiteverifyURL, dev-gated env), PLC healthcheck redirected to public web (→ /_health), 3 blueskypost tests dialed public.api.bsky.app (→ blueskyAPI seam), DNS-dependent 404 test (→ DID). Review: Codex "good" + Opus "safe as-is"; 9 fixes applied (unfurl E2E de-mocked via httptest OG, gate vets -tags live, non-dev override warns, prod default pinned, stub 127.0.0.1-only, GOPROXY=off, 404/500 handler tests, golden 200 parse, live cache purge+method asserts). 3281 tests / 14 allowlisted skips | | 3 | testkit core: db.go (template-clone, advisory lock), wait.go (WaitFor/Holds, terminal errs), fixtures.go (UniqueID run-prefix), scripts/test-db-prepare.sh, scripts/test-audit.sh (warn mode) + testkit's own tests | 1 | S | done | (see git log) | 47 testkit tests, -race -shuffle clean; clone ~30ms (cheaper than spec est). Review: Codex needs-work (5 high) + Opus not-safe-as-is (2 high) → 15-item batch applied: template-drop name rail (found a panic-in-error-path bug), drop-before-close teardown, 55006 retry, sweep de-FORCEd + error-accumulating, wait-primitive deadline contract, max_connections=200 + ParallelBudget (CI: -parallel 53, inert until ph.3), bounded lock wait w/ holder diagnostics, grep -H audit fix. ADJUDICATION: Codex RIGHT / Opus WRONG on advisory-lock leak (async cancel can grant lock after Go sees error; fixed via Conn.Raw(ErrBadConn) eviction). make ci GREEN 3347 tests / 14 allowlisted skips | -| 4 | testkit pds.go (absorb 4 factories, createPDSAccount×2, XRPC clients), firehose.go (generic cursor-gated), appview.go; fix 5 handle-collision sites | 1 ⛩ | S | pending | | then FULL PANEL review of tests/testkit (incl. pragma:security — PDS creds) | +| 4 | testkit pds.go (absorb 4 factories, createPDSAccount×2, XRPC clients), firehose.go (generic cursor-gated), appview.go; fix 5 handle-collision sites | 1 ⛩ | S | done | (see git log) | PHASE 1 COMPLETE. Worker found ALL 10 legacy subscribeToJetstream copies broken (gorilla corrupt-after-deadline → every "30s wait" gave up at ~5s — explains historical flakes). 5 factories not 4 (comments missed by helpers.go); old generateTID never emitted valid TIDs (now wraps indigo TIDClock); dep rule is TRANSITIVE (atproto/pds imports core/blobs → reimplemented). FULL PANEL (Codex+CR+SFH+TA+security): security CLEAN; ~28-item batch applied — same-time_us dedupe set, deadline-bounded dials, all-read-errors-recover, discard counting (clock-skew diagnosis), overflow=failure, lock-free blocking I/O, Event.Raw()/Into() (unblocks consumer migrations), XRPC-shaped 404 classification, PendingIfUnavailable, ConsumerHealth + WithConsumerHealth, option-pattern unification, testkit.Main(m, Require*...). CR false-positive on ParallelBudget wiring (discarded). testkit 117 tests -race -shuffle green; make ci GREEN 3429/14 | | 5 | Kill the lies: delete 6 debt tests; lexicon validator stops generating defs-only subtests (retire 8 allowlist entries); move 2 ratelimit files to internal/api/middleware (T0); fold tests/unit into internal/core/communities | 2 | M | pending | | | | 6 | Split multi-tier files by test func (manifest in commit msg); add build tags in place; retarget Makefile to tags; delete -short/testing.Short(); delete test-all | 2 ⛩ | S | pending | | identity_resolution, bluesky_post (what's left post-task-2), post_unfurl | | 7 | Migrate setupTestDB call sites → testkit.DB(t), batch 1 (~25 files) + delete their DELETE FROMs/cleanups | 3 | M | pending | | mechanical; gates are the reviewer | @@ -131,3 +131,20 @@ Stop the loop when every task is done, or on any blocked task. until task 6. REVIEWER CALIBRATION: on the lock-leak disagreement Codex was right, Opus wrong (missed the failure path, traced only cancellation) — weight Codex on DB/concurrency semantics. +- **From task 4 (testkit complete — MIGRATION CONTRACTS for tasks 7-8+)**: + canonical package setup is `testkit.Main(m, testkit.RequirePostgres, + testkit.RequirePDS, ...)` (opt-in probes; see testkit.go Layout doc). + Consumer-feeding tests use `ev.Into(&jetstreamEvent)` — never reconstruct + wire structs by hand. Options are component-qualified: WithAppViewBearer, + WithAppViewURL, WithFirehoseCursor. DeleteRecord is idempotent-silent on + the PDS (200 + no commit for missing rkey) — pair Await with + DeleteExistingRecord. `package testkit_test` (external test pkg) is the + sanctioned way to pin testkit's duplicated types against internal pkgs it + may not import (firehose_pin_test.go is the example). GORILLA TRAP + (tree-wide): any websocket loop that `continue`s after a read-deadline + timeout has a latent ~5s ceiling — the conn is CORRUPT after expiry and + must be re-dialled; all 10 legacy subscribeToJetstream copies had this + bug, so historical "Timeout: No Jetstream event received" failures were + often this, not slow indexing. testkit firehose recovers on ALL read + errors (only undecodable frames are terminal), counts cursor-predating + discards (clock-skew diagnosis), and FAILS on pending-buffer overflow. diff --git a/tests/e2e/user_signup_test.go b/tests/e2e/user_signup_test.go index a372ef9..a755607 100644 --- a/tests/e2e/user_signup_test.go +++ b/tests/e2e/user_signup_test.go @@ -12,6 +12,8 @@ import ( "testing" "time" + "Coves/tests/testkit" + _ "github.com/lib/pq" "github.com/pressly/goose/v3" ) @@ -66,8 +68,9 @@ func TestE2E_UserSignup(t *testing.T) { // Test 1: Create account on PDS t.Run("Create account on PDS and verify indexing", func(t *testing.T) { - handle := fmt.Sprintf("alice-%d.local.coves.dev", time.Now().Unix()) - email := fmt.Sprintf("alice-%d@test.com", time.Now().Unix()) + label := testkit.UniqueIDWithPrefix(t, "alice") + handle := label + ".local.coves.dev" + email := label + "@test.com" t.Logf("Creating account: %s", handle) @@ -107,8 +110,9 @@ func TestE2E_UserSignup(t *testing.T) { // Test 2: Idempotency (verify same user from multiple API calls) t.Run("Idempotent indexing on duplicate events", func(t *testing.T) { - handle := fmt.Sprintf("bob-%d.local.coves.dev", time.Now().Unix()) - email := fmt.Sprintf("bob-%d@test.com", time.Now().Unix()) + label := testkit.UniqueIDWithPrefix(t, "bob") + handle := label + ".local.coves.dev" + email := label + "@test.com" // Create account via AppView signup endpoint did, err := createPDSAccount(t, handle, email, "test1234") @@ -150,8 +154,9 @@ func TestE2E_UserSignup(t *testing.T) { handles := make([]string, numUsers) for i := 0; i < numUsers; i++ { - handle := fmt.Sprintf("user%d-%d.local.coves.dev", i, time.Now().Unix()) - email := fmt.Sprintf("user%d-%d@test.com", i, time.Now().Unix()) + label := testkit.UniqueIDWithPrefix(t, fmt.Sprintf("user%d", i)) + handle := label + ".local.coves.dev" + email := label + "@test.com" did, err := createPDSAccount(t, handle, email, "test1234") if err != nil { diff --git a/tests/integration/community_e2e_test.go b/tests/integration/community_e2e_test.go index cd08baf..cdf947e 100644 --- a/tests/integration/community_e2e_test.go +++ b/tests/integration/community_e2e_test.go @@ -8,6 +8,7 @@ import ( "Coves/internal/core/communities" "Coves/internal/core/users" "Coves/internal/db/postgres" + "Coves/tests/testkit" "bytes" "context" "database/sql" @@ -174,7 +175,7 @@ func TestCommunity_E2E(t *testing.T) { t.Run("1. Write-Forward to PDS", func(t *testing.T) { // Use shorter names to avoid "Handle too long" errors // atProto handles max: 63 chars, format: c-name.coves.social - communityName := fmt.Sprintf("e2e-%d", time.Now().Unix()) + communityName := testkit.UniqueIDWithPrefix(t, "e2e") createReq := communities.CreateCommunityRequest{ Name: communityName, @@ -377,7 +378,7 @@ func TestCommunity_E2E(t *testing.T) { // - createdByDid: from JWT token (authenticated user) // - hostedByDid: from instance configuration (security: prevents spoofing) createReq := map[string]interface{}{ - "name": fmt.Sprintf("xrpc-%d", time.Now().Unix()), + "name": testkit.UniqueIDWithPrefix(t, "xrpc"), "displayName": "XRPC E2E Test", "description": "Testing true end-to-end flow", "visibility": "public", diff --git a/tests/testkit/appview.go b/tests/testkit/appview.go new file mode 100644 index 0000000..f208d00 --- /dev/null +++ b/tests/testkit/appview.go @@ -0,0 +1,548 @@ +package testkit + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" +) + +// The XRPC client, and the AppView it talks to. +// +// XRPCClient is the one HTTP/JSON client in the kit: appview.go uses it against +// the AppView, pds.go uses it against the PDS, and neither hand-rolls a request. +// It is deliberately small — headers, a JSON body, a typed error — because the +// interesting behaviour belongs to the callers. +// +// WHY THE ERRORS ARE TYPED +// +// wait.go's Probe contract splits every failure in two: "not yet" is +// (false, nil) and gets retried, anything else is terminal and fails the test +// immediately with the reason attached. Making that split correctly needs the +// HTTP status, so a client that returns fmt.Errorf("unexpected status 401") +// forces every probe to either string-match or give up and retry everything — +// which is how "the session was rejected" becomes "timed out after 30s waiting +// for the post to appear". StatusError carries the status, the XRPC error name +// and the body; PendingIfNotFound turns the common case into one line. + +// maxErrorBody bounds how much of a failing response is captured into an error +// message. A stack trace or an HTML error page is useful; a paginated feed that +// answered 500 halfway through is not worth a megabyte in the test log. +const maxErrorBody = 64 << 10 + +// defaultXRPCTimeout bounds a single request. Long enough for a blob upload on a +// loaded CI machine, short enough that a hung service fails the test instead of +// the go test timeout killing the whole binary with no attribution. +const defaultXRPCTimeout = 30 * time.Second + +// StatusError is a non-2xx answer from an XRPC endpoint. +// +// Both the transport status and the lexicon-level error name are kept: a PDS +// answers 400 with error "InvalidRequest" for a malformed record and 400 with +// "RecordNotFound" for a missing one, so the status alone cannot tell a test +// what happened. +type StatusError struct { + // Method is the NSID that was called, e.g. "com.atproto.repo.createRecord". + Method string + StatusCode int + // XRPCError and XRPCMessage come from the {"error", "message"} body every + // atProto service returns on failure. Both are empty when the body was not + // XRPC-shaped — an HTML 502 from a proxy, for instance. + XRPCError string + XRPCMessage string + // XRPCShaped records whether the body was an XRPC error envelope at all. + // + // It is the difference between a service answering "no such record" and a + // router answering "no such route". Both are 404s, and only the first is + // worth waiting on: see IsNotFound. + XRPCShaped bool + // Body is the raw response, truncated to maxErrorBody. + Body string +} + +func (e *StatusError) Error() string { + var b strings.Builder + fmt.Fprintf(&b, "%s: HTTP %d", e.Method, e.StatusCode) + if e.XRPCError != "" { + fmt.Fprintf(&b, " %s", e.XRPCError) + } + switch { + case e.XRPCMessage != "": + fmt.Fprintf(&b, ": %s", e.XRPCMessage) + case e.Body != "": + fmt.Fprintf(&b, ": %s", e.Body) + } + return b.String() +} + +// StatusOf returns the HTTP status err carries, or 0 if it is not a +// StatusError — a connection refused, a DNS failure, a cancelled context. +func StatusOf(err error) int { + var se *StatusError + if errors.As(err, &se) { + return se.StatusCode + } + return 0 +} + +// IsStatus reports whether err is a StatusError with the given status. +func IsStatus(err error, code int) bool { return StatusOf(err) == code } + +// IsNotFound reports whether err is a service saying the thing is not there. +// +// A 404 ALONE IS NOT ENOUGH, and the distinction is load-bearing. A mistyped +// NSID, a route that was never registered, or a reverse proxy in front of the +// wrong upstream all answer 404 with a plain-text body — chi's is literally +// "404 page not found". Treating those as "not indexed yet" makes every such +// typo cost the full WaitFor timeout and then report the wrong problem: the +// wait says the record never appeared, when the truth is that nothing ever +// asked for it. So a 404 counts only when the body is an XRPC error envelope, +// which is what a service that understood the request produces. +// +// atProto also expresses "no such record" as a 400 with the XRPC error name +// RecordNotFound, so that spelling counts wherever it appears — otherwise a +// probe waiting for a record to be indexed would treat the PDS' own "not there +// yet" as terminal. +func IsNotFound(err error) bool { + var se *StatusError + if !errors.As(err, &se) { + return false + } + switch { + case strings.EqualFold(se.XRPCError, "RecordNotFound"), strings.EqualFold(se.XRPCError, "NotFound"): + return true + case se.StatusCode == http.StatusNotFound: + return se.XRPCShaped + default: + return false + } +} + +// IsTransient reports whether err is a service that is momentarily unable to +// answer rather than one that has answered. +// +// These are the statuses a restarting or rate-limited service returns: 429, and +// the gateway family 502/503/504. They say nothing about the request, so a +// wait that is going to keep asking anyway should keep asking. +func IsTransient(err error) bool { + switch StatusOf(err) { + case http.StatusTooManyRequests, http.StatusBadGateway, + http.StatusServiceUnavailable, http.StatusGatewayTimeout: + return true + default: + return false + } +} + +// PendingIfNotFound converts a lookup error into a Probe result: a service +// saying "not there" is "not yet", and everything else is terminal. +// +// It is the body of nearly every WaitFor probe against a serving endpoint: +// +// testkit.WaitFor(t, 10*time.Second, func() (bool, error) { +// err := appview.Query(ctx, "social.coves.actor.getProfile", params, &got) +// return testkit.PendingIfNotFound(err) +// }, testkit.WithDescription("profile for %s indexed", did)) +// +// A nil error reports done, so returning it directly is also correct when err +// may be nil. +// +// It is STRICT on purpose: a 401, a 400, a 500 and a bare router 404 all fail +// the wait immediately with the response attached. Use it whenever the service +// under observation is expected to stay up for the duration of the wait, which +// is every steady-state contract. When the wait deliberately spans a restart — +// the reliability suite's cursor-resume and replay cases — use +// PendingIfUnavailable instead. +func PendingIfNotFound(err error) (bool, error) { + switch { + case err == nil: + return true, nil + case IsNotFound(err): + return false, nil + default: + return false, err + } +} + +// PendingIfUnavailable is PendingIfNotFound plus tolerance for a service that is +// momentarily down: 429 and the 502/503/504 gateway family are "not yet" as +// well. +// +// Use it ONLY where a restart is part of what the test is doing. Everywhere else +// the strict version is what you want, because a 503 that nobody expected is a +// finding, and converting it into thirty seconds of patient retrying is how it +// stops being one. +// +// Retry-After is not honoured; the WaitFor poll interval governs. Nothing in +// this stack sends it, and guessing at a server's pacing hint would make the +// wait's timing depend on a header no test controls. +func PendingIfUnavailable(err error) (bool, error) { + switch { + case err == nil: + return true, nil + case IsNotFound(err), IsTransient(err): + return false, nil + default: + return false, err + } +} + +// --------------------------------------------------------------------------- +// XRPC client +// --------------------------------------------------------------------------- + +// XRPCClient calls XRPC endpoints on one service, optionally authenticated. +// +// Zero HTTP is exposed to callers: give it an NSID, get a decoded response or a +// StatusError. It uses its own http.Client rather than http.DefaultClient, +// which the suite has historically mutated in places. +type XRPCClient struct { + // BaseURL is the service root, without a trailing slash or the /xrpc path. + BaseURL string + // Bearer, when set, is sent as the Authorization header. + Bearer string + // HTTP is the transport. Never nil for a client built by NewXRPCClient. + HTTP *http.Client +} + +// NewXRPCClient builds a client for a service root. +func NewXRPCClient(baseURL string) *XRPCClient { + return &XRPCClient{ + BaseURL: trimURL(baseURL), + HTTP: &http.Client{Timeout: defaultXRPCTimeout}, + } +} + +// WithBearer returns a copy of the client authenticated as the holder of token. +// +// A copy, not a mutation: a test that holds one client per identity must not be +// able to change what another identity's client sends. +func (c *XRPCClient) WithBearer(token string) *XRPCClient { + clone := *c + clone.Bearer = token + return &clone +} + +// URL renders the absolute URL of an XRPC method on this service. +func (c *XRPCClient) URL(nsid string) string { + return c.BaseURL + "/xrpc/" + nsid +} + +// Query performs a GET against an XRPC query method, decoding a JSON response +// into out. A nil out discards the body. +func (c *XRPCClient) Query(ctx context.Context, nsid string, params url.Values, out any) error { + target := c.URL(nsid) + if len(params) > 0 { + target += "?" + params.Encode() + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil) + if err != nil { + return fmt.Errorf("%s: building request: %w", nsid, err) + } + return c.do(req, nsid, out) +} + +// Procedure performs a POST against an XRPC procedure method, sending in as +// JSON and decoding the response into out. Either may be nil. +func (c *XRPCClient) Procedure(ctx context.Context, nsid string, in, out any) error { + var body io.Reader + if in != nil { + encoded, err := json.Marshal(in) + if err != nil { + return fmt.Errorf("%s: encoding request: %w", nsid, err) + } + body = bytes.NewReader(encoded) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.URL(nsid), body) + if err != nil { + return fmt.Errorf("%s: building request: %w", nsid, err) + } + if in != nil { + req.Header.Set("Content-Type", "application/json") + } + return c.do(req, nsid, out) +} + +// Upload performs a POST whose body is raw bytes under the given content type, +// for the binary procedures (com.atproto.repo.uploadBlob). +// +// contentType must be the payload's concrete MIME type. A PDS enforcing the +// granular blob:*/* scope matches the granted accept patterns against it, and +// "*/*" matches nothing — an upload that sends a wildcard fails with a scope +// error that says nothing about content types. +func (c *XRPCClient) Upload(ctx context.Context, nsid, contentType string, data []byte, out any) error { + if contentType == "" || strings.Contains(contentType, "*") { + return fmt.Errorf("%s: contentType must be a concrete MIME type such as \"image/png\", got %q", nsid, contentType) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.URL(nsid), bytes.NewReader(data)) + if err != nil { + return fmt.Errorf("%s: building request: %w", nsid, err) + } + req.Header.Set("Content-Type", contentType) + return c.do(req, nsid, out) +} + +// Get performs a GET against a plain path on the service, outside the /xrpc +// namespace: /health, /health/consumers, the OAuth metadata documents. +// +// path is joined to the base URL as given, so it must start with "/". +func (c *XRPCClient) Get(ctx context.Context, path string, out any) error { + if !strings.HasPrefix(path, "/") { + return fmt.Errorf("testkit: path %q must start with \"/\"", path) + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.BaseURL+path, nil) + if err != nil { + return fmt.Errorf("%s: building request: %w", path, err) + } + return c.do(req, path, out) +} + +// Health calls the service's _health endpoint. +// +// It asserts only that the service answered 2xx. Both the AppView and the PDS +// answer this endpoint with a version document and no status field, so there is +// nothing further to check here — in particular, an AppView whose consumers have +// all stalled still answers /xrpc/_health with a 200. Consumer liveness is a +// separate question with a separate endpoint: see AppView.ConsumerHealth. +func (c *XRPCClient) Health(ctx context.Context) error { + return c.Query(ctx, "_health", nil, nil) +} + +func (c *XRPCClient) do(req *http.Request, nsid string, out any) error { + if c.Bearer != "" { + req.Header.Set("Authorization", "Bearer "+c.Bearer) + } + req.Header.Set("Accept", "application/json") + + resp, err := c.HTTP.Do(req) + if err != nil { + return fmt.Errorf("%s: %w", nsid, err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return newStatusError(nsid, resp) + } + if out == nil { + // Drained rather than abandoned, so the connection returns to the pool + // instead of being torn down after every discarded response. + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, maxErrorBody)) + return nil + } + if err := json.NewDecoder(resp.Body).Decode(out); err != nil { + return fmt.Errorf("%s: decoding %d response: %w", nsid, resp.StatusCode, err) + } + return nil +} + +// newStatusError reads a failing response into a StatusError, parsing the XRPC +// error envelope when there is one. +func newStatusError(nsid string, resp *http.Response) *StatusError { + body, readErr := io.ReadAll(io.LimitReader(resp.Body, maxErrorBody)) + se := &StatusError{ + Method: nsid, + StatusCode: resp.StatusCode, + Body: strings.TrimSpace(string(body)), + } + if readErr != nil { + // Said rather than swallowed. An empty Body reads as "the service + // answered with nothing", which would be a lie about a response that was + // cut off mid-transfer — and the difference matters when the next + // question is whether the service is healthy. + se.Body = strings.TrimSpace(se.Body) + + fmt.Sprintf(" [body truncated: %v]", readErr) + return se + } + + var envelope struct { + Error string `json:"error"` + Message string `json:"message"` + } + if err := json.Unmarshal(body, &envelope); err == nil && envelope.Error != "" { + se.XRPCError = envelope.Error + se.XRPCMessage = envelope.Message + se.XRPCShaped = true + } + return se +} + +// --------------------------------------------------------------------------- +// The AppView +// --------------------------------------------------------------------------- + +// AppView is an XRPC client pointed at the running AppView container. +// +// It is thin on purpose. The pipeline tier's semantics — which endpoint proves +// which contract, what a consumer-health snapshot should say when a wait times +// out — land in phase 4; this is the transport those tests will be written on. +type AppView struct { + *XRPCClient +} + +type appViewConfig struct { + baseURL string + bearer string +} + +// AppViewOption customises NewAppView. +// +// Options configure a private struct rather than mutating the client, so an +// option cannot be applied to a live client that another test is already using. +type AppViewOption func(*appViewConfig) + +// WithAppViewBearer authenticates every call as the holder of token. +func WithAppViewBearer(token string) AppViewOption { + return func(c *appViewConfig) { c.bearer = token } +} + +// WithAppViewURL overrides the AppView address from Endpoints(), for the rare +// test that runs its own server (an httptest instance standing in for the real +// one). +func WithAppViewURL(baseURL string) AppViewOption { + return func(c *appViewConfig) { c.baseURL = trimURL(baseURL) } +} + +// NewAppView returns a client for the AppView the test stack is running. +func NewAppView(t TestingT, opts ...AppViewOption) *AppView { + t.Helper() + cfg := appViewConfig{baseURL: Endpoints().AppView.BaseURL} + for _, opt := range opts { + opt(&cfg) + } + client := NewXRPCClient(cfg.baseURL) + client.Bearer = cfg.bearer + return &AppView{XRPCClient: client} +} + +// As returns a copy of the client authenticated as the holder of token, so one +// test can hold a client per identity. +func (a *AppView) As(token string) *AppView { + return &AppView{XRPCClient: a.WithBearer(token)} +} + +// WaitHealthy blocks until the AppView answers its health endpoint, failing the +// test if it has not within timeout. +func (a *AppView) WaitHealthy(t TestingT, timeout time.Duration) { + t.Helper() + waitHealthy(t, timeout, "the AppView", a.BaseURL, a.Health) +} + +// waitHealthy is the shared body of AppView.WaitHealthy and PDS.WaitHealthy. +// +// It does NOT swallow every failure into "not yet", which is the trap a health +// wait falls into: a service reachable at the wrong path answers 404 instantly +// and forever, and a wait that treats that as "still starting" spends its whole +// timeout and then reports that the service never answered — when in truth it +// answered every single time, with the news that the URL is wrong. +// +// So the three cases are kept apart. A transport failure is genuinely "not yet": +// nothing is listening, which is what a starting service looks like. A 5xx or +// 429 is also "not yet": it is listening and not ready. Any other status is an +// ANSWER, and answering 404 or 401 to a health check is a configuration finding +// that should surface in the first second rather than the thirtieth. +func waitHealthy(t TestingT, timeout time.Duration, subject, baseURL string, probe func(context.Context) error) { + t.Helper() + var last error + WaitFor(t, timeout, func() (bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + last = probe(ctx) + switch { + case last == nil: + return true, nil + case StatusOf(last) == 0, IsTransient(last): + return false, nil + default: + return false, fmt.Errorf( + "%s at %s answered its health check with HTTP %d rather than becoming healthy — "+ + "that is a reachable service saying no, so check the address before the service: %w", + subject, baseURL, StatusOf(last), last) + } + }, + WithDescription("%s at %s to answer its health endpoint", subject, baseURL), + WithDiagnostics(func() string { + if last == nil { + return "" + } + return "last health probe: " + last.Error() + })) +} + +// ConsumerHealth reads the AppView's /health/consumers document: per-consumer +// connection state, cursor positions and dead-letter backlog. +// +// This is the endpoint the pipeline tier attaches to its timeouts. "The record +// never appeared" and "the consumer that indexes that record has been +// disconnected for four minutes with 12 dead letters" are the same failure, and +// only the second one can be acted on. +func (a *AppView) ConsumerHealth(ctx context.Context) (ConsumerHealthReport, error) { + var report ConsumerHealthReport + err := a.Get(ctx, "/health/consumers", &report) + return report, err +} + +// ConsumerHealthReport mirrors cmd/server's /health/consumers response. +// +// Only the fields a failing test would want to read are modelled; the endpoint +// is a diagnostic surface, not a contract, and an unmodelled field costs a line +// of a failure message rather than a wrong result. +type ConsumerHealthReport struct { + Status string `json:"status"` // "ok", "degraded", "stalled" + DeadLetterBacklogUnknown bool `json:"deadLetterBacklogUnknown"` + Consumers []ConsumerState `json:"consumers"` +} + +// ConsumerState is one consumer's entry in a ConsumerHealthReport. +type ConsumerState struct { + Name string `json:"name"` + Connected bool `json:"connected"` + CursorTimeUS int64 `json:"cursorTimeUs"` + EventsProcessed uint64 `json:"eventsProcessed"` + EventsDeadLettered uint64 `json:"eventsDeadLettered"` + DeadLetterBacklog int64 `json:"deadLetterBacklog"` + LastEventAgeSeconds *int64 `json:"lastEventAgeSeconds,omitempty"` +} + +// String renders a report compactly, for failure messages. +func (r ConsumerHealthReport) String() string { + var b strings.Builder + fmt.Fprintf(&b, "consumers: %s", r.Status) + if r.DeadLetterBacklogUnknown { + b.WriteString(" (dead-letter backlog uncountable)") + } + for _, c := range r.Consumers { + fmt.Fprintf(&b, "\n %s: connected=%t cursor=%d processed=%d deadLettered=%d backlog=%d", + c.Name, c.Connected, c.CursorTimeUS, c.EventsProcessed, c.EventsDeadLettered, c.DeadLetterBacklog) + } + return b.String() +} + +// WithConsumerHealth snapshots the AppView's consumer health into a wait's +// failure message, and only into the failure message. +// +// This is docs/TEST_ARCHITECTURE.md §3.3's promise that a T2 timeout arrives +// with cursor positions and dead-letter counts attached: +// +// testkit.WaitFor(t, 30*time.Second, probe, +// testkit.WithDescription("the post to be indexed"), +// testkit.WithConsumerHealth(appview)) +func WithConsumerHealth(a *AppView) WaitOption { + return WithDiagnostics(func() string { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + report, err := a.ConsumerHealth(ctx) + if err != nil { + // Best effort: this runs on a path that is already failing, and + // must not fail differently. Saying why it could not be read is + // still worth a line. + return "consumer health unavailable: " + err.Error() + } + return report.String() + }) +} diff --git a/tests/testkit/appview_test.go b/tests/testkit/appview_test.go new file mode 100644 index 0000000..0997035 --- /dev/null +++ b/tests/testkit/appview_test.go @@ -0,0 +1,546 @@ +package testkit + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "net/url" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The AppView client is pure client logic — headers in, status and JSON out — +// so it is tested against an httptest server rather than the running AppView. +// That is not a mock standing in for infrastructure: the behaviour under test IS +// the client, and a real AppView cannot be asked to answer 401 on demand. The +// contracts that prove the AppView itself are phase 4, and they run against the +// container. + +// stubService answers XRPC calls from a table of canned responses. +type stubService struct { + *httptest.Server + // A clone, not the *http.Request: the request is not valid once the + // handler returns, so keeping it and reading headers from the test + // goroutine afterwards is a use-after-free with extra steps. + lastHeaders atomic.Pointer[http.Header] + lastBody atomic.Pointer[[]byte] +} + +func newStubService(t *testing.T, handler func(http.ResponseWriter, *http.Request)) *stubService { + t.Helper() + stub := &stubService{} + stub.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body []byte + if r.Body != nil { + body, _ = io.ReadAll(r.Body) + // Put it back. Recording the body must not consume it, or a handler + // that decodes the request sees an empty one — which reads as the + // client having sent nothing. + r.Body = io.NopCloser(bytes.NewReader(body)) + } + headers := r.Header.Clone() + stub.lastHeaders.Store(&headers) + stub.lastBody.Store(&body) + handler(w, r) + })) + t.Cleanup(stub.Close) + return stub +} + +func writeJSON(w http.ResponseWriter, status int, payload any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(payload) +} + +func TestXRPCClient_QuerySendsParamsAndDecodes(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/xrpc/social.coves.actor.getProfile", r.URL.Path) + assert.Equal(t, "did:plc:alice", r.URL.Query().Get("actor")) + writeJSON(w, http.StatusOK, map[string]any{"did": "did:plc:alice", "handle": "alice.test"}) + }) + + var profile struct { + DID string `json:"did"` + Handle string `json:"handle"` + } + err := NewXRPCClient(stub.URL).Query(context.Background(), + "social.coves.actor.getProfile", url.Values{"actor": {"did:plc:alice"}}, &profile) + + require.NoError(t, err) + assert.Equal(t, "did:plc:alice", profile.DID) + assert.Equal(t, "alice.test", profile.Handle) +} + +func TestXRPCClient_ProcedureSendsJSON(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + writeJSON(w, http.StatusOK, map[string]any{"uri": "at://did:plc:alice/c/1"}) + }) + + var out struct { + URI string `json:"uri"` + } + err := NewXRPCClient(stub.URL).Procedure(context.Background(), + "social.coves.community.create", map[string]any{"name": "testcove"}, &out) + + require.NoError(t, err) + assert.Equal(t, "at://did:plc:alice/c/1", out.URI) + assert.Contains(t, string(*stub.lastBody.Load()), `"name":"testcove"`) +} + +func TestXRPCClient_SendsTheBearerToken(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{}) + }) + + anonymous := NewXRPCClient(stub.URL) + authenticated := anonymous.WithBearer("token-abc") + + require.NoError(t, authenticated.Query(context.Background(), "some.method", nil, nil)) + assert.Equal(t, "Bearer token-abc", stub.lastHeaders.Load().Get("Authorization")) + + // WithBearer copies rather than mutating: one test holding a client per + // identity must not be able to change what another one sends. + require.NoError(t, anonymous.Query(context.Background(), "some.method", nil, nil)) + assert.Empty(t, stub.lastHeaders.Load().Get("Authorization")) +} + +func TestXRPCClient_UploadSendsRawBytes(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "image/png", r.Header.Get("Content-Type")) + writeJSON(w, http.StatusOK, map[string]any{"ok": true}) + }) + + err := NewXRPCClient(stub.URL).Upload(context.Background(), + "com.atproto.repo.uploadBlob", "image/png", TestPNG(4, 4), nil) + + require.NoError(t, err) +} + +func TestXRPCClient_UploadRejectsAnUnusableContentType(t *testing.T) { + client := NewXRPCClient("http://unused.test") + for _, mime := range []string{"", "*/*", "image/*"} { + err := client.Upload(context.Background(), "com.atproto.repo.uploadBlob", mime, []byte("x"), nil) + require.Error(t, err, "content type %q should be rejected before the request", mime) + assert.Contains(t, err.Error(), "concrete MIME type") + } +} + +// --------------------------------------------------------------------------- +// Typed errors: the split WaitFor probes depend on +// --------------------------------------------------------------------------- + +func TestStatusError_CarriesTheXRPCEnvelope(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, http.StatusUnauthorized, map[string]any{ + "error": "AuthenticationRequired", + "message": "the session has expired", + }) + }) + + err := NewXRPCClient(stub.URL).Query(context.Background(), "social.coves.feed.getTimeline", nil, nil) + + require.Error(t, err) + assert.Equal(t, http.StatusUnauthorized, StatusOf(err)) + assert.True(t, IsStatus(err, http.StatusUnauthorized)) + assert.False(t, IsNotFound(err)) + + var statusErr *StatusError + require.ErrorAs(t, err, &statusErr) + assert.Equal(t, "AuthenticationRequired", statusErr.XRPCError) + assert.Equal(t, "the session has expired", statusErr.XRPCMessage) + + message := err.Error() + assert.Contains(t, message, "social.coves.feed.getTimeline") + assert.Contains(t, message, "401") + assert.Contains(t, message, "the session has expired") +} + +func TestStatusError_HandlesANonXRPCBody(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadGateway) + _, _ = w.Write([]byte("upstream is down")) + }) + + err := NewXRPCClient(stub.URL).Query(context.Background(), "some.method", nil, nil) + + require.Error(t, err) + assert.Equal(t, http.StatusBadGateway, StatusOf(err)) + // A proxy's HTML page is not an XRPC envelope, and swallowing it would leave + // "HTTP 502" with no indication of what answered. + assert.Contains(t, err.Error(), "upstream is down") +} + +func TestStatusOf_IsZeroForTransportFailures(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + address := server.URL + server.Close() + + err := NewXRPCClient(address).Query(context.Background(), "some.method", nil, nil) + + require.Error(t, err) + assert.Zero(t, StatusOf(err), "a refused connection is not a status") + assert.False(t, IsNotFound(err)) +} + +func TestPendingIfNotFound_SplitsRetryableFromTerminal(t *testing.T) { + serviceNotFound := &StatusError{Method: "m", StatusCode: http.StatusNotFound, + XRPCError: "NotFound", XRPCShaped: true} + recordNotFound := &StatusError{Method: "m", StatusCode: http.StatusBadRequest, + XRPCError: "RecordNotFound", XRPCShaped: true} + routerNotFound := &StatusError{Method: "m", StatusCode: http.StatusNotFound, + Body: "404 page not found"} + unauthorized := &StatusError{Method: "m", StatusCode: http.StatusUnauthorized} + serverError := &StatusError{Method: "m", StatusCode: http.StatusInternalServerError} + unavailable := &StatusError{Method: "m", StatusCode: http.StatusServiceUnavailable} + + done, err := PendingIfNotFound(nil) + assert.True(t, done) + assert.NoError(t, err) + + // Not yet: a service that understood the request and has nothing to return. + for _, pending := range []error{serviceNotFound, recordNotFound} { + done, err = PendingIfNotFound(pending) + assert.False(t, done) + assert.NoError(t, err, "%v should be waited out", pending) + } + + // Terminal. The router 404 is the interesting one: a mistyped NSID answers + // 404 forever, and waiting it out spends the whole timeout before reporting + // that the record never appeared — when the truth is that nothing ever + // asked for it. + for _, terminal := range []error{routerNotFound, unauthorized, serverError, unavailable} { + done, err = PendingIfNotFound(terminal) + assert.False(t, done) + assert.ErrorIs(t, err, terminal, "%v should fail the wait immediately", terminal) + } +} + +func TestPendingIfUnavailable_ToleratesARestartingService(t *testing.T) { + notFound := &StatusError{Method: "m", StatusCode: http.StatusNotFound, + XRPCError: "NotFound", XRPCShaped: true} + unauthorized := &StatusError{Method: "m", StatusCode: http.StatusUnauthorized} + badRequest := &StatusError{Method: "m", StatusCode: http.StatusBadRequest} + + done, err := PendingIfUnavailable(nil) + assert.True(t, done) + assert.NoError(t, err) + + // The statuses a service answers while it is coming back up. They say + // nothing about the request, so a wait that is going to keep asking anyway + // should keep asking. + for _, status := range []int{ + http.StatusTooManyRequests, http.StatusBadGateway, + http.StatusServiceUnavailable, http.StatusGatewayTimeout, + } { + transient := &StatusError{Method: "m", StatusCode: status} + assert.True(t, IsTransient(transient)) + done, err = PendingIfUnavailable(transient) + assert.False(t, done) + assert.NoError(t, err, "HTTP %d should be waited out by the restart-tolerant probe", status) + + // ...and NOT by the strict one, which is the default for a reason. + _, strictErr := PendingIfNotFound(transient) + assert.Error(t, strictErr, "HTTP %d must stay terminal for the strict probe", status) + } + + // Everything the strict version fails on that is not transient still fails. + for _, terminal := range []error{unauthorized, badRequest} { + done, err = PendingIfUnavailable(terminal) + assert.False(t, done) + assert.ErrorIs(t, err, terminal) + } + + // Not-found is still not-found. + done, err = PendingIfUnavailable(notFound) + assert.False(t, done) + assert.NoError(t, err) +} + +func TestIsNotFound_RequiresAnXRPCAnswerNotJustA404(t *testing.T) { + // chi's own 404 body, which is what a mistyped NSID produces. + router := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + }) + err := NewXRPCClient(router.URL).Query(context.Background(), "social.coves.actor.getProfyle", nil, nil) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, StatusOf(err)) + assert.False(t, IsNotFound(err), "a router 404 is a wrong address, not a missing record") + assert.Contains(t, err.Error(), "404 page not found") + + // The same status from a service that understood the request. + service := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, http.StatusNotFound, map[string]any{ + "error": "NotFound", "message": "profile not indexed", + }) + }) + err = NewXRPCClient(service.URL).Query(context.Background(), "social.coves.actor.getProfile", nil, nil) + require.Error(t, err) + assert.True(t, IsNotFound(err)) +} + +// TestPendingIfNotFound_DrivesWaitFor is the contract in situ: a 404 is waited +// out, a 401 fails immediately with the reason attached. +func TestPendingIfNotFound_DrivesWaitFor(t *testing.T) { + t.Run("waits out a 404", func(t *testing.T) { + var calls atomic.Int32 + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + if calls.Add(1) < 3 { + writeJSON(w, http.StatusNotFound, map[string]any{"error": "NotFound"}) + return + } + writeJSON(w, http.StatusOK, map[string]any{"did": "did:plc:alice"}) + }) + client := NewXRPCClient(stub.URL) + + WaitFor(t, 5*time.Second, func() (bool, error) { + return PendingIfNotFound(client.Query(context.Background(), "social.coves.actor.getProfile", nil, nil)) + }, WithPollInterval(10*time.Millisecond), WithDescription("the profile to be indexed")) + + assert.EqualValues(t, 3, calls.Load()) + }) + + t.Run("fails immediately on a 401", func(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, http.StatusUnauthorized, map[string]any{ + "error": "AuthenticationRequired", "message": "no session", + }) + }) + client := NewXRPCClient(stub.URL) + + ft := &fakeT{} + start := time.Now() + runIsolated(func() { + WaitFor(ft, 30*time.Second, func() (bool, error) { + return PendingIfNotFound(client.Query(context.Background(), "social.coves.actor.getProfile", nil, nil)) + }, WithDescription("the profile to be indexed")) + }) + + require.True(t, ft.failed()) + assert.Less(t, time.Since(start), 5*time.Second, "a terminal error must not be retried to the deadline") + assert.Contains(t, ft.message(), "no session") + }) +} + +// --------------------------------------------------------------------------- +// The AppView wrapper +// --------------------------------------------------------------------------- + +func TestNewAppView_UsesTheConfiguredEndpoint(t *testing.T) { + appview := NewAppView(t) + assert.Equal(t, Endpoints().AppView.BaseURL, appview.BaseURL) + assert.Empty(t, appview.Bearer) + + authenticated := NewAppView(t, WithAppViewBearer("token-abc"), WithAppViewURL("http://appview.test:9999/")) + assert.Equal(t, "http://appview.test:9999", authenticated.BaseURL, "a trailing slash is trimmed") + assert.Equal(t, "token-abc", authenticated.Bearer) +} + +func TestAppView_AsIsIndependentPerIdentity(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{}) + }) + appview := NewAppView(t, WithAppViewURL(stub.URL)) + + alice := appview.As("alice-token") + bob := appview.As("bob-token") + + require.NoError(t, alice.Query(context.Background(), "m", nil, nil)) + assert.Equal(t, "Bearer alice-token", stub.lastHeaders.Load().Get("Authorization")) + require.NoError(t, bob.Query(context.Background(), "m", nil, nil)) + assert.Equal(t, "Bearer bob-token", stub.lastHeaders.Load().Get("Authorization")) + assert.Empty(t, appview.Bearer, "the original client stays anonymous") +} + +func TestAppView_WaitHealthy(t *testing.T) { + var healthy atomic.Bool + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/xrpc/_health", r.URL.Path) + if !healthy.Load() { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + writeJSON(w, http.StatusOK, map[string]any{"version": "test"}) + }) + appview := NewAppView(t, WithAppViewURL(stub.URL)) + + go func() { + time.Sleep(150 * time.Millisecond) + healthy.Store(true) + }() + appview.WaitHealthy(t, 5*time.Second) +} + +func TestAppView_WaitHealthyReportsTheLastFailure(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + // 503: listening, not ready. Waited out, then reported. + writeJSON(w, http.StatusServiceUnavailable, map[string]any{ + "error": "NotReady", "message": "the database is unreachable", + }) + }) + appview := NewAppView(t, WithAppViewURL(stub.URL)) + + ft := &fakeT{} + runIsolated(func() { appview.WaitHealthy(ft, 300*time.Millisecond) }) + + require.True(t, ft.failed()) + // A health probe that times out with no explanation is the failure this + // whole tier exists to stop producing. + assert.Contains(t, ft.message(), "timed out") + assert.Contains(t, ft.message(), "the database is unreachable") + assert.Contains(t, ft.message(), stub.URL) +} + +// TestAppView_WaitHealthyDoesNotWaitOutAnAnswer is the fix for a health check +// that lied by omission. +// +// Swallowing every error into "not yet" means a service reachable at the WRONG +// PATH — a proxy in front of the wrong upstream, a stale APPVIEW_URL — answers +// 404 instantly and forever, and the wait spends its whole timeout before +// reporting that the service never answered. It answered every time. What it +// said was that the address is wrong. +func TestAppView_WaitHealthyDoesNotWaitOutAnAnswer(t *testing.T) { + var probes atomic.Int32 + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + probes.Add(1) + http.NotFound(w, r) + }) + appview := NewAppView(t, WithAppViewURL(stub.URL)) + + ft := &fakeT{} + start := time.Now() + runIsolated(func() { appview.WaitHealthy(ft, 30*time.Second) }) + + require.True(t, ft.failed()) + assert.Less(t, time.Since(start), 5*time.Second, "an answered 404 is not something to wait out") + assert.EqualValues(t, 1, probes.Load(), "the first answer was already conclusive") + message := ft.message() + assert.Contains(t, message, "HTTP 404") + assert.Contains(t, message, "check the address before the service") + assert.NotContains(t, message, "timed out") +} + +func TestPDS_WaitHealthyDoesNotWaitOutAnAnswer(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + }) + pds := NewPDS(t, WithPDSURL(stub.URL)) + + ft := &fakeT{} + runIsolated(func() { pds.WaitHealthy(ft, 30*time.Second) }) + + require.True(t, ft.failed()) + assert.Contains(t, ft.message(), "the PDS") + assert.Contains(t, ft.message(), "HTTP 404") +} + +// --------------------------------------------------------------------------- +// Consumer health: the diagnostic surface the pipeline tier attaches to timeouts +// --------------------------------------------------------------------------- + +func TestXRPCClient_GetReachesPlainPaths(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/health/consumers", r.URL.Path, "not everything lives under /xrpc") + writeJSON(w, http.StatusOK, map[string]any{"status": "ok"}) + }) + + var out struct { + Status string `json:"status"` + } + require.NoError(t, NewXRPCClient(stub.URL).Get(context.Background(), "/health/consumers", &out)) + assert.Equal(t, "ok", out.Status) + + err := NewXRPCClient(stub.URL).Get(context.Background(), "health/consumers", nil) + require.Error(t, err, "a path without a leading slash would silently join wrong") +} + +func TestAppView_ConsumerHealth(t *testing.T) { + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{ + "status": "stalled", + "consumers": []map[string]any{{ + "name": "posts", "connected": false, "cursorTimeUs": 1751000000000000, + "eventsProcessed": 12, "eventsDeadLettered": 3, "deadLetterBacklog": 3, + }}, + }) + }) + appview := NewAppView(t, WithAppViewURL(stub.URL)) + + report, err := appview.ConsumerHealth(context.Background()) + require.NoError(t, err) + assert.Equal(t, "stalled", report.Status) + require.Len(t, report.Consumers, 1) + assert.Equal(t, "posts", report.Consumers[0].Name) + assert.False(t, report.Consumers[0].Connected) + assert.EqualValues(t, 3, report.Consumers[0].DeadLetterBacklog) + assert.Contains(t, report.String(), "posts: connected=false") +} + +// TestWithConsumerHealth_AttachesToATimeout is docs/TEST_ARCHITECTURE.md §3.3's +// promise made good: a pipeline wait that times out says what the consumers were +// doing, so "the record never appeared" arrives with the reason attached. +func TestWithConsumerHealth_AttachesToATimeout(t *testing.T) { + var healthProbes atomic.Int32 + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/health/consumers" { + healthProbes.Add(1) + writeJSON(w, http.StatusOK, map[string]any{ + "status": "stalled", + "consumers": []map[string]any{ + {"name": "posts", "connected": false, "deadLetterBacklog": 7}, + }, + }) + return + } + writeJSON(w, http.StatusNotFound, map[string]any{"error": "NotFound"}) + }) + appview := NewAppView(t, WithAppViewURL(stub.URL)) + + ft := &fakeT{} + runIsolated(func() { + WaitFor(ft, 200*time.Millisecond, func() (bool, error) { + return PendingIfNotFound(appview.Query(context.Background(), "social.coves.feed.getPost", nil, nil)) + }, + WithPollInterval(50*time.Millisecond), + WithDescription("the post to be indexed"), + WithConsumerHealth(appview)) + }) + + require.True(t, ft.failed()) + message := ft.message() + assert.Contains(t, message, "the post to be indexed") + assert.Contains(t, message, "consumers: stalled") + assert.Contains(t, message, "posts: connected=false") + assert.Contains(t, message, "backlog=7") + // Diagnostics run on the failure path only; a passing wait must not be + // polling the health endpoint every interval. + assert.EqualValues(t, 1, healthProbes.Load()) +} + +func TestWithConsumerHealth_SurvivesAnUnreachableAppView(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + address := server.URL + server.Close() + appview := NewAppView(t, WithAppViewURL(address)) + + ft := &fakeT{} + runIsolated(func() { + WaitFor(ft, 50*time.Millisecond, func() (bool, error) { return false, nil }, + WithDescription("something"), WithConsumerHealth(appview)) + }) + + require.True(t, ft.failed()) + // Best effort: a diagnostic hook runs on a path that is already failing and + // must not fail differently. + assert.Contains(t, ft.message(), "consumer health unavailable") +} diff --git a/tests/testkit/firehose.go b/tests/testkit/firehose.go new file mode 100644 index 0000000..87bbe9e --- /dev/null +++ b/tests/testkit/firehose.go @@ -0,0 +1,820 @@ +package testkit + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net" + "net/url" + "slices" + "strconv" + "strings" + "sync" + "time" + + "github.com/gorilla/websocket" +) + +// The one cursor-gated Jetstream subscriber. +// +// # What this is for, and what it is NOT for +// +// This is a T1 and debugging aid: consumer-plumbing tests that need to see a +// commit arrive, and a human asking "did the PDS actually emit that?". It is +// NOT the end-to-end mechanism. Pipeline contracts (docs/TEST_ARCHITECTURE.md +// §3.4, rule 1) never dial a websocket — the AppView container's own consumers +// do the consuming, exactly as deployed, and the test observes through serving +// endpoints. A T2 test that subscribes here is testing a socket it opened +// itself, which is the one thing production does not do. +// +// scripts/test-audit.sh counts websocket.DefaultDialer everywhere except this +// file, which is the mechanical form of that rule. +// +// # The race this API makes unrepresentable +// +// The ten hand-rolled copies this replaces all had the same shape: write to the +// PDS, then dial Jetstream, then wait for the event. A cursorless subscription +// only streams commits emitted after the socket is dialled, so that ordering +// races the PDS→Jetstream relay and silently drops the event under load. The +// copies papered over it with sleeps. +// +// NewFirehose captures a replay cursor at CONSTRUCTION and Await replays from +// it, so the correct ordering is the only one the API can express: +// +// fh := testkit.NewFirehose(t, testkit.WithCollections(collection)) // cursor captured here +// rec := account.CreateRecord(t, collection, payload) // write +// ev := fh.Await(t, 30*time.Second, testkit.MatchRecord(rec)) // dials, replays +// +// Jetstream stamps each event's time_us when it ingests the commit, which is +// necessarily after a write that had not happened when the cursor was taken. So +// the cursor is always below the event's time_us (we receive it) and above every +// earlier event's (no stale matches). Test and Jetstream share a host clock — +// the dev stack publishes on loopback, the CI stack shares a network namespace — +// so there is no skew to compensate for. Where that assumption fails it is +// reported rather than silently starving the wait: see the discarded-event +// accounting in render. +// +// The cursor is also a BOUND, not merely a starting hint: events older than it +// are discarded on arrival. A Jetstream subscribed "from now" on a quiet stream +// has been observed replaying its entire retained store, and a negative +// assertion ("no delete event arrived") written against an unbounded stream is +// answered by whatever a previous run left behind. +// +// # Feeding a real consumer +// +// The decoded Event carries only what a matcher needs. A test that wants to run +// a production consumer over the same event keeps the bytes: +// +// var je jetstream.JetstreamEvent +// require.NoError(t, ev.Into(&je)) +// require.NoError(t, consumer.HandleEvent(ctx, &je)) +// +// which is what the ten copies did inline, and is why Event does not need to +// grow every field the consumers read. +// +// # Event types are declared here, not imported +// +// internal/atproto/jetstream owns JetstreamEvent, and testkit cannot import it: +// its consumers pull in communities, posts, comments, users, votes, userblocks +// and aggregators, and testkit importing anything under internal/core makes +// those packages' own tests an import cycle (see the package doc in testkit.go). +// +// The four structs below are therefore a second declaration of the same wire +// format. firehose_pin_test.go pins them against the real ones from an EXTERNAL +// test package, which may import jetstream without putting the cycle into +// testkit's own import graph. + +const ( + // firehoseReadDeadline bounds a single websocket read. It has to be finite: + // a blocking read on a connection whose peer died never returns, and the + // test's own deadline would then be enforced by the go test timeout killing + // the whole binary. + // + // It is a WINDOW, not a budget. Reaching it means "nothing arrived in five + // seconds", which for a quiet stream is not an error at all — the wait + // continues on a fresh connection until the caller's deadline. + firehoseReadDeadline = 5 * time.Second + + // maxRecoveryFailures bounds consecutive FAILED recoveries — a dial that + // errors, or a connection that breaks again as soon as it is established. + // Once it trips, the subscription is reported as gone. + // + // It does not count quiet windows, and that distinction is the fix for a bug + // this guard had in all ten copies it replaces. They answered a + // read-deadline expiry with `continue`, and gorilla documents a connection + // whose read deadline has been exceeded as CORRUPT — every subsequent read + // fails instantly with the same timeout error. The counter therefore reached + // ten within microseconds of the FIRST expiry, so every "wait up to 30 + // seconds for the event" subscription in the old suite actually gave up + // after five, reporting "connection appears stale". Recovering means + // dialling again, which is what the production Connector does too. + maxRecoveryFailures = 10 + + // defaultMaxPendingEvents bounds the buffer of received-but-unmatched + // events. Successive Awaits on one Firehose share a subscription, so an + // event read while waiting for a different one has to be kept. + // + // Overflow FAILS THE TEST rather than discarding the oldest entry: the + // buffer holds evidence a later assertion may be about to ask for, and a + // harness that silently drops it produces a green run that proves nothing. + // A test that legitimately streams more than this is subscribing too + // broadly and should filter with WithCollections. + defaultMaxPendingEvents = 512 + + // firehoseDialTimeout caps a single handshake. The Await deadline caps it + // further whenever it is nearer. + firehoseDialTimeout = 15 * time.Second + + // recoveryBackoff paces reconnection after a FAILED recovery, so a server + // that refuses or immediately drops connections is retried a few times a + // second rather than as fast as the scheduler allows. + recoveryBackoff = 100 * time.Millisecond +) + +// --------------------------------------------------------------------------- +// Wire format +// --------------------------------------------------------------------------- + +// Event is one Jetstream event, decoded far enough to match on. +type Event struct { + DID string `json:"did"` + Kind string `json:"kind"` // "commit", "identity", "account" + TimeUS int64 `json:"time_us"` + + Commit *Commit `json:"commit,omitempty"` + Identity *IdentityEvent `json:"identity,omitempty"` + Account *AccountEvent `json:"account,omitempty"` + + // raw is the frame as it arrived. Unexported so it cannot be compared, + // copied or marshalled by accident; reached through Raw and Into. + raw []byte +} + +// Commit is a record write carried by a commit event. +type Commit struct { + Rev string `json:"rev"` + Operation string `json:"operation"` // "create", "update", "delete" + Collection string `json:"collection"` + RKey string `json:"rkey"` + CID string `json:"cid,omitempty"` + Record map[string]any `json:"record,omitempty"` +} + +// IdentityEvent is a handle or DID-document change. +type IdentityEvent struct { + DID string `json:"did"` + Handle string `json:"handle"` + Seq int64 `json:"seq"` + Time string `json:"time"` +} + +// AccountEvent is an account activation or deactivation. +type AccountEvent struct { + DID string `json:"did"` + Active bool `json:"active"` + Seq int64 `json:"seq"` + Time string `json:"time"` +} + +// Raw returns a copy of the frame exactly as Jetstream sent it. +// +// A copy, because a test that kept the slice and a later decode of the same +// buffer would otherwise be able to interfere with each other — a subtle bug to +// design into a harness in exchange for saving one allocation per event. +func (e *Event) Raw() []byte { + if e == nil { + return nil + } + return slices.Clone(e.raw) +} + +// Into decodes the original frame into v, which is how a test hands a real event +// to a real consumer: +// +// var je jetstream.JetstreamEvent +// require.NoError(t, ev.Into(&je)) +// require.NoError(t, consumer.HandleEvent(ctx, &je)) +// +// Decoding the bytes rather than translating this package's structs is +// deliberate: a field testkit does not model still reaches the consumer, so the +// two declarations of the wire format cannot drift into a silently truncated +// event. +func (e *Event) Into(v any) error { + if e == nil { + return errors.New("testkit: decoding a nil event") + } + if len(e.raw) == 0 { + return errors.New("testkit: this event carries no raw frame (it was not read from a subscription)") + } + if err := json.Unmarshal(e.raw, v); err != nil { + return fmt.Errorf("testkit: decoding a %s event into %T: %w", e.Kind, v, err) + } + return nil +} + +// URI renders the AT-URI of the record a commit event carries, or "" for an +// event that is not a commit. +func (e *Event) URI() string { + if e == nil || e.Commit == nil { + return "" + } + return "at://" + e.DID + "/" + e.Commit.Collection + "/" + e.Commit.RKey +} + +// String renders an event compactly, for failure messages. +func (e *Event) String() string { + if e == nil { + return "" + } + if e.Commit != nil { + return fmt.Sprintf("%s %s %s (time_us %d)", e.Kind, e.Commit.Operation, e.URI(), e.TimeUS) + } + return fmt.Sprintf("%s %s (time_us %d)", e.Kind, e.DID, e.TimeUS) +} + +// identity is what makes two frames the same event, for the deduplication a +// resumed subscription needs. +// +// Rev and CID are part of it because one atProto commit can apply several +// writes, and those events share a time_us and a repo while differing in +// nothing else a matcher looks at. Collapsing them would drop an event a test +// is waiting for. +func (e *Event) identity() string { + switch { + case e.Commit != nil: + return fmt.Sprintf("c|%s|%s|%s|%s", e.URI(), e.Commit.Operation, e.Commit.Rev, e.Commit.CID) + case e.Identity != nil: + return fmt.Sprintf("i|%s|%d|%s", e.Identity.DID, e.Identity.Seq, e.Identity.Handle) + case e.Account != nil: + return fmt.Sprintf("a|%s|%d|%t", e.Account.DID, e.Account.Seq, e.Account.Active) + default: + // An event kind this package does not model. Falling back to the frame + // keeps two genuinely different events distinguishable. + return "r|" + string(e.raw) + } +} + +// --------------------------------------------------------------------------- +// Matchers +// --------------------------------------------------------------------------- + +// Matcher selects the event an Await is waiting for. +type Matcher func(*Event) bool + +// MatchDID matches every event for one repo. +func MatchDID(did string) Matcher { + return func(e *Event) bool { return e.DID == did } +} + +// MatchCollection matches commit events in one collection. +func MatchCollection(collection string) Matcher { + return func(e *Event) bool { return e.Commit != nil && e.Commit.Collection == collection } +} + +// MatchRKey matches commit events with one record key. +func MatchRKey(rkey string) Matcher { + return func(e *Event) bool { return e.Commit != nil && e.Commit.RKey == rkey } +} + +// MatchOperation matches commit events by operation: "create", "update" or +// "delete". +func MatchOperation(operation string) Matcher { + return func(e *Event) bool { return e.Commit != nil && e.Commit.Operation == operation } +} + +// MatchURI matches the commit event for exactly one record. +func MatchURI(uri string) Matcher { + return func(e *Event) bool { return e.URI() == uri } +} + +// MatchRecord matches the commit events for a record just written, which is the +// common case: the write returns a Record, and that Record is the matcher. +func MatchRecord(r Record) Matcher { return MatchURI(r.URI) } + +// MatchAll matches events that satisfy every matcher. With no arguments it +// matches everything. +func MatchAll(matchers ...Matcher) Matcher { + return func(e *Event) bool { + for _, m := range matchers { + if !m(e) { + return false + } + } + return true + } +} + +// --------------------------------------------------------------------------- +// The subscriber +// --------------------------------------------------------------------------- + +// Firehose is a cursor-gated Jetstream subscription. +// +// One test goroutine drives it. The lock protects its bookkeeping and is +// deliberately NOT held across the handshake or a blocking read, so Close — from +// a cleanup, or from another goroutine deciding the test is over — takes effect +// promptly instead of waiting out the current read window. +type Firehose struct { + cursor int64 + baseURL string + path string + collections []string + + // Tunables, fields rather than constants so this package's own tests can + // trip the guards in under a second instead of fifty. Not exported: a test + // that needs a shorter read deadline is testing this file. + readDeadline time.Duration + maxRecoveryFailures int + maxPending int + + // ctx is cancelled by Close, which is what aborts a handshake in flight. + ctx context.Context + cancel context.CancelFunc + + mu sync.Mutex + conn *websocket.Conn + pending []*Event + + // lastTimeUS and seenAtLast are the resume bookkeeping. A re-dialled + // subscription resumes from lastTimeUS INCLUSIVE — rewinding by a + // microsecond would skip anything sharing that timestamp, and one atProto + // commit applying several writes emits several events with a single time_us + // — so the events already taken at that timestamp are remembered + // individually and skipped on the way back through. + lastTimeUS int64 + seenAtLast map[string]bool + + received int + discarded int + // newestDiscarded is the largest time_us thrown away for predating the + // cursor. Under a clock skew between this process and Jetstream it is the + // only evidence distinguishing "nothing arrived" from "everything arrived + // and was rejected", which otherwise reads as an unfixable timeout. + newestDiscarded int64 + duplicates int + reconnects int + closed bool +} + +type firehoseConfig struct { + baseURL string + path string + collections []string + cursor int64 + cursorSet bool +} + +// FirehoseOption customises NewFirehose. +type FirehoseOption func(*firehoseConfig) + +// WithCollections restricts the subscription to the named collections, via +// Jetstream's wantedCollections filter. Without it every commit on the stream is +// delivered, which is legal but wasteful. +func WithCollections(collections ...string) FirehoseOption { + return func(c *firehoseConfig) { c.collections = append(c.collections, collections...) } +} + +// WithFirehoseURL overrides the Jetstream address from Endpoints(), for tests of +// this file that point it at a server they control. +func WithFirehoseURL(baseURL string) FirehoseOption { + return func(c *firehoseConfig) { c.baseURL = trimURL(baseURL) } +} + +// WithFirehoseCursor replays from an explicit time_us instead of the moment of +// construction. +// +// The honest use is resuming from an event a previous Await returned — bounding +// a "and nothing further happened" assertion by an OBSERVED time_us rather than +// by a wall clock. Passing time.Now() is the anti-pattern the type exists to +// prevent; NewFirehose already does that, at the only moment when it is safe. +func WithFirehoseCursor(timeUS int64) FirehoseOption { + return func(c *firehoseConfig) { c.cursor, c.cursorSet = timeUS, true } +} + +// NewFirehose captures a replay cursor and returns a subscription that has not +// dialled yet. +// +// Construct it BEFORE the write whose event you intend to await. Nothing about +// the connection matters until Await; the cursor is the load-bearing part, and +// it is taken here. +func NewFirehose(t TestingT, opts ...FirehoseOption) *Firehose { + t.Helper() + endpoint := Endpoints().Jetstream + cfg := firehoseConfig{baseURL: endpoint.BaseURL, path: endpoint.SubscribePath} + for _, opt := range opts { + opt(&cfg) + } + if !cfg.cursorSet { + cfg.cursor = time.Now().UnixMicro() + } + + ctx, cancel := context.WithCancel(context.Background()) + f := &Firehose{ + cursor: cfg.cursor, + baseURL: cfg.baseURL, + path: cfg.path, + collections: cfg.collections, + readDeadline: firehoseReadDeadline, + maxRecoveryFailures: maxRecoveryFailures, + maxPending: defaultMaxPendingEvents, + ctx: ctx, + cancel: cancel, + seenAtLast: map[string]bool{}, + } + t.Cleanup(f.Close) + return f +} + +// Cursor is the replay cursor this subscription is bounded by, in unix +// microseconds. +func (f *Firehose) Cursor() int64 { return f.cursor } + +// SubscribeURL is the URL the next dial will use: the configured filters, and +// the cursor this subscription would resume from right now — the construction +// cursor until an event has been read, and that event's time_us afterwards. +func (f *Firehose) SubscribeURL() string { + f.mu.Lock() + defer f.mu.Unlock() + return f.subscribeURLLocked() +} + +func (f *Firehose) subscribeURLLocked() string { + params := url.Values{} + for _, c := range f.collections { + params.Add("wantedCollections", c) + } + params.Set("cursor", strconv.FormatInt(max(f.cursor, f.lastTimeUS), 10)) + return f.baseURL + f.path + "?" + params.Encode() +} + +// Await waits for an event matching match and returns it, failing the test if +// none arrives within timeout. +// +// The subscription is dialled on the first call and reused afterwards, so events +// read while waiting for one match are held for the next Await rather than being +// lost. Every event is delivered to at most one Await. +// +// WithDescription and WithDiagnostics shape the failure message; +// WithPollInterval is meaningless here and ignored — this reads a stream, it +// does not poll. +// +// timeout is the ONLY limit on how long this waits. A quiet stream costs +// re-dials, not a failure: the read deadline is a window, and reaching it means +// resuming from the last event seen. What ends an Await unsuccessfully is either +// that timeout or maxRecoveryFailures consecutive failures to re-establish the +// subscription at all — reported as two different findings, because "nothing was +// published" and "Jetstream is gone" want different answers. +func (f *Firehose) Await(t TestingT, timeout time.Duration, match Matcher, opts ...WaitOption) *Event { + t.Helper() + cfg := newWaitConfig(opts) + if cfg.description == "" { + cfg.description = "a matching firehose event" + } + start := time.Now() + deadline := start.Add(timeout) + + if event := f.takePending(match); event != nil { + return event + } + + recoveryFailures := 0 + for { + remaining := time.Until(deadline) + if remaining <= 0 { + t.Fatalf("%s", f.render(cfg, fmt.Sprintf( + "timed out after %s (limit %s) waiting for %s", + time.Since(start).Round(time.Millisecond), timeout, cfg.subject()))) + return nil + } + + conn, err := f.connect(deadline) + if err != nil { + if f.isClosed() { + t.Fatalf("%s", f.render(cfg, fmt.Sprintf( + "waiting for %s: the subscription was closed after %s", + cfg.subject(), time.Since(start).Round(time.Millisecond)))) + return nil + } + recoveryFailures++ + if recoveryFailures >= f.maxRecoveryFailures { + t.Fatalf("%s", f.render(cfg, fmt.Sprintf( + "gave up after %s waiting for %s: %d consecutive failures to establish the subscription; "+ + "the last was: %v", + time.Since(start).Round(time.Millisecond), cfg.subject(), recoveryFailures, err))) + return nil + } + f.pause(deadline) + continue + } + + // The read window is clamped to the caller's deadline, so the last read + // of an Await ends exactly at it rather than past it. + window, atDeadline := f.readDeadline, false + if remaining <= window { + window, atDeadline = remaining, true + } + + raw, readErr := readFrame(conn, window) + if readErr != nil { + if f.isClosed() { + t.Fatalf("%s", f.render(cfg, fmt.Sprintf( + "waiting for %s: the subscription was closed after %s", + cfg.subject(), time.Since(start).Round(time.Millisecond)))) + return nil + } + // Every read error is recoverable, and every recovery is a fresh + // connection: a websocket past its read deadline is corrupt, and a + // stream that ended (EOF, a reset, a Jetstream restart) is precisely + // the case cursor-resume exists for. Only a frame that will not + // decode is terminal, and that is handled below. + f.dropConnection() + if atDeadline { + // The window that expired WAS the caller's deadline. Re-dialling + // to read for zero more seconds would replace an honest timeout + // with whatever that dial happened to return. + continue + } + if quietWindow(readErr) { + // A silent stream is not a failure. Resume and keep waiting. + recoveryFailures = 0 + f.countReconnect() + continue + } + recoveryFailures++ + if recoveryFailures >= f.maxRecoveryFailures { + t.Fatalf("%s", f.render(cfg, fmt.Sprintf( + "gave up after %s waiting for %s: the subscription broke %d times in a row; "+ + "the last error was: %v", + time.Since(start).Round(time.Millisecond), cfg.subject(), recoveryFailures, readErr))) + return nil + } + f.countReconnect() + f.pause(deadline) + continue + } + + event := &Event{raw: raw} + if err := json.Unmarshal(raw, event); err != nil { + // Terminal: a frame this package cannot parse is a protocol change + // or a different service on the port, and neither is fixed by + // reading again. + t.Fatalf("%s", f.render(cfg, fmt.Sprintf( + "waiting for %s: Jetstream sent a frame that is not an event: %v\n frame: %s", + cfg.subject(), err, truncate(string(raw), 512)))) + return nil + } + recoveryFailures = 0 + + matched, err := f.accept(event, match) + if err != nil { + t.Fatalf("%s", f.render(cfg, fmt.Sprintf("waiting for %s: %v", cfg.subject(), err))) + return nil + } + if matched { + return event + } + } +} + +// accept applies the cursor bound and the duplicate filter, then either reports +// the event as the match or buffers it. +// +// It returns an error when the pending buffer is full: the buffered events are +// evidence for the assertions still to come, and quietly dropping the oldest +// would let a run go green having thrown away the thing it was about to check. +func (f *Firehose) accept(event *Event, match Matcher) (matched bool, err error) { + f.mu.Lock() + defer f.mu.Unlock() + + switch { + case event.TimeUS < f.cursor: + f.discarded++ + f.newestDiscarded = max(f.newestDiscarded, event.TimeUS) + return false, nil + case event.TimeUS < f.lastTimeUS: + // Older than the resume point: a stream replaying more than was asked + // for. Not a duplicate of anything held, but not new either. + f.duplicates++ + return false, nil + case event.TimeUS == f.lastTimeUS && f.seenAtLast[event.identity()]: + f.duplicates++ + return false, nil + } + + if event.TimeUS > f.lastTimeUS { + f.lastTimeUS = event.TimeUS + clear(f.seenAtLast) + } + f.seenAtLast[event.identity()] = true + f.received++ + + if match(event) { + return true, nil + } + if len(f.pending) >= f.maxPending { + return false, fmt.Errorf( + "the buffer of unmatched events is full at %d, and discarding one to make room would "+ + "throw away evidence a later assertion may need; narrow the subscription with "+ + "WithCollections, or await events closer to when they are written", + f.maxPending) + } + f.pending = append(f.pending, event) + return false, nil +} + +// Close ends the subscription. Registered as a test cleanup by NewFirehose; +// calling it early is safe, idempotent, and unblocks an Await's handshake. +func (f *Firehose) Close() { + f.mu.Lock() + f.closed = true + conn := f.conn + f.conn = nil + f.mu.Unlock() + + // Cancelling interrupts a handshake in flight; closing the socket interrupts + // a blocking read. Neither needs the lock, and holding it across them would + // reintroduce the stall this ordering exists to avoid. + f.cancel() + if conn != nil { + _ = conn.Close() + } +} + +func (f *Firehose) isClosed() bool { + f.mu.Lock() + defer f.mu.Unlock() + return f.closed +} + +// connect returns the live subscription, dialling if there is none. +// +// The handshake happens OUTSIDE the lock: it is a network round trip, and +// holding the lock across it would make Close wait for a connection it is trying +// to abandon. +func (f *Firehose) connect(deadline time.Time) (*websocket.Conn, error) { + f.mu.Lock() + if f.closed { + f.mu.Unlock() + return nil, errors.New("this firehose has been closed") + } + if f.conn != nil { + conn := f.conn + f.mu.Unlock() + return conn, nil + } + target := f.subscribeURLLocked() + f.mu.Unlock() + + // Bounded by the caller's deadline as well as by the handshake cap: a dial + // allowed to run past the deadline would make a sub-second Await take + // fifteen seconds and report a dial error in place of an honest timeout. + dialDeadline := deadline + if capped := time.Now().Add(firehoseDialTimeout); capped.Before(dialDeadline) { + dialDeadline = capped + } + ctx, cancel := context.WithDeadline(f.ctx, dialDeadline) + defer cancel() + + conn, resp, err := websocket.DefaultDialer.DialContext(ctx, target, nil) + if err != nil { + // The handshake response carries the reason a Jetstream rejects a + // subscription (a malformed collection filter, for instance), and + // discarding it leaves only "bad handshake". + status := "" + if resp != nil { + status = fmt.Sprintf(" (HTTP %d)", resp.StatusCode) + _ = resp.Body.Close() + } + return nil, fmt.Errorf("dialling Jetstream at %s%s: %w", target, status, err) + } + if resp != nil { + _ = resp.Body.Close() + } + + f.mu.Lock() + if f.closed { + f.mu.Unlock() + _ = conn.Close() + return nil, errors.New("this firehose has been closed") + } + f.conn = conn + f.mu.Unlock() + return conn, nil +} + +// dropConnection abandons the current socket so the next connect dials afresh. +func (f *Firehose) dropConnection() { + f.mu.Lock() + conn := f.conn + f.conn = nil + f.mu.Unlock() + if conn != nil { + _ = conn.Close() + } +} + +func (f *Firehose) countReconnect() { + f.mu.Lock() + f.reconnects++ + f.mu.Unlock() +} + +// pause waits out the recovery backoff, so a server that refuses or immediately +// drops connections is not retried in a hot loop. +func (f *Firehose) pause(deadline time.Time) { + remaining := time.Until(deadline) + if remaining <= 0 { + return + } + timer := time.NewTimer(min(recoveryBackoff, remaining)) + defer timer.Stop() + select { + case <-timer.C: + case <-f.ctx.Done(): + } +} + +// readFrame reads one message with a bounded deadline. It touches no shared +// state, so it runs without the lock. +func readFrame(conn *websocket.Conn, window time.Duration) ([]byte, error) { + if err := conn.SetReadDeadline(time.Now().Add(window)); err != nil { + return nil, fmt.Errorf("setting the read deadline: %w", err) + } + _, data, err := conn.ReadMessage() + if err != nil { + return nil, err + } + return data, nil +} + +// quietWindow reports whether a read failed because nothing arrived, as opposed +// to because the connection broke. +func quietWindow(err error) bool { + var netErr net.Error + return errors.As(err, &netErr) && netErr.Timeout() +} + +// takePending removes and returns the first buffered event that matches. +func (f *Firehose) takePending(match Matcher) *Event { + f.mu.Lock() + defer f.mu.Unlock() + for i, event := range f.pending { + if match(event) { + f.pending = append(f.pending[:i], f.pending[i+1:]...) + return event + } + } + return nil +} + +// render builds a failure message with the subscription's state attached, then +// the caller's own diagnostics. +// +// The state is the point. "Timed out waiting for an event" is the failure this +// tier exists to stop producing: what a reader needs is whether ANY event +// arrived (a live stream with a wrong matcher), whether none did (a dead +// consumer, a wrong collection filter, a PDS that never committed), or whether +// they all arrived and were rejected for predating the cursor (a clock skew +// between this process and Jetstream, which no amount of waiting fixes). +func (f *Firehose) render(cfg waitConfig, msg string) string { + f.mu.Lock() + defer f.mu.Unlock() + + var b strings.Builder + b.WriteString(msg) + fmt.Fprintf(&b, "\n subscription: %s", f.subscribeURLLocked()) + fmt.Fprintf(&b, "\n events past the cursor: %d received, %d held unmatched", f.received, len(f.pending)) + if f.duplicates > 0 { + fmt.Fprintf(&b, ", %d replayed duplicates ignored", f.duplicates) + } + if f.discarded > 0 { + fmt.Fprintf(&b, "\n %d event(s) arrived but PREDATED the cursor (newest time_us %d, cursor %d, "+ + "%s earlier) — that reads as a clock skew between this process and Jetstream, not as a missing event", + f.discarded, f.newestDiscarded, f.cursor, + time.Duration(f.cursor-f.newestDiscarded)*time.Microsecond) + } + if f.reconnects > 0 { + fmt.Fprintf(&b, "\n subscription re-dialled %d time(s); resuming from time_us %d", + f.reconnects, max(f.cursor, f.lastTimeUS)) + } + if n := len(f.pending); n > 0 { + shown := f.pending + if n > 5 { + shown = shown[n-5:] + } + b.WriteString("\n most recent unmatched:") + for _, event := range shown { + fmt.Fprintf(&b, "\n %s", event) + } + } + return cfg.render(b.String()) +} + +// truncate bounds a quoted payload in a failure message. +func truncate(s string, limit int) string { + if len(s) <= limit { + return s + } + return s[:limit] + "… (truncated)" +} diff --git a/tests/testkit/firehose_pin_test.go b/tests/testkit/firehose_pin_test.go new file mode 100644 index 0000000..97cb226 --- /dev/null +++ b/tests/testkit/firehose_pin_test.go @@ -0,0 +1,149 @@ +// Package testkit_test holds the wire-format pin, and it is an EXTERNAL test +// package for exactly one reason. +// +// testkit itself may not import internal/atproto/jetstream: that package's +// consumers pull in communities, posts, comments, users, votes, userblocks and +// aggregators, and testkit importing anything under internal/core would make +// those packages' own in-package tests import cycles. +// +// An external test package has no such constraint. `package testkit_test` +// compiles into the test binary rather than into testkit, so nothing that +// imports testkit inherits this import — the dependency rule holds, and the +// duplicated structs still get checked against the originals. +package testkit_test + +import ( + "encoding/json" + "testing" + + "Coves/internal/atproto/jetstream" + "Coves/tests/testkit" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestEventMirrorsTheProductionWireFormat is the guard on testkit's second +// declaration of the Jetstream event. +// +// firehose.go redeclares these structs because it cannot import the originals. +// The cost of that is drift: a renamed JSON tag in internal/atproto/jetstream +// would leave every testkit matcher quietly matching nothing, and the symptom +// would be a timeout in whichever test happened to run first — not a compile +// error, and not a message mentioning the rename. +// +// So the check runs in the only direction that catches it: marshal a PRODUCTION +// value, decode it with testkit's structs, and assert every field a matcher +// reads survived the trip. +func TestEventMirrorsTheProductionWireFormat(t *testing.T) { + t.Run("commit", func(t *testing.T) { + production := jetstream.JetstreamEvent{ + Did: "did:plc:alice", + Kind: "commit", + TimeUS: 1751000000000000, + Commit: &jetstream.CommitEvent{ + Rev: "3kabcdefghij", + Operation: "update", + Collection: "social.coves.community.post", + RKey: "3kzzzzzzzzzzz", + CID: "bafyreiabc", + Record: map[string]any{"$type": "social.coves.community.post", "title": "hello"}, + }, + } + + var mirrored testkit.Event + requireRoundTrip(t, production, &mirrored) + + assert.Equal(t, production.Did, mirrored.DID) + assert.Equal(t, production.Kind, mirrored.Kind) + assert.Equal(t, production.TimeUS, mirrored.TimeUS) + require.NotNil(t, mirrored.Commit, "the commit payload did not survive: a JSON tag has moved") + assert.Equal(t, production.Commit.Rev, mirrored.Commit.Rev) + assert.Equal(t, production.Commit.Operation, mirrored.Commit.Operation) + assert.Equal(t, production.Commit.Collection, mirrored.Commit.Collection) + assert.Equal(t, production.Commit.RKey, mirrored.Commit.RKey) + assert.Equal(t, production.Commit.CID, mirrored.Commit.CID) + assert.Equal(t, "hello", mirrored.Commit.Record["title"]) + + // The AT-URI every matcher is built from is assembled from three of + // those fields, so it is worth asserting as a whole. + assert.Equal(t, "at://did:plc:alice/social.coves.community.post/3kzzzzzzzzzzz", mirrored.URI()) + }) + + t.Run("identity", func(t *testing.T) { + production := jetstream.JetstreamEvent{ + Did: "did:plc:alice", + Kind: "identity", + TimeUS: 1751000000000001, + Identity: &jetstream.IdentityEvent{ + Did: "did:plc:alice", + Handle: "alice.local.coves.dev", + Seq: 42, + Time: "2026-07-29T00:00:00Z", + }, + } + + var mirrored testkit.Event + requireRoundTrip(t, production, &mirrored) + + require.NotNil(t, mirrored.Identity) + assert.Equal(t, production.Identity.Did, mirrored.Identity.DID) + assert.Equal(t, production.Identity.Handle, mirrored.Identity.Handle) + assert.Equal(t, production.Identity.Seq, mirrored.Identity.Seq) + assert.Equal(t, production.Identity.Time, mirrored.Identity.Time) + }) + + t.Run("account", func(t *testing.T) { + production := jetstream.JetstreamEvent{ + Did: "did:plc:alice", + Kind: "account", + TimeUS: 1751000000000002, + Account: &jetstream.AccountEvent{ + Did: "did:plc:alice", + Active: true, + Seq: 43, + Time: "2026-07-29T00:00:00Z", + }, + } + + var mirrored testkit.Event + requireRoundTrip(t, production, &mirrored) + + require.NotNil(t, mirrored.Account) + assert.Equal(t, production.Account.Did, mirrored.Account.DID) + assert.Equal(t, production.Account.Active, mirrored.Account.Active) + assert.Equal(t, production.Account.Seq, mirrored.Account.Seq) + assert.Equal(t, production.Account.Time, mirrored.Account.Time) + }) +} + +// TestEventIntoDecodesBackIntoTheProductionType closes the other half of the +// loop: Event.Into is how a test feeds a real consumer, so a frame testkit +// received must decode into the type the consumer expects. +func TestEventIntoDecodesBackIntoTheProductionType(t *testing.T) { + const frame = `{"did":"did:plc:alice","kind":"commit","time_us":1751000000000000,` + + `"commit":{"rev":"3kabcdefghij","operation":"create","collection":"social.coves.community.post",` + + `"rkey":"3kzzzzzzzzzzz","cid":"bafyreiabc","record":{"$type":"social.coves.community.post","title":"hello"}}}` + + // Decoding the frame the way Await does, then handing it on the way a + // migrated test would. + var mirrored testkit.Event + require.NoError(t, json.Unmarshal([]byte(frame), &mirrored)) + + var production jetstream.JetstreamEvent + require.NoError(t, json.Unmarshal([]byte(frame), &production)) + + assert.Equal(t, mirrored.DID, production.Did) + assert.Equal(t, mirrored.TimeUS, production.TimeUS) + require.NotNil(t, production.Commit) + assert.Equal(t, mirrored.Commit.RKey, production.Commit.RKey) + assert.Equal(t, mirrored.Commit.Collection, production.Commit.Collection) + assert.Equal(t, "hello", production.Commit.Record["title"]) +} + +func requireRoundTrip(t *testing.T, from any, into any) { + t.Helper() + encoded, err := json.Marshal(from) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(encoded, into)) +} diff --git a/tests/testkit/firehose_test.go b/tests/testkit/firehose_test.go new file mode 100644 index 0000000..c1b983a --- /dev/null +++ b/tests/testkit/firehose_test.go @@ -0,0 +1,658 @@ +package testkit + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// --------------------------------------------------------------------------- +// The claim the whole file exists for +// --------------------------------------------------------------------------- + +// TestFirehose_CursorGatingBeatsSubscribeAfterWrite is the load-bearing test. +// +// The record is written, AND read back from the PDS, before Await opens the +// websocket. Every hand-rolled subscriber this replaces would miss that event +// by construction: a cursorless subscription only carries commits emitted after +// the dial. It is caught here because NewFirehose took a replay cursor before +// the write, and Await replays from it. +func TestFirehose_CursorGatingBeatsSubscribeAfterWrite(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + // Cursor captured here. Nothing is dialled yet. + firehose := NewFirehose(t, WithCollections(testCollection)) + + record := account.CreateRecord(t, testCollection, map[string]any{ + "$type": testCollection, + "text": "cursor gating", + }) + // A full round trip to the PDS after the write, so the socket is opened + // well after the commit that produced the event. + require.Equal(t, record.URI, account.GetRecord(t, testCollection, record.RKey).URI) + + event := firehose.Await(t, 30*time.Second, MatchRecord(record), + WithDescription("the commit for %s", record.URI)) + + require.NotNil(t, event) + assert.Equal(t, "commit", event.Kind) + assert.Equal(t, account.DID, event.DID) + require.NotNil(t, event.Commit) + assert.Equal(t, "create", event.Commit.Operation) + assert.Equal(t, testCollection, event.Commit.Collection) + assert.Equal(t, record.RKey, event.Commit.RKey) + assert.Equal(t, "cursor gating", event.Commit.Record["text"]) + assert.Greater(t, event.TimeUS, firehose.Cursor(), + "an event matched by a cursor-gated subscription is always newer than the cursor") +} + +// TestFirehose_DeliversUpdateAndDeleteOnOneSubscription covers the shape every +// ingestion contract has — create, update, delete — and with it the buffering +// that makes successive Awaits on one connection safe. +func TestFirehose_DeliversUpdateAndDeleteOnOneSubscription(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + firehose := NewFirehose(t, WithCollections(testCollection)) + + record := account.CreateRecord(t, testCollection, map[string]any{"$type": testCollection, "text": "v1"}) + account.PutRecord(t, testCollection, record.RKey, map[string]any{"$type": testCollection, "text": "v2"}) + account.DeleteRecord(t, testCollection, record.RKey) + + deleted := firehose.Await(t, 30*time.Second, MatchAll(MatchRecord(record), MatchOperation("delete"))) + require.NotNil(t, deleted) + + // Awaited last but emitted first: the create and update events arrived while + // the delete was being waited for, and were held rather than dropped. + created := firehose.Await(t, 10*time.Second, MatchAll(MatchRecord(record), MatchOperation("create"))) + require.NotNil(t, created) + assert.Equal(t, "v1", created.Commit.Record["text"]) + + updated := firehose.Await(t, 10*time.Second, MatchAll(MatchRecord(record), MatchOperation("update"))) + require.NotNil(t, updated) + assert.Equal(t, "v2", updated.Commit.Record["text"]) + assert.Less(t, created.TimeUS, updated.TimeUS) +} + +// --------------------------------------------------------------------------- +// A Jetstream under the test's control +// --------------------------------------------------------------------------- + +// fakeJetstream serves websocket subscriptions from a handler the test writes. +// +// The guards below — the stale-connection counter, the deadline, the cursor +// bound — are about what happens when the stream misbehaves, and a real +// Jetstream cannot be asked to misbehave on cue. +func fakeJetstream(t *testing.T, handle func(*websocket.Conn, *url.URL)) (baseURL string) { + t.Helper() + upgrader := websocket.Upgrader{} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + return + } + defer func() { _ = conn.Close() }() + handle(conn, r.URL) + })) + t.Cleanup(server.Close) + + return "ws://" + strings.TrimPrefix(server.URL, "http://") +} + +// silentStream accepts the connection and sends nothing, until the client goes +// away. +func silentStream(conn *websocket.Conn, _ *url.URL) { + for { + if _, _, err := conn.NextReader(); err != nil { + return + } + } +} + +// sendEvents replays a fixed list from the subscription's cursor and then stays +// silent. +// +// Honouring the cursor is what makes this a stand-in for Jetstream rather than +// a tape recorder: a firehose that has to re-dial resumes from the last event it +// saw, and a fake that replayed everything regardless would test the fake's +// amnesia instead of the resume. +func sendEvents(events ...Event) func(*websocket.Conn, *url.URL) { + return func(conn *websocket.Conn, requested *url.URL) { + cursor, _ := strconv.ParseInt(requested.Query().Get("cursor"), 10, 64) + for _, event := range events { + if event.TimeUS < cursor { + continue + } + if err := conn.WriteJSON(event); err != nil { + return + } + } + silentStream(conn, requested) + } +} + +func commitEvent(did, collection, rkey, operation string, timeUS int64) Event { + return Event{ + DID: did, + Kind: "commit", + TimeUS: timeUS, + Commit: &Commit{Operation: operation, Collection: collection, RKey: rkey}, + } +} + +// --------------------------------------------------------------------------- +// Guards +// --------------------------------------------------------------------------- + +// TestFirehose_QuietStreamIsBoundedByTheDeadlineNotTheGuard is the regression +// test for the bug that made every subscriber in the old suite lie about its +// timeout. +// +// A read-deadline expiry corrupts a gorilla connection, so recovering from a +// quiet window means re-dialling. The ten copies this replaces instead counted +// those expiries toward a stale-connection guard and re-read the corrupt +// connection, which made the counter hit its limit microseconds after the first +// expiry: a "wait up to 30 seconds" subscription really waited five. Here the +// stream is silent for far more windows than the guard's limit, and the wait +// must still run to the caller's deadline and blame nothing but the clock. +func TestFirehose_QuietStreamIsBoundedByTheDeadlineNotTheGuard(t *testing.T) { + baseURL := fakeJetstream(t, silentStream) + + firehose := NewFirehose(t, WithFirehoseURL(baseURL)) + firehose.readDeadline = 20 * time.Millisecond + firehose.maxRecoveryFailures = 3 + + ft := &fakeT{} + runIsolated(func() { + firehose.Await(ft, 400*time.Millisecond, MatchAll(), WithDescription("anything at all")) + }) + + require.True(t, ft.failed(), "a silent stream still has to fail eventually") + message := ft.message() + assert.Contains(t, message, "timed out") + assert.Contains(t, message, "limit 400ms", "the caller's deadline is what ended the wait") + assert.NotContains(t, message, "consecutive failures", + "a quiet window is not a failed recovery, however many of them there are") + assert.Contains(t, message, "anything at all") + + firehose.mu.Lock() + reconnects := firehose.reconnects + firehose.mu.Unlock() + assert.Greater(t, reconnects, firehose.maxRecoveryFailures, + "the wait should have re-dialled past the guard's limit without tripping it") +} + +// TestFirehose_UnreachableJetstreamTripsTheRecoveryGuard covers the other half: +// a subscription that cannot be established at all is a finding, reported with +// the dial error rather than as a bare timeout. +func TestFirehose_UnreachableJetstreamTripsTheRecoveryGuard(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + address := "ws://" + strings.TrimPrefix(server.URL, "http://") + server.Close() // nothing is listening now + + firehose := NewFirehose(t, WithFirehoseURL(address)) + firehose.maxRecoveryFailures = 3 + + ft := &fakeT{} + runIsolated(func() { + firehose.Await(ft, time.Minute, MatchAll(), WithDescription("anything at all")) + }) + + require.True(t, ft.failed()) + message := ft.message() + assert.Contains(t, message, "3 consecutive failures to establish the subscription") + assert.Contains(t, message, "dialling Jetstream") + assert.Contains(t, message, address) +} + +func TestFirehose_TimeoutReportsWhatTheStreamDid(t *testing.T) { + did := "did:plc:firehosetest" + baseURL := fakeJetstream(t, sendEvents( + commitEvent(did, testCollection, "aaa", "create", time.Now().UnixMicro()+1_000_000), + commitEvent(did, testCollection, "bbb", "create", time.Now().UnixMicro()+2_000_000), + )) + + firehose := NewFirehose(t, WithFirehoseURL(baseURL), WithCollections(testCollection)) + firehose.readDeadline = 50 * time.Millisecond + + ft := &fakeT{} + runIsolated(func() { + // Well inside the stale guard's own window (10 × 50ms here), so the + // deadline is unambiguously the binding constraint and the verdict is + // "timed out" rather than "stale connection". + firehose.Await(ft, 300*time.Millisecond, MatchRKey("never-sent"), + WithDescription("a record that was never written"), + WithDiagnostics(func() string { return "caller diagnostics" })) + }) + + require.True(t, ft.failed()) + message := ft.message() + // The distinction a timeout must make: a live stream whose events did not + // match reads very differently from a stream that delivered nothing. + assert.Contains(t, message, "timed out") + assert.Contains(t, message, "a record that was never written") + assert.Contains(t, message, "2 received, 2 held unmatched") + assert.Contains(t, message, "wantedCollections="+url.QueryEscape(testCollection)) + assert.Contains(t, message, "aaa", "the unmatched events should be listed") + assert.Contains(t, message, "caller diagnostics") +} + +// TestFirehose_CursorIsABoundNotAHint is the defence against the Jetstream +// behaviour where a subscription on a quiet stream replays the entire retained +// store: an event older than the cursor must not be able to satisfy an Await, +// however well it matches. +// +// The fake IGNORES the cursor here, deliberately. A server that filtered by +// cursor would make this test pass without the client bound existing at all — +// which is exactly what it looked like before, and exactly the misbehaving +// server the bound is for. +func TestFirehose_CursorIsABoundNotAHint(t *testing.T) { + did := "did:plc:firehosetest" + now := time.Now().UnixMicro() + ancient := commitEvent(did, testCollection, "ancient", "create", now-3_600_000_000) + baseURL := fakeJetstream(t, func(conn *websocket.Conn, requested *url.URL) { + require.NotEmpty(t, requested.Query().Get("cursor"), "the subscription must still ask for a cursor") + _ = conn.WriteJSON(ancient) + silentStream(conn, requested) + }) + + firehose := NewFirehose(t, WithFirehoseURL(baseURL), WithFirehoseCursor(now)) + firehose.readDeadline = 50 * time.Millisecond + + ft := &fakeT{} + runIsolated(func() { + firehose.Await(ft, 300*time.Millisecond, MatchRKey("ancient")) + }) + + require.True(t, ft.failed(), "an event older than the cursor must not match") + message := ft.message() + assert.Contains(t, message, "0 received") + // The count is the difference between "the write never happened" and "your + // clock disagrees with Jetstream's", which no amount of waiting fixes. + assert.Contains(t, message, "PREDATED the cursor") + assert.Contains(t, message, "clock skew") +} + +// TestFirehose_ResumesWithoutLosingOrRepeatingEvents is the guarantee that makes +// a quiet window survivable: the connection is replaced, and the gap between the +// two connections is covered by replay rather than lost. +func TestFirehose_ResumesWithoutLosingOrRepeatingEvents(t *testing.T) { + did := "did:plc:firehosetest" + base := time.Now().UnixMicro() + first := commitEvent(did, testCollection, "first", "create", base+1_000) + // Stamped INSIDE the silent gap: it exists on the server before the second + // connection is made, so only a resume from the right cursor delivers it. + second := commitEvent(did, testCollection, "second", "create", base+2_000) + + var connections atomic.Int32 + baseURL := fakeJetstream(t, func(conn *websocket.Conn, requested *url.URL) { + switch connections.Add(1) { + case 1: + // One event, then silence until the read deadline expires. + _ = conn.WriteJSON(first) + default: + // A real Jetstream replays from the cursor inclusive. + cursor, _ := strconv.ParseInt(requested.Query().Get("cursor"), 10, 64) + assert.Equal(t, first.TimeUS, cursor, + "the resume must start at the last event seen, not at the original cursor") + for _, event := range []Event{first, second} { + if event.TimeUS >= cursor { + _ = conn.WriteJSON(event) + } + } + } + silentStream(conn, requested) + }) + + firehose := NewFirehose(t, WithFirehoseURL(baseURL), WithFirehoseCursor(base)) + firehose.readDeadline = 50 * time.Millisecond + + // The first event is buffered while waiting for the second, which only the + // resumed connection carries. + got := firehose.Await(t, 5*time.Second, MatchRKey("second")) + require.NotNil(t, got) + assert.Equal(t, second.TimeUS, got.TimeUS) + + buffered := firehose.Await(t, time.Second, MatchRKey("first")) + require.NotNil(t, buffered, "the event read before the gap must still be available") + + firehose.mu.Lock() + received, duplicates, reconnects := firehose.received, firehose.duplicates, firehose.reconnects + firehose.mu.Unlock() + + assert.Equal(t, 2, received, "each event is delivered exactly once across the reconnect") + assert.Equal(t, 1, duplicates, "the inclusive resume re-sends exactly one event, which is dropped") + assert.GreaterOrEqual(t, reconnects, 1) +} + +// TestFirehose_KeepsEventsSharingOneMicrosecond covers the boundary the resume +// is built around. +// +// One atProto commit can apply several writes, and Jetstream stamps them all +// with the same time_us. Resuming from that timestamp re-delivers every one of +// them, so remembering only the LAST event seen would drop its siblings — and +// resuming from time_us+1 instead would skip them outright. Both failures are +// silent: the test just waits forever for an event that already went past. +func TestFirehose_KeepsEventsSharingOneMicrosecond(t *testing.T) { + did := "did:plc:firehosetest" + base := time.Now().UnixMicro() + shared := base + 1_000 + siblingA := commitEvent(did, testCollection, "sibling-a", "create", shared) + siblingB := commitEvent(did, testCollection, "sibling-b", "create", shared) + + var connections atomic.Int32 + baseURL := fakeJetstream(t, func(conn *websocket.Conn, requested *url.URL) { + if connections.Add(1) == 1 { + _ = conn.WriteJSON(siblingA) + _ = conn.WriteJSON(siblingB) + } else { + cursor, _ := strconv.ParseInt(requested.Query().Get("cursor"), 10, 64) + assert.Equal(t, shared, cursor, "an inclusive resume cannot skip the shared microsecond") + _ = conn.WriteJSON(siblingA) + _ = conn.WriteJSON(siblingB) + } + silentStream(conn, requested) + }) + + firehose := NewFirehose(t, WithFirehoseURL(baseURL), WithFirehoseCursor(base)) + firehose.readDeadline = 50 * time.Millisecond + + require.NotNil(t, firehose.Await(t, 5*time.Second, MatchRKey("sibling-b"))) + require.NotNil(t, firehose.Await(t, time.Second, MatchRKey("sibling-a"))) + + // Force a reconnect and prove the replayed pair is recognised as already + // seen rather than delivered twice. + ft := &fakeT{} + runIsolated(func() { firehose.Await(ft, 300*time.Millisecond, MatchRKey("never-sent")) }) + require.True(t, ft.failed()) + + firehose.mu.Lock() + received := firehose.received + firehose.mu.Unlock() + assert.Equal(t, 2, received, "both siblings arrive once; the replayed copies are ignored") +} + +// TestFirehose_BufferOverflowFailsRatherThanDiscarding: the buffer holds events +// a later Await may ask for, so silently dropping the oldest would let a run go +// green having thrown away the evidence it was about to check. +func TestFirehose_BufferOverflowFailsRatherThanDiscarding(t *testing.T) { + did := "did:plc:firehosetest" + base := time.Now().UnixMicro() + var events []Event + for i := range 5 { + events = append(events, commitEvent(did, testCollection, + fmt.Sprintf("event-%d", i), "create", base+int64(i+1)*1_000)) + } + baseURL := fakeJetstream(t, sendEvents(events...)) + + firehose := NewFirehose(t, WithFirehoseURL(baseURL), WithFirehoseCursor(base)) + firehose.readDeadline = 50 * time.Millisecond + firehose.maxPending = 3 + + ft := &fakeT{} + runIsolated(func() { firehose.Await(ft, 5*time.Second, MatchRKey("never-sent")) }) + + require.True(t, ft.failed()) + message := ft.message() + assert.Contains(t, message, "buffer of unmatched events is full at 3") + assert.Contains(t, message, "WithCollections", "the failure should say how to fix it") +} + +// TestFirehose_CloseInterruptsAnAwaitInFlight is why the lock is not held across +// the handshake or a blocking read. +// +// A Firehose registers Close as a test cleanup, and an Await that held the mutex +// for its whole wait would make Close block until the read window expired — so a +// test abandoning a wait would sit there for the rest of the deadline. +func TestFirehose_CloseInterruptsAnAwaitInFlight(t *testing.T) { + baseURL := fakeJetstream(t, silentStream) + firehose := NewFirehose(t, WithFirehoseURL(baseURL)) + + failed := make(chan struct{}) + ft := &fakeT{} + go func() { + defer close(failed) + runIsolated(func() { firehose.Await(ft, time.Minute, MatchAll()) }) + }() + + // Let the subscription get as far as a blocking read. + WaitFor(t, 5*time.Second, func() (bool, error) { + firehose.mu.Lock() + defer firehose.mu.Unlock() + return firehose.conn != nil, nil + }, WithPollInterval(10*time.Millisecond), WithDescription("the subscription to be dialled")) + + closed := make(chan struct{}) + go func() { defer close(closed); firehose.Close() }() + + select { + case <-closed: + case <-time.After(5 * time.Second): + t.Fatal("Close blocked behind an in-flight Await") + } + select { + case <-failed: + case <-time.After(5 * time.Second): + t.Fatal("Await did not notice the subscription had been closed") + } + assert.Contains(t, ft.message(), "closed") +} + +func TestFirehose_AMalformedFrameIsTerminal(t *testing.T) { + baseURL := fakeJetstream(t, func(conn *websocket.Conn, requested *url.URL) { + _ = conn.WriteMessage(websocket.TextMessage, []byte("this is not an event")) + silentStream(conn, requested) + }) + firehose := NewFirehose(t, WithFirehoseURL(baseURL)) + firehose.readDeadline = 50 * time.Millisecond + + ft := &fakeT{} + runIsolated(func() { firehose.Await(ft, 10*time.Second, MatchAll()) }) + + require.True(t, ft.failed()) + message := ft.message() + // Not retried: a frame this package cannot parse is a protocol change or a + // different service on the port, and reading again fixes neither. + assert.Contains(t, message, "not an event") + assert.Contains(t, message, "this is not an event", "the frame itself is the evidence") +} + +func TestFirehose_AwaitAfterCloseFails(t *testing.T) { + baseURL := fakeJetstream(t, silentStream) + firehose := NewFirehose(t, WithFirehoseURL(baseURL)) + firehose.Close() + firehose.Close() // idempotent + + ft := &fakeT{} + runIsolated(func() { firehose.Await(ft, time.Second, MatchAll()) }) + + require.True(t, ft.failed()) + assert.Contains(t, ft.message(), "closed") +} + +// --------------------------------------------------------------------------- +// URL construction and matchers +// --------------------------------------------------------------------------- + +func TestFirehose_SubscribeURLCarriesTheCursorAndFilters(t *testing.T) { + firehose := NewFirehose(t, + WithFirehoseURL("ws://jetstream.test:6008/"), + WithCollections("social.coves.community.post", "social.coves.community.comment"), + WithFirehoseCursor(1234567890)) + + parsed, err := url.Parse(firehose.SubscribeURL()) + require.NoError(t, err) + assert.Equal(t, "ws", parsed.Scheme) + assert.Equal(t, "jetstream.test:6008", parsed.Host) + assert.Equal(t, "/subscribe", parsed.Path) + assert.Equal(t, "1234567890", parsed.Query().Get("cursor")) + assert.Equal(t, + []string{"social.coves.community.post", "social.coves.community.comment"}, + parsed.Query()["wantedCollections"]) +} + +func TestFirehose_CursorIsCapturedAtConstruction(t *testing.T) { + before := time.Now().UnixMicro() + firehose := NewFirehose(t, WithFirehoseURL("ws://jetstream.test:6008")) + after := time.Now().UnixMicro() + + assert.GreaterOrEqual(t, firehose.Cursor(), before) + assert.LessOrEqual(t, firehose.Cursor(), after) +} + +func TestMatchers(t *testing.T) { + event := commitEvent("did:plc:alice", "social.coves.community.post", "3kabc", "create", 42) + identity := &Event{DID: "did:plc:alice", Kind: "identity", TimeUS: 43, + Identity: &IdentityEvent{DID: "did:plc:alice", Handle: "alice.local.coves.dev"}} + + assert.True(t, MatchDID("did:plc:alice")(&event)) + assert.False(t, MatchDID("did:plc:bob")(&event)) + assert.True(t, MatchCollection("social.coves.community.post")(&event)) + assert.False(t, MatchCollection("social.coves.community.post")(identity), + "a non-commit event has no collection to match") + assert.True(t, MatchRKey("3kabc")(&event)) + assert.True(t, MatchOperation("create")(&event)) + assert.False(t, MatchOperation("delete")(&event)) + assert.True(t, MatchURI("at://did:plc:alice/social.coves.community.post/3kabc")(&event)) + assert.True(t, MatchRecord(Record{URI: "at://did:plc:alice/social.coves.community.post/3kabc"})(&event)) + assert.True(t, MatchAll()(&event), "no matchers matches everything") + assert.True(t, MatchAll(MatchDID("did:plc:alice"), MatchOperation("create"))(&event)) + assert.False(t, MatchAll(MatchDID("did:plc:alice"), MatchOperation("delete"))(&event)) +} + +// TestFirehose_EventCarriesTheRawFrame proves the property the ten consumer +// migrations depend on: a test can hand the event it awaited to the production +// consumer, fields testkit does not model included. +func TestFirehose_EventCarriesTheRawFrame(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + firehose := NewFirehose(t, WithCollections(testCollection)) + + record := account.CreateRecord(t, testCollection, map[string]any{ + "$type": testCollection, + "text": "raw frames", + }) + event := firehose.Await(t, 30*time.Second, MatchRecord(record)) + require.NotNil(t, event) + + // A struct shaped like a consumer's, including a field Event does not model. + var decoded struct { + DID string `json:"did"` + Kind string `json:"kind"` + TimeUS int64 `json:"time_us"` + Commit struct { + Rev string `json:"rev"` + Operation string `json:"operation"` + Collection string `json:"collection"` + RKey string `json:"rkey"` + CID string `json:"cid"` + Record map[string]any `json:"record"` + } `json:"commit"` + } + require.NoError(t, event.Into(&decoded)) + assert.Equal(t, account.DID, decoded.DID) + assert.Equal(t, record.RKey, decoded.Commit.RKey) + assert.Equal(t, record.CID, decoded.Commit.CID) + assert.NotEmpty(t, decoded.Commit.Rev) + assert.Equal(t, "raw frames", decoded.Commit.Record["text"]) + + raw := event.Raw() + assert.Contains(t, string(raw), `"time_us"`) + // A copy: mutating what Raw handed back must not corrupt a later decode. + for i := range raw { + raw[i] = 'x' + } + require.NoError(t, event.Into(&decoded)) + assert.Equal(t, record.RKey, decoded.Commit.RKey) +} + +func TestEvent_IntoRejectsWhatItCannotDecode(t *testing.T) { + var missing *Event + assert.Error(t, missing.Into(&struct{}{})) + assert.Nil(t, missing.Raw()) + + // An event built by a test rather than read from a subscription has no + // frame, and saying so beats decoding silence into a zero value. + handmade := commitEvent("did:plc:alice", testCollection, "abc", "create", 1) + err := handmade.Into(&struct{}{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "no raw frame") +} + +func TestEvent_URIAndString(t *testing.T) { + event := commitEvent("did:plc:alice", "social.coves.community.post", "3kabc", "create", 42) + assert.Equal(t, "at://did:plc:alice/social.coves.community.post/3kabc", event.URI()) + assert.Contains(t, event.String(), "create") + + identity := &Event{DID: "did:plc:alice", Kind: "identity", TimeUS: 43} + assert.Empty(t, identity.URI(), "only commits name a record") + assert.Contains(t, identity.String(), "identity") + + var missing *Event + assert.Empty(t, missing.URI()) + assert.Equal(t, "", missing.String()) +} + +// TestEvent_DecodesTheJetstreamWireFormat pins the field names against a +// payload in the shape Jetstream actually publishes. +// +// firehose_pin_test.go pins the same structs against internal/atproto/jetstream's, +// and the two catch different things. That one catches this package drifting +// from the consumers. This one catches BOTH of them drifting from Jetstream, +// which a struct-to-struct comparison cannot see, because the wire format is +// owned by neither. +func TestEvent_DecodesTheJetstreamWireFormat(t *testing.T) { + const payload = `{ + "did": "did:plc:alice", + "time_us": 1751000000000000, + "kind": "commit", + "commit": { + "rev": "3kabcdefghij", + "operation": "create", + "collection": "social.coves.community.post", + "rkey": "3kzzzzzzzzzzz", + "record": {"$type": "social.coves.community.post", "title": "hello"}, + "cid": "bafyreiabc" + } + }` + + var event Event + require.NoError(t, json.Unmarshal([]byte(payload), &event)) + + assert.Equal(t, "did:plc:alice", event.DID) + assert.Equal(t, "commit", event.Kind) + assert.Equal(t, int64(1751000000000000), event.TimeUS) + require.NotNil(t, event.Commit) + assert.Equal(t, "3kabcdefghij", event.Commit.Rev) + assert.Equal(t, "create", event.Commit.Operation) + assert.Equal(t, "social.coves.community.post", event.Commit.Collection) + assert.Equal(t, "3kzzzzzzzzzzz", event.Commit.RKey) + assert.Equal(t, "bafyreiabc", event.Commit.CID) + assert.Equal(t, "hello", event.Commit.Record["title"]) + + var identity Event + require.NoError(t, json.Unmarshal([]byte( + `{"did":"did:plc:alice","kind":"identity","time_us":1,"identity":{"did":"did:plc:alice","handle":"alice.test","seq":7,"time":"2026-07-29T00:00:00Z"}}`), &identity)) + require.NotNil(t, identity.Identity) + assert.Equal(t, "alice.test", identity.Identity.Handle) + assert.Equal(t, int64(7), identity.Identity.Seq) + + var account Event + require.NoError(t, json.Unmarshal([]byte( + `{"did":"did:plc:alice","kind":"account","time_us":1,"account":{"did":"did:plc:alice","active":true,"seq":9,"time":"2026-07-29T00:00:00Z"}}`), &account)) + require.NotNil(t, account.Account) + assert.True(t, account.Account.Active) + assert.Equal(t, int64(9), account.Account.Seq) +} diff --git a/tests/testkit/harness_test.go b/tests/testkit/harness_test.go index bffb1f4..5985841 100644 --- a/tests/testkit/harness_test.go +++ b/tests/testkit/harness_test.go @@ -10,9 +10,11 @@ import ( "testing" ) +// TestMain is also the worked example of the TestMain every migrating package +// gets: testkit's own tests exercise Postgres, the PDS and Jetstream, so all +// three are probed once here rather than failing test by test. func TestMain(m *testing.M) { - SilenceLogs() - os.Exit(m.Run()) + os.Exit(Main(m, RequirePostgres, RequirePDS, RequireJetstream)) } // fakeT records what a testkit helper did to the test it was handed, so the @@ -104,6 +106,31 @@ func resetTemplateVerification() { templateVerified = false } +// swapEndpoints rebinds only the endpoint singleton, re-reading it from the +// current environment, and registers the restore as a cleanup. +// +// Assigning endpointsOnce directly is a data race the moment `-shuffle=on` +// reorders tests or anything runs in parallel — the accessors read it under +// singletonMu, so writers must take that lock too. Every endpoint test needs to +// re-read the environment after t.Setenv, so the locking lives here once rather +// than being retyped (and forgotten) at each site. +func swapEndpoints(t *testing.T) { + t.Helper() + singletonMu.Lock() + savedEndpoints, savedTemplateName := endpointsOnce, templateNameOnce + endpointsOnce = sync.OnceValue(loadEndpoints) + // The template name is validated against the maintenance database, so it + // has to be re-derived whenever the endpoints change. + templateNameOnce = sync.OnceValues(loadTemplateName) + singletonMu.Unlock() + + t.Cleanup(func() { + singletonMu.Lock() + endpointsOnce, templateNameOnce = savedEndpoints, savedTemplateName + singletonMu.Unlock() + }) +} + // swapSingletons rebinds the memoised process singletons and returns a function // that restores them. // diff --git a/tests/testkit/pds.go b/tests/testkit/pds.go new file mode 100644 index 0000000..00feda6 --- /dev/null +++ b/tests/testkit/pds.go @@ -0,0 +1,706 @@ +package testkit + +import ( + "context" + "crypto/rand" + "encoding/binary" + "fmt" + "net/url" + "strings" + "sync" + "time" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" +) + +// Accounts, sessions, records and blobs on the test PDS. +// +// This replaces four drifted copies of the same three hundred lines: the +// createPDSAccount in tests/integration/helpers.go, the differently-shaped one +// in tests/e2e/user_signup_test.go, the hand-rolled writePDSRecord, and the +// four *PasswordAuthPDSClientFactory adapters. What they had in common was +// net/http plus encoding/json plus a slightly different set of forgotten error +// checks; what they did not have in common was handle generation, which is why +// the suite collided handles between runs. +// +// # Why not internal/atproto/pds +// +// That package is the real client and this one is not a rival to it. testkit +// cannot import it: it returns *blobs.BlobRef, so it imports +// internal/core/blobs, and testkit importing anything under internal/core makes +// `go test ./internal/core/blobs` an import cycle (see the package doc in +// testkit.go). Production code should keep using it. Test code that needs a +// domain's PDS-client seam gets it through PasswordAuthFactory below, which +// wires internal/atproto/pds in at the CALL SITE, where the import is legal. +// +// # Failure model +// +// Every exported helper takes a TestingT and fails the test on error, like +// testkit.DB. A missing PDS is a failed test, never a skip: if the suite was +// invoked, the infrastructure was requested. Tests that need to assert on a +// failure — a rejected record, a bad token — use Account.XRPC(), which returns +// errors instead of consuming them. + +// DefaultPassword is the password every generated account gets. +// +// It is a throwaway local credential for a PDS that holds nothing: the same +// value is in .env.dev, .env.ci and docker-compose.dev.yml. It exists as a +// constant so that a test needing a second session for an account does not have +// to guess it. +const DefaultPassword = "test-password-123" + +// --------------------------------------------------------------------------- +// TIDs +// --------------------------------------------------------------------------- + +// tidClock issues record keys. Its clock id is drawn randomly per process, which +// is what the field is for: two processes writing to one repo in the same +// microsecond produce different TIDs rather than a duplicate-rkey conflict. +// +// syntax.TIDClock is monotonic under concurrency (it holds a mutex and bumps the +// microsecond when time has not moved), so this replaces the old generateTID's +// hand-rolled counter with the real thing — and the result is a valid TID, which +// "3k" + a decimal timestamp never was. +var tidClock = syntax.NewTIDClock(randomClockID()) + +func randomClockID() uint { + var b [8]byte + if _, err := rand.Read(b[:]); err != nil { + panic(fmt.Sprintf("testkit: reading random bytes for the TID clock id: %v", err)) + } + // TID clock ids are 10 bits. + return uint(binary.BigEndian.Uint64(b[:]) % 1024) +} + +// TID returns a fresh record key: a valid atProto TID, strictly increasing +// within this process. +// +// Records are created with a client-side TID rather than letting the PDS mint +// one, because the caller needs the rkey BEFORE the write — that is what a +// firehose matcher is built from, and a matcher that can only be built after the +// event has been emitted is the subscribe-after-write race in another costume. +func TID() string { return tidClock.Next().String() } + +// --------------------------------------------------------------------------- +// The PDS +// --------------------------------------------------------------------------- + +// PDS is the test stack's Personal Data Server: the thing that holds repos, +// mints accounts, and feeds the firehose. +type PDS struct { + Endpoint PDSEndpoint + // Anon is an unauthenticated client, for the endpoints that need no session + // (createAccount, createSession, _health). + Anon *XRPCClient +} + +type pdsConfig struct { + baseURL string + handleDomain string +} + +// PDSOption customises NewPDS. +// +// Options configure a private struct rather than mutating the returned client, +// matching the other constructors in the kit. +type PDSOption func(*pdsConfig) + +// WithPDSURL overrides the PDS address from Endpoints(), for tests of this file +// that point it at a server they control. +func WithPDSURL(baseURL string) PDSOption { + return func(c *pdsConfig) { c.baseURL = trimURL(baseURL) } +} + +// WithPDSHandleDomain overrides the domain generated handles are issued under. +func WithPDSHandleDomain(domain string) PDSOption { + return func(c *pdsConfig) { c.handleDomain = strings.TrimPrefix(domain, ".") } +} + +// NewPDS returns a handle on the PDS the test stack is running. +func NewPDS(t TestingT, opts ...PDSOption) *PDS { + t.Helper() + endpoint := Endpoints().PDS + cfg := pdsConfig{baseURL: endpoint.BaseURL, handleDomain: endpoint.HandleDomain} + for _, opt := range opts { + opt(&cfg) + } + resolved := PDSEndpoint{BaseURL: cfg.baseURL, HandleDomain: cfg.handleDomain} + return &PDS{Endpoint: resolved, Anon: NewXRPCClient(resolved.BaseURL)} +} + +// URL is the PDS' base URL. +func (p *PDS) URL() string { return p.Endpoint.BaseURL } + +// WaitHealthy blocks until the PDS answers, failing the test if it has not +// within timeout. +// +// A refused connection is "still starting"; an answered 4xx is a finding about +// the address rather than the service. See waitHealthy in appview.go. +func (p *PDS) WaitHealthy(t TestingT, timeout time.Duration) { + t.Helper() + waitHealthy(t, timeout, "the PDS", p.Endpoint.BaseURL, p.Anon.Health) +} + +// --------------------------------------------------------------------------- +// Accounts and sessions +// --------------------------------------------------------------------------- + +// Account is a PDS account together with an authenticated session on it. +// +// The password is kept so a test can open a second session — an OAuth flow, a +// token-expiry case — without inventing one that does not match. +// +// # Session lifetime +// +// AccessToken is a PDS access JWT, which expires (two hours on a stock PDS). +// Nothing in this package refreshes it in the background, and nothing needs to: +// a test that outlives its own session has other problems. A test that +// deliberately spans that boundary calls RefreshSession, which swaps both tokens +// and the authenticated client together. +// +// # Credentials in failure messages +// +// Account carries a password and two bearer tokens, and a test logging +// `t.Logf("%+v", account)` would otherwise print all three into CI output that +// outlives the run. String and GoString redact them, so every fmt verb is safe. +type Account struct { + Handle string + DID string + Email string + Password string + AccessToken string + RefreshToken string + + pds *PDS + + // mu guards the credentials and the derived client, so RefreshSession + // swaps them as one unit rather than leaving a window in which the token + // and the client disagree. + mu sync.Mutex + client *XRPCClient +} + +// String renders the account without its credentials. +// +// This is what makes %v and %+v safe. GoString covers %#v, which would +// otherwise dump the struct field by field. +func (a *Account) String() string { + if a == nil { + return "" + } + return fmt.Sprintf("testkit.Account{Handle: %q, DID: %q, Email: %q, Password: %s, AccessToken: %s, RefreshToken: %s}", + a.Handle, a.DID, a.Email, redacted(a.Password), redacted(a.AccessToken), redacted(a.RefreshToken)) +} + +// GoString renders the account without its credentials, for %#v. +func (a *Account) GoString() string { return a.String() } + +// redacted describes a secret without disclosing it. The length is kept because +// "REDACTED (0 chars)" and "REDACTED (183 chars)" answer different questions, +// and neither answer is the secret. +func redacted(secret string) string { + if secret == "" { + return "\"\"" + } + return fmt.Sprintf("REDACTED(%d chars)", len(secret)) +} + +type accountConfig struct { + handle string + label string + prefix string + email string + password string + inviteCode string +} + +// AccountOption customises CreateAccount. +type AccountOption func(*accountConfig) + +// WithHandle sets the account's full handle verbatim, bypassing generation. +// +// Use it only when the handle itself is what a test is about. Anything else +// should take the generated one: a hand-written handle on a PDS whose volume +// survives the run is a collision waiting for the second invocation. +func WithHandle(handle string) AccountOption { + return func(c *accountConfig) { c.handle = handle } +} + +// WithHandlePrefix makes the generated handle start with a readable prefix, so +// a leftover account says which test created it. +func WithHandlePrefix(prefix string) AccountOption { + return func(c *accountConfig) { c.prefix = prefix } +} + +// WithEmail overrides the generated email address. +func WithEmail(email string) AccountOption { + return func(c *accountConfig) { c.email = email } +} + +// WithPassword overrides DefaultPassword. +func WithPassword(password string) AccountOption { + return func(c *accountConfig) { c.password = password } +} + +// WithInviteCode supplies an invite code, for a PDS configured to require one. +func WithInviteCode(code string) AccountOption { + return func(c *accountConfig) { c.inviteCode = code } +} + +// CreateAccount registers a new account on the PDS and returns it with a live +// session. +// +// The handle is generated from UniqueID unless WithHandle says otherwise, which +// is the whole point of routing account creation through here: the local label +// is inside the PDS' 18-character cap, starts with a letter, and carries this +// process's random run prefix, so it cannot collide with another run's leftovers +// on a PDS volume that persists (which the dev stack's does). +func (p *PDS) CreateAccount(t TestingT, opts ...AccountOption) *Account { + t.Helper() + + cfg := accountConfig{password: DefaultPassword} + for _, opt := range opts { + opt(&cfg) + } + if cfg.handle == "" { + if cfg.prefix != "" { + cfg.label = UniqueIDWithPrefix(t, cfg.prefix) + } else { + cfg.label = UniqueID(t) + } + cfg.handle = p.Endpoint.Handle(cfg.label) + } + if cfg.email == "" { + // Derived from the handle so a stray account in the PDS' user table can + // be traced back to the run that made it. + cfg.email = handleLabel(cfg.handle) + "@test.com" + } + + payload := map[string]string{ + "handle": cfg.handle, + "email": cfg.email, + "password": cfg.password, + } + if cfg.inviteCode != "" { + payload["inviteCode"] = cfg.inviteCode + } + + var resp sessionResponse + ctx, cancel := context.WithTimeout(context.Background(), defaultXRPCTimeout) + defer cancel() + if err := p.Anon.Procedure(ctx, "com.atproto.server.createAccount", payload, &resp); err != nil { + t.Fatalf("testkit: creating account %q on %s: %v", cfg.handle, p.Endpoint.BaseURL, err) + return nil + } + acct, err := p.newAccount(resp, cfg.email, cfg.password) + if err != nil { + t.Fatalf("testkit: creating account %q on %s: %v", cfg.handle, p.Endpoint.BaseURL, err) + return nil + } + return acct +} + +// Login opens a session on an existing account. +// +// identifier is a handle or a DID — whatever com.atproto.server.createSession +// accepts. This is the path the instance account takes: it exists on the PDS +// already, so there is nothing to create. +// +// The returned Account has an EMPTY Email: createSession does not disclose it, +// and inventing one would put a value in the field that does not match the +// account. Tests that need it should carry it from CreateAccount. +func (p *PDS) Login(t TestingT, identifier, password string) *Account { + t.Helper() + + var resp sessionResponse + ctx, cancel := context.WithTimeout(context.Background(), defaultXRPCTimeout) + defer cancel() + err := p.Anon.Procedure(ctx, "com.atproto.server.createSession", map[string]string{ + "identifier": identifier, + "password": password, + }, &resp) + if err != nil { + t.Fatalf("testkit: authenticating %q against %s: %v", identifier, p.Endpoint.BaseURL, err) + return nil + } + acct, err := p.newAccount(resp, "", password) + if err != nil { + t.Fatalf("testkit: authenticating %q against %s: %v", identifier, p.Endpoint.BaseURL, err) + return nil + } + return acct +} + +// sessionResponse is the body com.atproto.server.createAccount and +// com.atproto.server.createSession both return. +type sessionResponse struct { + DID string `json:"did"` + Handle string `json:"handle"` + AccessJwt string `json:"accessJwt"` + RefreshJwt string `json:"refreshJwt"` +} + +// newAccount validates a session response before anything depends on it. +// +// A 200 carrying an empty did, accessJwt or refreshJwt means the PDS — or +// something in front of it — answered with a body that is not a session. +// Accepting it would send an unauthenticated client into the next twenty lines +// of the test, where it fails as a 401 on an unrelated call; accepting a missing +// refresh token would defer the same surprise to RefreshSession. +func (p *PDS) newAccount(resp sessionResponse, email, password string) (*Account, error) { + switch { + case resp.DID == "": + return nil, fmt.Errorf("PDS returned a session with no did (handle %q)", resp.Handle) + case resp.AccessJwt == "": + return nil, fmt.Errorf("PDS returned a session with no accessJwt (did %q)", resp.DID) + case resp.RefreshJwt == "": + return nil, fmt.Errorf("PDS returned a session with no refreshJwt (did %q)", resp.DID) + } + return &Account{ + Handle: resp.Handle, + DID: resp.DID, + Email: email, + Password: password, + AccessToken: resp.AccessJwt, + RefreshToken: resp.RefreshJwt, + pds: p, + client: p.Anon.WithBearer(resp.AccessJwt), + }, nil +} + +// RefreshSession exchanges the refresh token for a new session and swaps both +// tokens and the authenticated client together. +// +// com.atproto.server.refreshSession authenticates with the REFRESH token, not +// the access token, and returns a new pair — the old refresh token is spent. So +// this is not a retry-safe operation: calling it twice with the same starting +// state fails the second time, which is correct and worth knowing before +// wrapping it in a loop. +func (a *Account) RefreshSession(t TestingT) { + t.Helper() + + a.mu.Lock() + refreshToken := a.RefreshToken + a.mu.Unlock() + + var resp sessionResponse + ctx, cancel := context.WithTimeout(context.Background(), defaultXRPCTimeout) + defer cancel() + err := a.pds.Anon.WithBearer(refreshToken). + Procedure(ctx, "com.atproto.server.refreshSession", nil, &resp) + if err != nil { + t.Fatalf("testkit: refreshing the session for %s: %v", a.DID, err) + return + } + switch { + case resp.AccessJwt == "": + t.Fatalf("testkit: refreshing the session for %s: PDS returned no accessJwt", a.DID) + return + case resp.RefreshJwt == "": + t.Fatalf("testkit: refreshing the session for %s: PDS returned no refreshJwt", a.DID) + return + case resp.DID != "" && resp.DID != a.DID: + t.Fatalf("testkit: refreshing the session for %s returned a session for %s", a.DID, resp.DID) + return + } + + a.mu.Lock() + a.AccessToken, a.RefreshToken = resp.AccessJwt, resp.RefreshJwt + a.client = a.pds.Anon.WithBearer(resp.AccessJwt) + a.mu.Unlock() +} + +// PDS returns the server this account lives on. +func (a *Account) PDS() *PDS { return a.pds } + +// XRPC returns an error-returning client authenticated as this account. +// +// This is the escape hatch for the two things the helpers below deliberately do +// not do: call an endpoint the kit does not wrap, and assert that a call FAILS. +// Everything else should use the helpers, which fail the test for you. +// +// The client is a snapshot of the current session. Hold it across a +// RefreshSession and it will still carry the spent access token. +func (a *Account) XRPC() *XRPCClient { + a.mu.Lock() + defer a.mu.Unlock() + return a.client +} + +// handleLabel returns the part of a handle before the first dot. +func handleLabel(handle string) string { + label, _, _ := strings.Cut(handle, ".") + return label +} + +// --------------------------------------------------------------------------- +// Records +// --------------------------------------------------------------------------- + +// Record identifies a record written to a repo. +type Record struct { + URI string + CID string + Collection string + RKey string +} + +// RecordValue is a record read back from a repo. +type RecordValue struct { + URI string + CID string + Value map[string]any +} + +type recordConfig struct { + rkey string + swapRecord string +} + +// RecordOption customises a record write. +// +// Not every option applies to every write — a key is an argument to PutRecord +// rather than an option, and a compare-and-swap has nothing to compare on a +// create. Passing an inapplicable one FAILS THE TEST rather than being ignored: +// a swapRecord silently dropped from a create is a test that believes it proved +// optimistic locking and proved nothing. +type RecordOption func(*recordConfig) + +// WithRKey sets the record key instead of generating a TID. CreateRecord only. +func WithRKey(rkey string) RecordOption { + return func(c *recordConfig) { c.rkey = rkey } +} + +// WithSwapRecord makes the write conditional on the record's current CID, the +// compare-and-swap atProto offers for lost-update protection. PutRecord only. +func WithSwapRecord(cid string) RecordOption { + return func(c *recordConfig) { c.swapRecord = cid } +} + +// CreateRecord writes a new record to this account's repo. +// +// record may be a map or a struct; it is marshalled as JSON, so a struct needs +// its lexicon field names in tags, including "$type" where the lexicon requires +// one. +func (a *Account) CreateRecord(t TestingT, collection string, record any, opts ...RecordOption) Record { + t.Helper() + cfg := newRecordConfig(opts) + if cfg.swapRecord != "" { + t.Fatalf("testkit: WithSwapRecord does not apply to CreateRecord — there is no existing record " + + "to compare a CID against; use PutRecord for a conditional write") + return Record{} + } + if cfg.rkey == "" { + cfg.rkey = TID() + } + return a.writeRecord(t, "com.atproto.repo.createRecord", map[string]any{ + "repo": a.DID, + "collection": collection, + "rkey": cfg.rkey, + "record": record, + }, collection, cfg.rkey) +} + +// PutRecord creates or replaces a record at a known key. This is how a contract +// exercises the update half of create/update/delete. +func (a *Account) PutRecord(t TestingT, collection, rkey string, record any, opts ...RecordOption) Record { + t.Helper() + cfg := newRecordConfig(opts) + if cfg.rkey != "" { + t.Fatalf("testkit: WithRKey does not apply to PutRecord — the record key is the rkey argument, "+ + "and passing %q as an option would silently disagree with %q", cfg.rkey, rkey) + return Record{} + } + payload := map[string]any{ + "repo": a.DID, + "collection": collection, + "rkey": rkey, + "record": record, + } + if cfg.swapRecord != "" { + payload["swapRecord"] = cfg.swapRecord + } + return a.writeRecord(t, "com.atproto.repo.putRecord", payload, collection, rkey) +} + +func newRecordConfig(opts []RecordOption) recordConfig { + var cfg recordConfig + for _, opt := range opts { + opt(&cfg) + } + return cfg +} + +func (a *Account) writeRecord(t TestingT, nsid string, payload map[string]any, collection, rkey string) Record { + t.Helper() + var resp struct { + URI string `json:"uri"` + CID string `json:"cid"` + } + ctx, cancel := context.WithTimeout(context.Background(), defaultXRPCTimeout) + defer cancel() + if err := a.XRPC().Procedure(ctx, nsid, payload, &resp); err != nil { + t.Fatalf("testkit: writing %s/%s to %s: %v", collection, rkey, a.DID, err) + return Record{} + } + // A 200 with no uri means the body was not a record-write response. The + // caller is about to build a firehose matcher out of that URI. + if resp.URI == "" || resp.CID == "" { + t.Fatalf("testkit: writing %s/%s to %s: PDS answered 200 without uri/cid", + collection, rkey, a.DID) + return Record{} + } + return Record{URI: resp.URI, CID: resp.CID, Collection: collection, RKey: rkey} +} + +// GetRecord reads a record back from this account's repo. A missing record +// fails the test; use XRPC() with IsNotFound to assert absence. +func (a *Account) GetRecord(t TestingT, collection, rkey string) RecordValue { + t.Helper() + var resp struct { + URI string `json:"uri"` + CID string `json:"cid"` + Value map[string]any `json:"value"` + } + ctx, cancel := context.WithTimeout(context.Background(), defaultXRPCTimeout) + defer cancel() + err := a.XRPC().Query(ctx, "com.atproto.repo.getRecord", url.Values{ + "repo": {a.DID}, + "collection": {collection}, + "rkey": {rkey}, + }, &resp) + if err != nil { + t.Fatalf("testkit: reading %s/%s from %s: %v", collection, rkey, a.DID, err) + return RecordValue{} + } + return RecordValue{URI: resp.URI, CID: resp.CID, Value: resp.Value} +} + +// DeleteRecord removes a record from this account's repo. +// +// IT IS SILENT ABOUT A RECORD THAT WAS NOT THERE. com.atproto.repo.deleteRecord +// answers 200 for a key that does not exist, and — because nothing changed — the +// repo commits nothing and the firehose emits NOTHING. A test that deletes the +// wrong rkey and then awaits the delete event therefore waits out its whole +// timeout on a call that looked like it succeeded. +// +// Use DeleteExistingRecord whenever an Await is going to depend on the event. +func (a *Account) DeleteRecord(t TestingT, collection, rkey string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), defaultXRPCTimeout) + defer cancel() + err := a.XRPC().Procedure(ctx, "com.atproto.repo.deleteRecord", map[string]any{ + "repo": a.DID, + "collection": collection, + "rkey": rkey, + }, nil) + if err != nil { + t.Fatalf("testkit: deleting %s/%s from %s: %v", collection, rkey, a.DID, err) + } +} + +// DeleteExistingRecord reads a record and then deletes it, failing the test if +// it was not there to begin with. +// +// The read is what makes the delete observable: a delete that removed nothing +// emits no commit, so pairing DeleteRecord with an Await on the delete event +// turns a wrong rkey into a timeout that blames the firehose. Proving the record +// existed first turns it into a failure that names the record. +func (a *Account) DeleteExistingRecord(t TestingT, collection, rkey string) { + t.Helper() + a.GetRecord(t, collection, rkey) + a.DeleteRecord(t, collection, rkey) +} + +// --------------------------------------------------------------------------- +// Blobs +// --------------------------------------------------------------------------- + +// BlobRef is a reference to an uploaded blob, in the exact JSON shape a record +// must embed it in. +// +// It is testkit's own type for the same reason firehose.go declares its own +// event structs: the production one lives in internal/core/blobs, which testkit +// may not import. The wire format is what matters and it is fixed by the +// lexicon, so the risk of the two drifting apart is a spec change, not a +// refactor. +type BlobRef struct { + Type string `json:"$type"` + Ref BlobLink `json:"ref"` + MimeType string `json:"mimeType"` + Size int64 `json:"size"` +} + +// BlobLink is the CID link inside a BlobRef. +type BlobLink struct { + Link string `json:"$link"` +} + +// CID returns the blob's content identifier. +func (b BlobRef) CID() string { return b.Ref.Link } + +// UploadBlob stores bytes in this account's blob store and returns the reference +// a record embeds. +// +// mimeType must be concrete: see XRPCClient.Upload for why a wildcard fails in a +// way that does not mention content types. +func (a *Account) UploadBlob(t TestingT, data []byte, mimeType string) BlobRef { + t.Helper() + var resp struct { + Blob BlobRef `json:"blob"` + } + ctx, cancel := context.WithTimeout(context.Background(), defaultXRPCTimeout) + defer cancel() + if err := a.XRPC().Upload(ctx, "com.atproto.repo.uploadBlob", mimeType, data, &resp); err != nil { + t.Fatalf("testkit: uploading a %d-byte %s blob to %s: %v", len(data), mimeType, a.DID, err) + return BlobRef{} + } + if resp.Blob.Ref.Link == "" { + t.Fatalf("testkit: uploading a %d-byte %s blob to %s: PDS answered 200 without a blob ref", + len(data), mimeType, a.DID) + return BlobRef{} + } + return resp.Blob +} + +// --------------------------------------------------------------------------- +// Domain interfaces +// --------------------------------------------------------------------------- + +// PasswordAuthFactory adapts a PDS-client constructor into the PDS-client +// factory a domain service expects. +// +// Five domain packages declare the identical type under five names — +// votes.PDSClientFactory, communities.PDSClientFactory, +// userblocks.PDSClientFactory, comments.PDSClientFactory, and a bare function +// type in the user-profile wiring — all of them +// +// func(context.Context, *oauth.ClientSessionData) (pds.Client, error) +// +// testkit cannot name pds.Client (internal/atproto/pds imports +// internal/core/blobs; see the package doc), so it cannot return any of them. +// It can, however, be generic over the client type and take the constructor as +// an argument, which moves the illegal import to the call site where it is +// perfectly legal: +// +// svc := communities.NewService(repo, testkit.PasswordAuthFactory(pds.NewFromAccessToken)) +// +// The returned unnamed function type is assignable to each of the five named +// ones, so no conversion is needed. That one line replaces the four adapters in +// tests/integration/helpers.go, and the validation they each did slightly +// differently now happens once, here. +func PasswordAuthFactory[C any](newClient func(host, did, accessToken string) (C, error)) func(context.Context, *oauth.ClientSessionData) (C, error) { + return func(_ context.Context, session *oauth.ClientSessionData) (C, error) { + var zero C + switch { + case session == nil: + return zero, fmt.Errorf("testkit: no session") + case session.AccessToken == "": + return zero, fmt.Errorf("testkit: session for %s has no access token", session.AccountDID) + case session.HostURL == "": + return zero, fmt.Errorf("testkit: session for %s has no host URL", session.AccountDID) + } + return newClient(session.HostURL, session.AccountDID.String(), session.AccessToken) + } +} diff --git a/tests/testkit/pds_test.go b/tests/testkit/pds_test.go new file mode 100644 index 0000000..3c85927 --- /dev/null +++ b/tests/testkit/pds_test.go @@ -0,0 +1,564 @@ +package testkit + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// testCollection is a namespace no consumer subscribes to, so records written +// by this file's tests reach Jetstream (which is what firehose_test.go needs) +// without being indexed by a running AppView. +const testCollection = "social.coves.testkit.record" + +// testPDS returns the local PDS, failing the test if it is not there. +// +// Not a skip. testkit's own tests are the proof that the harness works against +// real infrastructure, and a harness whose tests quietly pass when the +// infrastructure is missing proves nothing at all — that is the failure mode +// the whole refactor exists to remove. +func testPDS(t *testing.T) *PDS { + t.Helper() + p := NewPDS(t) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := p.Anon.Health(ctx); err != nil { + t.Fatalf("testkit's PDS tests need the local PDS at %s: %v\n"+ + " start the stack with 'make dev-up', or run the whole suite through 'make ci'", p.URL(), err) + } + return p +} + +func TestPDS_CreateAccountIssuesAUsableSession(t *testing.T) { + p := testPDS(t) + + account := p.CreateAccount(t, WithHandlePrefix("kit")) + + require.NotEmpty(t, account.DID) + assert.True(t, strings.HasPrefix(account.DID, "did:"), "expected a DID, got %q", account.DID) + assert.NotEmpty(t, account.AccessToken) + assert.Equal(t, DefaultPassword, account.Password) + assert.Contains(t, account.Email, "@") + + // The handle has to fit the PDS' 18-character local-label cap and land in a + // domain the PDS serves — the two things every hand-rolled generator in the + // old suite got wrong at least once. + label, domain, found := strings.Cut(account.Handle, ".") + require.True(t, found, "handle %q has no domain", account.Handle) + assert.Equal(t, Endpoints().PDS.HandleDomain, domain) + assert.LessOrEqual(t, len(label), MaxIDLength, "local label %q exceeds the PDS cap", label) + assert.True(t, strings.HasPrefix(label, "kit"), "prefix should survive into %q", label) +} + +func TestPDS_LoginReopensTheSameAccount(t *testing.T) { + p := testPDS(t) + created := p.CreateAccount(t) + + reopened := p.Login(t, created.Handle, created.Password) + + assert.Equal(t, created.DID, reopened.DID) + assert.Equal(t, created.Handle, reopened.Handle) + assert.NotEmpty(t, reopened.AccessToken) +} + +func TestPDS_LoginWithABadPasswordFailsWithTheReason(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + ft := &fakeT{} + runIsolated(func() { p.Login(ft, account.Handle, "not-the-password") }) + + require.True(t, ft.failed()) + msg := ft.message() + assert.Contains(t, msg, account.Handle) + assert.Contains(t, msg, p.URL(), "the failure should say which PDS refused") + assert.Contains(t, msg, "com.atproto.server.createSession") + assert.NotContains(t, msg, "not-the-password", "a failure message must not echo a credential") +} + +func TestAccount_RecordRoundTrip(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + created := account.CreateRecord(t, testCollection, map[string]any{ + "$type": testCollection, + "text": "first", + }) + + require.NotEmpty(t, created.URI) + require.NotEmpty(t, created.CID) + assert.Equal(t, testCollection, created.Collection) + assert.Equal(t, "at://"+account.DID+"/"+testCollection+"/"+created.RKey, created.URI) + _, err := syntax.ParseTID(created.RKey) + assert.NoError(t, err, "the generated rkey should be a real TID") + + read := account.GetRecord(t, testCollection, created.RKey) + assert.Equal(t, created.URI, read.URI) + assert.Equal(t, created.CID, read.CID) + assert.Equal(t, "first", read.Value["text"]) + + updated := account.PutRecord(t, testCollection, created.RKey, map[string]any{ + "$type": testCollection, + "text": "second", + }) + assert.Equal(t, created.URI, updated.URI) + assert.NotEqual(t, created.CID, updated.CID, "a changed record must have a new CID") + assert.Equal(t, "second", account.GetRecord(t, testCollection, created.RKey).Value["text"]) + + account.DeleteRecord(t, testCollection, created.RKey) + + // Absence is asserted through the error-returning client: the helpers fail + // the test on error, which is the wrong shape for "this should be gone". + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + err = account.XRPC().Query(ctx, "com.atproto.repo.getRecord", recordParams(account.DID, testCollection, created.RKey), nil) + require.Error(t, err) + assert.True(t, IsNotFound(err), "a deleted record should read as not-found, got %v", err) +} + +func TestAccount_CreateRecordAcceptsAnExplicitRKey(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + rkey := UniqueID(t) + created := account.CreateRecord(t, testCollection, map[string]any{"$type": testCollection}, WithRKey(rkey)) + + assert.Equal(t, rkey, created.RKey) + assert.Equal(t, created.URI, account.GetRecord(t, testCollection, rkey).URI) +} + +func TestAccount_CreateRecordFailureNamesTheRecord(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + rkey := UniqueID(t) + account.CreateRecord(t, testCollection, map[string]any{"$type": testCollection}, WithRKey(rkey)) + + // Creating the same key twice is a conflict, which is a convenient way to + // make a real PDS reject a real write. + ft := &fakeT{} + runIsolated(func() { + account.CreateRecord(ft, testCollection, map[string]any{"$type": testCollection}, WithRKey(rkey)) + }) + + require.True(t, ft.failed()) + assert.Contains(t, ft.message(), testCollection) + assert.Contains(t, ft.message(), rkey) +} + +func TestAccount_UploadBlobRoundTripsIntoARecord(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + png := TestPNG(64, 48) + ref := account.UploadBlob(t, png, "image/png") + + assert.Equal(t, "blob", ref.Type) + assert.Equal(t, "image/png", ref.MimeType) + assert.Equal(t, int64(len(png)), ref.Size) + assert.NotEmpty(t, ref.CID()) + + // The reference has to survive being embedded in a record and read back: + // testkit declares its own BlobRef (it may not import internal/core/blobs), + // so the JSON shape is the thing actually under test here. + created := account.CreateRecord(t, testCollection, map[string]any{ + "$type": testCollection, + "image": ref, + }) + value := account.GetRecord(t, testCollection, created.RKey).Value + + embedded, ok := value["image"].(map[string]any) + require.True(t, ok, "the blob did not survive as an object: %#v", value["image"]) + assert.Equal(t, "blob", embedded["$type"]) + assert.Equal(t, "image/png", embedded["mimeType"]) + link, ok := embedded["ref"].(map[string]any) + require.True(t, ok, "the blob ref lost its $link wrapper: %#v", embedded["ref"]) + assert.Equal(t, ref.CID(), link["$link"]) +} + +func TestAccount_UploadBlobRejectsAWildcardMIMEType(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + // Rejected locally, before any request: a PDS enforcing the granular + // blob:*/* scope answers a wildcard content type with a scope error that + // never mentions content types. + err := account.XRPC().Upload(ctx, "com.atproto.repo.uploadBlob", "*/*", TestPNG(8, 8), nil) + + require.Error(t, err) + assert.Contains(t, err.Error(), "concrete MIME type") +} + +func TestPDS_WaitHealthy(t *testing.T) { + p := testPDS(t) + p.WaitHealthy(t, 10*time.Second) +} + +func TestAccount_PutRecordHonoursASwapCID(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + created := account.CreateRecord(t, testCollection, map[string]any{"$type": testCollection, "text": "v1"}) + updated := account.PutRecord(t, testCollection, created.RKey, + map[string]any{"$type": testCollection, "text": "v2"}, WithSwapRecord(created.CID)) + require.NotEqual(t, created.CID, updated.CID) + + // The stale CID now names a version that is no longer current, which is the + // lost-update the compare-and-swap exists to refuse. An option that was + // accepted and ignored would make this write succeed. + ft := &fakeT{} + runIsolated(func() { + account.PutRecord(ft, testCollection, created.RKey, + map[string]any{"$type": testCollection, "text": "v3"}, WithSwapRecord(created.CID)) + }) + require.True(t, ft.failed(), "a write against a stale CID must be refused") + assert.Contains(t, ft.message(), created.RKey) + + assert.Equal(t, "v2", account.GetRecord(t, testCollection, created.RKey).Value["text"], + "the refused write must not have landed") +} + +func TestAccount_InapplicableRecordOptionsFailLoudly(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + // Silently ignoring either of these would leave a test believing it proved + // something it never asked the PDS to do. + ft := &fakeT{} + runIsolated(func() { + account.CreateRecord(ft, testCollection, map[string]any{"$type": testCollection}, + WithSwapRecord("bafyreiabc")) + }) + require.True(t, ft.failed()) + assert.Contains(t, ft.message(), "WithSwapRecord does not apply to CreateRecord") + + ft = &fakeT{} + runIsolated(func() { + account.PutRecord(ft, testCollection, "the-argument", + map[string]any{"$type": testCollection}, WithRKey("the-option")) + }) + require.True(t, ft.failed()) + assert.Contains(t, ft.message(), "WithRKey does not apply to PutRecord") + assert.Contains(t, ft.message(), "the-option") +} + +// TestAccount_DeleteExistingRecordCatchesTheSilentNoOp covers the trap that +// makes a delete look successful when it deleted nothing. +// +// com.atproto.repo.deleteRecord answers 200 for a key that was never there, and +// commits nothing — so the firehose emits nothing, and a test that pairs the +// delete with an Await waits out its whole timeout blaming the pipeline. +func TestAccount_DeleteExistingRecordCatchesTheSilentNoOp(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + + // The plain delete is happy to do nothing at all. + account.DeleteRecord(t, testCollection, "never-existed") + + ft := &fakeT{} + runIsolated(func() { account.DeleteExistingRecord(ft, testCollection, "never-existed") }) + require.True(t, ft.failed(), "deleting a record that is not there should be a failure, not a no-op") + assert.Contains(t, ft.message(), "never-existed") + + // And it still deletes what is there. + created := account.CreateRecord(t, testCollection, map[string]any{"$type": testCollection}) + account.DeleteExistingRecord(t, testCollection, created.RKey) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + err := account.XRPC().Query(ctx, "com.atproto.repo.getRecord", + recordParams(account.DID, testCollection, created.RKey), nil) + assert.True(t, IsNotFound(err), "the record should be gone, got %v", err) +} + +func TestAccount_RefreshSessionSwapsBothTokens(t *testing.T) { + p := testPDS(t) + account := p.CreateAccount(t) + originalAccess, originalRefresh := account.AccessToken, account.RefreshToken + + account.RefreshSession(t) + + assert.NotEmpty(t, account.AccessToken) + assert.NotEmpty(t, account.RefreshToken) + assert.NotEqual(t, originalRefresh, account.RefreshToken, + "refreshSession spends the refresh token; keeping the old one would fail the next call") + + // The client has to move with the tokens, or the next write would go out + // under the previous session. + created := account.CreateRecord(t, testCollection, map[string]any{"$type": testCollection}) + assert.NotEmpty(t, created.URI) + _ = originalAccess +} + +// TestAccount_DoesNotPrintItsCredentials guards every fmt verb, because the one +// that leaks is the one nobody thought to check. +func TestAccount_DoesNotPrintItsCredentials(t *testing.T) { + account := &Account{ + Handle: "alice.local.coves.dev", + DID: "did:plc:alice", + Email: "alice@test.com", + Password: "super-secret-password", + AccessToken: "access-token-value", + RefreshToken: "refresh-token-value", + } + secrets := []string{"super-secret-password", "access-token-value", "refresh-token-value"} + + for _, format := range []string{"%v", "%+v", "%s", "%#v", "%q"} { + rendered := fmt.Sprintf(format, account) + for _, secret := range secrets { + assert.NotContains(t, rendered, secret, + "%s leaked a credential into what may become CI output", format) + } + // Still useful: the identity a reader actually needs is all there. + if format != "%q" { + assert.Contains(t, rendered, "alice.local.coves.dev", "%s should still identify the account", format) + assert.Contains(t, rendered, "REDACTED", "%s should say something was withheld", format) + } + } + + var missing *Account + assert.Equal(t, "", fmt.Sprintf("%v", missing)) +} + +// --------------------------------------------------------------------------- +// TIDs +// --------------------------------------------------------------------------- + +func TestTID_IsAValidStrictlyIncreasingTID(t *testing.T) { + const n = 2000 + previous := "" + for i := 0; i < n; i++ { + tid := TID() + require.Len(t, tid, 13, "a TID is 13 characters") + _, err := syntax.ParseTID(tid) + require.NoError(t, err, "TID() produced %q", tid) + // Strictly increasing AND lexicographically sortable: rkeys order + // records in a repo listing, so a TID that only increases numerically + // would sort wrongly. + require.Greater(t, tid, previous, "TIDs must increase; %q followed %q", tid, previous) + previous = tid + } +} + +func TestTID_IsSafeUnderConcurrency(t *testing.T) { + const goroutines, each = 8, 200 + results := make(chan string, goroutines*each) + done := make(chan struct{}) + for i := 0; i < goroutines; i++ { + go func() { + defer func() { done <- struct{}{} }() + for j := 0; j < each; j++ { + results <- TID() + } + }() + } + for i := 0; i < goroutines; i++ { + <-done + } + close(results) + + seen := map[string]bool{} + for tid := range results { + require.False(t, seen[tid], "TID %q was issued twice", tid) + seen[tid] = true + } + assert.Len(t, seen, goroutines*each) +} + +// --------------------------------------------------------------------------- +// Account creation against a PDS the test controls +// --------------------------------------------------------------------------- +// +// These cover what a real PDS cannot be asked for on demand: a malformed +// session response, and the exact request body sent for options whose effect is +// otherwise invisible. + +// capturingPDS records the createAccount payload it was sent and answers with a +// canned session. +func capturingPDS(t *testing.T, respond func(http.ResponseWriter)) (*PDS, func() map[string]any) { + t.Helper() + var captured atomic.Pointer[map[string]any] + stub := newStubService(t, func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err == nil { + captured.Store(&body) + } + respond(w) + }) + return NewPDS(t, WithPDSURL(stub.URL), WithPDSHandleDomain("stub.test")), + func() map[string]any { + if got := captured.Load(); got != nil { + return *got + } + return nil + } +} + +func validSession(w http.ResponseWriter) { + writeJSON(w, http.StatusOK, map[string]any{ + "did": "did:plc:stubbed", "handle": "stubbed.stub.test", + "accessJwt": "access-jwt", "refreshJwt": "refresh-jwt", + }) +} + +func TestPDS_WithHandleBypassesGeneration(t *testing.T) { + pds, payload := capturingPDS(t, validSession) + + // Deliberately over the 18-character budget: WithHandle is the escape hatch + // for tests that are ABOUT the handle, so it must pass the value through + // untouched rather than sanitising it into something that no longer tests + // what was asked. + const handle = "a-very-long-local-label-indeed.stub.test" + pds.CreateAccount(t, WithHandle(handle), WithEmail("someone@test.com"), WithPassword("pw")) + + body := payload() + require.NotNil(t, body) + assert.Equal(t, handle, body["handle"]) + assert.Equal(t, "someone@test.com", body["email"]) + assert.Equal(t, "pw", body["password"]) + assert.NotContains(t, body, "inviteCode", "an absent invite code must not be sent as an empty string") +} + +func TestPDS_WithInviteCodeIsSent(t *testing.T) { + pds, payload := capturingPDS(t, validSession) + + pds.CreateAccount(t, WithInviteCode("stub-invite-1234")) + + body := payload() + require.NotNil(t, body) + assert.Equal(t, "stub-invite-1234", body["inviteCode"]) + // The generated handle still lands in the configured domain. + assert.Contains(t, body["handle"], ".stub.test") +} + +func TestPDS_GeneratedEmailFollowsTheHandle(t *testing.T) { + pds, payload := capturingPDS(t, validSession) + + account := pds.CreateAccount(t, WithHandlePrefix("trace")) + + body := payload() + require.NotNil(t, body) + // Derived from the handle that was REQUESTED — the stub answers with a + // canned handle of its own, and the account carries that, so the request is + // the only place the generated pair can be compared. + requested, ok := body["handle"].(string) + require.True(t, ok) + assert.True(t, strings.HasPrefix(requested, "trace"), "the prefix should reach the PDS: %q", requested) + assert.Equal(t, handleLabel(requested)+"@test.com", body["email"]) + assert.Equal(t, account.Email, body["email"]) +} + +func TestPDS_RejectsAMalformedSessionResponse(t *testing.T) { + // A 200 whose body is not a session. Accepting it would send an + // unauthenticated client into the next twenty lines of the test, where it + // fails as a 401 on something unrelated. + for name, respond := range map[string]func(http.ResponseWriter){ + "no did": func(w http.ResponseWriter) { + writeJSON(w, http.StatusOK, map[string]any{"accessJwt": "a", "refreshJwt": "r"}) + }, + "no access token": func(w http.ResponseWriter) { + writeJSON(w, http.StatusOK, map[string]any{"did": "did:plc:x", "refreshJwt": "r"}) + }, + "no refresh token": func(w http.ResponseWriter) { + writeJSON(w, http.StatusOK, map[string]any{"did": "did:plc:x", "accessJwt": "a"}) + }, + "empty body": func(w http.ResponseWriter) { + writeJSON(w, http.StatusOK, map[string]any{}) + }, + } { + t.Run(name, func(t *testing.T) { + pds, _ := capturingPDS(t, respond) + ft := &fakeT{} + runIsolated(func() { pds.CreateAccount(ft) }) + require.True(t, ft.failed(), "a session missing a credential must not be accepted") + assert.Contains(t, ft.message(), "testkit: creating account") + }) + } +} + +// --------------------------------------------------------------------------- +// The factory adapter +// --------------------------------------------------------------------------- + +// fakeClient stands in for internal/atproto/pds.Client, which testkit may not +// import. The real assignability — that PasswordAuthFactory's return type +// satisfies votes.PDSClientFactory and its four siblings — is exercised at the +// call sites in phase 3, where importing a domain package is legal. +type fakeClient struct { + host, did, token string +} + +func newFakeClient(host, did, token string) (*fakeClient, error) { + return &fakeClient{host: host, did: did, token: token}, nil +} + +func TestPasswordAuthFactory_PassesTheSessionThrough(t *testing.T) { + factory := PasswordAuthFactory(newFakeClient) + + did, err := syntax.ParseDID("did:plc:abc123") + require.NoError(t, err) + client, err := factory(context.Background(), &oauth.ClientSessionData{ + AccountDID: did, + AccessToken: "token-abc", + HostURL: "http://pds.test", + }) + + require.NoError(t, err) + assert.Equal(t, "http://pds.test", client.host) + assert.Equal(t, "did:plc:abc123", client.did) + assert.Equal(t, "token-abc", client.token) +} + +func TestPasswordAuthFactory_PropagatesAConstructorRejection(t *testing.T) { + // The real constructor validates its arguments too. A factory that + // swallowed that error would hand back a nil client and move the failure to + // whichever line first used it. + failing := func(host, did, token string) (*fakeClient, error) { + return nil, fmt.Errorf("constructor refused host %q", host) + } + factory := PasswordAuthFactory(failing) + + did, err := syntax.ParseDID("did:plc:abc123") + require.NoError(t, err) + client, err := factory(context.Background(), &oauth.ClientSessionData{ + AccountDID: did, AccessToken: "token-abc", HostURL: "http://pds.test", + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "constructor refused host") + assert.Nil(t, client) +} + +func TestPasswordAuthFactory_RejectsUnusableSessions(t *testing.T) { + did, err := syntax.ParseDID("did:plc:abc123") + require.NoError(t, err) + + for name, session := range map[string]*oauth.ClientSessionData{ + "no session": nil, + "no access token": {AccountDID: did, HostURL: "http://pds.test"}, + "no host": {AccountDID: did, AccessToken: "token-abc"}, + } { + t.Run(name, func(t *testing.T) { + factory := PasswordAuthFactory(newFakeClient) + client, err := factory(context.Background(), session) + require.Error(t, err, "an unusable session must not produce a client") + assert.Nil(t, client) + }) + } +} + +// recordParams builds the query a repo read takes. +func recordParams(repo, collection, rkey string) url.Values { + return url.Values{"repo": {repo}, "collection": {collection}, "rkey": {rkey}} +} diff --git a/tests/testkit/testkit.go b/tests/testkit/testkit.go index 5b6430e..e7355a0 100644 --- a/tests/testkit/testkit.go +++ b/tests/testkit/testkit.go @@ -20,20 +20,64 @@ // domain's own _test.go files or in a small leaf test package — never // here. // +// THE RULE IS TRANSITIVE, and that is not a technicality — it decides the two +// biggest design questions in this package: +// +// - internal/atproto/pds is a perfectly good PDS client, and testkit cannot +// use it: it imports internal/core/blobs for its BlobRef type. Wrapping it +// would make `go test ./internal/core/blobs` an import cycle. pds.go +// therefore speaks XRPC over net/http directly, which is also what the +// helpers it replaces did. +// - internal/atproto/jetstream owns the JetstreamEvent structs, and testkit +// cannot use those either: its consumers import communities, posts, +// comments, users, votes, userblocks and aggregators. firehose.go declares +// the four wire structs it needs (~30 lines) rather than dragging seven +// domain packages into every test binary. +// +// Neither is a duplication testkit is free to remove later; both are load +// bearing. See the PACKAGE DOC in firehose.go for the field-drift risk this +// leaves and how it is contained. +// +// # Domain interfaces (the factory-adapter pattern) +// +// Several domain services take a PDS-client factory whose type is named by the +// domain itself — votes.PDSClientFactory, communities.PDSClientFactory, and so +// on — which are identical function types under four different names. testkit +// cannot return any of them, so instead of four adapters it exports one generic +// one, PasswordAuthFactory, which a call site instantiates with the domain's +// own constructor in a single line. See pds.go. +// // # Layout // // testkit.go — endpoints, log silencing, the TestingT contract // db.go — Postgres isolation: template-clone-per-test // wait.go — the only waiting primitives tests are allowed to use // fixtures.go — UniqueID, PNG/JPEG image bytes +// pds.go — accounts, sessions, records and blobs on the test PDS +// firehose.go — the one cursor-gated Jetstream subscriber +// appview.go — the XRPC client, and the AppView it talks to +// +// # The canonical TestMain +// +// Every package that uses this kit starts the same way, and Main is that +// starting point — log silencing plus an up-front probe of whatever +// infrastructure the package needs: +// +// func TestMain(m *testing.M) { os.Exit(testkit.Main(m, testkit.RequirePostgres)) } package testkit import ( + "context" "fmt" "io" "log" "log/slog" "os" + "strings" + "testing" + "time" + + "github.com/gorilla/websocket" ) // TestingT is the subset of *testing.T that testkit needs. @@ -82,6 +126,35 @@ func (p PostgresEndpoint) Redacted(database string) string { return fmt.Sprintf("postgres://%s@%s:%s/%s", p.User, p.Host, p.Port, database) } +// ServiceEndpoint is a plain HTTP service located by its base URL, with no +// trailing slash. +type ServiceEndpoint struct { + BaseURL string +} + +// PDSEndpoint locates the test PDS and names the domain it issues handles under. +type PDSEndpoint struct { + BaseURL string + // HandleDomain is the suffix every generated handle carries, without a + // leading dot: "local.coves.dev". A handle outside the PDS' configured + // service domains is rejected at account creation, so this is not + // cosmetic — it is the difference between a working account and an + // "InvalidHandle" that reads like a bug in the generator. + HandleDomain string +} + +// Handle renders a local label as a full handle on this PDS. +func (p PDSEndpoint) Handle(label string) string { + return label + "." + p.HandleDomain +} + +// JetstreamEndpoint locates the test Jetstream and the path its subscriptions +// are opened on. +type JetstreamEndpoint struct { + BaseURL string // ws://host:port + SubscribePath string // /subscribe +} + // EndpointSet holds the address of every service the test stack provides. // // This is the only place test code reads infrastructure coordinates from the @@ -89,11 +162,12 @@ func (p PostgresEndpoint) Redacted(database string) string { // silently talking to a developer's dev stack — or to the public internet — // so endpoint literals in tests are counted as violations by // scripts/test-audit.sh. -// -// Later phases extend this struct with the PDS, PLC, Jetstream and AppView -// addresses; the Postgres coordinates are all the database harness needs. type EndpointSet struct { - Postgres PostgresEndpoint + Postgres PostgresEndpoint + PDS PDSEndpoint + PLC ServiceEndpoint + Jetstream JetstreamEndpoint + AppView ServiceEndpoint } // Endpoints returns the test stack's coordinates, read from the environment @@ -119,9 +193,52 @@ func loadEndpoints() EndpointSet { Database: envOr("POSTGRES_TEST_DB", "coves_test"), SSLMode: envOr("POSTGRES_TEST_SSLMODE", "disable"), }, + PDS: PDSEndpoint{ + BaseURL: trimURL(envOr("PDS_URL", "http://localhost:3001")), + HandleDomain: firstHandleDomain(os.Getenv("PDS_SERVICE_HANDLE_DOMAINS")), + }, + PLC: ServiceEndpoint{ + BaseURL: trimURL(envOr("PLC_DIRECTORY_URL", "http://localhost:3002")), + }, + Jetstream: JetstreamEndpoint{ + // JETSTREAM_TEST_URL rather than the JETSTREAM_FEEDS the server + // reads: that variable is a list of key=url pairs for the AppView's + // consumers, and a test subscribing to one endpoint should not have + // to parse a consumer topology to find it. + BaseURL: trimURL(envOr("JETSTREAM_TEST_URL", "ws://localhost:6008")), + SubscribePath: "/subscribe", + }, + AppView: ServiceEndpoint{ + // APPVIEW_PUBLIC_URL is what the server itself publishes and what + // .env.ci sets, so it is the honest second choice: if the AppView + // believes it lives somewhere, that is where tests should call it. + BaseURL: trimURL(envOr("APPVIEW_URL", envOr("APPVIEW_PUBLIC_URL", "http://localhost:8081"))), + }, } } +// firstHandleDomain reads the PDS' service handle domains — the same +// ".local.coves.dev,.coves.social" the PDS container is configured with — and +// returns the first, without its leading dot. +// +// Sharing the variable with the container means a stack that changes its handle +// domain cannot leave the generator behind: handles would be rejected at the +// PDS with an error nobody would think to trace back to a test helper. +func firstHandleDomain(configured string) string { + first, _, _ := strings.Cut(configured, ",") + first = strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(first), ".")) + if first == "" { + return "local.coves.dev" + } + return first +} + +// trimURL drops a trailing slash, so joining a path onto a base URL never +// produces a double slash — which some routers treat as a different route. +func trimURL(raw string) string { + return strings.TrimRight(strings.TrimSpace(raw), "/") +} + func envOr(key, fallback string) string { if v := os.Getenv(key); v != "" { return v @@ -132,10 +249,11 @@ func envOr(key, fallback string) string { // SilenceLogs discards application log output when LOG_ENABLED=false, which is // what the CI runner and `make test` set. // -// Call it from TestMain. It mirrors the per-package TestMain blocks that -// already exist across the tree, and additionally silences slog, which those -// blocks miss — the server logs through slog, so silencing only the standard -// logger leaves most of the noise in place. +// Call it from TestMain — or better, call Main, which does this and more. It +// mirrors the per-package TestMain blocks that already exist across the tree, +// and additionally silences slog, which those blocks miss: the server logs +// through slog, so silencing only the standard logger leaves most of the noise +// in place. // // This mutates process-global state, so it belongs in TestMain and nowhere // else: a test calling it mid-run would race any test reading log output. @@ -146,3 +264,77 @@ func SilenceLogs() { log.SetOutput(io.Discard) slog.SetDefault(slog.New(slog.NewTextHandler(io.Discard, nil))) } + +// Main is the canonical TestMain body. Every migrated package uses it: +// +// func TestMain(m *testing.M) { os.Exit(testkit.Main(m)) } +// +// It silences logs, then runs the tests. A package whose tests need +// infrastructure names it, and the probe runs ONCE, before any test: +// +// func TestMain(m *testing.M) { os.Exit(testkit.Main(m, testkit.RequirePDS)) } +// +// The reason to probe up front rather than let the first test fail is +// attribution. Without it, a stack that is not running produces one confusing +// failure per test — thirty timeouts blaming thirty different features — and +// the reader has to notice they share a cause. With it, the package fails once +// with the address it could not reach and the command that starts it. +// +// Requirements are opt-in because most packages need none: a table-driven test +// of a pure function should not fail because Postgres is down. +func Main(m *testing.M, requires ...Requirement) int { + SilenceLogs() + for _, require := range requires { + if err := require(); err != nil { + fmt.Fprintf(os.Stderr, "\ntestkit: the test stack is not ready\n %v\n\n"+ + " start it with 'make dev-up', or run the whole suite through 'make ci'.\n\n", err) + return 1 + } + } + return m.Run() +} + +// A Requirement probes one service and explains what is missing. +type Requirement func() error + +// RequirePostgres fails the package unless the test database answers. +func RequirePostgres() error { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + return WaitForPostgres(ctx, 10*time.Second) +} + +// RequirePDS fails the package unless the PDS answers. +func RequirePDS() error { return probeHTTP("PDS", Endpoints().PDS.BaseURL) } + +// RequireAppView fails the package unless the AppView answers. +func RequireAppView() error { return probeHTTP("AppView", Endpoints().AppView.BaseURL) } + +// RequireJetstream fails the package unless a subscription can be opened. +// +// It dials rather than probing an HTTP port: the Jetstream image in this stack +// runs with its health endpoint disabled, so a TCP connect proves less than a +// handshake does. +func RequireJetstream() error { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + endpoint := Endpoints().Jetstream + conn, resp, err := websocket.DefaultDialer.DialContext(ctx, endpoint.BaseURL+endpoint.SubscribePath, nil) + if resp != nil { + _ = resp.Body.Close() + } + if err != nil { + return fmt.Errorf("Jetstream at %s did not accept a subscription: %w", endpoint.BaseURL, err) + } + _ = conn.Close() + return nil +} + +func probeHTTP(name, baseURL string) error { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if err := NewXRPCClient(baseURL).Health(ctx); err != nil { + return fmt.Errorf("%s at %s did not answer its health endpoint: %w", name, baseURL, err) + } + return nil +} diff --git a/tests/testkit/testkit_test.go b/tests/testkit/testkit_test.go index da1d5d0..b10e5d6 100644 --- a/tests/testkit/testkit_test.go +++ b/tests/testkit/testkit_test.go @@ -1,7 +1,6 @@ package testkit import ( - "sync" "testing" "github.com/stretchr/testify/assert" @@ -15,8 +14,7 @@ func TestEndpoints_DefaultsMatchTheTestStack(t *testing.T) { } { t.Setenv(key, "") } - endpointsOnce = sync.OnceValue(loadEndpoints) - t.Cleanup(func() { endpointsOnce = sync.OnceValue(loadEndpoints) }) + swapEndpoints(t) pg := Endpoints().Postgres // docker-compose.dev.yml publishes postgres-test here, and .env.ci's shared @@ -28,11 +26,62 @@ func TestEndpoints_DefaultsMatchTheTestStack(t *testing.T) { assert.Equal(t, "disable", pg.SSLMode) } +func TestEndpoints_ServiceDefaultsMatchTheTestStack(t *testing.T) { + for _, key := range []string{ + "PDS_URL", "PDS_SERVICE_HANDLE_DOMAINS", "PLC_DIRECTORY_URL", + "JETSTREAM_TEST_URL", "APPVIEW_URL", "APPVIEW_PUBLIC_URL", + } { + t.Setenv(key, "") + } + swapEndpoints(t) + + endpoints := Endpoints() + // The ports docker-compose.dev.yml publishes on the host, which .env.ci + // reproduces inside the hermetic stack's shared network namespace. + assert.Equal(t, "http://localhost:3001", endpoints.PDS.BaseURL) + assert.Equal(t, "local.coves.dev", endpoints.PDS.HandleDomain) + assert.Equal(t, "http://localhost:3002", endpoints.PLC.BaseURL) + assert.Equal(t, "ws://localhost:6008", endpoints.Jetstream.BaseURL) + assert.Equal(t, "/subscribe", endpoints.Jetstream.SubscribePath) + assert.Equal(t, "http://localhost:8081", endpoints.AppView.BaseURL) +} + +func TestEndpoints_AppViewFallsBackToThePublishedURL(t *testing.T) { + t.Setenv("APPVIEW_URL", "") + // What the server itself publishes, and what .env.ci sets. If the AppView + // believes it lives somewhere, that is where tests should call it. + t.Setenv("APPVIEW_PUBLIC_URL", "http://127.0.0.1:8081") + swapEndpoints(t) + + assert.Equal(t, "http://127.0.0.1:8081", Endpoints().AppView.BaseURL) +} + +func TestEndpoints_HandleDomainComesFromThePDSConfiguration(t *testing.T) { + // Verbatim from .env.dev and .env.ci: leading dots, more than one domain. + t.Setenv("PDS_SERVICE_HANDLE_DOMAINS", ".local.coves.dev,.coves.social") + swapEndpoints(t) + + pds := Endpoints().PDS + assert.Equal(t, "local.coves.dev", pds.HandleDomain) + assert.Equal(t, "alice.local.coves.dev", pds.Handle("alice")) +} + +func TestEndpoints_TrimTrailingSlashes(t *testing.T) { + t.Setenv("PDS_URL", "http://pds.test:3001/") + t.Setenv("APPVIEW_URL", "http://appview.test:8081//") + swapEndpoints(t) + + endpoints := Endpoints() + // Joining "/xrpc/..." onto a base ending in a slash yields "//xrpc/...", + // which some routers treat as a different path entirely. + assert.Equal(t, "http://pds.test:3001", endpoints.PDS.BaseURL) + assert.Equal(t, "http://appview.test:8081", endpoints.AppView.BaseURL) +} + func TestEndpoints_ReadTheEnvironment(t *testing.T) { t.Setenv("POSTGRES_TEST_HOST", "db.example.internal") t.Setenv("POSTGRES_TEST_PORT", "6543") - endpointsOnce = sync.OnceValue(loadEndpoints) - t.Cleanup(func() { endpointsOnce = sync.OnceValue(loadEndpoints) }) + swapEndpoints(t) pg := Endpoints().Postgres assert.Equal(t, "db.example.internal", pg.Host) @@ -40,8 +89,20 @@ func TestEndpoints_ReadTheEnvironment(t *testing.T) { } func TestEndpoints_MemoisedPerProcess(t *testing.T) { + swapEndpoints(t) + t.Setenv("PDS_URL", "http://first.test:3001") first := Endpoints() - require.Equal(t, first, Endpoints(), "endpoints are read once; a mid-run change would be invisible anyway") + require.Equal(t, "http://first.test:3001", first.PDS.BaseURL) + + // Changing the environment after the first read must not change the answer. + // Asserting only that two consecutive calls agree would pass against a + // function that re-read the environment every time, which is the property + // this test exists to rule out: a suite whose endpoints could change halfway + // through a run would have tests talking to two different stacks. + t.Setenv("PDS_URL", "http://second.test:3001") + + assert.Equal(t, first, Endpoints()) + assert.Equal(t, "http://first.test:3001", Endpoints().PDS.BaseURL) } func TestPostgresEndpoint_RedactedOmitsTheCredential(t *testing.T) { -- 2.51.2