-
Notifications
You must be signed in to change notification settings - Fork 5.8k
test(integration): add tests for standalone arguments handling #31337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/compile_tests.rs (1)
98-118: Consider adding a comment to clarify the purpose of the second compile cycle.The second compile+run cycle tests that the
--outputflag can appear in different positions relative to the source file, which is valuable for ensuring robust argument parsing. A brief comment would make this intent explicit.+ // Test with --output flag after the source file to ensure argument order flexibility context .new_command() .args_vec([
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/compile_tests.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build libs
- GitHub Check: test release linux-x86_64
- GitHub Check: test debug linux-x86_64
- GitHub Check: test debug linux-aarch64
- GitHub Check: test debug macos-aarch64
- GitHub Check: test debug macos-x86_64
- GitHub Check: test debug windows-x86_64
- GitHub Check: lint debug windows-x86_64
- GitHub Check: lint debug macos-x86_64
- GitHub Check: lint debug linux-x86_64
🔇 Additional comments (2)
tests/integration/compile_tests.rs (2)
91-97: LGTM! Good test coverage for argument forwarding.The test correctly verifies that:
- Compile-time arguments passed after
--(a, b) are embedded in the binary- Runtime arguments (foo, --bar, --unstable) are also passed through
- Both sets of arguments are accessible to the compiled program in the expected order
77-90: No issues found—test data file exists and is correctly implemented.The file
tests/testdata/compile/args.tscorrectly implements argument forwarding by usingDeno.argsto capture and echo all arguments (both compile-time forwarded args via--and runtime args) one per line, as expected by the test.
This pull request adds a test to check that the .js file comes before the CLI options. As issue 30334 mentioned.