From adccf307a853aaffa1df3cf23404524c05bfd214 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 6 Nov 2019 00:06:17 +0100 Subject: [PATCH 1/2] test: remove AtExit() addon test Move the one bit of the addon test that was not covered by the cctest into the latter and delete the former. Refs: https://github.com/nodejs/node/pull/30227#discussion_r341879242 --- test/addons/at-exit/binding.cc | 59 --------------------------------- test/addons/at-exit/binding.gyp | 9 ----- test/addons/at-exit/test.js | 3 -- test/cctest/test_environment.cc | 18 ++++++++++ 4 files changed, 18 insertions(+), 71 deletions(-) delete mode 100644 test/addons/at-exit/binding.cc delete mode 100644 test/addons/at-exit/binding.gyp delete mode 100644 test/addons/at-exit/test.js diff --git a/test/addons/at-exit/binding.cc b/test/addons/at-exit/binding.cc deleted file mode 100644 index b091c65ed91eeb..00000000000000 --- a/test/addons/at-exit/binding.cc +++ /dev/null @@ -1,59 +0,0 @@ -#include -#include -#include -#include - -// TODO(addaleax): Maybe merge this file with the cctest for AtExit()? - -using node::AtExit; -using node::Environment; -using node::GetCurrentEnvironment; -using v8::HandleScope; -using v8::Isolate; -using v8::Local; -using v8::Object; - -#if defined(_MSC_VER) -#pragma section(".CRT$XPU", read) -#define NODE_C_DTOR(fn) \ - NODE_CTOR_PREFIX void __cdecl fn(void); \ - __declspec(dllexport, allocate(".CRT$XPU")) \ - void (__cdecl*fn ## _)(void) = fn; \ - NODE_CTOR_PREFIX void __cdecl fn(void) -#else -#define NODE_C_DTOR(fn) \ - NODE_CTOR_PREFIX void fn(void) __attribute__((destructor)); \ - NODE_CTOR_PREFIX void fn(void) -#endif - -static char cookie[] = "yum yum"; -static int at_exit_cb1_called = 0; -static int at_exit_cb2_called = 0; - -static void at_exit_cb1(void* arg) { - Isolate* isolate = static_cast(arg); - HandleScope handle_scope(isolate); - Local obj = Object::New(isolate); - assert(!obj.IsEmpty()); // Assert VM is still alive. - assert(obj->IsObject()); - at_exit_cb1_called++; -} - -static void at_exit_cb2(void* arg) { - assert(arg == static_cast(cookie)); - at_exit_cb2_called++; -} - -NODE_C_DTOR(sanity_check) { - assert(at_exit_cb1_called == 1); - assert(at_exit_cb2_called == 2); -} - -void init(Local exports) { - Environment* env = GetCurrentEnvironment(exports->CreationContext()); - AtExit(env, at_exit_cb1, exports->GetIsolate()); - AtExit(env, at_exit_cb2, cookie); - AtExit(env, at_exit_cb2, cookie); -} - -NODE_MODULE(NODE_GYP_MODULE_NAME, init) diff --git a/test/addons/at-exit/binding.gyp b/test/addons/at-exit/binding.gyp deleted file mode 100644 index 55fbe7050f18e4..00000000000000 --- a/test/addons/at-exit/binding.gyp +++ /dev/null @@ -1,9 +0,0 @@ -{ - 'targets': [ - { - 'target_name': 'binding', - 'sources': [ 'binding.cc' ], - 'includes': ['../common.gypi'], - } - ] -} diff --git a/test/addons/at-exit/test.js b/test/addons/at-exit/test.js deleted file mode 100644 index 4e2ca810f01c87..00000000000000 --- a/test/addons/at-exit/test.js +++ /dev/null @@ -1,3 +0,0 @@ -'use strict'; -const common = require('../../common'); -require(`./build/${common.buildType}/binding`); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 4045a7d2a4bba5..9cbebe53adbb0a 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -16,6 +16,7 @@ static void at_exit_callback1(void* arg); static void at_exit_callback2(void* arg); static void at_exit_callback_ordered1(void* arg); static void at_exit_callback_ordered2(void* arg); +static void at_exit_js(void* arg); static std::string cb_1_arg; // NOLINT(runtime/string) class EnvironmentTest : public EnvironmentTestFixture { @@ -91,6 +92,15 @@ TEST_F(EnvironmentTest, AtExitWithArgument) { EXPECT_EQ(arg, cb_1_arg); } +TEST_F(EnvironmentTest, AtExitRunsJS) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env env {handle_scope, argv}; + + AtExit(*env, at_exit_js, static_cast(isolate_)); + RunAtExit(*env); +} + TEST_F(EnvironmentTest, MultipleEnvironmentsPerIsolate) { const v8::HandleScope handle_scope(isolate_); const Argv argv; @@ -163,3 +173,11 @@ static void at_exit_callback_ordered2(void* arg) { EXPECT_FALSE(called_cb_ordered_1); called_cb_ordered_2 = true; } + +static void at_exit_js(void* arg) { + v8::Isolate* isolate = static_cast(arg); + v8::HandleScope handle_scope(isolate); + v8::Local obj = v8::Object::New(isolate); + assert(!obj.IsEmpty()); // Assert VM is still alive. + assert(obj->IsObject()); +} From f1015385dd6ac0358c11554c51c11bebdbb42d3e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 6 Nov 2019 19:30:14 +0100 Subject: [PATCH 2/2] fixup! test: remove AtExit() addon test --- test/cctest/test_environment.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 9cbebe53adbb0a..cc9b8e4531f6ef 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -12,6 +12,7 @@ static bool called_cb_1 = false; static bool called_cb_2 = false; static bool called_cb_ordered_1 = false; static bool called_cb_ordered_2 = false; +static bool called_at_exit_js = false; static void at_exit_callback1(void* arg); static void at_exit_callback2(void* arg); static void at_exit_callback_ordered1(void* arg); @@ -98,7 +99,9 @@ TEST_F(EnvironmentTest, AtExitRunsJS) { Env env {handle_scope, argv}; AtExit(*env, at_exit_js, static_cast(isolate_)); + EXPECT_FALSE(called_at_exit_js); RunAtExit(*env); + EXPECT_TRUE(called_at_exit_js); } TEST_F(EnvironmentTest, MultipleEnvironmentsPerIsolate) { @@ -180,4 +183,5 @@ static void at_exit_js(void* arg) { v8::Local obj = v8::Object::New(isolate); assert(!obj.IsEmpty()); // Assert VM is still alive. assert(obj->IsObject()); + called_at_exit_js = true; }