Skip to content

Commit

Permalink
Merge pull request #994 from wagenet/better-onerror-handling
Browse files Browse the repository at this point in the history
  • Loading branch information
rwjblue authored Feb 23, 2021
2 parents 3d267cd + ff1867c commit 90fcea7
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 28 deletions.
3 changes: 3 additions & 0 deletions addon-test-support/@ember/test-helpers/setup-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -180,6 +181,8 @@ export default function setupContext(

registerDestructor(context, cleanup);

_prepareOnerror(context);

return Promise.resolve()
.then(() => {
let application = getApplication();
Expand Down
43 changes: 41 additions & 2 deletions addon-test-support/@ember/test-helpers/setup-onerror.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand All @@ -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);
}
3 changes: 3 additions & 0 deletions addon-test-support/@ember/test-helpers/teardown-context.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -29,6 +30,8 @@ export default function teardownContext(

return Promise.resolve()
.then(() => {
_cleanupOnerror(context);

destroy(context);
})
.finally(() => {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
63 changes: 42 additions & 21 deletions tests/unit/setup-ember-onerror-test.js
Original file line number Diff line number Diff line change
@@ -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/);
});
}
});
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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" "*"
Expand Down

0 comments on commit 90fcea7

Please sign in to comment.