Reformat URL tests #11492
Reformat URL tests #11492
Conversation
wpt-pr-bot
requested review from
annevk,
domenic,
GPHemsley,
mikewest,
Sebmaster,
smola and
TimothyGu
|
This does not appear to touch my precious .json files so this should not disrupt my workflow :). I'll leave reviews of the conversion of the wrappers to others. |
url/url-tojson.any.js
Outdated
| @@ -0,0 +1,4 @@ | |||
| test(() => { | |||
| const a = new URL('https://example.com/'); | |||
| assert_equals(JSON.stringify(a), '"https://example.com/"'); | |||
There was a problem hiding this comment.
Could you undo the quoting changes to see if GitHub then recognizes this as a rename + modifications rather than a new resource?
There was a problem hiding this comment.
@mariestaver Undid the quoting changes, but this is still being interpreted as a new file
| @@ -67,5 +60,4 @@ | |||
| } | |||
| }) | |||
| }, "response.formData() with input: " + val.input) | |||
| }) | |||
| </script> | |||
| }); | |||
There was a problem hiding this comment.
Done, though not recognized by GitHub
There was a problem hiding this comment.
FWIW, I think it is. If there's no newline there's a red indicator at the end of the line.
url/urlsearchparams-append.any.js
Outdated
| <script> | ||
| /* | ||
| https://url.spec.whatwg.org/#dom-urlsearchparams-append | ||
| */ |
There was a problem hiding this comment.
No need to keep this. Or any of the links. It's pretty self-explanatory where this is all documented.
There was a problem hiding this comment.
Thanks for the feedback @annevk!
I'm curious to hear why you think its obvious and self-explanatory. I agree it might be the case for spec authors and implementors. However, for new WPT contributors without a strong standards background, I think this link would be pretty helpful.
There was a problem hiding this comment.
I was thinking the README had a link to the standard (as a standalone section), but it seems like it doesn't. I think we should add that. Test metadata tends to go out-of-date.
There was a problem hiding this comment.
Sounds good @annevk. Will update.
| </head> | ||
| </html> | ||
|
|
||
| }, 'URLSearchParams connected to URL'); |
url/urlsearchparams-sort.any.js
Outdated
| @@ -53,5 +47,4 @@ | |||
| url.searchParams.sort() | |||
| assert_equals(url.href, "http://example.com/") | |||
| assert_equals(url.search, "") | |||
| }, "Sorting non-existent params removes ? from URL") | |||
| </script> | |||
| }, "Sorting non-existent params removes ? from URL") | |||
| assert_equals(url2.searchParams.toString(), '%3Fa=b') | ||
| }, 'URL.searchParams and URL.search setters, update propagation') | ||
| } | ||
| runURLSearchParamTests() |
There was a problem hiding this comment.
This was strictly copy-and-pasted, right?
There was a problem hiding this comment.
Yes, this was strictly copy-and-pasted. Its the 68 lines from url/url-constructor.html plus 4 for the copy of bURL.
jugglinmike
force-pushed
the
liberate-url-2
branch
2 times, most recently
from
d253419
to
23baa8f
Compare
|
Hi, any updates on this PR? Would really love to see this get merged and incorporated into the Node.js core test suites, thanks! |
|
@jugglinmike could you rebase, please? |
jugglinmike
force-pushed
the
liberate-url-2
branch
from
23baa8f
to
c1491f8
Compare
- Convert url/urlsearchparams-append.html to .any.js - Convert url/urlsearchparams-constructor.html to .any.js - Convert url/urlsearchparams-foreach.html to .any.js - Convert url/urlsearchparams-set.html to .any.js - Convert url/urlsearchparams-getall.html to .any.js
jugglinmike
force-pushed
the
liberate-url-2
branch
from
c1491f8
to
9acf4f1
Compare
|
@Ms2ger Sure. The prior version of this branch is available at https://github.com/web-platform-tests/wpt/compare/master...bocoup:liberate-url-3?expand=1 in case that's interesting to anyone. |
Refactor tests so that they may be consumed by non-browser JavaScript runtimes which implement the WHATWG URL standard such as Node.js [1]. Using WPT's
.any.jsconvention also extends test coverage in browsers by allowing them to be executed within a Web Worker.This change is in service of gh-11277.
[1] https://nodejs.org/dist/latest-v8.x/docs/api/url.html#url_the_whatwg_url_api
/cc @jorydotcom @mariestaver @mzgoddard @nomadtechie @spectranaut