test: reproduce issue with short option (alias) and =#230
Conversation
OrbisK
changed the title
=
|
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 two unit tests for parseRawArgs: one verifies Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
test/parser.test.ts
Outdated
| }) | ||
| }) | ||
|
|
||
| it('handles -<arg>=<value> with alias', ()=>{ |
There was a problem hiding this comment.
| it('handles -<arg>=<value> with alias', ()=>{ | |
| it.fails('handles -<arg>=<value> with alias', ()=>{ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/parser.test.ts (3)
63-64: Remove extra blank lines.Two consecutive blank lines were added between tests. One blank line is sufficient for separation, consistent with the rest of the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/parser.test.ts` around lines 63 - 64, There are two consecutive blank lines between tests in test/parser.test.ts; remove the extra blank line so there is only a single blank line separating test blocks to match the file's existing spacing conventions and keep consistent separation between tests.
18-27: Test looks correct, but formatting is inconsistent with existing tests.The test for
--name=Johnsyntax correctly validates the expected parsing behavior. However, the formatting differs from other tests in this file (e.g., missing spaces around arrow function()=>{vs() => {, missing semicolons).♻️ Suggested formatting for consistency
- it('handles --<arg>=<value>', ()=>{ - const result = parseRawArgs(["--name=John"], { - string: ["name"] - }); + it("handles --<arg>=<value>", () => { + const result = parseRawArgs(["--name=John"], { + string: ["name"], + }); expect(result).toEqual({ _: [], - name: "John" - }) - }) + name: "John", + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/parser.test.ts` around lines 18 - 27, The test for 'handles --<arg>=<value>' is functionally correct but inconsistent with the project's test formatting; update the test that calls parseRawArgs to match the file's style by adding spaces around the arrow function (use "() => { ... }"), ensure statement terminators (semicolons) are present where other tests use them, and follow the same brace/spacing conventions as neighboring tests so the test description, the call to parseRawArgs, and the expect(...) block use consistent spacing and semicolons.
29-42: Reproduction test expected to fail - consider marking it appropriately.Based on the relevant context snippets, Node.js's
parseArgsdoes not properly handle-short=valueformat. For-n=John, it returns the value as"=John"(including the equals sign). The alias propagation logic cannot fix this malformed value.Since this is a reproduction test for the linked issue (nuxt/nuxt#34430), consider:
- Marking the test with
it.fails()orit.skip()to document it's a known failing case until fixed- Adding a comment referencing the issue being reproduced
📝 Suggested approach to mark as known failing test
- it('handles -<arg>=<value> with alias', ()=>{ + // Reproduces https://github.com/nuxt/nuxt/issues/34430 + it.fails("handles -<arg>=<value> with alias", () => { const result = parseRawArgs(["-n=John"], { string: ["name"], alias: { - n: ["name"] - } - }) + n: ["name"], + }, + }); expect(result).toEqual({ _: [], n: "John", name: "John", - }) - }) + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/parser.test.ts` around lines 29 - 42, This test is a known reproduction of a Node.js parseArgs bug where "-n=John" yields a malformed value; update the test case in parser.test.ts (the "handles -<arg>=<value> with alias" spec that calls parseRawArgs) to mark it as a known failing case (e.g., change it(...) to it.fails(...) or it.skip(...)) and add a short inline comment referencing the upstream issue (nuxt/nuxt#34430) so it’s clear this is an intentional reproduction until parseRawArgs or the alias handling in parseRawArgs is fixed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/parser.test.ts`:
- Around line 63-64: There are two consecutive blank lines between tests in
test/parser.test.ts; remove the extra blank line so there is only a single blank
line separating test blocks to match the file's existing spacing conventions and
keep consistent separation between tests.
- Around line 18-27: The test for 'handles --<arg>=<value>' is functionally
correct but inconsistent with the project's test formatting; update the test
that calls parseRawArgs to match the file's style by adding spaces around the
arrow function (use "() => { ... }"), ensure statement terminators (semicolons)
are present where other tests use them, and follow the same brace/spacing
conventions as neighboring tests so the test description, the call to
parseRawArgs, and the expect(...) block use consistent spacing and semicolons.
- Around line 29-42: This test is a known reproduction of a Node.js parseArgs
bug where "-n=John" yields a malformed value; update the test case in
parser.test.ts (the "handles -<arg>=<value> with alias" spec that calls
parseRawArgs) to mark it as a known failing case (e.g., change it(...) to
it.fails(...) or it.skip(...)) and add a short inline comment referencing the
upstream issue (nuxt/nuxt#34430) so it’s clear this is an intentional
reproduction until parseRawArgs or the alias handling in parseRawArgs is fixed.
==
|
FWIW: This seems to be native node parseArgs behavior. (maybe wont-fix) |
|
I guess we need to revert back from node arg parser to allow Do you mind to please convert it to an issue to track @OrbisK ? 🙏🏼 |
Co-authored-by: Robin <robin.kehl@singular-it.de>
|
I have added the issue and prepared this pr so it includes the reproduction as failing test, so it is mergeable if needed. |
2 participants
This test reproduces nuxt/nuxt#34430 / #237
Summary by CodeRabbit