feat(ci): surface vitest parallelism metrics in the isolate:false test gate#6004
Conversation
The suite runs with isolate:false for full parallelism (made safe by the tegg TeggScope per-app isolation). Instrument the existing test gating job to show how parallel it actually ran, without changing gate semantics: - vitest.config.ts emits a JSON reporter in CI to benchmark/ci-test/ci-run - a "Report parallelism metrics" step (always()) summarizes that JSON via the ci-test-benchmark harness and appends it to the GitHub job summary per matrix entry - the harness gains a --report-only/--vitest-json mode and computes avg/peak concurrency, parallel efficiency (vs the effective worker ceiling) and the critical path from a per-file concurrency-timeline sweep Metrics are honest lower bounds: Vitest per-file spans exclude suite-level beforeAll/afterAll and module transform/import, so peak concurrency is the robust signal. Fully-skipped files are dropped from the timeline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…FUSED The "should handle connection error" test hardcodes 127.0.0.1:1234 and asserted the exact ECONNREFUSED message. A local listener on that port (e.g. a proxy) makes supertest surface "socket hang up" instead, failing the test in an environment-dependent way. Assert the connection-error family so the test verifies error handling regardless of what (if anything) holds the port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughAdds a non-gating CI step that emits Vitest test timing as JSON (via a conditional reporter in ChangesCI Parallelism Metrics Instrumentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces single-run parallelism metrics reporting for the isolate:false test suite in CI, automatically appending a Markdown summary to the GitHub Actions job summary. It also hardens a connection error test in supertest to prevent environmental failures. The review feedback suggests returning early if the Vitest JSON is missing to avoid empty reports in CI, and replacing the spread operator in Math.min and Math.max calls with a single loop to prevent potential call stack size exceeded errors on large test suites.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (!vitestJson) { | ||
| console.warn(`No Vitest JSON found at ${vitestJsonPath}; nothing to report.`); | ||
| } |
There was a problem hiding this comment.
If vitestJson is not found, the script currently logs a warning but continues execution, which results in writing empty reports with n/a values and appending them to the GitHub step summary. Returning early here avoids polluting the CI job summary with empty tables when tests fail or are skipped.
| if (!vitestJson) { | |
| console.warn(`No Vitest JSON found at ${vitestJsonPath}; nothing to report.`); | |
| } | |
| if (!vitestJson) { | |
| console.warn(`No Vitest JSON found at ${vitestJsonPath}; nothing to report.`); | |
| return; | |
| } |
| const firstStart = Math.min(...intervals.map((interval) => interval.start)); | ||
| const lastEnd = Math.max(...intervals.map((interval) => interval.end)); |
There was a problem hiding this comment.
Using the spread operator with Math.min and Math.max on an array of unbounded size (like test files in a monorepo) can cause a RangeError: Maximum call stack size exceeded if the number of files grows very large. A single loop is safer, more memory-efficient, and avoids creating intermediate arrays.
let firstStart = Infinity;
let lastEnd = -Infinity;
for (const interval of intervals) {
if (interval.start < firstStart) firstStart = interval.start;
if (interval.end > lastEnd) lastEnd = interval.end;
}| const firstStart = Math.min(...intervals.map((interval) => interval.start)); | ||
| const lastEnd = Math.max(...intervals.map((interval) => interval.end)); |
There was a problem hiding this comment.
Using the spread operator with Math.min and Math.max on an array of unbounded size (like test files in a monorepo) can cause a RangeError: Maximum call stack size exceeded if the number of files grows very large. A single loop is safer, more memory-efficient, and avoids creating intermediate arrays.
| const firstStart = Math.min(...intervals.map((interval) => interval.start)); | |
| const lastEnd = Math.max(...intervals.map((interval) => interval.end)); | |
| let firstStart = Infinity; | |
| let lastEnd = -Infinity; | |
| for (const interval of intervals) { | |
| if (interval.start < firstStart) firstStart = interval.start; | |
| if (interval.end > lastEnd) lastEnd = interval.end; | |
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #6004 +/- ##
=======================================
Coverage 84.88% 84.88%
=======================================
Files 674 674
Lines 20269 20269
Branches 4039 4039
=======================================
Hits 17205 17205
Misses 2633 2633
Partials 431 431 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
Motivation
The suite already runs with
pool: 'threads'+isolate: falsefor full parallelism. This PR makes the effect of that parallelism visible in CI — so we can see how parallel the run actually was, per OS/Node matrix entry — without changing gate semantics.What
vitest.config.ts— adds ajsonreporter only whenCIis set, writingbenchmark/ci-test/ci-run/vitest-results.jsonas a side effect of the gatingut run ci(default console reporter preserved; no extra test run)..github/workflows/ci.yml— aReport parallelism metricsstep (if: always()) in the existingtestjob summarizes that JSON and appends the report to the GitHub job summary. Gating still comes solely fromut run ci.scripts/ci-test-benchmark/— the existing harness gains a--report-only --vitest-jsonmode and computes peak/avg concurrency, parallel efficiency, and critical path from a per-file concurrency-timeline sweep.test(supertest)(separate commit) — harden the over-strictshould handle connection errortest (hardcoded127.0.0.1:1234) to accept the connection-error family instead of exactlyECONNREFUSED, so it isn't environment-dependent (a local proxy on that port made itsocket hang up).Reading the metrics honestly
Vitest 4 derives a file's interval from test-level timings only, so the spans cover test bodies + per-test
beforeEach/afterEachbut exclude suite-levelbeforeAll/afterAll(where egg boots its apps) and module transform/import. Therefore avg concurrency / parallel efficiency are lower bounds; peak concurrency is the robust signal. The worker ceiling mirrorsvitest.config.ts(Windows CI caps workers). Fully-skipped files are dropped from the timeline. The report footnote states all of this.Test evidence
isolate: falsesuite is GREEN (services up, CI-faithful--maxWorkers 4): 526 files / 3425 tests pass, 0 failures.--report-onlyexits0on missing JSON; step-summary append is cross-platform;oxfmt/oxlintclean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation