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

Separate tests into ones that need dom and ones that don't #11277

Closed
devsnek opened this issue May 31, 2018 · 20 comments
Closed

Separate tests into ones that need dom and ones that don't #11277

devsnek opened this issue May 31, 2018 · 20 comments
Labels

Comments

@devsnek
Copy link

devsnek commented May 31, 2018

A lot of tests in the suite are for things that don't inherently require a DOM to test in all situations (like the URL constructor as an example) but still are wrapped in html.

As the number of runtimes that don't have a dom but implement these features increases it would seem in the interest of wpt to help support making sure everyone can run tests and stay spec compliant. Node.js as an example has hand copied tests into a form they can run, which is fine at first glance but the tests they have aren't updated to the latest forms of the tests here. Additionally if you are a single person without a huge community, its pretty much impossible to rewrite all these hundreds of tests to be runnable and keep them updated and make sure you don't break any of them.

I think some initiative (at least moving forward) for tests to be more carefully written to just be plain js files if possible would be nice. As for existing tests, even if slowly, we could begin rewriting them. I for one would be happy to help with the rewrites.

@Ms2ger
Copy link
Contributor

Ms2ger commented May 31, 2018

I've been working on supporting tests that run in JavaScript shells. There's also #7174 about node.

@annevk annevk added the url label May 31, 2018
@annevk
Copy link
Member

annevk commented May 31, 2018

I'm supportive of this provided it does demonstrably benefit Node.js or any other such project. I can assist with reviews of this code.

@devsnek
Copy link
Author

devsnek commented Jun 1, 2018

@Ms2ger what's that work look like? are you screening new tests? modifying old ones? is what you're doing already enough or do we need to do more?

@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 1, 2018

I'm currently working on the infrastructure on the SpiderMonkey side; will then move on to writing new tests for wasm.

@jugglinmike
Copy link
Contributor

I believe we have the infrastructure necessary to support this. Are you familiar with WPT's "multi-global" tests? The WPT CLI interprets any JavaScript file with the suffix .any.js in a number of different contexts, including a generic HTML document. If the HTML-formatted tests in question shouldn't be run in those additional contexts, they can instead be named with the suffix .window.js and be limited to just a document.

"Don't inherently require a DOM" may not be stringent enough, though. For instance, some tests load expectation data using fetch, and that would also make them less portable. But maybe we can avoid problems like that socially through code review.

@devsnek
Copy link
Author

devsnek commented Jun 2, 2018

@jugglinmike loading fixtures could either be mocked or imported or maybe another metadata comment could be added for fixture data.

i guess the condition i'm really looking for is "needs to run in html" or however that can be better said.

@jugglinmike
Copy link
Contributor

I don't mean to mince words about the requirements, just wanted to make sure we didn't overlook other obstacles

@jugglinmike
Copy link
Contributor

