Skip to content

Conversation

@cjihrig
Copy link
Contributor

This simplifies the process of running tests on different versions of Node, which might have a different set of global variables.

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

This simplifies the process of running tests on different versions of Node, which might have a different set of global variables.
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Apr 15, 2019
@nodejs-github-bot
Copy link
Collaborator

@cjihrig
Copy link
ContributorAuthor

cc: @Trott

Copy link
Member

@TrottTrott left a comment

Choose a reason for hiding this comment

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

LGTM. Nit: Probably worth adding a note to test/common/README.md about it?

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 15, 2019
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@danbev
Copy link
Contributor

Landed in 57ab3b5.

if(process.env.NODE_TEST_KNOWN_GLOBALS!=='0'){
if(process.env.NODE_TEST_KNOWN_GLOBALS){
constknownFromEnv=process.env.NODE_TEST_KNOWN_GLOBALS.split(',');
allowGlobals(...knownFromEnv);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be something like:

allowGlobals(...knownFromEnv.map((name)=>globalThis[name]));

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@cjihrig@nodejs-github-bot@danbev@jasnell@kt3k@Trott@addaleax@lpinca@BridgeAR