Skip to content

Conversation

0hmX
Copy link
Contributor

@0hmX 0hmX commented Apr 8, 2025

closes #28606

I have the implementation working and validated against the Node.js tests. I'm looking for knowledge on integrating the tests properly.

The existing parseArgs tests seem to be managed by a script that pulls them from Node.js. What's the preferred approach: update the files generated by that script, or add these tests separately? Also, what version of Node will the newly pulled tests come from?

@dsherret dsherret requested a review from kt3k April 8, 2025 21:30
@dsherret dsherret changed the title feat(ext/node): add support for --no- prefix in parseArgs() fix(ext/node): add support for --no- prefix in parseArgs() Apr 8, 2025
@kt3k
Copy link
Member

kt3k commented Apr 9, 2025

The existing parseArgs tests seem to be managed by a script that pulls them from Node.js. What's the preferred approach: update the files generated by that script, or add these tests separately? Also, what version of Node will the newly pulled tests come from?

Updated the test case. Looks like the added case are passing with this implementation (with a small fix). Nice work!

You can check it with the command:

./target/debug/deno test --config tests/config/deno.json -A tests/node_compat/test.ts -- parallel/test-parse-args.mjs

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@kt3k kt3k merged commit 9bc9faf into denoland:main Apr 9, 2025
18 checks passed
nayeemrmn pushed a commit that referenced this pull request Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

missing allowNegative option in parseArgs from node:util
2 participants