From f086b7f1c704e8185d2377a78382d453137163b2 Mon Sep 17 00:00:00 2001 From: Roy Wright Date: Sat, 25 Jul 2020 10:40:28 -0700 Subject: [PATCH] test: string tests together * Use promises and async/await to ensure that one test finishes before another one starts. This prevents errors thrown in one test from appearing to originate from another test. * Perform garbage collection consistently, via `testUtil.runGCTests()`, to ensure that it is performed correctly and that its results are awaited. PR-URL: https://github.com/nodejs/node-addon-api/pull/773 Reviewed-By: Chengzhong Wu Reviewed-By: Michael Dawson --- test/addon_data.js | 47 ++++---- test/arraybuffer.js | 28 ++--- test/asyncprogressqueueworker.cc | 9 -- test/asyncprogressqueueworker.js | 81 ++++++-------- test/asyncprogressworker.js | 61 +++++----- test/asyncworker-nocallback.js | 17 ++- test/asyncworker-persistent.js | 2 +- test/asyncworker.js | 105 +++++++++++------- test/buffer.js | 6 +- test/external.js | 6 +- test/index.js | 11 +- test/object/finalizer.js | 25 +++-- test/objectreference.js | 6 +- test/objectwrap-removewrap.js | 31 ++++-- test/objectwrap.js | 43 ++++--- test/objectwrap_constructor_exception.js | 19 +++- test/promise.js | 10 +- test/reference.js | 12 +- test/run_script.js | 6 +- test/testUtil.js | 67 ++++++----- .../threadsafe_function.js | 6 +- .../threadsafe_function_ctx.js | 6 +- .../threadsafe_function_existing_tsfn.js | 6 +- .../threadsafe_function_sum.cc | 34 +++++- .../threadsafe_function_sum.js | 14 +-- .../threadsafe_function_unref.js | 46 ++++---- 26 files changed, 381 insertions(+), 323 deletions(-) diff --git a/test/addon_data.js b/test/addon_data.js index 0a38526..571b23e 100644 --- a/test/addon_data.js +++ b/test/addon_data.js @@ -5,29 +5,37 @@ const { spawn } = require('child_process'); const readline = require('readline'); const path = require('path'); -test(path.resolve(__dirname, `./build/${buildType}/binding.node`)); -test(path.resolve(__dirname, `./build/${buildType}/binding_noexcept.node`)); +module.exports = + test(path.resolve(__dirname, `./build/${buildType}/binding.node`)) + .then(() => + test(path.resolve(__dirname, + `./build/${buildType}/binding_noexcept.node`))); // 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) { - 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)); + 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(); + }); + }); } -function test(bindingName) { +async function test(bindingName) { const binding = require(bindingName).addon_data(0); // Make sure it is possible to get/set instance data. @@ -37,6 +45,7 @@ function test(bindingName) { binding.verbose = false; assert.strictEqual(binding.verbose.verbose, false); - testFinalizer(bindingName, 0, ['addon_data: Addon::~Addon']); - testFinalizer(bindingName, 42, ['addon_data: Addon::~Addon', 'hint: 42']); + await testFinalizer(bindingName, 0, ['addon_data: Addon::~Addon']); + await testFinalizer(bindingName, 42, + ['addon_data: Addon::~Addon', 'hint: 42']); } diff --git a/test/arraybuffer.js b/test/arraybuffer.js index 4d0681a..38d35d1 100644 --- a/test/arraybuffer.js +++ b/test/arraybuffer.js @@ -3,11 +3,11 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const testUtil = require('./testUtil'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); function test(binding) { - testUtil.runGCTests([ + return testUtil.runGCTests([ 'Internal ArrayBuffer', () => { const test = binding.arraybuffer.createBuffer(); @@ -25,10 +25,8 @@ function test(binding) { assert.ok(test instanceof ArrayBuffer); assert.strictEqual(0, binding.arraybuffer.getFinalizeCount()); }, - () => { - global.gc(); - assert.strictEqual(0, binding.arraybuffer.getFinalizeCount()); - }, + + () => assert.strictEqual(0, binding.arraybuffer.getFinalizeCount()), 'External ArrayBuffer with finalizer', () => { @@ -37,12 +35,8 @@ function test(binding) { assert.ok(test instanceof ArrayBuffer); assert.strictEqual(0, binding.arraybuffer.getFinalizeCount()); }, - () => { - global.gc(); - }, - () => { - assert.strictEqual(1, binding.arraybuffer.getFinalizeCount()); - }, + + () => assert.strictEqual(1, binding.arraybuffer.getFinalizeCount()), 'External ArrayBuffer with finalizer hint', () => { @@ -51,12 +45,8 @@ function test(binding) { assert.ok(test instanceof ArrayBuffer); assert.strictEqual(0, binding.arraybuffer.getFinalizeCount()); }, - () => { - global.gc(); - }, - () => { - assert.strictEqual(1, binding.arraybuffer.getFinalizeCount()); - }, + + () => assert.strictEqual(1, binding.arraybuffer.getFinalizeCount()), 'ArrayBuffer with constructor', () => { diff --git a/test/asyncprogressqueueworker.cc b/test/asyncprogressqueueworker.cc index b308633..23c6707 100644 --- a/test/asyncprogressqueueworker.cc +++ b/test/asyncprogressqueueworker.cc @@ -37,14 +37,6 @@ class TestWorker : public AsyncProgressQueueWorker { worker->Queue(); } - static void CancelWork(const CallbackInfo& info) { - auto wrap = info[0].As>(); - auto worker = wrap.Data(); - // We cannot cancel a worker if it got started. So we have to do a quick cancel. - worker->Queue(); - worker->Cancel(); - } - protected: void Execute(const ExecutionProgress& progress) override { using namespace std::chrono_literals; @@ -89,7 +81,6 @@ Object InitAsyncProgressQueueWorker(Env env) { Object exports = Object::New(env); exports["createWork"] = Function::New(env, TestWorker::CreateWork); exports["queueWork"] = Function::New(env, TestWorker::QueueWork); - exports["cancelWork"] = Function::New(env, TestWorker::CancelWork); return exports; } diff --git a/test/asyncprogressqueueworker.js b/test/asyncprogressqueueworker.js index 6fa6552..4bb525e 100644 --- a/test/asyncprogressqueueworker.js +++ b/test/asyncprogressqueueworker.js @@ -4,60 +4,45 @@ const common = require('./common') const assert = require('assert'); const os = require('os'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -function test({ asyncprogressqueueworker }) { - success(asyncprogressqueueworker); - fail(asyncprogressqueueworker); - cancel(asyncprogressqueueworker); - return; +async function test({ asyncprogressqueueworker }) { + await success(asyncprogressqueueworker); + await fail(asyncprogressqueueworker); } function success(binding) { - const expected = [0, 1, 2, 3]; - const actual = []; - const worker = binding.createWork(expected.length, - common.mustCall((err) => { - if (err) { - assert.fail(err); - } - // All queued items shall be invoked before complete callback. - assert.deepEqual(actual, expected); - }), - common.mustCall((_progress) => { - actual.push(_progress); - }, expected.length) - ); - binding.queueWork(worker); + return new Promise((resolve, reject) => { + const expected = [0, 1, 2, 3]; + const actual = []; + const worker = binding.createWork(expected.length, + common.mustCall((err) => { + if (err) { + reject(err); + } else { + // All queued items shall be invoked before complete callback. + assert.deepEqual(actual, expected); + resolve(); + } + }), + common.mustCall((_progress) => { + actual.push(_progress); + }, expected.length) + ); + binding.queueWork(worker); + }); } function fail(binding) { - const worker = binding.createWork(-1, - common.mustCall((err) => { - assert.throws(() => { throw err }, /test error/) - }), - () => { - assert.fail('unexpected progress report'); - } - ); - binding.queueWork(worker); -} - -function cancel(binding) { - // make sure the work we are going to cancel will not be - // able to start by using all the threads in the pool. - for (let i = 0; i < os.cpus().length; ++i) { - const worker = binding.createWork(-1, () => {}, () => {}); + return new Promise((resolve, reject) => { + const worker = binding.createWork(-1, + common.mustCall((err) => { + assert.throws(() => { throw err }, /test error/); + resolve(); + }), + common.mustNotCall() + ); binding.queueWork(worker); - } - const worker = binding.createWork(-1, - () => { - assert.fail('unexpected callback'); - }, - () => { - assert.fail('unexpected progress report'); - } - ); - binding.cancelWork(worker); + }); } diff --git a/test/asyncprogressworker.js b/test/asyncprogressworker.js index 0aee9b7..b2896a6 100644 --- a/test/asyncprogressworker.js +++ b/test/asyncprogressworker.js @@ -3,40 +3,43 @@ const buildType = process.config.target_defaults.default_configuration; const common = require('./common') const assert = require('assert'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -function test({ asyncprogressworker }) { - success(asyncprogressworker); - fail(asyncprogressworker); - return; +async function test({ asyncprogressworker }) { + await success(asyncprogressworker); + await fail(asyncprogressworker); } function success(binding) { - const expected = [0, 1, 2, 3]; - const actual = []; - binding.doWork(expected.length, - common.mustCall((err) => { - if (err) { - assert.fail(err); - } - }), - common.mustCall((_progress) => { - actual.push(_progress); - if (actual.length === expected.length) { - assert.deepEqual(actual, expected); - } - }, expected.length) - ); + return new Promise((resolve, reject) => { + const expected = [0, 1, 2, 3]; + const actual = []; + binding.doWork(expected.length, + common.mustCall((err) => { + if (err) { + reject(err); + } + }), + common.mustCall((_progress) => { + actual.push(_progress); + if (actual.length === expected.length) { + assert.deepEqual(actual, expected); + resolve(); + } + }, expected.length) + ); + }); } function fail(binding) { - binding.doWork(-1, - common.mustCall((err) => { - assert.throws(() => { throw err }, /test error/) - }), - () => { - assert.fail('unexpected progress report'); - } - ); + return new Promise((resolve) => { + binding.doWork(-1, + common.mustCall((err) => { + assert.throws(() => { throw err }, /test error/) + resolve(); + }), + common.mustNotCall() + ); + }); } diff --git a/test/asyncworker-nocallback.js b/test/asyncworker-nocallback.js index c873c5f..fa9c172 100644 --- a/test/asyncworker-nocallback.js +++ b/test/asyncworker-nocallback.js @@ -2,14 +2,13 @@ const buildType = process.config.target_defaults.default_configuration; const common = require('./common'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -function test(binding) { - const resolving = binding.asyncworker.doWorkNoCallback(true, {}); - resolving.then(common.mustCall()).catch(common.mustNotCall()); +async function test(binding) { + await binding.asyncworker.doWorkNoCallback(true, {}) + .then(common.mustCall()).catch(common.mustNotCall()); - const rejecting = binding.asyncworker.doWorkNoCallback(false, {}); - rejecting.then(common.mustNotCall()).catch(common.mustCall()); - return; -} \ No newline at end of file + await binding.asyncworker.doWorkNoCallback(false, {}) + .then(common.mustNotCall()).catch(common.mustCall()); +} diff --git a/test/asyncworker-persistent.js b/test/asyncworker-persistent.js index 90806eb..d584086 100644 --- a/test/asyncworker-persistent.js +++ b/test/asyncworker-persistent.js @@ -21,7 +21,7 @@ function test(binding, succeed) { })); } -test(binding.persistentasyncworker, false) +module.exports = test(binding.persistentasyncworker, false) .then(() => test(binding.persistentasyncworker, true)) .then(() => test(noexceptBinding.persistentasyncworker, false)) .then(() => test(noexceptBinding.persistentasyncworker, true)); diff --git a/test/asyncworker.js b/test/asyncworker.js index 04415d5..0f008bb 100644 --- a/test/asyncworker.js +++ b/test/asyncworker.js @@ -17,8 +17,8 @@ function checkAsyncHooks() { return false; } -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); function installAsyncHooksForTest() { return new Promise((resolve, reject) => { @@ -52,41 +52,54 @@ function installAsyncHooksForTest() { }); } -function test(binding) { +async function test(binding) { if (!checkAsyncHooks()) { - binding.asyncworker.doWork(true, {}, function (e) { - assert.strictEqual(typeof e, 'undefined'); - assert.strictEqual(typeof this, 'object'); - assert.strictEqual(this.data, 'test data'); - }, 'test data'); + await new Promise((resolve) => { + binding.asyncworker.doWork(true, {}, function (e) { + assert.strictEqual(typeof e, 'undefined'); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + resolve(); + }, 'test data'); + }); - binding.asyncworker.doWork(false, {}, function (e) { - assert.ok(e instanceof Error); - assert.strictEqual(e.message, 'test error'); - assert.strictEqual(typeof this, 'object'); - assert.strictEqual(this.data, 'test data'); - }, 'test data'); + await new Promise((resolve) => { + binding.asyncworker.doWork(false, {}, function (e) { + assert.ok(e instanceof Error); + assert.strictEqual(e.message, 'test error'); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + resolve(); + }, 'test data'); + }); + + await new Promise((resolve) => { + binding.asyncworker.doWorkWithResult(true, {}, function (succeed, succeedString) { + assert(arguments.length == 2); + assert(succeed); + assert(succeedString == "ok"); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + resolve(); + }, 'test data'); + }); - binding.asyncworker.doWorkWithResult(true, {}, function (succeed, succeedString) { - assert(arguments.length == 2); - assert(succeed); - assert(succeedString == "ok"); - assert.strictEqual(typeof this, 'object'); - assert.strictEqual(this.data, 'test data'); - }, 'test data'); return; } { const hooks = installAsyncHooksForTest(); const triggerAsyncId = async_hooks.executionAsyncId(); - binding.asyncworker.doWork(true, { foo: 'foo' }, function (e) { - assert.strictEqual(typeof e, 'undefined'); - assert.strictEqual(typeof this, 'object'); - assert.strictEqual(this.data, 'test data'); - }, 'test data'); + await new Promise((resolve) => { + binding.asyncworker.doWork(true, { foo: 'foo' }, function (e) { + assert.strictEqual(typeof e, 'undefined'); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + resolve(); + }, 'test data'); + }); - hooks.then(actual => { + await hooks.then(actual => { assert.deepStrictEqual(actual, [ { eventName: 'init', type: 'TestResource', @@ -102,15 +115,19 @@ function test(binding) { { const hooks = installAsyncHooksForTest(); const triggerAsyncId = async_hooks.executionAsyncId(); - binding.asyncworker.doWorkWithResult(true, { foo: 'foo' }, function (succeed, succeedString) { - assert(arguments.length == 2); - assert(succeed); - assert(succeedString == "ok"); - assert.strictEqual(typeof this, 'object'); - assert.strictEqual(this.data, 'test data'); - }, 'test data'); + await new Promise((resolve) => { + binding.asyncworker.doWorkWithResult(true, { foo: 'foo' }, + function (succeed, succeedString) { + assert(arguments.length == 2); + assert(succeed); + assert(succeedString == "ok"); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + resolve(); + }, 'test data'); + }); - hooks.then(actual => { + await hooks.then(actual => { assert.deepStrictEqual(actual, [ { eventName: 'init', type: 'TestResource', @@ -126,15 +143,17 @@ function test(binding) { { const hooks = installAsyncHooksForTest(); const triggerAsyncId = async_hooks.executionAsyncId(); + await new Promise((resolve) => { + binding.asyncworker.doWork(false, { foo: 'foo' }, function (e) { + assert.ok(e instanceof Error); + assert.strictEqual(e.message, 'test error'); + assert.strictEqual(typeof this, 'object'); + assert.strictEqual(this.data, 'test data'); + resolve(); + }, 'test data'); + }); - binding.asyncworker.doWork(false, { foo: 'foo' }, function (e) { - assert.ok(e instanceof Error); - assert.strictEqual(e.message, 'test error'); - assert.strictEqual(typeof this, 'object'); - assert.strictEqual(this.data, 'test data'); - }, 'test data'); - - hooks.then(actual => { + await hooks.then(actual => { assert.deepStrictEqual(actual, [ { eventName: 'init', type: 'TestResource', diff --git a/test/buffer.js b/test/buffer.js index 9b94a90..ff17d30 100644 --- a/test/buffer.js +++ b/test/buffer.js @@ -4,11 +4,11 @@ const assert = require('assert'); const testUtil = require('./testUtil'); const safeBuffer = require('safe-buffer'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); function test(binding) { - testUtil.runGCTests([ + return testUtil.runGCTests([ 'Internal Buffer', () => { const test = binding.buffer.createBuffer(); diff --git a/test/external.js b/test/external.js index e206700..85dbe70 100644 --- a/test/external.js +++ b/test/external.js @@ -3,11 +3,11 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const testUtil = require('./testUtil'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); function test(binding) { - testUtil.runGCTests([ + return testUtil.runGCTests([ 'External without finalizer', () => { const test = binding.external.createExternal(); diff --git a/test/index.js b/test/index.js index 137cad2..86939af 100644 --- a/test/index.js +++ b/test/index.js @@ -91,17 +91,22 @@ if (napiVersion < 6) { } if (typeof global.gc === 'function') { + (async function() { console.log(`Testing with N-API Version '${napiVersion}'.`); console.log('Starting test suite\n'); // Requiring each module runs tests in the module. - testModules.forEach(name => { + for (const name of testModules) { console.log(`Running test '${name}'`); - require('./' + name); - }); + await require('./' + name); + }; console.log('\nAll tests passed!'); + })().catch((error) => { + console.log(error); + process.exit(1); + }); } else { // Construct the correct (version-dependent) command-line args. let args = ['--expose-gc', '--no-concurrent-array-buffer-freeing']; diff --git a/test/object/finalizer.js b/test/object/finalizer.js index 26820a0..312b2de 100644 --- a/test/object/finalizer.js +++ b/test/object/finalizer.js @@ -2,20 +2,29 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); +const testUtil = require('../testUtil'); -test(require(`../build/${buildType}/binding.node`)); -test(require(`../build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`../build/${buildType}/binding.node`)) + .then(() => test(require(`../build/${buildType}/binding_noexcept.node`))); function createWeakRef(binding, bindingToTest) { return binding.object[bindingToTest]({}); } function test(binding) { - const obj1 = createWeakRef(binding, 'addFinalizer'); - global.gc(); - assert.deepStrictEqual(obj1, { finalizerCalled: true }); + let obj1; + let obj2; + return testUtil.runGCTests([ + 'addFinalizer', + () => { + obj1 = createWeakRef(binding, 'addFinalizer'); + }, + () => assert.deepStrictEqual(obj1, { finalizerCalled: true }), - const obj2 = createWeakRef(binding, 'addFinalizerWithHint'); - global.gc(); - assert.deepStrictEqual(obj2, { finalizerCalledWithCorrectHint: true }); + 'addFinalizerWithHint', + () => { + obj2 = createWeakRef(binding, 'addFinalizerWithHint'); + }, + () => assert.deepStrictEqual(obj2, { finalizerCalledWithCorrectHint: true }) + ]); } diff --git a/test/objectreference.js b/test/objectreference.js index 07de0bc..55b95db 100644 --- a/test/objectreference.js +++ b/test/objectreference.js @@ -14,8 +14,8 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const testUtil = require('./testUtil'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); function test(binding) { function testCastedEqual(testToCompare) { @@ -29,7 +29,7 @@ function test(binding) { } } - testUtil.runGCTests([ + return testUtil.runGCTests([ 'Weak Casted Array', () => { binding.objectreference.setCastedObjects(); diff --git a/test/objectwrap-removewrap.js b/test/objectwrap-removewrap.js index 01c8b91..560f61d 100644 --- a/test/objectwrap-removewrap.js +++ b/test/objectwrap-removewrap.js @@ -8,18 +8,25 @@ if (process.argv[2] === 'child') { const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const { spawnSync } = require('child_process'); +const testUtil = require('./testUtil'); -const test = (bindingName) => { - const binding = require(bindingName); - const Test = binding.objectwrap_removewrap.Test; - const getDtorCalled = binding.objectwrap_removewrap.getDtorCalled; +function test(bindingName) { + return testUtil.runGCTests([ + 'objectwrap removewrap test', + () => { + const binding = require(bindingName); + const Test = binding.objectwrap_removewrap.Test; + const getDtorCalled = binding.objectwrap_removewrap.getDtorCalled; - assert.strictEqual(getDtorCalled(), 0); - assert.throws(() => { - new Test(); - }); - assert.strictEqual(getDtorCalled(), 1); - global.gc(); // Does not crash. + assert.strictEqual(getDtorCalled(), 0); + assert.throws(() => { + new Test(); + }); + assert.strictEqual(getDtorCalled(), 1); + }, + // Test that gc does not crash. + () => {} + ]); // Start a child process that creates a single wrapped instance to ensure that // it is properly freed at its exit. It must not segfault. @@ -31,5 +38,5 @@ const test = (bindingName) => { assert.strictEqual(child.status, 0); } -test(`./build/${buildType}/binding.node`); -test(`./build/${buildType}/binding_noexcept.node`); +module.exports = test(`./build/${buildType}/binding.node`) + .then(() => test(`./build/${buildType}/binding_noexcept.node`)); diff --git a/test/objectwrap.js b/test/objectwrap.js index 02abf60..3a41683 100644 --- a/test/objectwrap.js +++ b/test/objectwrap.js @@ -1,8 +1,9 @@ 'use strict'; const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); +const testUtil = require('./testUtil'); -const test = (binding) => { +async function test(binding) { const Test = binding.objectwrap.Test; const testValue = (obj, clazz) => { @@ -237,22 +238,20 @@ const test = (binding) => { } }; - const testFinalize = (clazz) => { - + async function testFinalize(clazz) { let finalizeCalled = false; - const finalizeCb = function(called) { - finalizeCalled = called; - }; - - //Scope Test instance so that it can be gc'd. - (function() { - new Test(finalizeCb); - })(); - - global.gc(); - - assert.strictEqual(finalizeCalled, true); - + await testUtil.runGCTests([ + 'test finalize', + () => { + const finalizeCb = function(called) { + finalizeCalled = called; + }; + + //Scope Test instance so that it can be gc'd. + (() => { new Test(finalizeCb); })(); + }, + () => assert.strictEqual(finalizeCalled, true) + ]); }; const testObj = (obj, clazz) => { @@ -265,22 +264,22 @@ const test = (binding) => { testConventions(obj, clazz); } - const testClass = (clazz) => { + async function testClass(clazz) { testStaticValue(clazz); testStaticAccessor(clazz); testStaticMethod(clazz); testStaticEnumerables(clazz); - testFinalize(clazz); + await testFinalize(clazz); }; // `Test` is needed for accessing exposed symbols testObj(new Test(), Test); - testClass(Test); + await testClass(Test); // Make sure the C++ object can be garbage collected without issues. - setImmediate(global.gc); + await testUtil.runGCTests(['one last gc', () => {}, () => {}]); } -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); diff --git a/test/objectwrap_constructor_exception.js b/test/objectwrap_constructor_exception.js index 8428c80..02dff2c 100644 --- a/test/objectwrap_constructor_exception.js +++ b/test/objectwrap_constructor_exception.js @@ -1,12 +1,19 @@ 'use strict'; const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); +const testUtil = require('./testUtil'); -const test = (binding) => { - const { ConstructorExceptionTest } = binding.objectwrapConstructorException; - assert.throws(() => (new ConstructorExceptionTest()), /an exception/); - global.gc(); +function test(binding) { + return testUtil.runGCTests([ + 'objectwrap constructor exception', + () => { + const { ConstructorExceptionTest } = binding.objectwrapConstructorException; + assert.throws(() => (new ConstructorExceptionTest()), /an exception/); + }, + // Do on gc before returning. + () => {} + ]); } -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); diff --git a/test/promise.js b/test/promise.js index 4a04ab9..65544c6 100644 --- a/test/promise.js +++ b/test/promise.js @@ -3,17 +3,17 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const common = require('./common'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -function test(binding) { +async function test(binding) { assert.strictEqual(binding.promise.isPromise({}), false); const resolving = binding.promise.resolvePromise('resolved'); - assert.strictEqual(binding.promise.isPromise(resolving), true); + await assert.strictEqual(binding.promise.isPromise(resolving), true); resolving.then(common.mustCall()).catch(common.mustNotCall()); const rejecting = binding.promise.rejectPromise('error'); - assert.strictEqual(binding.promise.isPromise(rejecting), true); + await assert.strictEqual(binding.promise.isPromise(rejecting), true); rejecting.then(common.mustNotCall()).catch(common.mustCall()); } diff --git a/test/reference.js b/test/reference.js index 3a59e85..22ee8c8 100644 --- a/test/reference.js +++ b/test/reference.js @@ -5,11 +5,13 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const testUtil = require('./testUtil'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); function test(binding) { - binding.reference.createWeakArray(); - global.gc(); - assert.strictEqual(true, binding.reference.accessWeakArrayEmpty()); + return testUtil.runGCTests([ + 'test reference', + () => binding.reference.createWeakArray(), + () => assert.strictEqual(true, binding.reference.accessWeakArrayEmpty()) + ]); }; diff --git a/test/run_script.js b/test/run_script.js index 271eeb5..ec36dcf 100644 --- a/test/run_script.js +++ b/test/run_script.js @@ -3,11 +3,11 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const testUtil = require('./testUtil'); -test(require(`./build/${buildType}/binding.node`)); -test(require(`./build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`./build/${buildType}/binding.node`)) + .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); function test(binding) { - testUtil.runGCTests([ + return testUtil.runGCTests([ 'Plain C string', () => { const sum = binding.run_script.plainString(); diff --git a/test/testUtil.js b/test/testUtil.js index dfd7fab..b8777e7 100644 --- a/test/testUtil.js +++ b/test/testUtil.js @@ -1,38 +1,51 @@ // Run each test function in sequence, // with an async delay and GC call between each. -function tick(x, cb) { - function ontick() { - if (--x === 0) { - if (typeof cb === 'function') cb(); - } else { - setImmediate(ontick); - } - } - setImmediate(ontick); +function tick(x) { + return new Promise((resolve) => { + setImmediate(function ontick() { + if (--x === 0) { + resolve(); + } else { + setImmediate(ontick); + } + }); + }); }; -function runGCTests(tests, i, title) { - if (!i) { - i = 0; +async function runGCTests(tests) { + // Break up test list into a list of lists of the form + // [ [ 'test name', function() {}, ... ], ..., ]. + const testList = []; + let currentTest; + for (const item of tests) { + if (typeof item === 'string') { + currentTest = []; + testList.push(currentTest); + } + currentTest.push(item); } - if (tests[i]) { - if (typeof tests[i] === 'string') { - title = tests[i]; - runGCTests(tests, i + 1, title); - } else { - try { - tests[i](); - } catch (e) { - console.error('Test failed: ' + title); - throw e; + for (const test of testList) { + await (async function(test) { + let title; + for (let i = 0; i < test.length; i++) { + if (i === 0) { + title = test[i]; + } else { + try { + test[i](); + } catch (e) { + console.error('Test failed: ' + title); + throw e; + } + if (i < tests.length - 1) { + global.gc(); + await tick(10); + } + } } - setImmediate(() => { - global.gc(); - tick(10, runGCTests(tests, i + 1, title)); - }); - } + })(test); } } diff --git a/test/threadsafe_function/threadsafe_function.js b/test/threadsafe_function/threadsafe_function.js index 710c212..a3690fc 100644 --- a/test/threadsafe_function/threadsafe_function.js +++ b/test/threadsafe_function/threadsafe_function.js @@ -4,8 +4,8 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const common = require('../common'); -test(require(`../build/${buildType}/binding.node`)); -test(require(`../build/${buildType}/binding_noexcept.node`)); +module.exports = test(require(`../build/${buildType}/binding.node`)) + .then(() => test(require(`../build/${buildType}/binding_noexcept.node`))); function test(binding) { const expectedArray = (function(arrayLength) { @@ -43,7 +43,7 @@ function test(binding) { }); } - new Promise(function testWithoutJSMarshaller(resolve) { + return new Promise(function testWithoutJSMarshaller(resolve) { let callCount = 0; binding.threadsafe_function.startThreadNoNative(function testCallback() { callCount++; diff --git a/test/threadsafe_function/threadsafe_function_ctx.js b/test/threadsafe_function/threadsafe_function_ctx.js index d091bbb..2651586 100644 --- a/test/threadsafe_function/threadsafe_function_ctx.js +++ b/test/threadsafe_function/threadsafe_function_ctx.js @@ -3,10 +3,8 @@ const assert = require('assert'); const buildType = process.config.target_defaults.default_configuration; -module.exports = Promise.all[ - test(require(`../build/${buildType}/binding.node`)), - test(require(`../build/${buildType}/binding_noexcept.node`)) -]; +module.exports = test(require(`../build/${buildType}/binding.node`)) + .then(() => test(require(`../build/${buildType}/binding_noexcept.node`))); async function test(binding) { const ctx = { }; diff --git a/test/threadsafe_function/threadsafe_function_existing_tsfn.js b/test/threadsafe_function/threadsafe_function_existing_tsfn.js index 8843dec..d77f57e 100644 --- a/test/threadsafe_function/threadsafe_function_existing_tsfn.js +++ b/test/threadsafe_function/threadsafe_function_existing_tsfn.js @@ -4,10 +4,8 @@ const assert = require('assert'); const buildType = process.config.target_defaults.default_configuration; -module.exports = Promise.all([ - test(require(`../build/${buildType}/binding.node`)), - test(require(`../build/${buildType}/binding_noexcept.node`)) -]); +module.exports = test(require(`../build/${buildType}/binding.node`)) + .then(() => test(require(`../build/${buildType}/binding_noexcept.node`))); async function test(binding) { const testCall = binding.threadsafe_function_existing_tsfn.testCall; diff --git a/test/threadsafe_function/threadsafe_function_sum.cc b/test/threadsafe_function/threadsafe_function_sum.cc index eac2488..5134e58 100644 --- a/test/threadsafe_function/threadsafe_function_sum.cc +++ b/test/threadsafe_function/threadsafe_function_sum.cc @@ -22,6 +22,10 @@ struct TestData { std::vector threads = {}; ThreadSafeFunction tsfn = ThreadSafeFunction(); + + // These variables are only accessed from the main thread. + bool mainWantsRelease = false; + size_t expected_calls = 0; }; void FinalizerCallback(Napi::Env env, TestData* finalizeData){ @@ -142,10 +146,27 @@ static Value TestDelayedTSFN(const CallbackInfo &info) { return testData->deferred.Promise(); } +void AcquireFinalizerCallback(Napi::Env env, + TestData* finalizeData, + TestData* context) { + (void) context; + for (size_t i = 0; i < finalizeData->threads.size(); ++i) { + finalizeData->threads[i].join(); + } + finalizeData->deferred.Resolve(Boolean::New(env, true)); + delete finalizeData; +} + void entryAcquire(ThreadSafeFunction tsfn, int threadId) { tsfn.Acquire(); + TestData* testData = tsfn.GetContext(); std::this_thread::sleep_for(std::chrono::milliseconds(std::rand() % 100 + 1)); tsfn.BlockingCall( [=](Napi::Env env, Function callback) { + // This lambda runs on the main thread so it's OK to access the variables + // `expected_calls` and `mainWantsRelease`. + testData->expected_calls--; + if (testData->expected_calls == 0 && testData->mainWantsRelease) + testData->tsfn.Release(); callback.Call( { Number::New(env, static_cast(threadId))}); }); tsfn.Release(); @@ -153,6 +174,11 @@ void entryAcquire(ThreadSafeFunction tsfn, int threadId) { static Value CreateThread(const CallbackInfo& info) { TestData* testData = static_cast(info.Data()); + // Counting expected calls like this only works because on the JS side this + // binding is called from a synchronous loop. This means the main loop has no + // chance to run the tsfn JS callback before we've counted how many threads + // the JS intends to create. + testData->expected_calls++; ThreadSafeFunction tsfn = testData->tsfn; int threadId = testData->threads.size(); // A copy of the ThreadSafeFunction will go to the thread entry point @@ -162,8 +188,7 @@ static Value CreateThread(const CallbackInfo& info) { static Value StopThreads(const CallbackInfo& info) { TestData* testData = static_cast(info.Data()); - ThreadSafeFunction tsfn = testData->tsfn; - tsfn.Release(); + testData->mainWantsRelease = true; return info.Env().Undefined(); } @@ -176,8 +201,9 @@ static Value TestAcquire(const CallbackInfo& info) { TestData *testData = new TestData(Promise::Deferred::New(info.Env())); testData->tsfn = ThreadSafeFunction::New( - env, cb, "Test", 0, 1, - std::function(FinalizerCallback), testData); + env, cb, "Test", 0, 1, testData, + std::function(AcquireFinalizerCallback), + testData); Object result = Object::New(env); result["createThread"] = Function::New( env, CreateThread, "createThread", testData); diff --git a/test/threadsafe_function/threadsafe_function_sum.js b/test/threadsafe_function/threadsafe_function_sum.js index 4323dab..738e31d 100644 --- a/test/threadsafe_function/threadsafe_function_sum.js +++ b/test/threadsafe_function/threadsafe_function_sum.js @@ -29,10 +29,8 @@ const buildType = process.config.target_defaults.default_configuration; const THREAD_COUNT = 5; const EXPECTED_SUM = (THREAD_COUNT - 1) * (THREAD_COUNT) / 2; -module.exports = Promise.all([ - test(require(`../build/${buildType}/binding.node`)), - test(require(`../build/${buildType}/binding_noexcept.node`)) -]); +module.exports = test(require(`../build/${buildType}/binding.node`)) + .then(() => test(require(`../build/${buildType}/binding_noexcept.node`))); /** @param {number[]} N */ const sum = (N) => N.reduce((sum, n) => sum + n, 0); @@ -57,9 +55,7 @@ function test(binding) { assert.equal(sum(calls), EXPECTED_SUM); } - return Promise.all([ - check(binding.threadsafe_function_sum.testDelayedTSFN), - check(binding.threadsafe_function_sum.testWithTSFN), - checkAcquire() - ]); + return check(binding.threadsafe_function_sum.testDelayedTSFN) + .then(() => check(binding.threadsafe_function_sum.testWithTSFN)) + .then(() => checkAcquire()); } diff --git a/test/threadsafe_function/threadsafe_function_unref.js b/test/threadsafe_function/threadsafe_function_unref.js index 37ce56b..e8f0ee3 100644 --- a/test/threadsafe_function/threadsafe_function_unref.js +++ b/test/threadsafe_function/threadsafe_function_unref.js @@ -15,8 +15,8 @@ const isMainProcess = process.argv[1] != __filename; */ if (isMainProcess) { - test(`../build/${buildType}/binding.node`); - test(`../build/${buildType}/binding_noexcept.node`); + module.exports = test(`../build/${buildType}/binding.node`) + .then(() => test(`../build/${buildType}/binding_noexcept.node`)); } else { test(process.argv[2]); } @@ -24,27 +24,29 @@ if (isMainProcess) { function test(bindingFile) { if (isMainProcess) { // Main process - const child = require('../napi_child').spawn(process.argv[0], [ '--expose-gc', __filename, bindingFile ], { - stdio: 'inherit', + return new Promise((resolve, reject) => { + const child = require('../napi_child').spawn(process.argv[0], [ + '--expose-gc', __filename, bindingFile + ], { stdio: 'inherit' }); + + let timeout = setTimeout( function() { + child.kill(); + timeout = 0; + reject(new Error("Expected child to die")); + }, 5000); + + child.on("error", (err) => { + clearTimeout(timeout); + timeout = 0; + reject(new Error(err)); + }) + + child.on("close", (code) => { + if (timeout) clearTimeout(timeout); + assert.strictEqual(code, 0, "Expected return value 0"); + resolve(); + }); }); - - let timeout = setTimeout( function() { - child.kill(); - timeout = 0; - throw new Error("Expected child to die"); - }, 5000); - - child.on("error", (err) => { - clearTimeout(timeout); - timeout = 0; - throw new Error(err); - }) - - child.on("close", (code) => { - if (timeout) clearTimeout(timeout); - assert(!code, "Expected return value 0"); - }); - } else { // Child process const binding = require(bindingFile);