test: fix ineffective error tests

Fix tests whether errors are thrown correctly because they are successful when error doesn't get thrown. PR-URL: #27333 Fixes: #26385 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
nodejs · Apr 24, 2019 · fb3f600 · fb3f600
1 parent 9b982fe
commit fb3f600
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 72 deletions.
diff --git a/test/addons/non-node-context/test-make-buffer.js b/test/addons/non-node-context/test-make-buffer.js
@@ -9,14 +9,15 @@ const {
 // Because the `Buffer` function and its protoype property only (currently)
 // exist in a Node.js instance’s main context, trying to create buffers from
 // another context throws an exception.
+assert.throws(
+  () => makeBufferInNewContext(),
+  (exception) => {
+    assert.strictEqual(exception.constructor.name, 'Error');
+    assert(!(exception.constructor instanceof Error));
 
-try {
-  makeBufferInNewContext();
-} catch (exception) {
-  assert.strictEqual(exception.constructor.name, 'Error');
-  assert(!(exception.constructor instanceof Error));
-
-  assert.strictEqual(exception.code, 'ERR_BUFFER_CONTEXT_NOT_AVAILABLE');
-  assert.strictEqual(exception.message,
-                     'Buffer is not available for the current Context');
-}
+    assert.strictEqual(exception.code, 'ERR_BUFFER_CONTEXT_NOT_AVAILABLE');
+    assert.strictEqual(exception.message,
+                       'Buffer is not available for the current Context');
+    return true;
+  }
+);
diff --git a/test/es-module/test-esm-error-cache.js b/test/es-module/test-esm-error-cache.js
@@ -18,9 +18,11 @@ let error;
 
   assert(error);
 
-  try {
-    await import(file);
-  } catch (e) {
-    assert.strictEqual(error, e);
-  }
+  await assert.rejects(
+    () => import(file),
+    (e) => {
+      assert.strictEqual(error, e);
+      return true;
+    }
+  );
 })();
diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js
@@ -297,23 +297,21 @@ testAssertionMessage({ a: NaN, b: Infinity, c: -Infinity },
                      '{\n+   a: NaN,\n+   b: Infinity,\n+   c: -Infinity\n+ }');
 
 // https://github.com/nodejs/node-v0.x-archive/issues/5292
-try {
-  assert.strictEqual(1, 2);
-} catch (e) {
-  assert.strictEqual(
-    e.message,
-    `${strictEqualMessageStart}\n1 !== 2\n`
-  );
-  assert.ok(e.generatedMessage, 'Message not marked as generated');
-}
+assert.throws(
+  () => assert.strictEqual(1, 2),
+  {
+    message: `${strictEqualMessageStart}\n1 !== 2\n`,
+    generatedMessage: true
+  }
+);
 
-try {
-  assert.strictEqual(1, 2, 'oh no');
-} catch (e) {
-  assert.strictEqual(e.message, 'oh no');
-  // Message should not be marked as generated.
-  assert.strictEqual(e.generatedMessage, false);
-}
+assert.throws(
+  () => assert.strictEqual(1, 2, 'oh no'),
+  {
+    message: 'oh no',
+    generatedMessage: false
+  }
+);
 
 {
   let threw = false;

diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
@@ -538,11 +538,14 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
   ].forEach((err) => {
     assert.strictEqual(util.inspect(err), err.stack);
   });
-  try {
-    undef(); // eslint-disable-line no-undef
-  } catch (e) {
-    assert.strictEqual(util.inspect(e), e.stack);
-  }
+  assert.throws(
+    () => undef(), // eslint-disable-line no-undef
+    (e) => {
+      assert.strictEqual(util.inspect(e), e.stack);
+      return true;
+    }
+  );
+
   const ex = util.inspect(new Error('FAIL'), true);
   assert(ex.includes('Error: FAIL'));
   assert(ex.includes('[stack]'));

diff --git a/test/parallel/test-vm-codegen.js b/test/parallel/test-vm-codegen.js
@@ -8,19 +8,6 @@ const { createContext, runInContext, runInNewContext } = require('vm');
 const WASM_BYTES = Buffer.from(
   [0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00]);
 
