test: add shared CLI test helpers#30
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93aa102c0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
📝 WalkthroughWalkthroughExpanded rstack test helpers to export the CLI binary path, execute raw child processes, and provide asynchronous CLI execution with inherited environment and working-directory options. Process output is forwarded to logging and processes are cleaned up during teardown. Added utilities for selecting unused TCP ports, polling for file existence, standardizing 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/rstack/test/helpers/cliTest.ts (1)
102-107: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winQuote
RSTACK_BIN_PATHto handle paths with spaces.If the project directory contains spaces, the shell will misparse
node /path with spaces/bin/rs.jsas multiple arguments. A simple fix is to quote the path; for full robustness, considerspawn('node', [RSTACK_BIN_PATH, ...args])with an argument array.♻️ Proposed fix: quote the binary path
const execCliAsync: Exec = (command, options = {}) => { - return exec(`node ${RSTACK_BIN_PATH} ${command}`, options); + return exec(`node "${RSTACK_BIN_PATH}" ${command}`, options); };🤖 Prompt for 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. In `@packages/rstack/test/helpers/cliTest.ts` around lines 102 - 107, Update execCliAsync to quote RSTACK_BIN_PATH when constructing the node command, ensuring project paths containing spaces remain a single shell argument while preserving the existing command and options behavior.
🤖 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 `@packages/rstack/test/helpers/cliTest.ts`:
- Around line 77-101: Update the exec fixture around the Exec function to track
every spawned child process cleanup handler rather than overwriting a single
close callback. Store each process’s stdout/stderr listener removal and kill
logic in a collection, then run all handlers during teardown after await
use(exec).
In `@packages/rstack/test/helpers/utils.ts`:
- Around line 32-43: Update getRandomPort to enforce the valid TCP port upper
bound while scanning candidates. Stop the loop when port exceeds 65535 and
return or propagate a clear failure instead of continuing indefinitely; preserve
the existing portMap reservation and availability checks for valid ports.
- Around line 7-22: Update isPortAvailable to resolve true only from the
server.close callback, ensuring the port is fully released before callers
proceed. Make the catch path return a resolved Promise<boolean> as well, keeping
the function’s return type consistently Promise<boolean>.
---
Nitpick comments:
In `@packages/rstack/test/helpers/cliTest.ts`:
- Around line 102-107: Update execCliAsync to quote RSTACK_BIN_PATH when
constructing the node command, ensuring project paths containing spaces remain a
single shell argument while preserving the existing command and options
behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 871a3d19-5c19-42a7-9e47-4ba60e6c094a
📒 Files selected for processing (3)
packages/rstack/test/helpers/cli.tspackages/rstack/test/helpers/cliTest.tspackages/rstack/test/helpers/utils.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/rstack/test/helpers/cliTest.ts`:
- Line 81: Update the CLI process cleanup around childProcess in the test helper
to terminate the entire process tree cross-platform rather than only the shell,
then await the child process close event before the finally block completes.
Preserve existing teardown behavior while ensuring no spawned CLI descendants
outlive the fixture.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3296e190-426a-4059-96c2-d2f8365f06b0
📒 Files selected for processing (2)
packages/rstack/test/helpers/cliTest.tspackages/rstack/test/helpers/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rstack/test/helpers/utils.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9777e6f37a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Rstack's e2e tests need reusable process and polling utilities for long-running CLI scenarios. This PR adds Rsbuild-aligned async command, random port, and file polling helpers while retaining the explicit types required by isolated declarations. The process cleanup and port-allocation behavior includes the fixes already landed in Rsbuild.
Related Links