A still simpler solution for metadata would be to reformat the existing JSON files into JavaScript files that assigned the data to a global variable. The two downsides to this are that it makes dependencies implicit and that it would be easier for folks to do scripting within the fixture data (which I think we'd generally want to discourage). Still, when compared to the complexity of a new rule for test interpreting (i.e. // META fixture=data.json), it might be wise to prefer a more primitive solution.

@annevk how would you feel about adopting that pattern in the URL tests?

@devsnek I've only been considering Node.js and the tests in url/ so far. Node might find html/webappapis/timers/ useful. Maybe wasm/ as well. streams/ sounds like a candidate, but I'm not aware of any effort within Node.js to follow the DOM there. Would any of these be relevant? And do you know any others?

@devsnek
Copy link
Author

devsnek commented Jun 5, 2018

@jugglinmike as node implements esm there is like a cascade of standards it might gain. if we want to support https imports as an example we'll need to add fetch and then response and blob and body and stream etc etc.

aside from that in the last year or so there has been more and more of a push for it to support standards in general, like with Encoding and URL

i'm also working on my own runtime mostly as a learning experience and I'm just trying to add as much as possible so the more open available tests the better.

@jugglinmike
Copy link
Contributor

Got it. It's hard for me (or any one person) to approve the adoption of this pattern across all of WPT. My recommendation is to file a separate issue for each specification whose tests you'd like to see generalized. This will help get your request in front of the people who can greenlight it.

I'm suggesting this because I may have some bandwidth to help out, but I'd like some indication that patches would be accepted before starting.

@annevk
Copy link
Member

annevk commented Jun 6, 2018

@jugglinmike moving away from JSON to JavaScript doesn't work for non-JavaScript consumers. I don't think that's a good idea.

@jugglinmike
Copy link
Contributor

@annevk Supporting non-JavaScript consumers is a much more restrictive requirement than what we have discussed in this issue. We'll need to list out all the constraints before we can make progress.

As a start, can you give some examples of consumers and tests?

@annevk
Copy link
Member

annevk commented Jun 6, 2018

Consumers might be all kinds of software libraries for tests such as url/resources/urltestdata.json and mimesniff/mime-types/resources/mime-types.json. We provide a wrapper to run them in the browser.

@jugglinmike
Copy link
Contributor

That's great!

@devsnek: this means that for the URL tests, we don't need to worry about fetch. You/Node.js can rely on the JSON-formatted files in the "resources" sub-directory--instructions for interpreting them are available in this README file. There are other tests for the specifics of the JavaScript API, but those don't rely on fetch, so the ".any.js"/".window.js" approach should get us the rest of the way there.

@devsnek
Copy link
Author

devsnek commented Jun 6, 2018

@jugglinmike indeed, however the runner that wraps those includes more tests and is wrapped in html, so there is still some to be desired.

@joyeecheung
Copy link
Contributor

joyeecheung commented Jun 7, 2018

this means that for the URL tests, we don't need to worry about fetch. You/Node.js can rely on the JSON-formatted files in the "resources" sub-directory--instructions for interpreting them are available in this README file. There are other tests for the specifics of the JavaScript API, but those don't rely on fetch, so the ".any.js"/".window.js" approach should get us the rest of the way there.

Just FYI, we already have ported most of the URL tests into our code base about a year ago. The current way of porting URL WPT into Node.js is:

  1. Copy the resources into our fixtures directory. The JSON files are formatted like this, with a module.exports wrapper around the JSON text and some license text on top. The files are renamed .js because before our URL implementation became WPT compliant, we needed to comment out certain cases in the JSON, it should be fine to just rename them to JSON and import them as JSON now.
  2. We have some mocked harness (assert_false, test, etc.) in test/common/wpt.js
  3. We copy the JS block of .html tests into test/parallel, rename them as .js (like this), add some code on top of that:
    • Import the harness and the URL/URLParams constructors into the scope (the port was primarily done before we decided to expose them as globals. For future implementation I'd anticipate that we need to add // Flag: --experiment-feature before we complete the implementation)
    • Skip certain tests if icu is not available, require the test common for global leak check
    • Add a license header and a link to the original file
    • Add comments to disable eslint because WPT does not follow our JS style
  4. We comment out certain part of the .html tests that rely on the DOM or APIs that we don't have (e.g. XMLHttpRequest, like this).

Currently only the 4th step needs manual maintenance. Step 1 and 3 can be automated (I have a prototype of node-core-utils that does half of the job).

@joyeecheung
Copy link
Contributor

joyeecheung commented Jun 7, 2018

I've only been considering Node.js and the tests in url/ so far. Node might find html/webappapis/timers/ useful. Maybe wasm/ as well. streams/ sounds like a candidate, but I'm not aware of any effort within Node.js to follow the DOM there.

AFAIK our timer implementation is pretty far from the web and it cannot be changed for compatibility reasons. e.g. you get a Timer object from setTimeout and setInterval() instead of an integer.

There has been resistance against implementing the WHATWG stream and there is also a new Stream API being proposed (openjs-foundation/summit#82) that's not WHATWG stream. But implementation of other standards may need part of WHATWG stream, e.g. fetch (being considered but it's on and off), esm.

wasm could be a good fit.

@joyeecheung
Copy link
Contributor

joyeecheung commented Jun 7, 2018

Node.js also implemented performance-timeline but we have not ported the tests. We also implemented encoding and some tests has been ported by replacing the harness in-place manually (in our test/parallel/test-whatwg-encoding-* files, like this).

jugglinmike pushed a commit to bocoup/wpt that referenced this issue Jun 13, 2018
Refactor tests so that they may be consumed by non-browser JavaScript
runtimes which implement the standard (e.g. Node.js [1]). Use WPT's
`.any.js` convention to extend test coverage in browsers by allowing
them to be executed within a Web Worker.

This change is in service of web-platform-testsgh-11277 [2]

[1] https://nodejs.org/api/perf_hooks.html
[2] web-platform-tests#11277
plehegar pushed a commit that referenced this issue Jun 19, 2018
Refactor tests so that they may be consumed by non-browser JavaScript
runtimes which implement the standard (e.g. Node.js [1]). Use WPT's
`.any.js` convention to extend test coverage in browsers by allowing
them to be executed within a Web Worker.

This change is in service of gh-11277 [2]

[1] https://nodejs.org/api/perf_hooks.html
[2] #11277
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 11, 2018
…=testonly

Automatic update from web-platform-tests[performance-timeline] Reformat tests (#11495)

Refactor tests so that they may be consumed by non-browser JavaScript
runtimes which implement the standard (e.g. Node.js [1]). Use WPT's
`.any.js` convention to extend test coverage in browsers by allowing
them to be executed within a Web Worker.

This change is in service of gh-11277 [2]

[1] https://nodejs.org/api/perf_hooks.html
[2] web-platform-tests/wpt#11277
--

wpt-commits: 50f1ceebcf83482f7ce4edd5a33f25cd27ee6e38
wpt-pr: 11495
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…=testonly

Automatic update from web-platform-tests[performance-timeline] Reformat tests (#11495)

Refactor tests so that they may be consumed by non-browser JavaScript
runtimes which implement the standard (e.g. Node.js [1]). Use WPT's
`.any.js` convention to extend test coverage in browsers by allowing
them to be executed within a Web Worker.

This change is in service of gh-11277 [2]

[1] https://nodejs.org/api/perf_hooks.html
[2] web-platform-tests/wpt#11277
--

wpt-commits: 50f1ceebcf83482f7ce4edd5a33f25cd27ee6e38
wpt-pr: 11495

UltraBlame original commit: 0e7163be63eb16cba21354a120dfd84f11a56b03
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 3, 2019
…=testonly

Automatic update from web-platform-tests[performance-timeline] Reformat tests (#11495)

Refactor tests so that they may be consumed by non-browser JavaScript
runtimes which implement the standard (e.g. Node.js [1]). Use WPT's
`.any.js` convention to extend test coverage in browsers by allowing
them to be executed within a Web Worker.

This change is in service of gh-11277 [2]

[1] https://nodejs.org/api/perf_hooks.html
[2] web-platform-tests/wpt#11277
--

wpt-commits: 50f1ceebcf83482f7ce4edd5a33f25cd27ee6e38
wpt-pr: 11495

UltraBlame original commit: 0e7163be63eb16cba21354a120dfd84f11a56b03
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…=testonly

Automatic update from web-platform-tests[performance-timeline] Reformat tests (#11495)

Refactor tests so that they may be consumed by non-browser JavaScript
runtimes which implement the standard (e.g. Node.js [1]). Use WPT's
`.any.js` convention to extend test coverage in browsers by allowing
them to be executed within a Web Worker.

This change is in service of gh-11277 [2]

[1] https://nodejs.org/api/perf_hooks.html
[2] web-platform-tests/wpt#11277
--

wpt-commits: 50f1ceebcf83482f7ce4edd5a33f25cd27ee6e38
wpt-pr: 11495

UltraBlame original commit: 0e7163be63eb16cba21354a120dfd84f11a56b03
@jimmywarting
Copy link
Contributor

jimmywarting commented Jul 29, 2021

It would be cool if the idlharness.js could be rewritten to not add any global functions and also be rewritten to ESM
or at least assign the functions to the global namespace using something like Object.assign(globalThis, all_assertions)

It would be pretty sweet if it was just possible to import some related tests in nodejs that have written some polyfill and just do:

globalThis.formData = await import('./formdata-polyfill.js')
await import('http://wpt.live/FileAPI/file/send-file-formdata-controls.any.js')

now thanks to NodeJS and Deno's http loader

@annevk
Copy link
Member

annevk commented Dec 16, 2022

I think we can consider this closed given there's now any.js and that works for Deno.

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

No branches or pull requests

6 participants