-
-function expectsError(fn, type) {
-  try {
-    fn();
-    assert.fail('expected fn to error');
-  } catch (err) {
-    if (typeof type === 'string')
-      assert.strictEqual(err.name, type);
-    else
-      assert(err instanceof type);
-  }
-}
-
 {
   const ctx = createContext({ WASM_BYTES });
   const test = 'eval(""); new WebAssembly.Module(WASM_BYTES);';
@@ -39,7 +26,7 @@ function expectsError(fn, type) {
   });
 
   const EvalError = runInContext('EvalError', ctx);
-  expectsError(() => {
+  assert.throws(() => {
     runInContext('eval("x")', ctx);
   }, EvalError);
 }
@@ -52,26 +39,30 @@ function expectsError(fn, type) {
   });
 
   const CompileError = runInContext('WebAssembly.CompileError', ctx);
-  expectsError(() => {
+  assert.throws(() => {
     runInContext('new WebAssembly.Module(WASM_BYTES)', ctx);
   }, CompileError);
 }
 
-expectsError(() => {
+assert.throws(() => {
   runInNewContext('eval("x")', {}, {
     contextCodeGeneration: {
       strings: false,
     },
   });
-}, 'EvalError');
+}, {
+  name: 'EvalError'
+});
 
-expectsError(() => {
+assert.throws(() => {
   runInNewContext('new WebAssembly.Module(WASM_BYTES)', { WASM_BYTES }, {
     contextCodeGeneration: {
       wasm: false,
     },
   });
-}, 'CompileError');
+}, {
+  name: 'CompileError'
+});
 
 common.expectsError(() => {
   createContext({}, {

diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js
@@ -201,27 +201,33 @@ assert.throws(
 
   assert.strictEqual(require(`${loadOrder}file1`).file1, 'file1');
   assert.strictEqual(require(`${loadOrder}file2`).file2, 'file2.js');
-  try {
-    require(`${loadOrder}file3`);
-  } catch (e) {
-    // Not a real .node module, but we know we require'd the right thing.
-    if (common.isOpenBSD) // OpenBSD errors with non-ELF object error
-      assert.ok(/File not an ELF object/.test(e.message.replace(backslash, '/')));
-    else
-      assert.ok(/file3\.node/.test(e.message.replace(backslash, '/')));
-  }
+  assert.throws(
+    () => require(`${loadOrder}file3`),
+    (e) => {
+      // Not a real .node module, but we know we require'd the right thing.
+      if (common.isOpenBSD) { // OpenBSD errors with non-ELF object error
+        assert.ok(/File not an ELF object/.test(e.message.replace(backslash, '/')));
+      } else {
+        assert.ok(/file3\.node/.test(e.message.replace(backslash, '/')));
+      }
+      return true;
+    }
+  );
 
   assert.strictEqual(require(`${loadOrder}file4`).file4, 'file4.reg');
   assert.strictEqual(require(`${loadOrder}file5`).file5, 'file5.reg2');
   assert.strictEqual(require(`${loadOrder}file6`).file6, 'file6/index.js');
-  try {
-    require(`${loadOrder}file7`);
-  } catch (e) {
-    if (common.isOpenBSD)
-      assert.ok(/File not an ELF object/.test(e.message.replace(backslash, '/')));
-    else
-      assert.ok(/file7\/index\.node/.test(e.message.replace(backslash, '/')));
-  }
+  assert.throws(
+    () => require(`${loadOrder}file7`),
+    (e) => {
+      if (common.isOpenBSD) {
+        assert.ok(/File not an ELF object/.test(e.message.replace(backslash, '/')));
+      } else {
+        assert.ok(/file7\/index\.node/.test(e.message.replace(backslash, '/')));
+      }
+      return true;
+    }
+  );
 
   assert.strictEqual(require(`${loadOrder}file8`).file8, 'file8/index.reg');
   assert.strictEqual(require(`${loadOrder}file9`).file9, 'file9/index.reg2');