From 6c1906ab3e0093a4f5c765de5a709abbe6b32d6b Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Tue, 23 Jan 2018 22:18:38 -0800
Subject: [PATCH] test: refactor addons-napi/test_exception/test.js

* provide block scoping to prevent unintended side effects
* remove confusing and unnecessary assertion message
* use consisitent `actual, expected` argument order for assertions

PR-URL: https://github.com/nodejs/node/pull/18340
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 test/addons-napi/test_exception/test.js | 86 ++++++++++++-------------
 1 file changed, 42 insertions(+), 44 deletions(-)

diff --git a/test/addons-napi/test_exception/test.js b/test/addons-napi/test_exception/test.js
index 8bd2f50b12b15f..83961411df7574 100644
--- a/test/addons-napi/test_exception/test.js
+++ b/test/addons-napi/test_exception/test.js
@@ -4,49 +4,47 @@ const common = require('../../common');
 const test_exception = require(`./build/${common.buildType}/test_exception`);
 const assert = require('assert');
 const theError = new Error('Some error');
-function throwTheError() {
-  throw theError;
+
+{
+  const throwTheError = () => { throw theError; };
+
+  // Test that the native side successfully captures the exception
+  let returnedError = test_exception.returnException(throwTheError);
+  assert.strictEqual(theError, returnedError);
+
+  // Test that the native side passes the exception through
+  assert.throws(
+    () => { test_exception.allowException(throwTheError); },
+    (err) => err === theError
+  );
+
+  // Test that the exception thrown above was marked as pending
+  // before it was handled on the JS side
+  assert.strictEqual(test_exception.wasPending(), true,
+                     'VM was marked as having an exception pending' +
+                     ' when it was allowed through');
+
+  // Test that the native side does not capture a non-existing exception
+  returnedError = test_exception.returnException(common.mustCall());
+  assert.strictEqual(returnedError, undefined,
+                     'Returned error should be undefined when no exception is' +
+                     ` thrown, but ${returnedError} was passed`);
 }
-let caughtError;
-
-// Test that the native side successfully captures the exception
-let returnedError = test_exception.returnException(throwTheError);
-assert.strictEqual(theError, returnedError);
-
-// Test that the native side passes the exception through
-assert.throws(
-  () => {
-    test_exception.allowException(throwTheError);
-  },
-  function(err) {
-    return err === theError;
-  },
-  'Thrown exception was allowed to pass through unhindered'
-);
-
-// Test that the exception thrown above was marked as pending
-// before it was handled on the JS side
-assert.strictEqual(test_exception.wasPending(), true,
-                   'VM was marked as having an exception pending' +
-                   ' when it was allowed through');
-
-// Test that the native side does not capture a non-existing exception
-returnedError = test_exception.returnException(common.mustCall());
-assert.strictEqual(undefined, returnedError,
-                   'Returned error should be undefined when no exception is' +
-                   ` thrown, but ${returnedError} was passed`);
-
-// Test that no exception appears that was not thrown by us
-try {
-  test_exception.allowException(common.mustCall());
-} catch (anError) {
-  caughtError = anError;
+
+{
+  // Test that no exception appears that was not thrown by us
+  let caughtError;
+  try {
+    test_exception.allowException(common.mustCall());
+  } catch (anError) {
+    caughtError = anError;
+  }
+  assert.strictEqual(caughtError, undefined,
+                     'No exception originated on the native side, but' +
+                     ` ${caughtError} was passed`);
+
+  // Test that the exception state remains clear when no exception is thrown
+  assert.strictEqual(test_exception.wasPending(), false,
+                     'VM was not marked as having an exception pending' +
+                     ' when none was allowed through');
 }
-assert.strictEqual(undefined, caughtError,
-                   'No exception originated on the native side, but' +
-                   ` ${caughtError} was passed`);
-
-// Test that the exception state remains clear when no exception is thrown
-assert.strictEqual(test_exception.wasPending(), false,
-                   'VM was not marked as having an exception pending' +
-                   ' when none was allowed through');