Support .any.js and .window.js tests #12
Conversation
|
If you want to give the updated streams tests a spin for yourself in your local copy of
|
There was a problem hiding this comment.
Gosh, this is a lot more code than I was anticipating. But, I suppose this is the right way to do things to support all the related features, and do things right going forward. Nice work.
Only one nit, which is to use the correct URL parser package (in all places).
lib/internal/handlers.js
Outdated
| // https://github.com/web-platform-tests/wpt/blob/master/tools/wptserve/wptserve/handlers.py | ||
|
|
||
| const path = require("path"); | ||
| const { parse: parseUrl } = require("url"); |
There was a problem hiding this comment.
Can you use the modern WHATWG URL parser, so const { URL } = require("url")? That may also do decoding for you, not sure.
There was a problem hiding this comment.
Sure, I can do that.
This would require Node 7 or higher, perhaps we should signal the minimum Node version somewhere, e.g. in engines.node of package.json?
I noticed that there's a .travis.yml file in the project that targets Node 4, but there aren't any tests to run on Travis. 🤷♂️
There was a problem hiding this comment.
Haha, oh dear. Yeah, let's bump to Node 7 min version, and I'll release this as a new major.
There was a problem hiding this comment.
Hmm, the WHATWG URL parser can't handle relative URLs without passing an absolute base URL. I'll need to check what base URL to use for every parse. I'll look into it.
MattiasBuelens
force-pushed
the
any-js
branch
from
9e80eb6 to
b601a32
Compare
| @@ -25,6 +25,9 @@ | |||
| "prepare": "npm run copy-testharness", | |||
| "copy-testharness": "copyfiles -u 2 wpt/resources/testharness.js testharness/" | |||
| }, | |||
| "engines": { | |||
| "node": ">= 8" | |||
There was a problem hiding this comment.
I went with Node 8 instead of Node 7, since Node 7 is already end-of-life. Plus, Node 8 supports async/await if we later on want to use that. 😄
This adds support for running tests with auto-generated test boilerplate, i.e.
.any.jsand.window.jstests. (Worker tests are ignored sincejsdomdoesn't support workers yet.)I've tried to keep the JavaScript implementation as close as possible to the original Python implementation from web-platform-tests. This should (hopefully) make maintaining our implementation a bit easier, in the (rare) case the test format changes in the future.
I have a branch ready which ports the streams tests to the new format. The new test runner executes all tests, and they all pass on the reference implementation! 🎉 After the streams reference implementation is updated to use this new runner, I will submit a PR to
web-platform-testswith this branch to migrate the streams tests.Fixes #11