diff --git a/test/addon.cc b/test/addon.cc index 6940edb6f..1ec9343f0 100644 --- a/test/addon.cc +++ b/test/addon.cc @@ -17,6 +17,8 @@ class TestAddon : public Napi::Addon { {InstanceMethod("decrement", &TestAddon::Decrement)}))}); } + ~TestAddon() { fprintf(stderr, "TestAddon::~TestAddon\n"); } + private: Napi::Value Increment(const Napi::CallbackInfo& info) { return Napi::Number::New(info.Env(), ++value); @@ -29,10 +31,14 @@ class TestAddon : public Napi::Addon { uint32_t value = 42; }; +Napi::Value CreateAddon(const Napi::CallbackInfo& info) { + return TestAddon::Init(info.Env(), Napi::Object::New(info.Env())); +} + } // end of anonymous namespace Napi::Object InitAddon(Napi::Env env) { - return TestAddon::Init(env, Napi::Object::New(env)); + return Napi::Function::New(env, "CreateAddon"); } #endif // (NAPI_VERSION > 5) diff --git a/test/addon.js b/test/addon.js index 78abaf6f7..c326b68e3 100644 --- a/test/addon.js +++ b/test/addon.js @@ -1,11 +1,7 @@ 'use strict'; -const assert = require('assert'); - -module.exports = require('./common').runTest(test); - -function test (binding) { - assert.strictEqual(binding.addon.increment(), 43); - assert.strictEqual(binding.addon.increment(), 44); - assert.strictEqual(binding.addon.subObject.decrement(), 43); -} +module.exports = require('./common').runTestInChildProcess({ + suite: 'addon', + testName: 'workingCode', + expectedStderr: ['TestAddon::~TestAddon'] +}); diff --git a/test/addon_data.cc b/test/addon_data.cc index bb650b5b2..2a62ebdde 100644 --- a/test/addon_data.cc +++ b/test/addon_data.cc @@ -4,10 +4,10 @@ #include "test_helper.h" // An overly elaborate way to get/set a boolean stored in the instance data: -// 0. A boolean named "verbose" is stored in the instance data. The constructor -// for JS `VerboseIndicator` instances is also stored in the instance data. +// 0. The constructor for JS `VerboseIndicator` instances, which have a private +// member named "verbose", is stored in the instance data. // 1. Add a property named "verbose" onto exports served by a getter/setter. -// 2. The getter returns a object of type VerboseIndicator, which itself has a +// 2. The getter returns an object of type VerboseIndicator, which itself has a // property named "verbose", also served by a getter/setter: // * The getter returns a boolean, indicating whether "verbose" is set. // * The setter sets "verbose" on the instance data. diff --git a/test/addon_data.js b/test/addon_data.js index 6fdbdd977..900aa32c8 100644 --- a/test/addon_data.js +++ b/test/addon_data.js @@ -1,46 +1,24 @@ 'use strict'; -const assert = require('assert'); -const { spawn } = require('child_process'); -const readline = require('readline'); +const common = require('./common'); -module.exports = require('./common').runTestWithBindingPath(test); +module.exports = common.runTest(test); -// Make sure the instance data finalizer is called at process exit. If the hint -// is non-zero, it will be printed out by the child process. -function testFinalizer (bindingName, hint, expected) { - return new Promise((resolve) => { - bindingName = bindingName.split('\\').join('\\\\'); - const child = spawn(process.execPath, [ - '-e', - `require('${bindingName}').addon_data(${hint}).verbose = true;` - ]); - const actual = []; - readline - .createInterface({ input: child.stderr }) - .on('line', (line) => { - if (expected.indexOf(line) >= 0) { - actual.push(line); - } - }) - .on('close', () => { - assert.deepStrictEqual(expected, actual); - resolve(); - }); +async function test () { + await common.runTestInChildProcess({ + suite: 'addon_data', + testName: 'workingCode' }); -} - -async function test (bindingName) { - const binding = require(bindingName).addon_data(0); - // Make sure it is possible to get/set instance data. - assert.strictEqual(binding.verbose.verbose, false); - binding.verbose = true; - assert.strictEqual(binding.verbose.verbose, true); - binding.verbose = false; - assert.strictEqual(binding.verbose.verbose, false); + await common.runTestInChildProcess({ + suite: 'addon_data', + testName: 'cleanupWithoutHint', + expectedStderr: ['addon_data: Addon::~Addon'] + }); - await testFinalizer(bindingName, 0, ['addon_data: Addon::~Addon']); - await testFinalizer(bindingName, 42, - ['addon_data: Addon::~Addon', 'hint: 42']); + await common.runTestInChildProcess({ + suite: 'addon_data', + testName: 'cleanupWithHint', + expectedStderr: ['addon_data: Addon::~Addon', 'hint: 42'] + }); } diff --git a/test/child_processes/addon.js b/test/child_processes/addon.js new file mode 100644 index 000000000..33e80d798 --- /dev/null +++ b/test/child_processes/addon.js @@ -0,0 +1,11 @@ +'use strict'; +const assert = require('assert'); + +module.exports = { + workingCode: binding => { + const addon = binding.addon(); + assert.strictEqual(addon.increment(), 43); + assert.strictEqual(addon.increment(), 44); + assert.strictEqual(addon.subObject.decrement(), 43); + } +}; diff --git a/test/child_processes/addon_data.js b/test/child_processes/addon_data.js new file mode 100644 index 000000000..82d1aa317 --- /dev/null +++ b/test/child_processes/addon_data.js @@ -0,0 +1,24 @@ +'use strict'; + +const assert = require('assert'); + +// Make sure the instance data finalizer is called at process exit. If the hint +// is non-zero, it will be printed out by the child process. +const cleanupTest = (binding, hint) => { + binding.addon_data(hint).verbose = true; +}; + +module.exports = { + workingCode: binding => { + const addonData = binding.addon_data(0); + + // Make sure it is possible to get/set instance data. + assert.strictEqual(addonData.verbose.verbose, false); + addonData.verbose = true; + assert.strictEqual(addonData.verbose.verbose, true); + addonData.verbose = false; + assert.strictEqual(addonData.verbose.verbose, false); + }, + cleanupWithHint: binding => cleanupTest(binding, 42), + cleanupWithoutHint: binding => cleanupTest(binding, 0) +}; diff --git a/test/child_processes/objectwrap_function.js b/test/child_processes/objectwrap_function.js new file mode 100644 index 000000000..2ee83cb5e --- /dev/null +++ b/test/child_processes/objectwrap_function.js @@ -0,0 +1,22 @@ +'use strict'; + +const assert = require('assert'); +const testUtil = require('../testUtil'); + +module.exports = { + runTest: function (binding) { + return testUtil.runGCTests([ + 'objectwrap function', + () => { + const { FunctionTest } = binding.objectwrap_function(); + const newConstructed = new FunctionTest(); + const functionConstructed = FunctionTest(); + assert(newConstructed instanceof FunctionTest); + assert(functionConstructed instanceof FunctionTest); + assert.throws(() => (FunctionTest(true)), /an exception/); + }, + // Do one gc before returning. + () => {} + ]); + } +}; diff --git a/test/common/index.js b/test/common/index.js index e6a7c2b62..a82d49f50 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -3,6 +3,11 @@ const assert = require('assert'); const path = require('path'); const { access } = require('node:fs/promises'); +const { spawn } = require('child_process'); +const { EOL } = require('os'); +const readline = require('readline'); + +const escapeBackslashes = (pathString) => pathString.split('\\').join('\\\\'); const noop = () => {}; @@ -26,7 +31,7 @@ function runCallChecks (exitCode) { context.name, context.messageSegment, context.actual); - console.log(context.stack.split('\n').slice(2).join('\n')); + console.log(context.stack.split(EOL).slice(2).join(EOL)); }); if (failed.length) process.exit(1); @@ -190,3 +195,51 @@ exports.runTestWithBuildType = async function (test, buildType) { await Promise.resolve(test(buildType)) .finally(exports.mustCall()); }; + +// Some tests have to run in their own process, otherwise they would interfere +// with each other. Such tests export a factory function rather than the test +// itself so as to avoid automatic instantiation, and therefore interference, +// in the main process. Two examples are addon and addon_data, both of which +// use Napi::Env::SetInstanceData(). This helper function provides a common +// approach for running such tests. +exports.runTestInChildProcess = function ({ suite, testName, expectedStderr }) { + return exports.runTestWithBindingPath((bindingName) => { + return new Promise((resolve) => { + bindingName = escapeBackslashes(bindingName); + // Test suites are assumed to be located here. + const suitePath = escapeBackslashes(path.join(__dirname, '..', 'child_processes', suite)); + const child = spawn(process.execPath, [ + '--expose-gc', + '-e', + `require('${suitePath}').${testName}(require('${bindingName}'))` + ]); + const resultOfProcess = { stderr: [] }; + + // Capture the exit code and signal. + child.on('close', (code, signal) => resolve(Object.assign(resultOfProcess, { code, signal }))); + + // Capture the stderr as an array of lines. + readline + .createInterface({ input: child.stderr }) + .on('line', (line) => { + resultOfProcess.stderr.push(line); + }); + }).then(actual => { + // Back up the stderr in case the assertion fails. + const fullStderr = actual.stderr.map(item => `from child process: ${item}`); + const expected = { stderr: expectedStderr, code: 0, signal: null }; + + if (!expectedStderr) { + // If we don't care about stderr, delete it. + delete actual.stderr; + delete expected.stderr; + } else { + // Otherwise we only care about expected lines in the actual stderr, so + // filter out everything else. + actual.stderr = actual.stderr.filter(line => expectedStderr.includes(line)); + } + + assert.deepStrictEqual(actual, expected, `Assertion for child process test ${suite}.${testName} failed:${EOL}` + fullStderr.join(EOL)); + }); + }); +}; diff --git a/test/index.js b/test/index.js index eb6530fb6..f0c034584 100644 --- a/test/index.js +++ b/test/index.js @@ -60,6 +60,7 @@ function loadTestModules (currentDirectory = __dirname, pre = '') { file === 'binding.gyp' || file === 'build' || file === 'common' || + file === 'child_processes' || file === 'napi_child.js' || file === 'testUtil.js' || file === 'thunking_manual.cc' || diff --git a/test/objectwrap_function.cc b/test/objectwrap_function.cc index 4c2e1bbe2..0ce074c68 100644 --- a/test/objectwrap_function.cc +++ b/test/objectwrap_function.cc @@ -18,28 +18,26 @@ class FunctionTest : public Napi::ObjectWrap { return MaybeUnwrap(GetConstructor(info.Env()).New(args)); } - // Constructor-per-env map in a static member because env.SetInstanceData() - // would interfere with Napi::Addon - static std::unordered_map constructors; - static void Initialize(Napi::Env env, Napi::Object exports) { const char* name = "FunctionTest"; Napi::Function func = DefineClass(env, name, {}); - constructors[env] = Napi::Persistent(func); - env.AddCleanupHook([env] { constructors.erase(env); }); + Napi::FunctionReference* ctor = new Napi::FunctionReference(); + *ctor = Napi::Persistent(func); + env.SetInstanceData(ctor); exports.Set(name, func); } static Napi::Function GetConstructor(Napi::Env env) { - return constructors[env].Value(); + return env.GetInstanceData()->Value(); } }; -std::unordered_map - FunctionTest::constructors; +Napi::Value ObjectWrapFunctionFactory(const Napi::CallbackInfo& info) { + Napi::Object exports = Napi::Object::New(info.Env()); + FunctionTest::Initialize(info.Env(), exports); + return exports; +} Napi::Object InitObjectWrapFunction(Napi::Env env) { - Napi::Object exports = Napi::Object::New(env); - FunctionTest::Initialize(env, exports); - return exports; + return Napi::Function::New(env, "FunctionFactory"); } diff --git a/test/objectwrap_function.js b/test/objectwrap_function.js index 7bcf6c087..671833911 100644 --- a/test/objectwrap_function.js +++ b/test/objectwrap_function.js @@ -1,22 +1,6 @@ 'use strict'; -const assert = require('assert'); -const testUtil = require('./testUtil'); - -function test (binding) { - return testUtil.runGCTests([ - 'objectwrap function', - () => { - const { FunctionTest } = binding.objectwrap_function; - const newConstructed = new FunctionTest(); - const functionConstructed = FunctionTest(); - assert(newConstructed instanceof FunctionTest); - assert(functionConstructed instanceof FunctionTest); - assert.throws(() => (FunctionTest(true)), /an exception/); - }, - // Do on gc before returning. - () => {} - ]); -} - -module.exports = require('./common').runTest(test); +module.exports = require('./common').runTestInChildProcess({ + suite: 'objectwrap_function', + testName: 'runTest' +});