From 5d36882bced8f1ec83c8524fcbbed87b2974259a Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sat, 25 Jul 2020 10:40:28 -0700 Subject: [PATCH 1/5] 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. --- test/addon_data.js | 47 ++++---- test/arraybuffer.js | 30 ++--- test/asyncprogressqueueworker.js | 97 ++++++++-------- test/asyncprogressworker.js | 61 +++++----- test/asyncworker-nocallback.js | 17 ++- test/asyncworker-persistent.js | 2 +- test/asyncworker.js | 105 +++++++++++------- test/buffer.js | 8 +- test/external.js | 8 +- test/index.js | 10 +- test/object/finalizer.js | 32 ++++-- test/objectreference.js | 8 +- test/objectwrap-removewrap.js | 31 ++++-- test/objectwrap.js | 43 ++++--- test/objectwrap_constructor_exception.js | 19 +++- test/promise.js | 10 +- test/reference.js | 14 ++- test/run_script.js | 8 +- test/testUtil.js | 67 ++++++----- .../threadsafe_function.js | 6 +- .../threadsafe_function_ctx.js | 6 +- .../threadsafe_function_existing_tsfn.js | 6 +- .../threadsafe_function_sum.js | 14 +-- .../threadsafe_function_unref.js | 46 ++++---- 24 files changed, 380 insertions(+), 315 deletions(-) diff --git a/test/addon_data.js b/test/addon_data.js index 0a3852696..571b23e72 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 4d0681ac3..681c7a959 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([ +async function test(binding) { + await 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.js b/test/asyncprogressqueueworker.js index 6fa65520e..4ad8c43d5 100644 --- a/test/asyncprogressqueueworker.js +++ b/test/asyncprogressqueueworker.js @@ -4,60 +4,67 @@ 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); + await cancel(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); + 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); + }); } 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, () => {}, () => {}); - binding.queueWork(worker); - } - const worker = binding.createWork(-1, - () => { - assert.fail('unexpected callback'); - }, - () => { - assert.fail('unexpected progress report'); + return new Promise((resolve, reject) => { + // 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, () => {}, () => {}); + binding.queueWork(worker); } - ); - binding.cancelWork(worker); + const worker = binding.createWork(-1, + () => { + assert.fail('unexpected callback'); + }, + () => { + assert.fail('unexpected progress report'); + } + ); + binding.cancelWork(worker); + resolve(); + }); } diff --git a/test/asyncprogressworker.js b/test/asyncprogressworker.js index 0aee9b7f8..35633733f 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, reject) => { + 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 c873c5fbf..fa9c172c0 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 90806ebf9..d584086e7 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 04415d522..0f008bb33 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 9b94a9069..127a3cf2c 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([ +async function test(binding) { + await testUtil.runGCTests([ 'Internal Buffer', () => { const test = binding.buffer.createBuffer(); diff --git a/test/external.js b/test/external.js index e2067006a..3cda548f6 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([ +async function test(binding) { + await testUtil.runGCTests([ 'External without finalizer', () => { const test = binding.external.createExternal(); diff --git a/test/index.js b/test/index.js index 137cad238..33d2e2dba 100644 --- a/test/index.js +++ b/test/index.js @@ -91,17 +91,21 @@ 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); + }); } 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 26820a05a..40535e1f7 100644 --- a/test/object/finalizer.js +++ b/test/object/finalizer.js @@ -2,20 +2,32 @@ 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 }); - - const obj2 = createWeakRef(binding, 'addFinalizerWithHint'); - global.gc(); - assert.deepStrictEqual(obj2, { finalizerCalledWithCorrectHint: true }); +async function test(binding) { + let obj1; + let obj2; + await testUtil.runGCTests([ + 'addFinalizer', + () => { + obj1 = createWeakRef(binding, 'addFinalizer'); + }, + () => { + assert.deepStrictEqual(obj1, { finalizerCalled: true }); + }, + 'addFinalizerWithHint', + () => { + obj2 = createWeakRef(binding, 'addFinalizerWithHint'); + }, + () => { + assert.deepStrictEqual(obj2, { finalizerCalledWithCorrectHint: true }); + } + ]); } diff --git a/test/objectreference.js b/test/objectreference.js index 07de0bc77..c0e811655 100644 --- a/test/objectreference.js +++ b/test/objectreference.js @@ -14,10 +14,10 @@ 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) { +async function test(binding) { function testCastedEqual(testToCompare) { var compare_test = ["hello", "world", "!"]; if (testToCompare instanceof Array) { @@ -29,7 +29,7 @@ function test(binding) { } } - testUtil.runGCTests([ + await testUtil.runGCTests([ 'Weak Casted Array', () => { binding.objectreference.setCastedObjects(); diff --git a/test/objectwrap-removewrap.js b/test/objectwrap-removewrap.js index 01c8b9169..2e1604830 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; +async function test(bindingName) { + await 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 02abf60b9..3a4168359 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 8428c8034..754a138af 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(); +async function test(binding) { + await 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 4a04ab9a0..65544c648 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 3a59e850f..312e81aaa 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()); +async function test(binding) { + await 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 271eeb50c..4789ac2e0 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([ +async function test(binding) { + await testUtil.runGCTests([ 'Plain C string', () => { const sum = binding.run_script.plainString(); diff --git a/test/testUtil.js b/test/testUtil.js index dfd7fab26..b8777e781 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 710c21212..a3690fcf3 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 d091bbbdd..2651586a0 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 8843decd1..d77f57ef1 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.js b/test/threadsafe_function/threadsafe_function_sum.js index 4323dabeb..738e31db2 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 37ce56b4d..e8f0ee391 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); From 506b9959b28cee296e21b9ff30636c05387dcde0 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 26 Jul 2020 09:08:04 -0700 Subject: [PATCH 2/5] remove async/await pairs where a promise could be returned This reduces the number of promises created internally --- test/arraybuffer.js | 4 ++-- test/buffer.js | 4 ++-- test/external.js | 4 ++-- test/index.js | 1 + test/object/finalizer.js | 13 +++++-------- test/objectreference.js | 4 ++-- test/objectwrap-removewrap.js | 4 ++-- test/objectwrap_constructor_exception.js | 4 ++-- test/reference.js | 4 ++-- test/run_script.js | 4 ++-- 10 files changed, 22 insertions(+), 24 deletions(-) diff --git a/test/arraybuffer.js b/test/arraybuffer.js index 681c7a959..38d35d1e7 100644 --- a/test/arraybuffer.js +++ b/test/arraybuffer.js @@ -6,8 +6,8 @@ const testUtil = require('./testUtil'); module.exports = test(require(`./build/${buildType}/binding.node`)) .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -async function test(binding) { - await testUtil.runGCTests([ +function test(binding) { + return testUtil.runGCTests([ 'Internal ArrayBuffer', () => { const test = binding.arraybuffer.createBuffer(); diff --git a/test/buffer.js b/test/buffer.js index 127a3cf2c..ff17d30e7 100644 --- a/test/buffer.js +++ b/test/buffer.js @@ -7,8 +7,8 @@ const safeBuffer = require('safe-buffer'); module.exports = test(require(`./build/${buildType}/binding.node`)) .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -async function test(binding) { - await testUtil.runGCTests([ +function test(binding) { + return testUtil.runGCTests([ 'Internal Buffer', () => { const test = binding.buffer.createBuffer(); diff --git a/test/external.js b/test/external.js index 3cda548f6..85dbe702f 100644 --- a/test/external.js +++ b/test/external.js @@ -6,8 +6,8 @@ const testUtil = require('./testUtil'); module.exports = test(require(`./build/${buildType}/binding.node`)) .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -async function test(binding) { - await testUtil.runGCTests([ +function test(binding) { + return testUtil.runGCTests([ 'External without finalizer', () => { const test = binding.external.createExternal(); diff --git a/test/index.js b/test/index.js index 33d2e2dba..86939af84 100644 --- a/test/index.js +++ b/test/index.js @@ -105,6 +105,7 @@ if (typeof global.gc === 'function') { console.log('\nAll tests passed!'); })().catch((error) => { console.log(error); + process.exit(1); }); } else { // Construct the correct (version-dependent) command-line args. diff --git a/test/object/finalizer.js b/test/object/finalizer.js index 40535e1f7..312b2de6d 100644 --- a/test/object/finalizer.js +++ b/test/object/finalizer.js @@ -11,23 +11,20 @@ function createWeakRef(binding, bindingToTest) { return binding.object[bindingToTest]({}); } -async function test(binding) { +function test(binding) { let obj1; let obj2; - await testUtil.runGCTests([ + return testUtil.runGCTests([ 'addFinalizer', () => { obj1 = createWeakRef(binding, 'addFinalizer'); }, - () => { - assert.deepStrictEqual(obj1, { finalizerCalled: true }); - }, + () => assert.deepStrictEqual(obj1, { finalizerCalled: true }), + 'addFinalizerWithHint', () => { obj2 = createWeakRef(binding, 'addFinalizerWithHint'); }, - () => { - assert.deepStrictEqual(obj2, { finalizerCalledWithCorrectHint: true }); - } + () => assert.deepStrictEqual(obj2, { finalizerCalledWithCorrectHint: true }) ]); } diff --git a/test/objectreference.js b/test/objectreference.js index c0e811655..55b95dba6 100644 --- a/test/objectreference.js +++ b/test/objectreference.js @@ -17,7 +17,7 @@ const testUtil = require('./testUtil'); module.exports = test(require(`./build/${buildType}/binding.node`)) .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -async function test(binding) { +function test(binding) { function testCastedEqual(testToCompare) { var compare_test = ["hello", "world", "!"]; if (testToCompare instanceof Array) { @@ -29,7 +29,7 @@ async function test(binding) { } } - await testUtil.runGCTests([ + return testUtil.runGCTests([ 'Weak Casted Array', () => { binding.objectreference.setCastedObjects(); diff --git a/test/objectwrap-removewrap.js b/test/objectwrap-removewrap.js index 2e1604830..560f61d46 100644 --- a/test/objectwrap-removewrap.js +++ b/test/objectwrap-removewrap.js @@ -10,8 +10,8 @@ const assert = require('assert'); const { spawnSync } = require('child_process'); const testUtil = require('./testUtil'); -async function test(bindingName) { - await testUtil.runGCTests([ +function test(bindingName) { + return testUtil.runGCTests([ 'objectwrap removewrap test', () => { const binding = require(bindingName); diff --git a/test/objectwrap_constructor_exception.js b/test/objectwrap_constructor_exception.js index 754a138af..02dff2c48 100644 --- a/test/objectwrap_constructor_exception.js +++ b/test/objectwrap_constructor_exception.js @@ -3,8 +3,8 @@ const buildType = process.config.target_defaults.default_configuration; const assert = require('assert'); const testUtil = require('./testUtil'); -async function test(binding) { - await testUtil.runGCTests([ +function test(binding) { + return testUtil.runGCTests([ 'objectwrap constructor exception', () => { const { ConstructorExceptionTest } = binding.objectwrapConstructorException; diff --git a/test/reference.js b/test/reference.js index 312e81aaa..22ee8c842 100644 --- a/test/reference.js +++ b/test/reference.js @@ -8,8 +8,8 @@ const testUtil = require('./testUtil'); module.exports = test(require(`./build/${buildType}/binding.node`)) .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -async function test(binding) { - await testUtil.runGCTests([ +function test(binding) { + 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 4789ac2e0..ec36dcf51 100644 --- a/test/run_script.js +++ b/test/run_script.js @@ -6,8 +6,8 @@ const testUtil = require('./testUtil'); module.exports = test(require(`./build/${buildType}/binding.node`)) .then(() => test(require(`./build/${buildType}/binding_noexcept.node`))); -async function test(binding) { - await testUtil.runGCTests([ +function test(binding) { + return testUtil.runGCTests([ 'Plain C string', () => { const sum = binding.run_script.plainString(); From a15238726c10ffba3f3007afa167af39cf7f3e63 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Sun, 26 Jul 2020 09:10:48 -0700 Subject: [PATCH 3/5] remove unused reject() --- test/asyncprogressworker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/asyncprogressworker.js b/test/asyncprogressworker.js index 35633733f..b2896a6ca 100644 --- a/test/asyncprogressworker.js +++ b/test/asyncprogressworker.js @@ -33,7 +33,7 @@ function success(binding) { } function fail(binding) { - return new Promise((resolve, reject) => { + return new Promise((resolve) => { binding.doWork(-1, common.mustCall((err) => { assert.throws(() => { throw err }, /test error/) From 5ba003c276549368dc5a21531d25facd24686555 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 27 Jul 2020 00:25:28 -0700 Subject: [PATCH 4/5] remove race from tsfn sum test --- .../threadsafe_function_sum.cc | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/test/threadsafe_function/threadsafe_function_sum.cc b/test/threadsafe_function/threadsafe_function_sum.cc index eac248816..5134e5816 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); From 72920647b39cd80718484787124636b521deafb0 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 27 Jul 2020 14:59:53 -0700 Subject: [PATCH 5/5] remove cancel test --- test/asyncprogressqueueworker.cc | 9 --------- test/asyncprogressqueueworker.js | 22 ---------------------- 2 files changed, 31 deletions(-) diff --git a/test/asyncprogressqueueworker.cc b/test/asyncprogressqueueworker.cc index b30863301..23c6707ff 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 4ad8c43d5..4bb525e9a 100644 --- a/test/asyncprogressqueueworker.js +++ b/test/asyncprogressqueueworker.js @@ -10,7 +10,6 @@ module.exports = test(require(`./build/${buildType}/binding.node`)) async function test({ asyncprogressqueueworker }) { await success(asyncprogressqueueworker); await fail(asyncprogressqueueworker); - await cancel(asyncprogressqueueworker); } function success(binding) { @@ -47,24 +46,3 @@ function fail(binding) { binding.queueWork(worker); }); } - -function cancel(binding) { - return new Promise((resolve, reject) => { - // 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, () => {}, () => {}); - binding.queueWork(worker); - } - const worker = binding.createWork(-1, - () => { - assert.fail('unexpected callback'); - }, - () => { - assert.fail('unexpected progress report'); - } - ); - binding.cancelWork(worker); - resolve(); - }); -}