Skip to content
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

URL test data: expected query and fragment should have no leading ?/# #499

Closed
smola opened this issue Jan 9, 2014 · 5 comments
Closed

Comments

@smola
Copy link
Contributor

smola commented Jan 9, 2014

In url/urltestdata.txt, expected query and fragment always have a leading '?' and '#'. Example:

http://user:pass@foo:21/bar;par?b#c s:http u:user pass:pass h:foo port:21 p:/bar;par q:?b f:#c

The spec says the leading '?' and '#' are not part of query and fragment themselves, but these test could misguide implementors into including them "to pass the tests".

If you give ok to this change, I can provide a quick pull request.

@rubys
Copy link
Contributor

rubys commented Nov 23, 2014

I'm OK with this change

@annevk
Copy link
Member

annevk commented Feb 15, 2017

@domenic @jasnell what do you think? Making urltestdata.json reflect URL records might be somewhat better than having it reflect the JavaScript API.

@annevk
Copy link
Member

annevk commented Feb 15, 2017

(That might also make it easier to omit fields that have their default value?)

@domenic
Copy link
Member

domenic commented Feb 15, 2017

Strong -1 from me. Being able to do simple assert_equals against the public API is what makes this a valuable way of testing the public API. If we start reconstructing the expected values from the JSON plus added test-specific logic, that'd be a big step backward to the old days where urltestdata just contained URL strings and the tests had to have their own mini-parser to reconstruct the expected values.

@annevk
Copy link
Member

annevk commented Feb 15, 2017

Okay, not a hill I'm dying on.

@annevk annevk closed this as completed Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants