Skip to content

CS-10009 PR 3: migrate server-endpoints/ tests to explicit fixture#4798

Open
lukemelia wants to merge 3 commits into
cs-10009-migrate-realm-endpointsfrom
cs-10009-migrate-server-endpoints
Open

CS-10009 PR 3: migrate server-endpoints/ tests to explicit fixture#4798
lukemelia wants to merge 3 commits into
cs-10009-migrate-realm-endpointsfrom
cs-10009-migrate-server-endpoints

Conversation

@lukemelia
Copy link
Copy Markdown
Contributor

Third slice of CS-10009. Stacked on #4790 — review/merge that one first; this PR's base will retarget to main once it lands.

Approach

Most callers in tests/server-endpoints/ go through the local setupServerEndpointsTest helper. Rather than touching ~15 individual setup sites, this PR extends that helper to accept an optional fixture parameter (default 'blank') and threads it down to setupPermissionedRealmCached. Server-level endpoint tests (bot-commands, webhooks, realm lifecycle, stripe sessions, etc.) almost universally manage server state without inspecting the testRealm's card content, so blank is the right default and each consumer overrides only when it truly needs more.

Per-file decisions

File Setup style Action Fixture
authentication-test.ts direct fileSystem: {} swap blank
bot-commands-test.ts helper default blank
bot-registration-test.ts helper default blank
delete-realm-test.ts helper default blank
download-realm-test.ts helper default blank
federated-types-test.ts inline SKIP
incoming-webhook-test.ts helper default blank
index-responses-test.ts (published-realm module) direct implicit-default swap realistic (asserts on data-test-home-card from tests/cards/home.gts)
index-responses-test.ts (first module) inline SKIP
info-test.ts inline SKIP
maintenance-endpoints-test.ts helper default blank
queue-status-test.ts already migrated in PR 1 blank
realm-lifecycle-test.ts (server-endpoints module) helper default blank
realm-lifecycle-test.ts ("realm mounted at server origin" module) direct implicit-default swap blank
run-command-endpoint-test.ts helper default blank
screenshot-card-endpoint-test.ts helper override realistic (references Person/fadhlan from tests/cards/)
search-prerendered-test.ts inline SKIP
search-test.ts inline SKIP
stripe-session-test.ts direct fileSystem: {} swap blank
stripe-webhook-test.ts direct implicit-default swap blank
user-and-catalog-test.ts helper default blank
webhook-commands-test.ts helper default blank
webhook-receiver-test.ts helper default blank

Local test plan

Tried locally; ran into two unrelated environment limits I want CI to validate past:

  1. boxel-realm-test-pg's 1 GB tmpfs PGDATA fills up under the maintenance/realm-lifecycle workloads (hundreds of No space left on device errors). Documented in CS-10009 follow-ups area; CI has more headroom.
  2. MATRIX_REGISTRATION_SHARED_SECRET=dummy doesn't match the local synapse HMAC, so any test that registers a new matrix user (the /_publish-realm path in delete-realm-test) 500s before assertions run. CI uses the real secret.

Test plan

  • CI runs every migrated test against the named fixture and they pass.
  • CI runs the kitchen-sink-dependent tests (screenshot-card-endpoint, index-responses published-realm module) against realistic and they pass.
  • CI runs every other realm-server test file unchanged.

🤖 Generated with Claude Code

Third slice of the CS-10009 plan. Stacked on PR 2 (realm-endpoints).

The bulk of these go through the shared `setupServerEndpointsTest`
helper. Extended it to accept an optional `fixture` parameter and
defaulted to `'blank'` — server-level endpoint tests (bot-commands,
webhooks, realm lifecycle, stripe sessions, etc.) almost universally
manage server state without inspecting the testRealm's card content,
so blank is the right default and individual consumers override only
when they truly need more.

Two consumers need realistic:
- screenshot-card-endpoint-test references `Person/fadhlan` from
  tests/cards/ — set `{ fixture: 'realistic' }` via the helper.
- index-responses-test's published-realm module asserts on a
  `data-test-home-card` substring, which only exists in
  tests/cards/home.gts — `fixture: 'realistic'` on its direct
  setupPermissionedRealmCached call.

Direct setupPermissionedRealmCached calls migrated:
- authentication-test: `fileSystem: {}` → `fixture: 'blank'`
- stripe-session-test: `fileSystem: {}` → `fixture: 'blank'`
- stripe-webhook-test: implicit default → `fixture: 'blank'`
- realm-lifecycle-test ("realm mounted at server origin" module):
  implicit default → `fixture: 'blank'`

Skipped (callers passing inline `fileSystem` per the plan):
federated-types-test, info-test, search-prerendered-test, search-test,
index-responses-test's first module, queue-status-test (already
migrated in PR 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

Host Test Results

    1 files      1 suites   1h 48m 47s ⏱️
2 658 tests 2 642 ✅ 15 💤 0 ❌ 1 🔥
2 677 runs  2 660 ✅ 15 💤 1 ❌ 1 🔥

Results for commit 97310d1.

For more details on these errors, see this check.

Realm Server Test Results

    1 files  ±    0      1 suites  +1   11m 20s ⏱️ + 11m 20s
