fix(ci): skip oclif shell probe in egg-bin#6014
Conversation
|
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 (5)
📝 WalkthroughWalkthroughAdds a ChangesWindows SHELL preset and dynamic import
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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: |
c8e3851
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://253ceb83.egg-cci.pages.dev |
| Branch Preview URL: | https://codex-egg-bin-ci-diagnostics.egg-cci.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 CI runner diagnostics and improves the test benchmarking tool to support sharding and long-tail test reporting. Specifically, it adds a new script scripts/ci-runner-diagnostics.js to measure hardware, process startup, and I/O baselines, and updates the benchmarking tool to capture --shard parameters and report individual slow tests. Feedback was provided on scripts/ci-runner-diagnostics.js to wrap the temporary directory I/O operations in a try...finally block to guarantee cleanup and prevent resource leaks if an error occurs.
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.
| async function measureTempFileIo(bytes) { | ||
| const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'egg-ci-diagnostics-')); | ||
| const file = path.join(dir, 'io.bin'); | ||
| const buffer = Buffer.alloc(bytes, 7); | ||
| const writeStart = performance.now(); | ||
| await fs.writeFile(file, buffer); | ||
| const writeMs = Math.round(performance.now() - writeStart); | ||
| const readStart = performance.now(); | ||
| const readBuffer = await fs.readFile(file); | ||
| const readMs = Math.round(performance.now() - readStart); | ||
| await fs.rm(dir, { force: true, recursive: true }); | ||
| return { | ||
| bytes: readBuffer.length, | ||
| readMs, | ||
| writeMs, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The temporary directory created by fs.mkdtemp is not guaranteed to be cleaned up if an error occurs during fs.writeFile or fs.readFile (for example, due to disk space or permission issues). Wrapping the I/O operations in a try...finally block ensures that the temporary directory is always deleted, preventing resource leaks on the CI runner.
async function measureTempFileIo(bytes) {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'egg-ci-diagnostics-'));
try {
const file = path.join(dir, 'io.bin');
const buffer = Buffer.alloc(bytes, 7);
const writeStart = performance.now();
await fs.writeFile(file, buffer);
const writeMs = Math.round(performance.now() - writeStart);
const readStart = performance.now();
const readBuffer = await fs.readFile(file);
const readMs = Math.round(performance.now() - readStart);
return {
bytes: readBuffer.length,
readMs,
writeMs,
};
} finally {
await fs.rm(dir, { force: true, recursive: true });
}
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #6014 +/- ##
=======================================
Coverage 81.94% 81.94%
=======================================
Files 677 677
Lines 20651 20651
Branches 4099 4099
=======================================
Hits 16922 16922
Misses 3215 3215
Partials 514 514 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Deploying egg-v3 with
|
| Latest commit: |
c8e3851
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://031223d2.egg-v3.pages.dev |
| Branch Preview URL: | https://codex-egg-bin-ci-diagnostics.egg-v3.pages.dev |
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
killagu
force-pushed
the
codex/egg-bin-ci-diagnostics
branch
2 times, most recently
from
36b1341 to
8060729
Compare
killagu
changed the title
|
排查结论如下，方便集中看；排查过程中加过的 runner/timing/benchmark 诊断代码已经从最终 PR 移除了，过程和数据保留在
|
killagu
force-pushed
the
codex/egg-bin-ci-diagnostics
branch
from
8060729 to
051f007
Compare
killagu
force-pushed
the
codex/egg-bin-ci-diagnostics
branch
from
051f007 to
c8e3851
Compare
|
|
||
| - Before the fix, `test/commands/test.test.ts` on Windows took about 1,077s. | ||
| - After the main entrypoint preset, `test/commands/test.test.ts` dropped to about | ||
| 42-48s. |
Summary
SHELLbefore loading oclif in the mainegg-binentrypoint when Windows children start without itmy-egg-binfixture entrypointRoot Cause
On GitHub-hosted Windows runners, spawned
egg-binchildren can start withoutSHELL. oclif then synchronously probes the parent process shell through PowerShell/CIM during startup.tools/egg-bin/test/commands/test.test.tsstarts many child CLI processes, so that startup probe dominated the file runtime.Validation
git diff --check origin/next...HEADnode --check tools/egg-bin/bin/run.jsnode --check tools/egg-bin/test/fixtures/my-egg-bin/bin/run.jsCI=1andSHELLunset:test/commands/test.test.ts -t "should only test files specified by TESTS argv"test/my-egg-bin.test.ts -t "should my-egg-bin nsp success"Test bin (windows-latest, 24)passed in about 3m06s after diagnostic code was removedEvidence
The diagnostic code used to isolate the hotspot was removed from this PR. The durable notes and CI evidence are recorded in
wiki/workflows/egg-bin-windows-shell-probe.md.test/commands/test.test.tson Windows took about 1,077stest/commands/test.test.tsdropped to about 42-49stest/my-egg-bin.test.tsdropped from about 299s to about 8.5stest/commands/test.test.ts48.8s,test/my-egg-bin.test.ts8.5s,test/commands/cov.test.ts30.0sSummary by CodeRabbit
Bug Fixes
Documentation