Merge pull request #994 from wagenet/better-onerror-handling

Author: rwjblue

addon-test-support/@ember/test-helpers/setup-context.ts

@@ -5,6 +5,7 @@ import { setOwner } from '@ember/application';
 
 import buildOwner, { Owner } from './build-owner';
 import { _setupAJAXHooks, _teardownAJAXHooks } from './settled';
+import { _prepareOnerror } from './setup-onerror';
 import Ember from 'ember';
 import { assert } from '@ember/debug';
 import global from './global';
@@ -180,6 +181,8 @@ export default function setupContext(
 
   registerDestructor(context, cleanup);
 
+  _prepareOnerror(context);
+
   return Promise.resolve()
     .then(() => {
       let application = getApplication();

addon-test-support/@ember/test-helpers/setup-onerror.ts

@@ -1,6 +1,7 @@
 import Ember from 'ember';
+import { BaseContext, getContext } from './setup-context';
 
-const ORIGINAL_EMBER_ONERROR: (error: Error) => void | undefined = Ember.onerror;
+let cachedOnerror: Map<BaseContext, ((error: Error) => void) | undefined> = new Map();
 
 /**
  * Sets the `Ember.onerror` function for tests. This value is intended to be reset after
@@ -21,8 +22,20 @@ const ORIGINAL_EMBER_ONERROR: (error: Error) => void | undefined = Ember.onerror
  * });
  */
 export default function setupOnerror(onError?: (error: Error) => void): void {
+  let context = getContext();
+
+  if (!context) {
+    throw new Error('Must setup test context before calling setupOnerror');
+  }
+
+  if (!cachedOnerror.has(context)) {
+    throw new Error(
+      '_cacheOriginalOnerror must be called before setupOnerror. Normally, this will happen as part of your test harness.'
+    );
+  }
+
   if (typeof onError !== 'function') {
-    onError = ORIGINAL_EMBER_ONERROR;
+    onError = cachedOnerror.get(context);
   }
 
   Ember.onerror = onError;
@@ -42,3 +55,29 @@ export default function setupOnerror(onError?: (error: Error) => void): void {
  * })
  */
 export const resetOnerror: Function = setupOnerror;
+
+/**
+ * Caches the current value of Ember.onerror. When `setupOnerror` is called without a value
+ * or when `resetOnerror` is called the value will be set to what was cached here.
+ *
+ * @private
+ * @param {BaseContext} context the text context
+ */
+export function _prepareOnerror(context: BaseContext) {
+  if (cachedOnerror.has(context)) {
+    throw new Error('_prepareOnerror should only be called once per-context');
+  }
+
+  cachedOnerror.set(context, Ember.onerror);
+}
+
+/**
+ * Removes the cached value of Ember.onerror.
+ *
+ * @private
+ * @param {BaseContext} context the text context
+ */
+export function _cleanupOnerror(context: BaseContext) {
+  resetOnerror();
+  cachedOnerror.delete(context);
+}

addon-test-support/@ember/test-helpers/teardown-context.ts

@@ -1,6 +1,7 @@
 import { TestContext } from './setup-context';
 import { Promise } from './-utils';
 import settled from './settled';
+import { _cleanupOnerror } from './setup-onerror';
 import { destroy } from '@ember/destroyable';
 
 /**
@@ -29,6 +30,8 @@ export default function teardownContext(
 
   return Promise.resolve()
     .then(() => {
+      _cleanupOnerror(context);
+
       destroy(context);
     })
     .finally(() => {

package.json

@@ -45,7 +45,7 @@
   },
   "devDependencies": {
     "@ember/optional-features": "^2.0.0",
-    "@types/ember": "^3.16.3",
+    "@types/ember": "^3.16.4",
     "@types/ember-data": "^3.16.8",
     "@types/ember-testing-helpers": "^0.0.4",
     "@types/rsvp": "^4.0.3",

tests/unit/setup-ember-onerror-test.js

@@ -1,46 +1,67 @@
 import Ember from 'ember';
 import { module, test } from 'qunit';
 import hasEmberVersion from '@ember/test-helpers/has-ember-version';
+import { setupContext, teardownContext } from '@ember/test-helpers';
 import { setupOnerror, resetOnerror } from '@ember/test-helpers';
 
 module('setupOnerror', function (hooks) {
-  hooks.afterEach(function () {
-    resetOnerror();
+  let context;
+
+  hooks.beforeEach(function () {
+    context = {};
+  });
+
+  hooks.afterEach(async function () {
+    if (context.owner) {
+      await teardownContext(context);
+    }
   });
 
   if (hasEmberVersion(2, 4)) {
-    test('Ember.onerror is undefined by default', function (assert) {
-      assert.expect(1);
+    module('with context set', function (hooks) {
+      hooks.beforeEach(async function () {
+        await setupContext(context);
+      });
 
-      assert.equal(Ember.onerror, undefined);
-    });
+      test('Ember.onerror is undefined by default', function (assert) {
+        assert.expect(1);
 
-    test('Ember.onerror is setup correctly', async function (assert) {
-      assert.expect(2);
+        assert.equal(Ember.onerror, undefined);
+      });
 
-      let onerror = err => err;
+      test('Ember.onerror is setup correctly', async function (assert) {
+        assert.expect(2);
 
-      assert.equal(Ember.onerror, undefined);
+        let onerror = err => err;
 
-      setupOnerror(onerror);
+        assert.equal(Ember.onerror, undefined);
 
-      assert.equal(Ember.onerror, onerror);
-    });
+        setupOnerror(onerror);
+
+        assert.equal(Ember.onerror, onerror);
+      });
 
-    test('Ember.onerror is reset correctly', async function (assert) {
-      assert.expect(3);
+      test('Ember.onerror is reset correctly', async function (assert) {
+        assert.expect(3);
 
-      let onerror = err => err;
+        let onerror = err => err;
 
-      assert.equal(Ember.onerror, undefined);
+        assert.equal(Ember.onerror, undefined);
 
-      setupOnerror(onerror);
+        setupOnerror(onerror);
 
-      assert.equal(Ember.onerror, onerror);
+        assert.equal(Ember.onerror, onerror);
 
-      resetOnerror();
+        resetOnerror();
+
+        assert.equal(Ember.onerror, undefined);
+      });
+    });
 
-      assert.equal(Ember.onerror, undefined);
+    test('it raises an error without context', function (assert) {
+      assert.throws(() => {
+        setupOnerror();
+      }, /Must setup test context before calling setupOnerror/);
     });
   }
 });

yarn.lock

@@ -1776,10 +1776,10 @@
     "@types/jquery" "*"
     "@types/rsvp" "*"
 
-"@types/ember@*", "@types/ember@^3.16.3":
-  version "3.16.3"
-  resolved "https://registry.yarnpkg.com/@types/ember/-/ember-3.16.3.tgz#e2e9c24e56d8161ffcdbfa16d6a1e7e032bb557b"
-  integrity sha512-KkRs2F2dv6CuNfj9GFpJULL+8g+foJNEkOwSIqf5+RxaHjsg9NSVg9phTK4RItbndMyECCtOGOwWQAJ4cQgiiA==
+"@types/ember@*", "@types/ember@^3.16.4":
+  version "3.16.4"
+  resolved "https://registry.yarnpkg.com/@types/ember/-/ember-3.16.4.tgz#bfccd8ed198ca7bee09878a3423ca6e1a9caac17"
+  integrity sha512-kCZNxuCofZN2sYUltfUmPegqAr1wvZ4b6aH0i8AsG+AsUiaWCDzVfCayMfr4CRUOhUiQ2VA9AOgnZT+JgBvjXQ==
   dependencies:
     "@types/ember__application" "*"
     "@types/ember__array" "*"