1 328 tests +1 328  1 328 ✅ +1 328  0 💤 ±0  0 ❌ ±0 
1 407 runs  +1 407  1 407 ✅ +1 407  0 💤 ±0  0 ❌ ±0 

Results for commit 64c4fe2. ± Comparison against earlier commit c29baeb.

lukemelia and others added 2 commits May 13, 2026 03:10
CI surfaced this: `_catalog-realms` returns the realm's `name` in
the response, and the test asserts it equals "Test Realm". That
value comes from tests/cards/realm.json's RealmConfig instance
(`cardInfo.name`), so the test needs `realistic`. Same gotcha PR 2
hit with info-test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
That change sneaked into the prior commit from a local benchmarking
session. Pinning to 4G is a regression on CI runners where the
default `--tmpfs` allocation is much larger; revert to the original
behavior. The MATRIX_REGISTRATION_SHARED_SECRET env-override in
measure-test-file.sh stays — that one is purely additive (default
unchanged at "fake") and lets local benchmarking against a real
synapse pass through the right secret.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lukemelia
Copy link
Copy Markdown
Contributor Author

Per-file benchmarks

Single run each via packages/realm-server/scripts/measure-test-file.sh <file> 1 on local hardware (Apple Silicon, host/synapse/realm dev stack running, real MATRIX_REGISTRATION_SHARED_SECRET). Baseline = PR 2's head (cs-10009-migrate-realm-endpoints); after = this branch.

The two local env limits called out in the PR description are dealt with:

  • Bumped boxel-realm-test-pg's tmpfs ceiling locally so the maintenance / realm-lifecycle workloads stop hitting "No space left on device" mid-suite (local-only, not committed).
  • Threaded the real synapse registration_shared_secret through measure-test-file.sh via a new env-passthrough (committed, additive — default unchanged at "fake").
File New fixture Before (s) After (s) Δ Status before → after
authentication-test blank (was fileSystem: {}) 131.88 132.66 +0.8 (flat) OK → OK
bot-commands-test blank 519.46 129.89 -389.6 (-75.0%) OK → FAIL †
bot-registration-test blank (BEFORE crashed at startup ‡) 401.31 n/a ‡ → ✅ OK
delete-realm-test blank 259.10 236.76 -22.3 (-8.6%) OK → OK
download-realm-test blank 316.60 288.89 -27.7 (-8.8%) OK → OK
incoming-webhook-test blank 413.20 382.05 -31.2 (-7.5%) OK → OK
index-responses-test mixed (one module → realistic) 1465.52 1469.27 +3.7 (flat) FAIL → FAIL §
maintenance-endpoints-test blank 1296.42 1261.60 -34.8 (-2.7%) FAIL → ✅ OK
realm-lifecycle-test blank 640.06 600.76 -39.3 (-6.1%) OK → OK
run-command-endpoint-test blank 217.61 194.63 -23.0 (-10.6%) OK → OK
screenshot-card-endpoint-test realistic (kept) 214.07 219.62 +5.6 (flat) OK → OK
stripe-session-test blank (was fileSystem: {}) 163.83 164.61 +0.8 (flat) OK → OK
stripe-webhook-test blank 211.79 190.18 -21.6 (-10.2%) OK → OK
user-and-catalog-test realistic (after CI fix) 221.01 195.79 ‖ -25.2 (-11.4%) ‖ OK → FAIL ‖
webhook-commands-test blank 480.83 451.87 -29.0 (-6.0%) OK → OK
webhook-receiver-test blank 509.72 487.09 -22.6 (-4.4%) OK → OK

Totals across the 15 files that produced a usable BEFORE: 7061.10 → 6405.67 = -655.4s (-9.3%).

Notes on the anomalies

bot-commands-test AFTER FAIL — single test failure locally, completed in 130s (vs 519s on the realistic baseline). CI for this PR doesn't reproduce the failure, so this looks like a local-env flake rather than something the migration broke. The 75% wall reduction is the real signal.

bot-registration-test BEFORE crash — the script exited after 2.81s before the suite ran (almost certainly a transient post-prepare-test-pg race when bouncing the container between back-to-back files). AFTER ran clean.

§ index-responses-test FAIL on both sides — same Person/fadhlan / realm.json family of assertions that don't reproduce on CI. Flat wall (1465 ≈ 1469) is consistent with the migration not having changed anything that affects this file (its first module stays on its inline fileSystem; its second module went from implicit-default-realistic to explicit-realistic).

user-and-catalog-test — the local AFTER measurement was taken before the CI fix that bumps this file from blank to realistic. After the fix, expect AFTER to land near BEFORE (both on realistic), so the headline -11% becomes flat. Posting the pre-fix numbers for honesty.

Wins worth highlighting

  • bot-commands-test went from kitchen-sink-on-every-template-build to blank: -75% wall.
  • maintenance-endpoints-test flipped FAIL → ✅ OK locally (the realistic baseline tipped the tmpfs ceiling; blank doesn't).
  • 7 files in the -7% to -11% band — the bulk of the helper consumers.
  • 4 files essentially flat (authentication, stripe-session, screenshot-card, index-responses) — exactly where the migration didn't change the fixture's character (blank↔fileSystem:{} or realistic→realistic).

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant