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

test: automate WPT updates with drivers in test/wpt #24035

Closed
wants to merge 7 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Nov 2, 2018

This is a big diff but note that the biggest commit (the first one) is automated by git node wpt. For a high-level design see test/wpt/README.md in this diff.

There is still work to do with the upstream to port more tests to .js files and split them properly so that we only need to skip the parts that truely cannot be run by us (e.g. ones that requires Response/Request objects).

Currently there are still a few test-whatwg-* tests in test/parallel that are copy-pasted with manually added fixtures/linter switches, but I think we can land these patches first to eliminate the manual maintenance of of the tests that can be migrated, and gradually migrate the rest of them later while working with the upstream.

Refs: #23192

Current status:

  • console tests: all migrated here
  • URL tests:
    • Some of them are still in .html files upstream
    • Some of them mixes JS tests and <a>/<area> tests with references to document.createElement in .window.js, or needs something like Request/Response which we don't have
    • There are a bunch of test-whatwg-url-custom-* tests that can be upstreamed and pulled here later
    • Others are migrated here
  • encoding tests: I have a WIP branch for them - but will need to figure out the ICU requirements in the CI first as some of the subtests need more support than the others, and we will probably want to split them into separate files somehow in the upstream as well, so I'd prefer to leave them in a subsequent PR

Note that none of the IDL tests currently pass - I plan to fix them later after this PR lands.

test: use git node wpt to pull WPT into test/fixtures

This patch uses the git node wpt command in node-core-utils
to initialize test/fixtures/wpt with a subset of the Web
Platform Tests (https://github.com/web-platform-tests/wpt),
including:

  • console tests
  • url tests
  • interfaces (.idl WebIDL files)
  • test harness located in resources, including the WebIDL
    parser

Also initializes the README.md, LICENSE and versions.json in the
test/fixtures/wpt.

In later patches, we will use these files to run WPT
and move away from the manually maintained copy-pasted files
in test/parallel/test-whatwg-*.

https://github.com/nodejs/node-core-utils/blob/master/docs/git-node.md#git-node-wpt

test: remove WPT tests that are now .any.js in the upstream

In favor of actual .any.js tests that will be run with the WPT
harness in a subsequent patch.

The other types of .js tests (e.g. .window.js tests) and the .html
tests remained to be supported or migrated in the upstream.

test: use URL fixtures under test/fixtures/wpt/url/resources

Removes the following files:

  • test/fixtures/url-tests.js
  • test/fixtures/url-setter-tests.js
  • test/fixtures/url-toascii.js

in favor of:

  • test/fixtures/wpt/url/resources/urltestdata.json
  • test/fixtures/wpt/url/resources/setters_tests.json
  • test/fixtures/wpt/url/resources/toascii.json

Also removes dependency of fixtures/url-tests.js in http2 tests
and use fixtures/person-large.jpg instead since they are just
looking for a big enough file to transfer.

test: initialize test/wpt to run URL and console .js tests

This patch:

  • Creates a new test suite wpt that can be used to run a subset
    of Web Platform Tests
  • Adds a WPTRunner in test/common/wpt.js that can run the WPT
    subset in test/fixtures/wpt with a vm and the WPT harness
    while taking the status file in test/wpt/status into account.
    Here we use a new format of status file (in JSON) to handle specific
    requirements (like ICU requirements) in the tests and to handle
    expected failures and TODOs.
  • Adds documentation on how the runner and the update automation works
  • Runs the WHATWG URL tests and the console tests with the new test
    runner.

With this patch we eliminates the need of copy-pasting with manual
modifications to update a large chunk of our WPT subset previously
maintained in test/parallel. Now the tests run in test/wpt can
be automatically updated with git node wpt without modifications
by the actual WPT harness instead of our home-grown mock.

There are still a few URL tests left that need to be migrated in the
upstream to be placed in .js instead of .html - we currently still use
the legacy harness mock in the test files.

doc: remove legacy WPT integration guide

Point to the new guide in test/wpt/README.md instead.

This patch uses the `git node wpt` command in node-core-utils
to initialize `test/fixtures/wpt` with a subset of the Web
Platform Tests (https://github.com/web-platform-tests/wpt),
including:

- console tests
- url tests
- interfaces (.idl WebIDL files)
- test harness located in `resources`, including the WebIDL
  parser

Also initializes the README.md, LICENSE and versions.json in the
`test/fixtures/wpt`.

In later patches, we will use these files to run WPT
and move away from the manually maintained copy-pasted files
in `test/parallel/test-whatwg-*`.

https://github.com/nodejs/node-core-utils/blob/master/docs/git-node.md#git-node-wpt
In favor of actual .any.js tests that will be run with the WPT
harness in a subsequent patch.

The other types of .js tests (e.g. .window.js tests) and the .html
tests remained to be supported or migrated in the upstream.
Removes the following files:

- test/fixtures/url-tests.js
- test/fixtures/url-setter-tests.js
- test/fixtures/url-toascii.js

in favor of:

- test/fixtures/wpt/url/resources/urltestdata.json
- test/fixtures/wpt/url/resources/setters_tests.json
- test/fixtures/wpt/url/resources/toascii.json

Also removes dependency of `fixtures/url-tests.js` in http2 tests
and use `fixtures/person-large.jpg` instead since they are just
looking for a big enough file to transfer.
This patch:

- Creates a new test suite `wpt` that can be used to run a subset
  of Web Platform Tests
- Adds a `WPTRunner` in `test/common/wpt.js` that can run the WPT
  subset in `test/fixtures/wpt` with a vm and the WPT harness
  while taking the status file in `test/wpt/status` into account.
  Here we use a new format of status file (in JSON) to handle specific
  requirements (like ICU requirements) in the tests and to handle
  expected failures and TODOs.
- Adds documentation on how the runner and the update automation works
- Runs the WHATWG URL tests and the console tests with the new test
  runner.

With this patch we eliminates the need of copy-pasting with manual
modifications to update a large chunk of our WPT subset previously
maintained in `test/parallel`. Now the tests run in `test/wpt` can
be automatically updated with `git node wpt` without modifications
by the actual WPT harness instead of our home-grown mock.

There are still a few URL tests left that need to be migrated in the
upstream to be placed in .js instead of .html - we currently still use
the legacy harness mock in the test files.
Point to the new guide in test/wpt/README.md instead.
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Nov 2, 2018
@joyeecheung
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/18300/

cc @nodejs/testing @nodejs/url

@joyeecheung
Copy link
Member Author

Also maybe @nodejs/open-standards would be interested in taking a look

@joyeecheung
Copy link
Member Author

@targos
Copy link
Member

targos commented Nov 2, 2018

@targos
Copy link
Member

targos commented Nov 2, 2018

06:44:59 + python tools/test.py -j 4 -p tap --logfile test.tap --mode=release --flaky-tests=dontcare wpt
06:45:00 TAP version 13
06:45:00 1..2
06:45:00 ok 1 wpt/test-whatwg-console
06:45:00   ---
06:45:00   duration_ms: 0.332
06:45:00   ...
06:45:00 ok 2 wpt/test-whatwg-url
06:45:00   ---
06:45:00   duration_ms: 0.412
06:45:00   ...

🎉

@joyeecheung
Copy link
Member Author

@joyeecheung
Copy link
Member Author

The CI is green. Can I have some reviews please? @nodejs/testing @nodejs/url

Makefile Show resolved Hide resolved
@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 8, 2018

I plan to land this after the 7-day limit passes to continue fixing these failures:

// test/wpt/status/console.json
{
  "idlharness.any.js": {
    "fail": ".table, .dir and .timeLog parameter lengths are wrong"
  }
} 
// test/wpt/status/url.json
  "toascii.window.js": {
    "requires": ["intl"],
    "skip": "TODO: port from .window.js"
  },
  "historical.any.js": {
    "requires": ["intl"]
  },
  "urlencoded-parser.any.js": {
    "fail": "missing Request and Response"
  },
  "idlharness.any.js": {
    "fail": "getter/setter names are wrong, etc."
  }
}

And continue porting encoding, timer, performance and messaging (worker even?) tests into this repo without manual copy-pasting - I don't expect all of them to pass but at least it would be nice to know where the inconsistencies are.

So please voice your opinion if you spot something that's just incorrect in theory and cannot be fixed later once this lands (I hope there isn't since this is just home-grown test changes that's completely up to us to fix anytime we want...).

@joyeecheung
Copy link
Member Author

Landed in 9d434db...22f3ff9, thanks!

@joyeecheung joyeecheung closed this Nov 9, 2018
joyeecheung added a commit that referenced this pull request Nov 9, 2018
This patch uses the `git node wpt` command in node-core-utils
to initialize `test/fixtures/wpt` with a subset of the Web
Platform Tests (https://github.com/web-platform-tests/wpt),
including:

- console tests
- url tests
- interfaces (.idl WebIDL files)
- test harness located in `resources`, including the WebIDL
  parser

Also initializes the README.md, LICENSE and versions.json in the
`test/fixtures/wpt`.

In later patches, we will use these files to run WPT
and move away from the manually maintained copy-pasted files
in `test/parallel/test-whatwg-*`.

https://github.com/nodejs/node-core-utils/blob/master/docs/git-node.md#git-node-wpt

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
joyeecheung added a commit that referenced this pull request Nov 9, 2018
In favor of actual .any.js tests that will be run with the WPT
harness in a subsequent patch.

The other types of .js tests (e.g. .window.js tests) and the .html
tests remained to be supported or migrated in the upstream.

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
joyeecheung added a commit that referenced this pull request Nov 9, 2018
Removes the following files:

- test/fixtures/url-tests.js
- test/fixtures/url-setter-tests.js
- test/fixtures/url-toascii.js

in favor of:

- test/fixtures/wpt/url/resources/urltestdata.json
- test/fixtures/wpt/url/resources/setters_tests.json
- test/fixtures/wpt/url/resources/toascii.json

Also removes dependency of `fixtures/url-tests.js` in http2 tests
and use `fixtures/person-large.jpg` instead since they are just
looking for a big enough file to transfer.

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
joyeecheung added a commit that referenced this pull request Nov 9, 2018
This patch:

- Creates a new test suite `wpt` that can be used to run a subset
  of Web Platform Tests
- Adds a `WPTRunner` in `test/common/wpt.js` that can run the WPT
  subset in `test/fixtures/wpt` with a vm and the WPT harness
  while taking the status file in `test/wpt/status` into account.
  Here we use a new format of status file (in JSON) to handle specific
  requirements (like ICU requirements) in the tests and to handle
  expected failures and TODOs.
- Adds documentation on how the runner and the update automation works
- Runs the WHATWG URL tests and the console tests with the new test
  runner.

With this patch we eliminates the need of copy-pasting with manual
modifications to update a large chunk of our WPT subset previously
maintained in `test/parallel`. Now the tests run in `test/wpt` can
be automatically updated with `git node wpt` without modifications
by the actual WPT harness instead of our home-grown mock.

There are still a few URL tests left that need to be migrated in the
upstream to be placed in .js instead of .html - we currently still use
the legacy harness mock in the test files.

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
joyeecheung added a commit that referenced this pull request Nov 9, 2018
Point to the new guide in test/wpt/README.md instead.

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
This patch uses the `git node wpt` command in node-core-utils
to initialize `test/fixtures/wpt` with a subset of the Web
Platform Tests (https://github.com/web-platform-tests/wpt),
including:

- console tests
- url tests
- interfaces (.idl WebIDL files)
- test harness located in `resources`, including the WebIDL
  parser

Also initializes the README.md, LICENSE and versions.json in the
`test/fixtures/wpt`.

In later patches, we will use these files to run WPT
and move away from the manually maintained copy-pasted files
in `test/parallel/test-whatwg-*`.

https://github.com/nodejs/node-core-utils/blob/master/docs/git-node.md#git-node-wpt

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
In favor of actual .any.js tests that will be run with the WPT
harness in a subsequent patch.

The other types of .js tests (e.g. .window.js tests) and the .html
tests remained to be supported or migrated in the upstream.

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
Removes the following files:

- test/fixtures/url-tests.js
- test/fixtures/url-setter-tests.js
- test/fixtures/url-toascii.js

in favor of:

- test/fixtures/wpt/url/resources/urltestdata.json
- test/fixtures/wpt/url/resources/setters_tests.json
- test/fixtures/wpt/url/resources/toascii.json

Also removes dependency of `fixtures/url-tests.js` in http2 tests
and use `fixtures/person-large.jpg` instead since they are just
looking for a big enough file to transfer.

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
This patch:

- Creates a new test suite `wpt` that can be used to run a subset
  of Web Platform Tests
- Adds a `WPTRunner` in `test/common/wpt.js` that can run the WPT
  subset in `test/fixtures/wpt` with a vm and the WPT harness
  while taking the status file in `test/wpt/status` into account.
  Here we use a new format of status file (in JSON) to handle specific
  requirements (like ICU requirements) in the tests and to handle
  expected failures and TODOs.
- Adds documentation on how the runner and the update automation works
- Runs the WHATWG URL tests and the console tests with the new test
  runner.

With this patch we eliminates the need of copy-pasting with manual
modifications to update a large chunk of our WPT subset previously
maintained in `test/parallel`. Now the tests run in `test/wpt` can
be automatically updated with `git node wpt` without modifications
by the actual WPT harness instead of our home-grown mock.

There are still a few URL tests left that need to be migrated in the
upstream to be placed in .js instead of .html - we currently still use
the legacy harness mock in the test files.

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
Point to the new guide in test/wpt/README.md instead.

PR-URL: #24035
Refs: #23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
This patch uses the `git node wpt` command in node-core-utils
to initialize `test/fixtures/wpt` with a subset of the Web
Platform Tests (https://github.com/web-platform-tests/wpt),
including:

- console tests
- url tests
- interfaces (.idl WebIDL files)
- test harness located in `resources`, including the WebIDL
  parser

Also initializes the README.md, LICENSE and versions.json in the
`test/fixtures/wpt`.

In later patches, we will use these files to run WPT
and move away from the manually maintained copy-pasted files
in `test/parallel/test-whatwg-*`.

https://github.com/nodejs/node-core-utils/blob/master/docs/git-node.md#git-node-wpt

PR-URL: nodejs#24035
Refs: nodejs#23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
In favor of actual .any.js tests that will be run with the WPT
harness in a subsequent patch.

The other types of .js tests (e.g. .window.js tests) and the .html
tests remained to be supported or migrated in the upstream.

PR-URL: nodejs#24035
Refs: nodejs#23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
Removes the following files:

- test/fixtures/url-tests.js
- test/fixtures/url-setter-tests.js
- test/fixtures/url-toascii.js

in favor of:

- test/fixtures/wpt/url/resources/urltestdata.json
- test/fixtures/wpt/url/resources/setters_tests.json
- test/fixtures/wpt/url/resources/toascii.json

Also removes dependency of `fixtures/url-tests.js` in http2 tests
and use `fixtures/person-large.jpg` instead since they are just
looking for a big enough file to transfer.

PR-URL: nodejs#24035
Refs: nodejs#23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
This patch:

- Creates a new test suite `wpt` that can be used to run a subset
  of Web Platform Tests
- Adds a `WPTRunner` in `test/common/wpt.js` that can run the WPT
  subset in `test/fixtures/wpt` with a vm and the WPT harness
  while taking the status file in `test/wpt/status` into account.
  Here we use a new format of status file (in JSON) to handle specific
  requirements (like ICU requirements) in the tests and to handle
  expected failures and TODOs.
- Adds documentation on how the runner and the update automation works
- Runs the WHATWG URL tests and the console tests with the new test
  runner.

With this patch we eliminates the need of copy-pasting with manual
modifications to update a large chunk of our WPT subset previously
maintained in `test/parallel`. Now the tests run in `test/wpt` can
be automatically updated with `git node wpt` without modifications
by the actual WPT harness instead of our home-grown mock.

There are still a few URL tests left that need to be migrated in the
upstream to be placed in .js instead of .html - we currently still use
the legacy harness mock in the test files.

PR-URL: nodejs#24035
Refs: nodejs#23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
Point to the new guide in test/wpt/README.md instead.

PR-URL: nodejs#24035
Refs: nodejs#23192
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
@codebytere
Copy link
Member

@joyeecheung this doesn't land cleanly on v10.x; if you think it's a backport candidate could you please backport it manually?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants