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: remove useless require('../common') from WPTs #46796

Merged
merged 1 commit into from
Feb 25, 2023

Conversation

panva
Copy link
Member

@panva panva commented Feb 23, 2023

Removes the need to put a useless require('../common'); at the top of every WPT startup file.

@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 Feb 23, 2023
@panva panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 23, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 25, 2023
@nodejs-github-bot nodejs-github-bot merged commit 11e37e0 into nodejs:main Feb 25, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 11e37e0

@panva panva deleted the remove-useless-requires branch February 25, 2023 16:05
targos pushed a commit that referenced this pull request Mar 13, 2023
PR-URL: #46796
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@danielleadams danielleadams added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Apr 11, 2023
@danielleadams
Copy link
Contributor

@panva this broke tests in v18.x-staging. do you mind opening a backport PR?

panva added a commit to panva/node that referenced this pull request Apr 11, 2023
PR-URL: nodejs#46796
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Backport-PR-URL: https://github.com/nodejs/node/pull/?????
panva added a commit to panva/node that referenced this pull request Apr 11, 2023
PR-URL: nodejs#46796
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Backport-PR-URL: nodejs#47506
@panva
Copy link
Member Author

panva commented Apr 11, 2023

@danielleadams

this broke tests in v18.x-staging. do you mind opening a backport PR?

Didn't break anything, just didn't land clean. #47506

@panva panva removed the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Apr 11, 2023
@panva panva added the backport-open-v18.x Indicate that the PR has an open backport. label Apr 11, 2023
@Trott
Copy link
Member

Trott commented Apr 11, 2023

FWIW, the reason for the inclusion was because that adds a check on exit for leaking things into the global space.

@panva
Copy link
Member Author

panva commented Apr 11, 2023

FWIW, the reason for the inclusion was because that adds a check on exit for leaking things into the global space.

Which some of the WPTs explicitly do to be able to run 🤷‍♂️. But they do so in their own spawned workers the WPTs execute in.

panva added a commit to panva/node that referenced this pull request Apr 11, 2023
PR-URL: nodejs#46796
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Backport-PR-URL: nodejs#47506
panva added a commit to panva/node that referenced this pull request Apr 12, 2023
PR-URL: nodejs#46796
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Backport-PR-URL: nodejs#47506
danielleadams pushed a commit that referenced this pull request May 17, 2023
PR-URL: #46796
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Backport-PR-URL: #47506
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. backport-open-v18.x Indicate that the PR has an open backport. 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.

9 participants