test: introduce SetUpTestCase/TearDownTestCase #18558

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 ok
gyp info using node-gyp@3.6.2
gyp info using node@10.0.0-pre | freebsd | x64
gyp 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/python
gyp 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.gyp
gyp ERR! configure error 
gyp ERR! stack Error: `gyp` failed with exit code: 1
gyp 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-p7
gyp 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-path
gyp ERR! node -v v10.0.0-pre
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok 
Makefile:329: recipe for target 'test/addons/.buildstamp' failed
gmake[1]: *** [test/addons/.buildstamp] Error 1
Makefile:475: recipe for target 'run-ci' failed
gmake: *** [run-ci] Error 2
Build step 'Execute shell' marked build as failure

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

test/cctest/node_test_fixture.h

-
-  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

test/cctest/node_test_fixture.h

-  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 ad04fb9 Compare 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: introduce SetUpTestCase/TearDownTestCase #18558

test: introduce SetUpTestCase/TearDownTestCase #18558

danbev commented Feb 4, 2018

danbev commented Feb 4, 2018

danbev commented Feb 4, 2018

bnoordhuis Feb 4, 2018

danbev Feb 4, 2018

bnoordhuis Feb 4, 2018

danbev Feb 4, 2018

danbev commented Feb 6, 2018

MylesBorins commented Feb 21, 2018

test: introduce SetUpTestCase/TearDownTestCase #18558

test: introduce SetUpTestCase/TearDownTestCase #18558

Conversation

danbev commented Feb 4, 2018

Checklist

Affected core subsystem(s)

danbev commented Feb 4, 2018

danbev commented Feb 4, 2018

bnoordhuis Feb 4, 2018

Choose a reason for hiding this comment

danbev Feb 4, 2018

Choose a reason for hiding this comment

bnoordhuis Feb 4, 2018

Choose a reason for hiding this comment

danbev Feb 4, 2018

Choose a reason for hiding this comment

danbev commented Feb 6, 2018

MylesBorins commented Feb 21, 2018