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.

Sign up for GitHub

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

Jump to bottom

Suggested additional WHATWG URL test coverage #20720

Closed
domenic opened this issue May 14, 2018 · 1 comment
Closed

Suggested additional WHATWG URL test coverage #20720

domenic opened this issue May 14, 2018 · 1 comment
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@domenic
Copy link
Contributor

domenic commented May 14, 2018

In web-platform-tests/wpt#10955 we are slightly changing the way in which bad base URLs are tested for the WHATWG URL API's canonical urltestdata.json file. I believe Node also makes use of that file and so might want to adapt.

Basically, we are removing the three tests that currently exist which test bad base URLs, in favor of the following:

In addition to testing that parsing input against base gives the result, a test harness for the URL constructor (or similar APIs) should additionally test the following pattern: if failure is true, parsing about:blank against base must give failure. This tests that the logic for converting base URLs into strings properly fails the whole parsing algorithm if the base URL cannot be parsed.

You can see an example of how to do this in jsdom/whatwg-url#116; probably Node.js will want to do something similar.
@domenic domenic mentioned this issue May 14, 2018
Merged
@addaleax addaleax added the whatwg-url Issues and PRs related to the WHATWG URL implementation. label May 14, 2018
@addaleax
Copy link
Member

@nodejs/url

@joyeecheung joyeecheung mentioned this issue May 17, 2018
Closed
3 tasks
joyeecheung added a commit to joyeecheung/node that referenced this issue May 17, 2018
@joyeecheung
test: test about:blank against invalid WHATWG URL
c5e49e7 
> If `failure` is true, parsing `about:blank` against `base`
> must give failure. This tests that the logic for converting
> base URLs into strings properly fails the whole parsing
> algorithm if the base URL cannot be parsed.

Fixes: nodejs#20720
MylesBorins pushed a commit that referenced this issue May 22, 2018
@joyeecheung @MylesBorins
test: test about:blank against invalid WHATWG URL
235a272 
> If `failure` is true, parsing `about:blank` against `base`
> must give failure. This tests that the logic for converting
> base URLs into strings properly fails the whole parsing
> algorithm if the base URL cannot be parsed.

Fixes: #20720

PR-URL: #20796
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue May 23, 2018
@joyeecheung @MylesBorins
test: test about:blank against invalid WHATWG URL
5886b78 
> If `failure` is true, parsing `about:blank` against `base`
> must give failure. This tests that the logic for converting
> base URLs into strings properly fails the whole parsing
> algorithm if the base URL cannot be parsed.

Fixes: #20720

PR-URL: #20796
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: test about:blank against invalid WHATWG URL joyeecheung/node
2 participants
@domenic @addaleax