Skip to content

Conversation

myd7349
Copy link
Contributor

@myd7349 myd7349 commented Mar 1, 2025

On Windows, the equivalent of /dev/nul is nul. Using /dev/nul on Windows will cause the program to throw a runtime exception when the output parameter is not specified.

Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

Nice. Is it easy to write a test for this? (So we don't regress)

@myd7349
Copy link
Contributor Author

myd7349 commented Mar 1, 2025

Hello. @anonrig Adding a test sounds like a good idea.

To my understanding, this is a modification to adaparse, so should this test be added as a step in CI to execute adaparse and ensure it works(For example, checking whether it correctly parses a URL as expected or ensuring its exit code is 0?)? Or should a post-build event be added in CMake to run the test?

I took a look at the tests under https://github.com/ada-url/ada/tree/main/tests, and they all seem to be testing ada's API. So, I need some guidance.

@anonrig
Copy link
Member

anonrig commented Mar 1, 2025

I think we need to add a new test file under tests folder that links to adaparse. Let's merge this as it is, and appreciate it if you can add a test for it as a follow up.

@anonrig anonrig merged commit 30a2aaf into ada-url:main Mar 1, 2025
38 checks passed
@myd7349 myd7349 deleted the fix-devnull branch March 1, 2025 16:57
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.

2 participants