test: introduce SetUpTestCase/TearDownTestCase by danbev · Pull Request #18558 · nodejs/node

danbev · 2018-02-04T07:20:32Z

This commit add SetUpTestCase and TearDownTestCase functions that will
be called once per test case. Currently we only have SetUp/TearDown
which are called for each test.

This commit moves the initialization and configuration of Node and V8 to
be done on a per test case basis, but gives each test a new Isolate.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

test

danbev · 2018-02-04T07:22:26Z

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

danbev · 2018-02-04T07:41:51Z

test/node-test-commit-freebsd failure looks unrelated

console output:

Building addon /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/addons/load-long-path/gyp info it worked if it ends with okgyp info using node-gyp@3.6.2gyp info using node@10.0.0-pre | freebsd | x64gyp info chdir /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/addons/load-long-path/gyp info spawn /usr/local/bin/pythongyp info spawn args [ '/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/deps/npm/node_modules/node-gyp/gyp/gyp_main.py',gyp info spawn args 'binding.gyp',gyp info spawn args '-f',gyp info spawn args 'make',gyp info spawn args '-I',gyp info spawn args '/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/addons/load-long-path/build/config.gypi',gyp info spawn args '-I',gyp info spawn args '/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/deps/npm/node_modules/node-gyp/addon.gypi',gyp info spawn args '-I',gyp info spawn args '/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/common.gypi',gyp info spawn args '-Dlibrary=shared_library',gyp info spawn args '-Dvisibility=default',gyp info spawn args '-Dnode_root_dir=/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64',gyp info spawn args '-Dnode_gyp_dir=/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/deps/npm/node_modules/node-gyp',gyp info spawn args '-Dnode_lib_file=/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/$(Configuration)/node.lib',gyp info spawn args '-Dmodule_root_dir=/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/addons/load-long-path',gyp info spawn args '-Dnode_engine=v8',gyp info spawn args '--depth=.',gyp info spawn args '--no-parallel',gyp info spawn args '--generator-output',gyp info spawn args 'build',gyp info spawn args '-Goutput_dir=.' ]gyp: /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/common.gypi not found (cwd: /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/addons/load-long-path) while reading includes of binding.gyp while trying to load binding.gypgyp ERR! configure error gyp ERR! stack Error: `gyp` failed with exit code: 1gyp ERR! stack at ChildProcess.onCpExit (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/deps/npm/node_modules/node-gyp/lib/configure.js:336:16)gyp ERR! stack at ChildProcess.emit (events.js:131:13)gyp ERR! stack at Process.ChildProcess._handle.onexit (internal/child_process.js:209:12)gyp ERR! System FreeBSD 10.3-RELEASE-p7gyp ERR! command "/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/out/Release/node" "/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/deps/npm/node_modules/node-gyp/bin/node-gyp" "--loglevel=info" "rebuild" "--python=/usr/local/bin/python" "--directory=/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/addons/load-long-path/" "--nodedir=/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64"gyp ERR! cwd /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/addons/load-long-pathgyp ERR! node -v v10.0.0-pregyp ERR! node-gyp -v v3.6.2gyp ERR! not ok Makefile:329: recipe for target 'test/addons/.buildstamp' failedgmake[1]: *** [test/addons/.buildstamp] Error 1Makefile:475: recipe for target 'run-ci' failedgmake: *** [run-ci] Error 2Build step 'Execute shell' marked build as failure

bnoordhuis · 2018-02-04T11:44:42Z

-
- virtual void SetUp() {
+ static void SetUpTestCase() {
+ platform.reset(new node::NodePlatform(8, nullptr));


Maybe lower this to one to make the tests more deterministic.
As well, different cctests can (at least in theory) run concurrently; you don't want to have an explosion of threads.

Sounds good, I'll change it to 4.

bnoordhuis · 2018-02-04T11:47:55Z

- std::unique_ptr<v8::ArrayBuffer::Allocator> allocator_{
- v8::ArrayBuffer::Allocator::NewDefaultAllocator()};
+ virtual void SetUp() {
+ isolate_ = v8::Isolate::New(params);


Can you add a CHECK_NE(isolate_, nullptr)?

Sure thing, I'll add the check. Thanks

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate.

danbev · 2018-02-06T05:59:33Z

Landed in c64e130 and ca473be.

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate. PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins · 2018-02-21T00:07:02Z

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate. PR-URL: nodejs#18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate. Backport-PR-URL: #18957 PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate. PR-URL: nodejs#18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: nodejs#18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate. Backport-PR-URL: #19504 PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Backport-PR-URL: #19504 PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate. PR-URL: nodejs#18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: nodejs#18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate. PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This commit add SetUpTestCase and TearDownTestCase functions that will be called once per test case. Currently we only have SetUp/TearDown which are called for each test. This commit moves the initialization and configuration of Node and V8 to be done on a per test case basis, but gives each test a new Isolate. PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 4, 2018

bnoordhuis approved these changes Feb 4, 2018
View reviewed changes

danbev added 4 commits February 5, 2018 07:02

test: remove NodeTestFixture from Env constructor
df3800d

squash: set number of theads to 4
fdfd826

squash: add check for null isolate
ad04fb9

danbev force-pushed the test_fixture_refectoring branch from dc699ee to ad04fb9Compare February 5, 2018 06:06

danbev closed this Feb 6, 2018

danbev added a commit that referenced this pull request Feb 6, 2018
test: remove NodeTestFixture from Env constructor
ca473be
PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

danbev deleted the test_fixture_refectoring branch February 6, 2018 05:59

MylesBorins added the backport-requested-v9.x label Feb 21, 2018

danbev mentioned this pull request Feb 23, 2018
[v9.x backport] test: introduce SetUpTestCase/TearDownTestCase #18957
Closed
2 tasks

addaleax added backport-requested-v9.x and removed backport-requested-v9.x labels Feb 27, 2018

addaleax mentioned this pull request Feb 27, 2018
v9.7.0 proposal #19040
Closed

This was referenced Mar 20, 2018
src: set thread local env in CreateEnvironment #18573
Closed
n-api: take n-api out of experimental #19262
Closed

danbev added a commit to danbev/node that referenced this pull request Mar 21, 2018
test: remove NodeTestFixture from Env constructor
30090a3
PR-URL: nodejs#18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

danbev mentioned this pull request Mar 21, 2018
[v9.x backport] test: introduce SetUpTestCase/TearDownTestCase #19504
Closed
2 tasks

MylesBorins pushed a commit that referenced this pull request Mar 21, 2018
test: remove NodeTestFixture from Env constructor
4e9279d
Backport-PR-URL: #19504 PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added backported-to-v9.x and removed backport-requested-v9.x labels Mar 21, 2018

targos mentioned this pull request Mar 21, 2018
v9.9.0 proposal #19428
Merged

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
test: remove NodeTestFixture from Env constructor
32e304f
PR-URL: nodejs#18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added dont-land-on-v8.x and removed dont-land-on-v8.x labels Aug 7, 2018

MylesBorins pushed a commit that referenced this pull request Aug 7, 2018
test: remove NodeTestFixture from Env constructor
29c7eea
PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

rvagg pushed a commit that referenced this pull request Aug 16, 2018
test: remove NodeTestFixture from Env constructor
3cdae15
PR-URL: #18558 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins mentioned this pull request Aug 16, 2018
v8.12.0 proposal #21593
Merged

Navigation Menu

Uh oh!

Uh oh!

test: introduce SetUpTestCase/TearDownTestCase - #18558

test: introduce SetUpTestCase/TearDownTestCase#18558
danbev wants to merge 4 commits into
nodejs:masterfrom
danbev:test_fixture_refectoring

danbev commented Feb 4, 2018

Uh oh!

danbev commented Feb 4, 2018

Uh oh!

danbev commented Feb 4, 2018

Uh oh!

bnoordhuisFeb 4, 2018

Uh oh!

danbevFeb 4, 2018

Uh oh!

bnoordhuisFeb 4, 2018

Uh oh!

danbevFeb 4, 2018

Uh oh!

danbev commented Feb 6, 2018

Uh oh!

MylesBorins commented Feb 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

danbev commented Feb 4, 2018

Checklist

Affected core subsystem(s)

Uh oh!

danbev commented Feb 4, 2018

Uh oh!

danbev commented Feb 4, 2018

Uh oh!

bnoordhuisFeb 4, 2018

Choose a reason for hiding this comment

Uh oh!

danbevFeb 4, 2018

Choose a reason for hiding this comment

Uh oh!

bnoordhuisFeb 4, 2018

Choose a reason for hiding this comment

Uh oh!

danbevFeb 4, 2018

Choose a reason for hiding this comment

Uh oh!

danbev commented Feb 6, 2018

Uh oh!

MylesBorins commented Feb 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants