FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

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

/ node Public

test: introduce SetUpTestCase/TearDownTestCase - #18558

Closed
danbev wants to merge 4 commits into
nodejs:masterfrom
danbev:test_fixture_refectoring
ConversationCommits (4)ChecksFiles changed
Closed

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

Conversation

danbev commented Feb 4, 2018

Copy link
Copy Markdown
Contributor

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

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

danbev commented Feb 4, 2018

Copy link
Copy Markdown
Contributor Author

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

danbev commented Feb 4, 2018

Copy link
Copy Markdown
Contributor Author
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 approved these changes Feb 4, 2018
View reviewed changes
Comment thread test/cctest/node_test_fixture.h Outdated

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

bnoordhuis Feb 4, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

danbev Feb 4, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sounds good, I'll change it to 4.

Comment thread 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);

bnoordhuis Feb 4, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you add a CHECK_NE(isolate_, nullptr)?

danbev Feb 4, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sure thing, I'll add the check. Thanks

danbev added 4 commits February 5, 2018 07:02
test: introduce SetUpTestCase/TearDownTestCase
5f364ee 
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.
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 commented Feb 6, 2018

Copy link
Copy Markdown
Contributor Author

Landed in c64e130 and ca473be.

danbev closed this Feb 6, 2018
danbev added a commit that referenced this pull request Feb 6, 2018
test: introduce SetUpTestCase/TearDownTestCase
c64e130 
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>
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 commented Feb 21, 2018

Copy link
Copy Markdown
Contributor

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.

danbev added a commit to danbev/node that referenced this pull request Feb 23, 2018
test: introduce SetUpTestCase/TearDownTestCase
9632966 
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>
danbev mentioned this pull request Feb 23, 2018
Closed
2 tasks
addaleax pushed a commit that referenced this pull request Feb 26, 2018
test: introduce SetUpTestCase/TearDownTestCase
3e1d810 
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>
addaleax pushed a commit that referenced this pull request Feb 26, 2018
test: introduce SetUpTestCase/TearDownTestCase
ef14ef0 
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>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
test: introduce SetUpTestCase/TearDownTestCase
743cf33 
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>
addaleax mentioned this pull request Feb 27, 2018
Closed
This was referenced Mar 20, 2018
Closed
Closed
danbev added a commit to danbev/node that referenced this pull request Mar 21, 2018
test: introduce SetUpTestCase/TearDownTestCase
1474d9f 
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>
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
Closed
2 tasks
MylesBorins pushed a commit that referenced this pull request Mar 21, 2018
test: introduce SetUpTestCase/TearDownTestCase
22b8f9f 
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>
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>
targos mentioned this pull request Mar 21, 2018
Merged
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
test: introduce SetUpTestCase/TearDownTestCase
316ec62 
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>
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 pushed a commit that referenced this pull request Aug 7, 2018
test: introduce SetUpTestCase/TearDownTestCase
907a4b9 
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>
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: introduce SetUpTestCase/TearDownTestCase
25ce5c3 
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>
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
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnoordhuis bnoordhuis approved these changes

Assignees

No one assigned

Labels

test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL