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: change WPT status files into CommonJS files #45826

Closed
wants to merge 2 commits into from

Conversation

panva
Copy link
Member

@panva panva commented Dec 12, 2022

This PR updates the WPT status files to be .js files.

The motivation for this change is to be able to use the javascript syntax to minimize the likelyhood of conflicts when adding/removing expectations and to allow for comments to be added for test blocks that may be failing for a specificic reason.

This would also allow to make more failures optional depending on config rather than current workaround being flaky, e.g. the console expectations file could be:

'use strict';

const hasCrypto = Boolean(process.versions.openssl);
const hasIntl = !!process.config.variables.v8_enable_i18n_support;

const idlharnessFailures = [
];

if (!hasCrypto || !hasIntl) {
  idlharnessFailures.push(
    // https://github.com/nodejs/node/issues/44185#issuecomment-1219388742
    'console namespace: operation assert(optional boolean, any...)',
    'console namespace: operation table(optional any, optional sequence<DOMString>)',
    'console namespace: operation dir(optional any, optional object?)');
}

module.exports = {
  'idlharness.any.js': {
    fail: {
      expected: idlharnessFailures,
    },
  },
};

The linter is configured to support that (newline for every array element, newline for array brackets). max-len is disabled because we cannot control the length of WPT test names and want them to fit on a single line.

@panva panva added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Dec 12, 2022
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Dec 12, 2022
@panva
Copy link
Member Author

panva commented Dec 12, 2022

I've also considered yaml or json5 but felt CJS is more at home here and the present js linter can maintain the status files better.

@panva panva added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 12, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 12, 2022
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@panva
Copy link
Member Author

panva commented Dec 13, 2022

I will deal with merge conflicts when landing this.

@panva
Copy link
Member Author

panva commented Dec 20, 2022

cc @nodejs/testing

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 20, 2022
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

test/common/wpt.js Outdated Show resolved Hide resolved
@panva panva added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 23, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 23, 2022
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

RSLGTM with a nit.

test/common/wpt.js Outdated Show resolved Hide resolved
test/wpt/README.md Show resolved Hide resolved
@panva panva removed the review wanted PRs that need reviews. label Jan 9, 2023
@panva panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. review wanted PRs that need reviews. request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. review wanted PRs that need reviews. labels Feb 13, 2023
@panva
Copy link
Member Author

panva commented Feb 16, 2023

actually, not going to bother atm with ideas around the WPTRunner still in flight

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants