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: update WPT encoding tests and run WPT in subdirectories #27860

Closed
wants to merge 5 commits into from

Conversation

joyeecheung
Copy link
Member

test: expect wpt/encoding/streams to fail

Since we do not implement TextDecoderStream or TextEncoderStream.

test: run WPT in subdirectories

For a directory like this:

  • wpt
    • encoding
      • streams
        • backpressure.any.js
      • api-basics.any.js

Previously we only run api-basics.any.js, now we also run
backpressure.any.js (and any tests in more deeply nested
directories). This enables us to run more of WPT since not
every module put their tests at the top level directory.

test: update wpt/encoding to 7287608f90

Using git node wpt encoding

test: expect wpt/encoding/encodeInto.any.js to fail

Since we do not implement TextEncoder.prototype.encodeInto

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Since we do not implement TextDecoderStream or TextEncoderStream.
For a directory like this:

- wpt
  - encoding
    - streams
      - backpressure.any.js
    - api-basics.any.js

Previously we only run `api-basics.any.js`, now we also run
`backpressure.any.js` (and any tests in more deeply nested
directories). This enables us to run more of WPT since not
every module put their tests at the top level directory.
Using `git node wpt encoding`
Since we do not implement TextEncoder.prototype.encodeInto
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label May 24, 2019
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

The last commit could be moved up to help future bisectors

@joyeecheung
Copy link
Member Author

joyeecheung commented May 24, 2019

@targos wpt/encoding/encodeInto.any.js only exists after the WPT update so it probably does not make a lot of sense to skip a test that does not exist at that point of git history.

@targos
Copy link
Member

targos commented May 24, 2019

@joyeecheung I'd personally squash both commits

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 28, 2019
@danbev
Copy link
Contributor

danbev commented May 28, 2019

Landed in 86ed4ad, 6abeaac, 4ccc359, and aa42d37.

@danbev danbev closed this May 28, 2019
danbev pushed a commit that referenced this pull request May 28, 2019
Since we do not implement TextDecoderStream or TextEncoderStream.

PR-URL: #27860
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
danbev pushed a commit that referenced this pull request May 28, 2019
For a directory like this:

- wpt
  - encoding
    - streams
      - backpressure.any.js
    - api-basics.any.js

Previously we only run `api-basics.any.js`, now we also run
`backpressure.any.js` (and any tests in more deeply nested
directories). This enables us to run more of WPT since not
every module put their tests at the top level directory.

PR-URL: #27860
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
danbev pushed a commit that referenced this pull request May 28, 2019
Using `git node wpt encoding`

PR-URL: #27860
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
danbev pushed a commit that referenced this pull request May 28, 2019
Since we do not implement TextEncoder.prototype.encodeInto

PR-URL: #27860
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request May 28, 2019
Since we do not implement TextDecoderStream or TextEncoderStream.

PR-URL: #27860
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request May 28, 2019
For a directory like this:

- wpt
  - encoding
    - streams
      - backpressure.any.js
    - api-basics.any.js

Previously we only run `api-basics.any.js`, now we also run
`backpressure.any.js` (and any tests in more deeply nested
directories). This enables us to run more of WPT since not
every module put their tests at the top level directory.

PR-URL: #27860
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request May 28, 2019
Using `git node wpt encoding`

PR-URL: #27860
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
targos pushed a commit that referenced this pull request May 28, 2019
Since we do not implement TextEncoder.prototype.encodeInto

PR-URL: #27860
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
@targos targos mentioned this pull request Jun 3, 2019
let { read, written } = new TextEncoder().encodeInto("", view);
assert_equals(read, 0);
assert_equals(written, 0);
new MessageChannel().port1.postMessage(buffer, [buffer]);

Choose a reason for hiding this comment

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

I try to finish the feature encodeInto, after I finished I tested it with the test cases, but meet the failure as followed

[FAILURE] encodeInto() and a detached output buffer
MessageChannel is not defined
ReferenceError: MessageChannel is not defined
at Test. (node/test/fixtures/wpt/encoding/encodeInto.any.js:142:3) 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants