feat: route software-factory tests through boxel-cli's engine (drop host/dist dep)#5246
Conversation
Export the in-memory QUnit runner (runRealmQunit + RunTestsResult / qunit result types) from the importable api so consumers can drive a realm's tests without going through the boxel binary. Add an `authorization` option so a caller holding a realm JWT can authenticate without an active CLI profile, and lower the run timeout to 60s (env FACTORY_TEST_TIMEOUT_MS) with a diagnostic message in place of Playwright's opaque timeout. CS-11579 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The run_tests tool and the validation pipeline drove the factory's own host-dist-bound QUnit runner — the pre-extraction copy CS-11149 was meant to retire. Delegate the browser run to boxel-cli's runRealmQunit and delete the local test-page builder, asset server, and findHostDistPackageDir resolution. The factory no longer depends on a live packages/host/dist build; the harness is owned by boxel-cli's bundled copy. Public signatures, the validation-run cache, and TestRun-card creation are unchanged. CS-11579 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review, boxel-cli should only expose the runner — not change it. Restore the original 300s waitForFunction and drop the timeout helper and diagnostic message I'd added. The only new logging (the run-duration line) lives solely in the software-factory caller. CS-11579 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A numeric passthrough on runRealmQunit (default unchanged at 300s, no logging) so a caller can bound the wait and surface its own diagnostics. CS-11579 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bound the QUnit run to 60s (override via FACTORY_TEST_TIMEOUT_MS) by passing boxel-cli's timeoutMs knob, and translate the resulting opaque Playwright timeout into a diagnostic that names the likely cause. Keeps the timeout value and all logging in the factory layer; boxel-cli only exposes the knob. Re-homes the CS-11573 fix that the host/dist removal displaced. CS-11573, CS-11579 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR completes the migration of software-factory’s realm QUnit execution to the extracted boxel-cli test engine, removing the software-factory-specific host/dist-backed runner and exposing a programmatic runRealmQunit API in boxel-cli (with optional auth injection and configurable timeout).
Changes:
- software-factory: Replace the local Playwright/host-dist QUnit runner with a call to
runRealmQunit, adding a factory-owned timeout (default 60s) and improved timeout diagnostics. - boxel-cli: Export
runRealmQunitand the QUnit result types, and addauthorization+timeoutMsoptions to the underlying runner. - boxel-cli api surface: Re-export the new engine/types via
@cardstack/boxel-cli/api.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| packages/software-factory/src/test-run-execution.ts | Delegates browser QUnit execution to boxel-cli, adds factory-owned timeout parsing + error translation, and removes host/dist runner implementation. |
| packages/boxel-cli/src/commands/test.ts | Adds exportable QUnit result types, supports injected Authorization header + configurable runEnd wait timeout, and introduces runRealmQunit. |
| packages/boxel-cli/api.ts | Re-exports runRealmQunit and related types from the importable API entrypoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let authorization = | ||
| (await options.client.getRealmToken(options.targetRealm)) ?? undefined; | ||
| // The factory owns the timeout value (boxel-cli only exposes the knob) so a | ||
| // hung test page fails fast instead of blocking the full 300s default. | ||
| let timeoutMs = qunitTimeoutMs(); | ||
| let start = Date.now(); | ||
| let browser; | ||
| let testPageServer: Server | undefined; | ||
|
|
||
| let qunitResults: QunitResults; | ||
| let durationMs: number; | ||
| try { | ||
| // Locate the host app's dist directory — contains tests/index.html and assets. | ||
| // In worktrees, the local host/dist may not exist; fall back to the root | ||
| // repo checkout's host dist (same logic as the harness support services). | ||
| let hostDistDir = | ||
| options.hostDistDir ?? | ||
| join( | ||
| findHostDistPackageDir() ?? resolve(__dirname, '../../host'), | ||
| 'dist', | ||
| ); | ||
|
|
||
| // Start a local server to serve both the test HTML page and the host's | ||
| // dist assets. All asset references point to our server, so no external | ||
| // host app is needed — fully hermetic. | ||
| let { | ||
| url: testPageUrl, | ||
| server, | ||
| setHtml, | ||
| } = await startTestPageServer(hostDistDir); | ||
| testPageServer = server; | ||
|
|
||
| // Build HTML using our server URL for asset references. | ||
| // realmProxyUrl = hostAppUrl = the compat proxy that the browser can reach. | ||
| let html = buildQunitTestPageHtml({ | ||
| assetServerUrl: testPageUrl, | ||
| hostDistDir, | ||
| targetRealm: options.targetRealm, | ||
| realmProxyUrl: options.hostAppUrl, | ||
| slug: options.slug, | ||
| let run = await runRealmQunit(options.targetRealm, { | ||
| hostAppUrl: options.hostAppUrl, | ||
| timeoutMs, | ||
| ...(options.hostDistDir ? { hostDistDir: options.hostDistDir } : {}), | ||
| ...(options.debug ? { debug: options.debug } : {}), | ||
| ...(authorization ? { authorization } : {}), | ||
| }); |
There was a problem hiding this comment.
(Written by Claude on Matic's behalf.)
Fixed in 21b3870. The factory now normalizes once — let realmUrl = ensureTrailingSlash(options.targetRealm) — and uses it for both getRealmToken(realmUrl) and runRealmQunit(realmUrl, …), so the token lookup hits the prefix-matched cache and doesn't miss auth on private realms.
| // boxel-cli's QunitResults is structurally identical to the factory's; the | ||
| // cast keeps the parse/card layer typed against the local definition. | ||
| qunitResults = run.qunitResults as QunitResults; | ||
| durationMs = run.durationMs; |
There was a problem hiding this comment.
(Written by Claude on Matic's behalf.)
Fixed in 21b3870. Dropped the as QunitResults cast and assign run.qunitResults directly; the factory's and boxel-cli's QunitResults are structurally identical, so the compiler now enforces that contract (tsc passes on both packages).
| export { | ||
| runRealmQunit, | ||
| runTestsForRealm, | ||
| type RunRealmQunitOptions, | ||
| type RunTestsOptions, | ||
| type RunTestsResult, | ||
| type TestFailure, | ||
| type QunitResults, | ||
| type QunitTestResult, | ||
| type QunitRunSummary, | ||
| } from './src/commands/test.ts'; |
There was a problem hiding this comment.
(Written by Claude on Matic's behalf.)
Done in 21b3870. Extracted the reusable engine + types into src/lib/test-engine.ts (runRealmQunit, runTestsForRealm/runTestsLocally, the test-page server, host-dist resolution, and all QUnit types). src/commands/test.ts is now just the commander wiring (imports the engine), and api.ts imports from ./src/lib/test-engine.ts — so consumers of @cardstack/boxel-cli/api no longer load the command module's deps (commander / cliLog / colors).
| /** | ||
| * Explicit realm JWT to inject on requests to the target realm's origin | ||
| * (remote mode). Takes precedence over `pm.getRealmToken`, letting callers | ||
| * that hold a token directly (e.g. the software factory, which owns its own | ||
| * BoxelCLIClient) authenticate without an active CLI profile for the realm. | ||
| */ | ||
| authorization?: string; |
There was a problem hiding this comment.
(Written by Claude on Matic's behalf.)
Fixed in 21b3870. Reworded: it's the value for the HTTP Authorization header, sent verbatim (so the full header value, typically Bearer <jwt>), not a bare token. The realm server returns exactly this and we set it as-is on page.route.
| /** Realm JWT to inject; lets callers authenticate without a CLI profile. */ | ||
| authorization?: string; |
There was a problem hiding this comment.
(Written by Claude on Matic's behalf.)
Fixed in 21b3870 — same reword as the QunitRunnerOptions field: HTTP Authorization header value, sent verbatim (Bearer <jwt>), not a bare JWT.
| let message = err instanceof Error ? err.message : String(err); | ||
| // boxel-cli surfaces a timeout as Playwright's opaque "Timeout <n>ms | ||
| // exceeded". Replace it with a diagnostic that names the likely cause. | ||
| if (/Timeout\s+\d+ms exceeded/i.test(message)) { | ||
| let waited = Date.now() - start; |
There was a problem hiding this comment.
(Written by Claude on Matic's behalf.)
Fixed in 21b3870. boxel-cli now wraps only the runEnd waitForFunction in a try/catch and, on a Playwright TimeoutError, rethrows a specific marker — QUnit suite did not reach runEnd within <n>ms — with the original as cause. The page.goto/asset timeouts are outside that catch and propagate unchanged. The factory now matches only that marker (/did not reach runEnd within/i), so a navigation/fetch stall is no longer masked by the run-end diagnostic.
- Extract the reusable QUnit engine + types into src/lib/test-engine.ts; commands/test.ts is now just the commander wiring, and api.ts imports the engine from the lib module so library consumers no longer load command-only deps (commander / cliLog / colors). - boxel-cli: label only the run-end waitForFunction timeout (rethrow other Playwright timeouts untouched, with the original as `cause`); document `authorization` as a verbatim HTTP Authorization header value, not a bare JWT. - software-factory: normalize the realm URL with ensureTrailingSlash before fetching the token and running (token cache / private-realm auth); drop the `as QunitResults` cast so the compiler enforces structural compatibility; match only the run-end timeout marker and attach the original error as `cause`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@copilot resolve the merge conflicts in this pull request |
[Claude Code 🤖] Resolved the merge conflict and pushed merge commit |
|
Nice |
(Written by Claude on Matic's behalf.)
Closes the half-finished CS-11149 migration: the software factory still ran its own host-dist-bound QUnit runner instead of boxel-cli's extracted engine. That coupling is why a stale
packages/host/dist(missingRenderedHtmlResourceType) made every card test fail globally during a factory run, with no card edit able to fix it.What changed
boxel-cli — expose the engine, no behavior/logging change:
runRealmQunit(returns rawQunitResults+ duration) and the result/qunit types from the importableapi.authorizationoption so a caller holding a realm JWT authenticates without an active CLI profile.timeoutMsknob (default unchanged at 300s, no logging) so a caller can bound the wait and emit its own diagnostics.software-factory — consume it:
runQunitInBrowsernow delegates torunRealmQunit, passing the realm token the factory already holds.findHostDistPackageDirresolution (~250 lines). The factory no longer depends on a livepackages/host/dist.run_teststool and the validation pipeline migrate together.FACTORY_TEST_TIMEOUT_MS) and translates boxel-cli's opaque Playwright timeout into a diagnostic. The timeout value and all new logging live here, in the factory — not in boxel-cli. (Re-homes the CS-11573 fix the host/dist removal displaced.)Testing done here
pnpm lintclean in both packages (ember-tsc / tsc types, eslint, prettier).test:node: 452/453 (the one failure,port-allocator > IPv4 holder blocks a default (dual-stack) bind, is a pre-existing environment-dependent test that also fails on base).test:unit: 313/313.What still needs validation (why this is a draft)
.spec.tse2e runners (run-tests-in-memory.spec.ts,factory-test-realm.spec.ts) are Playwright, not run here; and a realfactory:gois the true gate.bundled-test-harness/in this checkout is the same staleruntime-common-CBCJRK_nbundle. To validate:pnpm --filter @cardstack/host buildthenpnpm --filter @cardstack/boxel-cli build:test-harness, then run the factory. That staleness is now boxel-cli's (bundled, CLI-versioned) concern — the architectural point of this PR.build:types) at release so the newapiexports ship; monorepo dev importsapi.tsdirectly, so workspace consumers are unaffected.Relationship to #5245
This PR carries the CS-11573 timeout fix (now in the factory's boxel-cli wrapper), so #5245's commit 3 should be dropped — its file is largely deleted here.
Tickets: CS-11579, CS-11573.
🤖 Generated with Claude Code