ci: harden smoke tests to catch Windows dev-mode SSR and perf regressions#161
ci: harden smoke tests to catch Windows dev-mode SSR and perf regressions#161rsbh wants to merge 6 commits into
Conversation
…cleanup Extract dev/start smoke checks into scripts/smoke-test.sh: - capture server output and dump it when a check fails - fail immediately if the server process dies during startup - add curl timeouts so a hung request can't stall the job - kill the full process tree on Windows via taskkill - run the page API check in dev mode too, not just start Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Windows dev smoke test forced NITRO_DEV_RUNNER=self, but real Windows users default to the node-process runner (set in dev.ts). CI was skipping the exact code path users run, letting dev-mode SSR errors ship undetected. Drop the override so CI tests the runner users get. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The smoke test only fetched the homepage, so an SSR failure on any other page shipped undetected. Crawl up to CRAWL_LIMIT (15) URLs from sitemap.xml and fail on any non-200 response or truncated HTML document. SSR errors always surface as HTTP 500 (including the dev MDX error page), so status checks catch them reliably. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Windows users hit very slow dev page loads that no test measured. Time the first homepage load (includes SSR compile in dev) and a warm reload, failing over FIRST_LOAD_BUDGET (60s) and WARM_LOAD_BUDGET (10s). Both are env-overridable for tuning once real CI numbers are known. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CI smoke-test job now calls a reusable Bash script for dev and start modes. The script manages the Chronicle process, checks health and load behavior, validates homepage and metadata responses, and crawls sitemap pages for complete successful HTML. ChangesSmoke test runner
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI as CI smoke-test job
participant Runner as smoke-test.sh
participant Chronicle as Chronicle server
CI->>Runner: Run dev or start mode with port
Runner->>Chronicle: Start server
Runner->>Chronicle: Poll /api/health
Chronicle-->>Runner: Return ok status
Runner->>Chronicle: Request homepage and page metadata
Chronicle-->>Runner: Return HTML and metadata
Runner->>Chronicle: Request sitemap.xml
Chronicle-->>Runner: Return page locations
loop Selected sitemap paths
Runner->>Chronicle: Request page HTML
Chronicle-->>Runner: Return status and complete HTML
end
Runner->>Chronicle: Terminate server on exit
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
head -n exits after CRAWL_LIMIT lines, upstream grep/sed get SIGPIPE, and pipefail turns that into a script failure (raced on ubuntu, passed on macos/windows). Use awk 'NR <= n' instead, which drains the full stream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/smoke-test.sh`:
- Around line 59-62: Update the warm page load curl invocation in the warm_time
check to use WARM_LOAD_BUDGET as its request timeout instead of the hardcoded
30-second value, preserving the existing timing comparison and failure messages.
- Around line 32-43: Update the health-check loop in the smoke-test script to
use a wall-clock deadline representing 120 seconds instead of a fixed 60-attempt
count. Before each curl invocation, calculate the remaining time, stop and fail
when the deadline is reached, and set curl’s max-time to no greater than that
remaining duration; retain the existing server-exit and healthy-response checks.
- Around line 81-83: Update the crawl loop around crawled and the curl
invocation in the smoke-test script to ensure the entire crawl completes within
the workflow’s 15-minute job limit, including setup overhead. Add a total crawl
deadline or reduce the per-page timeout so slow pages cannot consume the full
timeout sequentially, while preserving failure handling and diagnostic output.
- Around line 52-60: Update both homepage curl invocations in the smoke test,
including the first_time and warm_time requests, to use curl's HTTP-failure
behavior so non-2xx responses cause the existing failure handlers to run.
Preserve the current content check, timing capture, budgets, and success
messages.
- Around line 73-79: Update the sitemap URL extraction assigned to paths so an
empty grep result does not terminate the script under set -euo pipefail.
Temporarily handle or suppress the pipeline’s non-zero status, while preserving
the existing paths empty check so fail "sitemap.xml returned no URLs" executes
and prints the server log.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eaaa1ce6-cf4c-411a-a497-80e6c301dceb
📒 Files selected for processing (2)
.github/workflows/ci.ymlscripts/smoke-test.sh
- bound health wait by wall-clock HEALTH_BUDGET (120s), not attempt count, so worst case can no longer stretch to ~420s - add curl -f to homepage/warm loads so HTTP errors fail the check - use WARM_LOAD_BUDGET as the warm request timeout instead of a hardcoded 30s - guard sitemap extraction with || true so an empty sitemap reaches the explicit fail (with server log) instead of dying on pipefail - cap total crawl at CRAWL_BUDGET (300s) and per-page timeout at 30s so the job timeout can't kill the script before diagnostics print Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 participants
Problem
Windows users hit dev-mode SSR errors and very slow page loads that CI never caught, because:
NITRO_DEV_RUNNER=self, while real Windows users default to thenode-processrunner (dev.ts) — CI was skipping the exact code path users run.Changes (one commit each)
scripts/smoke-test.sh— server log dumped on failure, fail-fast if the server process dies, curl timeouts, process-tree kill on Windows viataskkill //T, page API check now runs in dev mode too.NITRO_DEV_RUNNER=selfoverride — Windows CI now exercises thenode-processrunner users actually get. If this goes red on Windows, that failure is the user-facing bug, now with logs to diagnose it.CRAWL_LIMIT(15) URLs fromsitemap.xml, failing on non-200 responses or truncated HTML. SSR errors always surface as HTTP 500 (including the dev MDX error page), so status checks catch them reliably.FIRST_LOAD_BUDGET(60s), a warm reload must beatWARM_LOAD_BUDGET(10s). Both env-overridable for tuning against real CI numbers.Verification
Ran
scripts/smoke-test.shlocally (macOS) in both modes: all checks pass, 15 pages crawled, dev first load 1.5s, warm 0.03s.Notes
ci.ymlonce Windows CI numbers are known.🤖 Generated with Claude Code