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.

Sign up for GitHub

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

Jump to bottom

test: update to improve terminology #37011

Merged
merged 1 commit into from
Jan 27, 2021
Merged

test: update to improve terminology #37011

merged 1 commit into from
Jan 27, 2021

Conversation

mhdawson
Copy link
Member

Update common section in wpt to incorporate
improved terminology fixed upstream in
web-platform-tests/wpt#27152

Signed-off-by: Michael Dawson [email protected]

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jan 21, 2021
@mhdawson
Copy link
Member Author

The change in test/fixtures/wpt/common/sab.js is due to additional changes beyond web-platform-tests/wpt#27152 that were picked up by the update. It adds additional checks to avoid tests passing when they should not. Unfortunately that means we have more test failures and I had to mark additional test files as expecting a failure. I don't see any way to mark the tests as expecting a failure except at the file level from the current excludes.

@mhdawson
Copy link
Member Author

@joyeecheung can you confirm that in terms of excluding tests that fail there is no better way to handle excluding the tests than what I've done?

@joyeecheung
Copy link
Member

joyeecheung commented Jan 22, 2021 •
edited
Loading

@mhdawson You can use "skip" instead of "fail" if you don't need the test expectation to be changed once the failure is fixed. But using "fail" is OK as well, and it prompts people to update the expectation.

@mhdawson
Copy link
Member Author

@joyeecheung thanks

@nodejs-github-bot
Copy link
Collaborator

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

@mhdawson
Copy link
Member Author

Issue already open for test which failed in CI #36867

@nodejs-github-bot
Copy link
Collaborator

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

@mhdawson
Copy link
Member Author

Resume build to try to get to green: https://ci.nodejs.org/job/node-test-pull-request/35737/

@nodejs-github-bot
Copy link
Collaborator

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

@mhdawson
Copy link
Member Author

One more attempt: https://ci.nodejs.org/job/node-test-pull-request/35760/ as there was another unrelated failure (build time out)

@mhdawson @Trott
test: update to improve terminology
9603e4a 
Update common section in wpt to incorporate
improved terminology fixed upstream in
web-platform-tests/wpt#27152

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#37011
Reviewed-By: Milad Fa <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@Trott
Copy link
Member

Trott commented Jan 27, 2021

Landed in 9603e4a

@Trott Trott merged commit 9603e4a into nodejs:master Jan 27, 2021
targos pushed a commit that referenced this pull request Feb 2, 2021
@mhdawson @targos
test: update to improve terminology
55407b8 
Update common section in wpt to incorporate
improved terminology fixed upstream in
web-platform-tests/wpt#27152

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #37011
Reviewed-By: Milad Fa <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@targos targos mentioned this pull request Feb 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miladfarca miladfarca miladfarca approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@lpinca lpinca lpinca approved these changes

@Trott Trott Trott approved these changes

@danbev danbev danbev approved these changes

Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mhdawson @joyeecheung @nodejs-github-bot @Trott @danbev @lpinca @miladfarca @targos