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

feat: WPT runner #1662

Merged
merged 2 commits into from
Sep 23, 2022
Merged

feat: WPT runner #1662

merged 2 commits into from
Sep 23, 2022

Conversation

KhafraDev
Copy link
Member

Implements a very basic WPT runner that should allow every test to run (theoretically - this PR only enables a single test to ensure that the runner is working).

The implementation is taken from https://github.com/KhafraDev/node-xmlhttprequest/tree/main/tests with some minor adjustments (setting exitCode on test failure, exiting if there are no tests to run).

Pros:

  1. Uses the WPT test harness, unlike node's builtin wpt runner. See https://github.com/nodejs/node/blob/098eac7f272f5e720c9aff8b232501928a88b82a/test/common/wpt.js#L12-L14.
  2. Each test runs in its own context, and the tests are run in a Worker. The environment these tests run in should be identical to an actual node.js runtime, so hopefully there won't be any unexpected jest-like issues.
  3. Has a built-in server that allows serving resources for tests and enabling a very basic routing system. Note that the server runs in its own process (using child_process.fork) and communicates via ipc. This is because the xhr tests (from my package mentioned above) test synchronous requests that would block the thread and cause requests to never complete. This workaround probably isn't needed here but I decided not to make many changes.

Cons

  1. Uses esm because of personal preference. Also makes use of top level await and other esm-only things, however it could be re-written to use cjs.
  2. Probably only works on node v18 or above; hasn't been tested in v16.
  3. Doesn't log test completions; only test failures. Currently there isn't a way to enable flaky tests or expected test failures either.

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2022

Codecov Report

Base: 94.89% // Head: 94.89% // No change to project coverage 👍

Coverage data is based on head (4cefdc8) compared to base (e5e5b97).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1662   +/-   ##
=======================================
  Coverage   94.89%   94.89%           
=======================================
  Files          53       53           
  Lines        4803     4803           
=======================================
  Hits         4558     4558           
  Misses        245      245           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@ronag ronag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@mcollina
Copy link
Member

Can you add it to CI?

@KhafraDev
Copy link
Member Author

added to the CI

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@KhafraDev KhafraDev merged commit 9c3f34c into nodejs:main Sep 23, 2022
@KhafraDev KhafraDev deleted the wpt-runner branch September 23, 2022 18:14
metcoder95 pushed a commit to metcoder95/undici that referenced this pull request Dec 26, 2022
* feat: WPT runner

* fix: add runner to ci
crysmags pushed a commit to crysmags/undici that referenced this pull request Feb 27, 2024
* feat: WPT runner

* fix: add runner to ci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants