Add ember-test-waiters to markdown rendering pipeline#5072
Conversation
Preview deploymentsHost Test Results 1 files ± 0 1 suites ±0 1h 52m 33s ⏱️ + 2m 59s Results for commit 3b559cc. ± Comparison against earlier commit 0eb55bc. Realm Server Test Results 1 files ± 0 1 suites ±0 10m 16s ⏱️ + 1m 8s Results for commit 3b559cc. ± Comparison against earlier commit 0eb55bc. |
The markdown card-reference, Mermaid, and KaTeX rendering runs in async operations kicked off by modifiers and ember-concurrency tasks after the initial render settles, so `settled()` could not wait for them. The acceptance tests compensated with manual 10-15s `waitFor`/`waitUntil` timeouts, which were slow and flaky. Base card defs run in the card sandbox and cannot import `@ember/test-waiters` directly, so this extracts the existing waiter injection plumbing out of fetcher.ts into a shared runtime-common `test-waiters` module that exposes a lazy `buildWaiter`/`waitForPromise` backed by the host-injected implementation (no-op in production). `base/default-templates/markdown.gts` now registers a `markdown-rendering` waiter around the KaTeX and Mermaid lazy-load tasks and the deferred card-slot collection, with a teardown guard so a destroyed modifier can't leave a waiter pending. The acceptance test manual timeouts are replaced with `settled()`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6e04355 to
4c18295
Compare
render-service-test's throwaway VirtualNetworks register @test-prefix/ in the process-global card-reference prefix registry, which outlives them and leaked into sibling modules — realm indexing then collected a spurious @test-prefix/ dependency alias. Unregister the prefix in afterEach. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The host-merge-reports-and-publish job downloaded JUnit reports with `pattern: host-test-report-*`. That glob also matched the job's own `host-test-report-merged` output and every prior attempt's shard reports, and download-artifact has no attempt filter — so re-running a flaky shard re-merged the failed attempt's results (including its errors) back into the published "Host Test Results" check. The aggregate could never go green on a re-run even though all shards passed. Prefix the per-shard artifact name with the run attempt and scope the merge download pattern to that attempt, so each merge sees only the current attempt's shard reports and no longer matches the merged output artifact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR instruments the markdown rendering pipeline (card references, Mermaid, KaTeX, and deferred slot collection) with Ember test waiters so settled() correctly waits for post-render async work, eliminating slow/flaky manual timeouts in acceptance tests.
Changes:
- Introduces a sandbox-safe waiter plumbing module in
runtime-commonand refactorsfetcherto use it. - Wraps markdown async rendering work in a
markdown-renderingwaiter so acceptance tests can rely onsettled(). - Fixes CI report artifact naming so re-run attempts don’t merge stale shard results into the published “Host Test Results” check.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/test-waiters.ts | New shared waiter injection + lazy waiter helpers for sandboxed code. |
| packages/runtime-common/index.ts | Re-exports the new test-waiters module from the package entrypoint. |
| packages/runtime-common/fetcher.ts | Switches fetcher waiters to the shared waiter plumbing. |
| packages/base/default-templates/markdown.gts | Adds a markdown-rendering waiter around Mermaid/KaTeX tasks and deferred slot collection. |
| packages/host/tests/acceptance/markdown-file-def-test.gts | Removes manual waitFor/waitUntil timeouts in favor of settled(). |
| packages/host/tests/unit/services/render-service-test.ts | Adds afterEach cleanup to unregister globally-registered card-reference prefix mappings. |
| .github/workflows/ci-host.yaml | Scopes junit artifact names/patterns to github.run_attempt to avoid stale merges on re-run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…st-waiters-to-markdown-rendering-pipeline-to # Conflicts: # packages/host/tests/unit/services/render-service-test.ts
Observability diff (vs staging)Show diffdiff --git a/tmp/remote-canon.sMR0tB/dashboards/boxel-status/service-prerender-manager.json b/tmp/committed-canon.TJsdsN/dashboards/boxel-status/service-prerender-manager.json
index 517ba1c..c4fbf99 100644
--- a/tmp/remote-canon.sMR0tB/dashboards/boxel-status/service-prerender-manager.json
+++ b/tmp/committed-canon.TJsdsN/dashboards/boxel-status/service-prerender-manager.json
@@ -2618,7 +2618,7 @@
{
"hide": 2,
"name": "env",
- "query": "staging",
+ "query": "__ENV__",
"skipUrlSync": true,
"type": "constant"
},
(Run: https://github.com/cardstack/boxel/actions/runs/26871295567) |
Resolves CS-10661.
Problem
The markdown card-reference, Mermaid, and KaTeX rendering runs in async operations kicked off by modifiers and ember-concurrency tasks after the initial render settles, so
settled()could not wait for them. The acceptance tests inmarkdown-file-def-test.gtscompensated with manual 10–15swaitFor/waitUntiltimeouts, making them slow and flaky.Approach
Base card defs run in the card sandbox and cannot import
@ember/test-waitersdirectly — they go throughruntime-common's injected waiters (the host test setup callsuseTestWaiters, andexternals.tsshims@cardstack/runtime-commonto that same instance for card modules).packages/runtime-common/test-waiters.ts(new) — extracts the waiter-injection plumbing out offetcher.tsinto a shared module exposing a lazybuildWaiter(label)andwaitForPromise(). No-op in production; backs onto the real@ember/test-waitersonce the host injects. Lazy resolution lets a module build its waiter at import time, before injection.packages/runtime-common/fetcher.ts— refactored to consume the shared module (buildWaiter('fetcher')); behaviour unchanged.packages/base/default-templates/markdown.gts— registers amarkdown-renderingwaiter around:_loadKatexTaskand_renderMermaidTask(begin/end around the async body)beginAsyncwhenscheduleOnce('afterRender')is queued,endAsyncin theupdateSlotsfinally), plus a teardown guard so a destroyed modifier can't leavesettled()hanging.packages/host/tests/acceptance/markdown-file-def-test.gts— removes the manualwaitFor/waitUntiltimeouts (7× @10s + 4× @15s) and relies onsettled(). The overlaycursor:pointerbinding and URL-bar navigation now resolve naturally via the slot waiter.Testing
ember test --filter "markdown"→ 206/206 pass; the BFM module re-run 3× with no flakes. The instrumented acceptance tests now complete in ~0.7–1.5s each (Mermaid 715ms, KaTeX 715ms, card refs 1445ms) instead of being gated by the 10–15s ceilings.Out of scope
The one remaining
waitUntil(5s) incaptureFileExtractResultwaits on the prerender file-extract pipeline — a separate subsystem the three instrumented operations don't cover.🤖 Generated with Claude Code