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: nodejs/node-addon-api#773
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>

Author: kevindavies8

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']);
 }

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',
     () => {

test/asyncprogressqueueworker.cc

@@ -37,14 +37,6 @@ class TestWorker : public AsyncProgressQueueWorker<ProgressData> {
     worker->Queue();
   }
 
-  static void CancelWork(const CallbackInfo& info) {
-    auto wrap = info[0].As<Napi::External<TestWorker>>();
-    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;
 }
 

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);
+  });
 }

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()
+    );
+  });
 }

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;
-}
+  await binding.asyncworker.doWorkNoCallback(false, {})
+    .then(common.mustNotCall()).catch(common.mustCall());
+}

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));