From 5f364eef53dcadddcccdaa521844b202a2140082 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Sat, 3 Feb 2018 18:02:57 +0100 Subject: [PATCH 1/4] test: introduce SetUpTestCase/TearDownTestCase 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/cctest/node_test_fixture.cc | 6 +++- test/cctest/node_test_fixture.h | 54 +++++++++++++++----------------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/test/cctest/node_test_fixture.cc b/test/cctest/node_test_fixture.cc index 477adb5711af38..3e5a112240f6d3 100644 --- a/test/cctest/node_test_fixture.cc +++ b/test/cctest/node_test_fixture.cc @@ -1,3 +1,7 @@ #include "node_test_fixture.h" -uv_loop_t current_loop; +uv_loop_t NodeTestFixture::current_loop; +std::unique_ptr NodeTestFixture::platform; +std::unique_ptr NodeTestFixture::allocator; +std::unique_ptr NodeTestFixture::tracing_controller; +v8::Isolate::CreateParams NodeTestFixture::params; diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h index 48642d59493514..bdc088c9fb7bea 100644 --- a/test/cctest/node_test_fixture.h +++ b/test/cctest/node_test_fixture.h @@ -53,49 +53,45 @@ struct Argv { int nr_args_; }; -extern uv_loop_t current_loop; class NodeTestFixture : public ::testing::Test { - public: - static uv_loop_t* CurrentLoop() { return ¤t_loop; } - - node::MultiIsolatePlatform* Platform() const { return platform_; } - protected: + static std::unique_ptr allocator; + static std::unique_ptr tracing_controller; + static std::unique_ptr platform; + static v8::Isolate::CreateParams params; + static uv_loop_t current_loop; v8::Isolate* isolate_; - virtual void SetUp() { + static void SetUpTestCase() { + platform.reset(new node::NodePlatform(8, nullptr)); + tracing_controller.reset(new v8::TracingController()); + allocator.reset(v8::ArrayBuffer::Allocator::NewDefaultAllocator()); + params.array_buffer_allocator = allocator.get(); + node::tracing::TraceEventHelper::SetTracingController( + tracing_controller.get()); CHECK_EQ(0, uv_loop_init(¤t_loop)); - platform_ = new node::NodePlatform(8, nullptr); - v8::V8::InitializePlatform(platform_); + v8::V8::InitializePlatform(platform.get()); v8::V8::Initialize(); - v8::Isolate::CreateParams params_; - params_.array_buffer_allocator = allocator_.get(); - isolate_ = v8::Isolate::New(params_); - - // As the TracingController is stored globally, we only need to create it - // one time for all tests. - if (node::tracing::TraceEventHelper::GetTracingController() == nullptr) { - node::tracing::TraceEventHelper::SetTracingController( - new v8::TracingController()); - } } - virtual void TearDown() { - platform_->Shutdown(); + static void TearDownTestCase() { + platform->Shutdown(); while (uv_loop_alive(¤t_loop)) { uv_run(¤t_loop, UV_RUN_ONCE); } v8::V8::ShutdownPlatform(); - delete platform_; - platform_ = nullptr; CHECK_EQ(0, uv_loop_close(¤t_loop)); } - private: - node::NodePlatform* platform_ = nullptr; - std::unique_ptr allocator_{ - v8::ArrayBuffer::Allocator::NewDefaultAllocator()}; + virtual void SetUp() { + isolate_ = v8::Isolate::New(params); + } + + virtual void TearDown() { + isolate_->Dispose(); + isolate_ = nullptr; + } }; @@ -112,8 +108,8 @@ class EnvironmentTestFixture : public NodeTestFixture { context_->Enter(); isolate_data_ = node::CreateIsolateData(isolate, - NodeTestFixture::CurrentLoop(), - test_fixture->Platform()); + &NodeTestFixture::current_loop, + platform.get()); CHECK_NE(nullptr, isolate_data_); environment_ = node::CreateEnvironment(isolate_data_, context_, From df3800de609489140bb0053337c4c2905eac85bc Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Sat, 3 Feb 2018 18:25:59 +0100 Subject: [PATCH 2/4] test: remove NodeTestFixture from Env constructor --- test/cctest/node_test_fixture.h | 4 +--- test/cctest/test_environment.cc | 8 ++++---- test/cctest/test_node_postmortem_metadata.cc | 10 +++++----- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h index bdc088c9fb7bea..d50d891e7eb21e 100644 --- a/test/cctest/node_test_fixture.h +++ b/test/cctest/node_test_fixture.h @@ -99,9 +99,7 @@ class EnvironmentTestFixture : public NodeTestFixture { public: class Env { public: - Env(const v8::HandleScope& handle_scope, - const Argv& argv, - NodeTestFixture* test_fixture) { + Env(const v8::HandleScope& handle_scope, const Argv& argv) { auto isolate = handle_scope.GetIsolate(); context_ = node::NewContext(isolate); CHECK(!context_.IsEmpty()); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 352fed1fb62ed9..4575d3b65ae318 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -32,7 +32,7 @@ class EnvironmentTest : public EnvironmentTestFixture { TEST_F(EnvironmentTest, AtExitWithEnvironment) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env env {handle_scope, argv, this}; + Env env {handle_scope, argv}; AtExit(*env, at_exit_callback1); RunAtExit(*env); @@ -42,7 +42,7 @@ TEST_F(EnvironmentTest, AtExitWithEnvironment) { TEST_F(EnvironmentTest, AtExitWithArgument) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env env {handle_scope, argv, this}; + Env env {handle_scope, argv}; std::string arg{"some args"}; AtExit(*env, at_exit_callback1, static_cast(&arg)); @@ -53,8 +53,8 @@ TEST_F(EnvironmentTest, AtExitWithArgument) { TEST_F(EnvironmentTest, MultipleEnvironmentsPerIsolate) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env env1 {handle_scope, argv, this}; - Env env2 {handle_scope, argv, this}; + Env env1 {handle_scope, argv}; + Env env2 {handle_scope, argv}; AtExit(*env1, at_exit_callback1); AtExit(*env2, at_exit_callback2); diff --git a/test/cctest/test_node_postmortem_metadata.cc b/test/cctest/test_node_postmortem_metadata.cc index 72e6abb585f75c..be5cc7ce8ad2c5 100644 --- a/test/cctest/test_node_postmortem_metadata.cc +++ b/test/cctest/test_node_postmortem_metadata.cc @@ -50,7 +50,7 @@ TEST_F(DebugSymbolsTest, ExternalStringDataOffset) { TEST_F(DebugSymbolsTest, BaseObjectPersistentHandle) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env env{handle_scope, argv, this}; + Env env{handle_scope, argv}; v8::Local object = v8::Object::New(isolate_); node::BaseObject obj(*env, object); @@ -67,7 +67,7 @@ TEST_F(DebugSymbolsTest, BaseObjectPersistentHandle) { TEST_F(DebugSymbolsTest, EnvironmentHandleWrapQueue) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env env{handle_scope, argv, this}; + Env env{handle_scope, argv}; auto expected = reinterpret_cast((*env)->handle_wrap_queue()); auto calculated = reinterpret_cast(*env) + @@ -78,7 +78,7 @@ TEST_F(DebugSymbolsTest, EnvironmentHandleWrapQueue) { TEST_F(DebugSymbolsTest, EnvironmentReqWrapQueue) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env env{handle_scope, argv, this}; + Env env{handle_scope, argv}; auto expected = reinterpret_cast((*env)->req_wrap_queue()); auto calculated = reinterpret_cast(*env) + @@ -89,7 +89,7 @@ TEST_F(DebugSymbolsTest, EnvironmentReqWrapQueue) { TEST_F(DebugSymbolsTest, HandleWrapList) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env env{handle_scope, argv, this}; + Env env{handle_scope, argv}; uv_tcp_t handle; @@ -118,7 +118,7 @@ TEST_F(DebugSymbolsTest, HandleWrapList) { TEST_F(DebugSymbolsTest, ReqWrapList) { const v8::HandleScope handle_scope(isolate_); const Argv argv; - Env env{handle_scope, argv, this}; + Env env{handle_scope, argv}; auto obj_template = v8::FunctionTemplate::New(isolate_); obj_template->InstanceTemplate()->SetInternalFieldCount(1); From fdfd8262fe7eb4b5c56ff75f2d10cee36dcbb6d8 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Sun, 4 Feb 2018 15:12:37 +0100 Subject: [PATCH 3/4] squash: set number of theads to 4 --- test/cctest/node_test_fixture.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h index d50d891e7eb21e..5fe1fa0b6fbf84 100644 --- a/test/cctest/node_test_fixture.h +++ b/test/cctest/node_test_fixture.h @@ -64,7 +64,7 @@ class NodeTestFixture : public ::testing::Test { v8::Isolate* isolate_; static void SetUpTestCase() { - platform.reset(new node::NodePlatform(8, nullptr)); + platform.reset(new node::NodePlatform(4, nullptr)); tracing_controller.reset(new v8::TracingController()); allocator.reset(v8::ArrayBuffer::Allocator::NewDefaultAllocator()); params.array_buffer_allocator = allocator.get(); From ad04fb96890c1972d802c4b0f291871bdb806c0c Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Sun, 4 Feb 2018 15:14:39 +0100 Subject: [PATCH 4/4] squash: add check for null isolate --- test/cctest/node_test_fixture.h | 1 + 1 file changed, 1 insertion(+) diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h index 5fe1fa0b6fbf84..660111c5a90c6f 100644 --- a/test/cctest/node_test_fixture.h +++ b/test/cctest/node_test_fixture.h @@ -86,6 +86,7 @@ class NodeTestFixture : public ::testing::Test { virtual void SetUp() { isolate_ = v8::Isolate::New(params); + CHECK_NE(isolate_, nullptr); } virtual void TearDown() {