test: add type tests for public exports#75
Merged
Conversation
Deploying rstack-cli with
|
| Latest commit: |
a2937d5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4031148c.rstack-cli.pages.dev |
| Branch Preview URL: | https://chenjiahan-test-type-tests.rstack-cli.pages.dev |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (52)
📝 WalkthroughWalkthroughUpdates test TypeScript paths and lint project references for the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)packages/rstack/tests/config/define-lib/package.jsonTraceback (most recent call last): packages/rstack/tests/tsconfig.jsonTraceback (most recent call last): packages/rstack/tests/types/core-types/tsconfig.jsonTraceback (most recent call last):
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
🤖 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/tests/config/define-lib/index.test.ts`:
- Around line 6-14: Update both test cases in
packages/rstack/tests/config/define-lib/index.test.ts (lines 6-14) and
packages/rstack/tests/config/define-doc/index.test.ts (lines 6-14) to wrap the
build, file lookup, and assertions in try/finally blocks, removing each prepared
distPath in finally as demonstrated by define-app/index.test.ts.
In `@packages/rstack/tests/exports/lint-subpath/index.test.ts`:
- Around line 8-10: Replace the ineffective typeof truthiness assertion with an
exact function-type assertion in the commonLintMethods loop in
packages/rstack/tests/exports/lint-subpath/index.test.ts:8-10. Apply the same
change to the corresponding test API loop in
packages/rstack/tests/exports/test-subpath/index.test.ts:8-10, while retaining
the existing toHaveProperty checks.
In `@packages/rstack/tests/helpers/cli.ts`:
- Line 28: Update the synchronous CLI invocation around execSync to invoke the
script through Node and quote RSTACK_BIN_PATH, matching the approach used by
execCliAsync so repository paths containing spaces are handled correctly.
🪄 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: 4bd3d765-f0dd-4d6d-8456-31308499f655
📒 Files selected for processing (52)
packages/rstack/rstack.config.tspackages/rstack/tests/cli/specify-config/custom.config.tspackages/rstack/tests/cli/specify-config/index.test.tspackages/rstack/tests/cli/specify-config/src/index.jspackages/rstack/tests/cli/staged/index.test.tspackages/rstack/tests/config/define-app-lib/fixture.tspackages/rstack/tests/config/define-app-lib/index.test.tspackages/rstack/tests/config/define-app-lib/rstack.config.tspackages/rstack/tests/config/define-app/index.test.tspackages/rstack/tests/config/define-app/rstack.config.tspackages/rstack/tests/config/define-app/src/index.jspackages/rstack/tests/config/define-doc/docs/index.mdpackages/rstack/tests/config/define-doc/index.test.tspackages/rstack/tests/config/define-doc/rstack.config.tspackages/rstack/tests/config/define-lib/index.test.tspackages/rstack/tests/config/define-lib/package.jsonpackages/rstack/tests/config/define-lib/rstack.config.tspackages/rstack/tests/config/define-lib/src/index.jspackages/rstack/tests/config/define-lint/index.test.tspackages/rstack/tests/config/define-lint/rstack.config.tspackages/rstack/tests/config/define-lint/src/index.jspackages/rstack/tests/config/define-test-projects-app/custom.tspackages/rstack/tests/config/define-test-projects-app/first.tspackages/rstack/tests/config/define-test-projects-app/index.test.tspackages/rstack/tests/config/define-test-projects-app/rstack.config.tspackages/rstack/tests/config/define-test-projects-app/second.tspackages/rstack/tests/config/define-test-projects-lib/first.tspackages/rstack/tests/config/define-test-projects-lib/index.test.tspackages/rstack/tests/config/define-test-projects-lib/rstack.config.tspackages/rstack/tests/config/define-test-projects-lib/second.tspackages/rstack/tests/config/load-state/index.test.tspackages/rstack/tests/config/load-state/rstack.config.tspackages/rstack/tests/config/reload-app-config/index.test.tspackages/rstack/tests/config/reload-app-config/src/index.jspackages/rstack/tests/exports/app-subpath/index.test.tspackages/rstack/tests/exports/lib-subpath/index.test.tspackages/rstack/tests/exports/lint-subpath/index.test.tspackages/rstack/tests/exports/test-subpath/index.test.tspackages/rstack/tests/helpers/cli.tspackages/rstack/tests/helpers/cliTest.tspackages/rstack/tests/helpers/constants.tspackages/rstack/tests/helpers/index.tspackages/rstack/tests/helpers/logs.tspackages/rstack/tests/tsconfig.jsonpackages/rstack/tests/types/core-types/index.tspackages/rstack/tests/types/core-types/tsconfig.jsonpackages/rstack/tests/types/resolution-bundler/index.tspackages/rstack/tests/types/resolution-bundler/tsconfig.jsonpackages/rstack/tests/types/resolution-nodenext/index.tspackages/rstack/tests/types/resolution-nodenext/tsconfig.jsonpackages/rstack/tests/types/tsconfig.jsonrstack.config.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🤖 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/tests/config/define-lib/index.test.ts`:
- Around line 6-14: Update both test cases in
packages/rstack/tests/config/define-lib/index.test.ts (lines 6-14) and
packages/rstack/tests/config/define-doc/index.test.ts (lines 6-14) to wrap the
build, file lookup, and assertions in try/finally blocks, removing each prepared
distPath in finally as demonstrated by define-app/index.test.ts.
In `@packages/rstack/tests/exports/lint-subpath/index.test.ts`:
- Around line 8-10: Replace the ineffective typeof truthiness assertion with an
exact function-type assertion in the commonLintMethods loop in
packages/rstack/tests/exports/lint-subpath/index.test.ts:8-10. Apply the same
change to the corresponding test API loop in
packages/rstack/tests/exports/test-subpath/index.test.ts:8-10, while retaining
the existing toHaveProperty checks.
In `@packages/rstack/tests/helpers/cli.ts`:
- Line 28: Update the synchronous CLI invocation around execSync to invoke the
script through Node and quote RSTACK_BIN_PATH, matching the approach used by
execCliAsync so repository paths containing spaces are handled correctly.
🪄 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: 4bd3d765-f0dd-4d6d-8456-31308499f655
📒 Files selected for processing (52)
packages/rstack/rstack.config.tspackages/rstack/tests/cli/specify-config/custom.config.tspackages/rstack/tests/cli/specify-config/index.test.tspackages/rstack/tests/cli/specify-config/src/index.jspackages/rstack/tests/cli/staged/index.test.tspackages/rstack/tests/config/define-app-lib/fixture.tspackages/rstack/tests/config/define-app-lib/index.test.tspackages/rstack/tests/config/define-app-lib/rstack.config.tspackages/rstack/tests/config/define-app/index.test.tspackages/rstack/tests/config/define-app/rstack.config.tspackages/rstack/tests/config/define-app/src/index.jspackages/rstack/tests/config/define-doc/docs/index.mdpackages/rstack/tests/config/define-doc/index.test.tspackages/rstack/tests/config/define-doc/rstack.config.tspackages/rstack/tests/config/define-lib/index.test.tspackages/rstack/tests/config/define-lib/package.jsonpackages/rstack/tests/config/define-lib/rstack.config.tspackages/rstack/tests/config/define-lib/src/index.jspackages/rstack/tests/config/define-lint/index.test.tspackages/rstack/tests/config/define-lint/rstack.config.tspackages/rstack/tests/config/define-lint/src/index.jspackages/rstack/tests/config/define-test-projects-app/custom.tspackages/rstack/tests/config/define-test-projects-app/first.tspackages/rstack/tests/config/define-test-projects-app/index.test.tspackages/rstack/tests/config/define-test-projects-app/rstack.config.tspackages/rstack/tests/config/define-test-projects-app/second.tspackages/rstack/tests/config/define-test-projects-lib/first.tspackages/rstack/tests/config/define-test-projects-lib/index.test.tspackages/rstack/tests/config/define-test-projects-lib/rstack.config.tspackages/rstack/tests/config/define-test-projects-lib/second.tspackages/rstack/tests/config/load-state/index.test.tspackages/rstack/tests/config/load-state/rstack.config.tspackages/rstack/tests/config/reload-app-config/index.test.tspackages/rstack/tests/config/reload-app-config/src/index.jspackages/rstack/tests/exports/app-subpath/index.test.tspackages/rstack/tests/exports/lib-subpath/index.test.tspackages/rstack/tests/exports/lint-subpath/index.test.tspackages/rstack/tests/exports/test-subpath/index.test.tspackages/rstack/tests/helpers/cli.tspackages/rstack/tests/helpers/cliTest.tspackages/rstack/tests/helpers/constants.tspackages/rstack/tests/helpers/index.tspackages/rstack/tests/helpers/logs.tspackages/rstack/tests/tsconfig.jsonpackages/rstack/tests/types/core-types/index.tspackages/rstack/tests/types/core-types/tsconfig.jsonpackages/rstack/tests/types/resolution-bundler/index.tspackages/rstack/tests/types/resolution-bundler/tsconfig.jsonpackages/rstack/tests/types/resolution-nodenext/index.tspackages/rstack/tests/types/resolution-nodenext/tsconfig.jsonpackages/rstack/tests/types/tsconfig.jsonrstack.config.ts
🛑 Comments failed to post (3)
packages/rstack/tests/config/define-lib/index.test.ts (1)
6-14: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the relevant tests git ls-files 'packages/rstack/tests/**/index.test.ts' | sed -n '1,200p' echo '--- define-lib ---' cat -n packages/rstack/tests/config/define-lib/index.test.ts echo '--- define-doc ---' cat -n packages/rstack/tests/config/define-doc/index.test.ts echo '--- app test candidates ---' rg -n "finally|remove|cleanup|prepareDist" packages/rstack/tests -g 'index.test.ts' -A 4 -B 4Repository: rstackjs/rstack-cli
Length of output: 8248
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- packages/rstack/tests/config/define-lib/index.test.ts ---' sed -n '1,220p' packages/rstack/tests/config/define-lib/index.test.ts echo '--- packages/rstack/tests/config/define-doc/index.test.ts ---' sed -n '1,220p' packages/rstack/tests/config/define-doc/index.test.ts echo '--- search for cleanup in nearby tests ---' rg -n "finally|removeSync|rmSync|cleanup|prepareDist" packages/rstack/tests -g 'index.test.ts'Repository: rstackjs/rstack-cli
Length of output: 2664
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find the fixture/helper implementation for prepareDist and any cleanup helpers. rg -n "prepareDist" packages -g '*.ts' -A 4 -B 4 rg -n "rmSync|removeSync|mkdtemp|tmpdir|prepareDist" packages -g '*.ts' -A 3 -B 3Repository: rstackjs/rstack-cli
Length of output: 12630
Clean up the prepared dist directories in these config tests. Both tests leave their temporary build output behind on success or failure; wrap the assertions in
try/finallyand removedistPathafterward, likepackages/rstack/tests/config/define-app/index.test.ts.📍 Affects 2 files
packages/rstack/tests/config/define-lib/index.test.ts#L6-L14(this comment)packages/rstack/tests/config/define-doc/index.test.ts#L6-L14🤖 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/tests/config/define-lib/index.test.ts` around lines 6 - 14, Update both test cases in packages/rstack/tests/config/define-lib/index.test.ts (lines 6-14) and packages/rstack/tests/config/define-doc/index.test.ts (lines 6-14) to wrap the build, file lookup, and assertions in try/finally blocks, removing each prepared distPath in finally as demonstrated by define-app/index.test.ts.packages/rstack/tests/exports/lint-subpath/index.test.ts (1)
8-10: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the callable-export checks meaningful.
Both tests apply
toBeTruthy()totypeof, which is truthy even for"undefined". AsserttoBe('function')at both sites.
packages/rstack/tests/exports/lint-subpath/index.test.ts#L8-L10: require each lint API to have function type.packages/rstack/tests/exports/test-subpath/index.test.ts#L8-L10: require each test API to have function type.📍 Affects 2 files
packages/rstack/tests/exports/lint-subpath/index.test.ts#L8-L10(this comment)packages/rstack/tests/exports/test-subpath/index.test.ts#L8-L10🤖 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/tests/exports/lint-subpath/index.test.ts` around lines 8 - 10, Replace the ineffective typeof truthiness assertion with an exact function-type assertion in the commonLintMethods loop in packages/rstack/tests/exports/lint-subpath/index.test.ts:8-10. Apply the same change to the corresponding test API loop in packages/rstack/tests/exports/test-subpath/index.test.ts:8-10, while retaining the existing toHaveProperty checks.packages/rstack/tests/helpers/cli.ts (1)
28-28: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Quote and invoke the CLI script through Node.
Line 28 splits
RSTACK_BIN_PATHwhen the repository path contains spaces. MatchexecCliAsyncby invokingnodeand quoting the script path.Proposed fix
- const output = execSync(`${RSTACK_BIN_PATH} ${command}`, { + const output = execSync(`node "${RSTACK_BIN_PATH}" ${command}`, {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const output = execSync(`node "${RSTACK_BIN_PATH}" ${command}`, {🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { type ExecSyncOptions, execSync } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').(detect-child-process-typescript)
🪛 OpenGrep (1.25.0)
[ERROR] 28-35: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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/tests/helpers/cli.ts` at line 28, Update the synchronous CLI invocation around execSync to invoke the script through Node and quote RSTACK_BIN_PATH, matching the approach used by execCliAsync so repository paths containing spaces are handled correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds type-level coverage for Rstack's public entry points under Bundler and NodeNext module resolution, while strictly checking the Rsbuild preset declarations exposed through
rstack/types.It also consolidates package tests under
packages/rstack/testsand updates the Rstest and lint configuration paths.