Skip to content

test(aggregator): keep the test worker off the network - #4321

Open
danielmlr wants to merge 2 commits into
mainfrom
daniel/aggregator-tests-offline
Open

danielmlr wants to merge 2 commits into
mainfrom
daniel/aggregator-tests-offline

Conversation

@danielmlr

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops "accepts uppercase BEARER scheme" and a few tests around it from timing out in the aggregator suite, on main too. Some tests call admin routes that start real work in the background: discovery on bsky.network, a Jetstream connection, and DID lookups on plc.directory. While the test worker waits for one of those host names to resolve, the other tests wait with it, so a slow DNS lookup in CI pushes whichever test is running past the 5 s limit.

The test config now answers every outbound request from the worker with a 503 stub, so no test waits on DNS or on a live service. Production config and the tests themselves are unchanged.

Type of change

  • Bug fix
  • Feature (requires an accepted feature plan or RFC)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Your connection to this change

  • I use EmDash and ran into the problem this PR addresses
  • I maintain a tool, dependency, or service that EmDash uses, and this change concerns that integration
  • A Maintainer approved this specific change before I opened the PR

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable) (n/a: only the test config changes; the reproduction is below)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include messages.po changes except in translation PRs — a workflow extracts catalogs on merge to main. (n/a: test config only)
  • I have added and reviewed the user-facing changeset (if this PR changes a published package) (n/a: test config of the private @emdash-cms/aggregator app)
  • New features link to their merged design PR: https://github.com/emdash-cms/emdash/pull/... (n/a: not a feature)
  • I have included screenshots below if this PR changes the UI (n/a: no UI change)

AI assistance disclosure

  • This PR includes AI-generated code or text — model/tool: Claude Opus 5.5

Screenshots / test output

  • Before, in CI: the failing main run on bed3e42 took 3,362 ms for "accepts an empty body and triggers discovery", 3,939 ms for "accepts canonical mixed-case Bearer" and 8,064 ms for "accepts uppercase BEARER scheme", which timed out. Relay discovery never finished in that run.
  • On a fork's Linux CI, with DNS lookups for the test run taking 10 s and failing: on main, 10 of the 50 tests in backfill.test.ts time out, among them "accepts uppercase BEARER scheme" and "returns 400 on a non-array dids value", whose request never leaves the worker. With this change, all 50 pass in 2 s.
  • Locally, host names that take 5 s to resolve make 5 of the 50 time out on main. A server that never answers, or an address that never connects, leaves all 50 passing, so it is the name lookup that holds the tests up.
  • Over the whole suite the stub gets 8 requests for bsky.network, 1 for Jetstream and 8 for plc.directory, which are all the hosts the worker reaches.

Some tests call admin routes that start background work against
bsky.network, the Jetstream firehose and plc.directory. While the worker
waits for one of those host names to resolve, the other tests in the
isolate wait with it, so a slow DNS lookup in CI made unrelated tests
such as "accepts uppercase BEARER scheme" hit the 5 s limit.

Answer every outbound request from the test worker with a 503 stub
instead. Production config is unchanged.
@changeset-bot

changeset-bot Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b48e407

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added query-count changed PR diff modifies query-count snapshot files review/needs-review No maintainer or bot review yet size/XS cla: signed labels Oct 21, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://daniel-aggregator-tests-offline.try.emdashcms.com, https://daniel-aggregator-tests-offline-emdash-playground.emdash-cms.workers.dev (commit b48e407)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://70c29be6.try.emdashcms.com, https://70c29be6-emdash-playground.emdash-cms.workers.dev b48e407 2026-10-21T02:45:23.066Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://5552177d.try.emdashcms.com, https://5552177d-emdash-playground.emdash-cms.workers.dev 63d7c4d 2026-10-21T02:35:45.420Z Visit the dashboard ↗

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach is sound. Stubbing outbound requests with a 503 at the Miniflare outboundService layer fixes the DNS-dependent test timeouts at their cause. It is narrowly scoped to apps/aggregator/vitest.config.ts, leaves production behavior and the tests unchanged, and needs no changeset because the package is private. Both reviewers approved with no findings. One caveat from the first reviewer: @cloudflare/vitest-plugin was not installed in the checkout, so it could not confirm the plugin passes outboundService through to Miniflare. The PR's per-host request counts suggest it does. The second reviewer's review was static only, with no tests, builds or lint run.


Reviewed by anthropic/claude-opus-5-5 and openai/gpt-6.1-sol.

@github-actions github-actions Bot removed the query-count changed PR diff modifies query-count snapshot files label Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: signed review/needs-review No maintainer or bot review yet size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant