chore(test): add ci benchmark harness#5912
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an npm script and a standalone Node CLI ( ChangesCI Benchmark Test Infrastructure
Sequence DiagramsequenceDiagram
participant User as CI/Developer
participant CLI as ci-test-benchmark.js
participant Vitest as Vitest Process
participant FS as Filesystem
User->>CLI: invoke `benchmark:ci-test` (npm script / node) with args/env
CLI->>CLI: parse args, normalize argv, resolve output dir, build command
CLI->>Vitest: spawn built command (may include JSON reporter/outputFile)
Vitest-->>CLI: exit code, signal, produce vitest JSON file (optional)
CLI->>FS: read vitest JSON (if exists) and vitest.config.ts
CLI->>CLI: summarize results, collect environment/parameters
CLI->>FS: write `report.json` and `report.md`
CLI-->>User: print report paths, exit with child exit code
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Deploying egg with
|
| Latest commit: |
c41200d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ca0672c0.egg-cci.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-e176f152.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5912 +/- ##
=======================================
Coverage 85.03% 85.03%
=======================================
Files 665 667 +2
Lines 19108 19110 +2
Branches 3719 3719
=======================================
+ Hits 16249 16251 +2
Misses 2466 2466
Partials 393 393 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying egg-v3 with
|
| Latest commit: |
c41200d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1666e7d1.egg-v3.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-e176f152.egg-v3.pages.dev |
![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 a new benchmarking script, scripts/ci-test-benchmark.js, along with a corresponding package.json script entry. The utility executes Vitest tests, gathers environment and performance data, and generates comprehensive reports in both Markdown and JSON formats. Review feedback focused on improving the robustness of the script, specifically by refining regex patterns used for parsing configuration files to avoid matching comments and fixing a logic error in the argument parser where flags could incorrectly consume subsequent options as values.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/ci-test-benchmark.js`:
- Around line 222-227: Guard the JSON parsing in readJsonIfExists so a
malformed/truncated reporter file doesn't throw and abort the harness: wrap the
fs.readFileSync + JSON.parse logic in a try/catch inside the readJsonIfExists
function, on any error return null (optionally log a warning with the filePath
and error) so callers can handle a missing/invalid report instead of crashing.
- Around line 159-176: The current hasReporter check skips adding
--reporter=json when any reporter is present; change it to detect only JSON
reporter explicitly: examine the replaced array and set hasReporter true only if
an element is '--reporter=json' or startsWith('--reporter=json') or if an
element is '--reporter' and the next element === 'json' (to handle separated
args); leave the hasOutputFile and reporterArgs logic unchanged and continue to
append '--reporter=json' and `--outputFile=${vitestJsonPath}` only when
hasReporter/hasOutputFile are false.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d60ec538-3c64-4db7-a317-2ae04db2ebac
📒 Files selected for processing (2)
package.jsonscripts/ci-test-benchmark.js
There was a problem hiding this comment.
Pull request overview
Adds a reusable CI test benchmark harness that runs (or wraps) a Vitest invocation and emits Markdown + JSON reports enriched with environment, command parameters, and per-file/per-project duration summaries.
Changes:
- Introduces
scripts/ci-test-benchmark.jsto run a Vitest command and generatereport.md,report.json, and rawvitest-results.json. - Adds a root npm script
benchmark:ci-testto invoke the harness.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/ci-test-benchmark.js | New CLI tool to execute (or dry-run) Vitest and generate benchmark reports from JSON reporter output. |
| package.json | Exposes the harness via pnpm run benchmark:ci-test. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/ci-test-benchmark.js`:
- Around line 177-188: The hasJsonReporter function currently uses a redundant
arg === '--reporter=json' check and an overly broad
arg.startsWith('--reporter=json') which can falsely match reporters like
'--reporter=json-compact'; update hasJsonReporter to remove the startsWith and
only treat '--reporter=json' as a match (keep the existing handling for the
split form where arg === '--reporter' and the next token === 'json'), so only
exact '--reporter=json' or the two-token form are considered true.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f8ff088-d3d3-4919-990d-86560f532460
📒 Files selected for processing (1)
scripts/ci-test-benchmark.js
Summary
Test
Summary by CodeRabbit