From 7a45874b34dd331c18c999cde5b998f10f234bf8 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 7 Dec 2018 18:06:14 -0500 Subject: [PATCH 1/3] Add ability to opt-out of automatic settledness waiting in teardown. In order to allow things like `ember-qunit`'s upcoming async leak detection, we need to avoid waiting for `settled()`. By default we should continue to effectively `await settled()` but when ember-qunit (and soon ember-mocha) is configured to use async leak detection they can appropriately pass `waitForSettled` when they call `teardownContext` / `teardownRenderingContext` / `teardownApplicationContext`. --- .../teardown-application-context.ts | 16 +++++++++++-- .../@ember/test-helpers/teardown-context.ts | 23 ++++++++++++++++--- .../teardown-rendering-context.ts | 18 +++++++++++++-- tests/unit/teardown-context-test.js | 21 ++++++++++++++++- tests/unit/teardown-rendering-context-test.js | 15 ++++++++++++ 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/addon-test-support/@ember/test-helpers/teardown-application-context.ts b/addon-test-support/@ember/test-helpers/teardown-application-context.ts index 9aab9c67d..176ce5b03 100644 --- a/addon-test-support/@ember/test-helpers/teardown-application-context.ts +++ b/addon-test-support/@ember/test-helpers/teardown-application-context.ts @@ -1,3 +1,4 @@ +import { nextTickPromise } from './-utils'; import settled from './settled'; /** @@ -5,8 +6,19 @@ import settled from './settled'; @public @param {Object} context the context to setup + @param {Object} [options] options used to override defaults + @param {boolean} [options.waitForSettled=true] should the teardown wait for `settled()`ness @returns {Promise} resolves when settled */ -export default function(context: object): Promise { - return settled(); +export default function(context: object, options?: { waitForSettled?: boolean }): Promise { + let waitForSettled = true; + if (options !== undefined && 'waitForSettled' in options) { + waitForSettled = options.waitForSettled!; + } + + if (waitForSettled) { + return settled(); + } + + return nextTickPromise(); } diff --git a/addon-test-support/@ember/test-helpers/teardown-context.ts b/addon-test-support/@ember/test-helpers/teardown-context.ts index b247b8c8a..e865a36d3 100644 --- a/addon-test-support/@ember/test-helpers/teardown-context.ts +++ b/addon-test-support/@ember/test-helpers/teardown-context.ts @@ -17,9 +17,18 @@ import Ember from 'ember'; @public @param {Object} context the context to setup + @param {Object} [options] options used to override defaults + @param {boolean} [options.waitForSettled=true] should the teardown wait for `settled()`ness @returns {Promise} resolves when settled */ -export default function teardownContext(context: TestContext): Promise { +export default function teardownContext( + context: TestContext, + options?: { waitForSettled?: boolean } +): Promise { + let waitForSettled = true; + if (options !== undefined && 'waitForSettled' in options) { + waitForSettled = options.waitForSettled!; + } return nextTickPromise() .then(() => { let { owner } = context; @@ -31,13 +40,21 @@ export default function teardownContext(context: TestContext): Promise { unsetContext(); - return settled(); + if (waitForSettled) { + return settled(); + } + + return nextTickPromise(); }) .finally(() => { let contextGuid = guidFor(context); runDestroyablesFor(CLEANUP, contextGuid); - return settled(); + if (waitForSettled) { + return settled(); + } + + return nextTickPromise(); }); } diff --git a/addon-test-support/@ember/test-helpers/teardown-rendering-context.ts b/addon-test-support/@ember/test-helpers/teardown-rendering-context.ts index 33d6c9502..1d1c8bfe6 100644 --- a/addon-test-support/@ember/test-helpers/teardown-rendering-context.ts +++ b/addon-test-support/@ember/test-helpers/teardown-rendering-context.ts @@ -13,14 +13,28 @@ import settled from './settled'; @public @param {Object} context the context to setup + @param {Object} [options] options used to override defaults + @param {boolean} [options.waitForSettled=true] should the teardown wait for `settled()`ness @returns {Promise} resolves when settled */ -export default function teardownRenderingContext(context: RenderingTestContext): Promise { +export default function teardownRenderingContext( + context: RenderingTestContext, + options?: { waitForSettled?: boolean } +): Promise { + let waitForSettled = true; + if (options !== undefined && 'waitForSettled' in options) { + waitForSettled = options.waitForSettled!; + } + return nextTickPromise().then(() => { let contextGuid = guidFor(context); runDestroyablesFor(RENDERING_CLEANUP, contextGuid); - return settled(); + if (waitForSettled) { + return settled(); + } + + return nextTickPromise(); }); } diff --git a/tests/unit/teardown-context-test.js b/tests/unit/teardown-context-test.js index 0e0eff70e..a757c952c 100644 --- a/tests/unit/teardown-context-test.js +++ b/tests/unit/teardown-context-test.js @@ -1,12 +1,19 @@ import { module, test } from 'qunit'; import Service from '@ember/service'; -import { getContext, setupContext, teardownContext, getSettledState } from '@ember/test-helpers'; +import { + getContext, + setupContext, + teardownContext, + getSettledState, + settled, +} from '@ember/test-helpers'; import { setResolverRegistry } from '../helpers/resolver'; import hasEmberVersion from '@ember/test-helpers/has-ember-version'; import Ember from 'ember'; import hasjQuery from '../helpers/has-jquery'; import ajax from '../helpers/ajax'; import Pretender from 'pretender'; +import { later } from '@ember/runloop'; module('teardownContext', function(hooks) { if (!hasEmberVersion(2, 4)) { @@ -69,4 +76,16 @@ module('teardownContext', function(hooks) { assert.equal(state.pendingRequestCount, 0, 'pendingRequestCount is 0'); }); } + + test('can opt out of waiting for settledness', async function(assert) { + later(() => assert.step('later'), 200); + + await teardownContext(context, { waitForSettled: false }); + + assert.step('after teardown'); + + await settled(); + + assert.verifySteps(['after teardown', 'later']); + }); }); diff --git a/tests/unit/teardown-rendering-context-test.js b/tests/unit/teardown-rendering-context-test.js index f0c35dcf6..8ce8fc833 100644 --- a/tests/unit/teardown-rendering-context-test.js +++ b/tests/unit/teardown-rendering-context-test.js @@ -4,8 +4,10 @@ import { setupRenderingContext, teardownContext, teardownRenderingContext, + settled, } from '@ember/test-helpers'; import hasEmberVersion from '@ember/test-helpers/has-ember-version'; +import { later } from '@ember/runloop'; module('teardownRenderingContext', function(hooks) { if (!hasEmberVersion(2, 4)) { @@ -54,4 +56,17 @@ module('teardownRenderingContext', function(hooks) { 'previous ember-testing element is no longer in DOM' ); }); + + test('can opt out of waiting for settledness', async function(assert) { + later(() => assert.step('later'), 200); + + await teardownRenderingContext(this, { waitForSettled: false }); + await teardownContext(this, { waitForSettled: false }); + + assert.step('after teardown'); + + await settled(); + + assert.verifySteps(['after teardown', 'later']); + }); }); From 501cafaedf8cc79f69430b4e0cb9b072456aa76e Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 10 Dec 2018 17:54:19 -0500 Subject: [PATCH 2/3] Add composable helper for manual test waiter testing. --- tests/helpers/manual-test-waiter.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 tests/helpers/manual-test-waiter.js diff --git a/tests/helpers/manual-test-waiter.js b/tests/helpers/manual-test-waiter.js new file mode 100644 index 000000000..956fec024 --- /dev/null +++ b/tests/helpers/manual-test-waiter.js @@ -0,0 +1,13 @@ +import { registerWaiter, unregisterWaiter } from '@ember/test'; + +export default function setupManualTestWaiter(hooks) { + hooks.beforeEach(function() { + this.shouldWait = false; + this._waiter = () => !this.shouldWait; + registerWaiter(this._waiter); + }); + + hooks.afterEach(function() { + unregisterWaiter(this._waiter); + }); +} From 3475d233bdc73647760f21dcd992f59da0c80f4b Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 10 Dec 2018 17:54:52 -0500 Subject: [PATCH 3/3] Migrate tests to use more controlled test waiter. Avoid possible timing issues in original tests. --- tests/unit/teardown-context-test.js | 14 ++++++++++---- tests/unit/teardown-rendering-context-test.js | 13 +++++++++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/tests/unit/teardown-context-test.js b/tests/unit/teardown-context-test.js index a757c952c..c6cbdcebc 100644 --- a/tests/unit/teardown-context-test.js +++ b/tests/unit/teardown-context-test.js @@ -6,6 +6,7 @@ import { teardownContext, getSettledState, settled, + isSettled, } from '@ember/test-helpers'; import { setResolverRegistry } from '../helpers/resolver'; import hasEmberVersion from '@ember/test-helpers/has-ember-version'; @@ -13,13 +14,15 @@ import Ember from 'ember'; import hasjQuery from '../helpers/has-jquery'; import ajax from '../helpers/ajax'; import Pretender from 'pretender'; -import { later } from '@ember/runloop'; +import setupManualTestWaiter from '../helpers/manual-test-waiter'; module('teardownContext', function(hooks) { if (!hasEmberVersion(2, 4)) { return; } + setupManualTestWaiter(hooks); + let context; hooks.beforeEach(function() { this.pretender = new Pretender(); @@ -78,14 +81,17 @@ module('teardownContext', function(hooks) { } test('can opt out of waiting for settledness', async function(assert) { - later(() => assert.step('later'), 200); + this.shouldWait = true; + + assert.equal(isSettled(), false, 'should not be settled'); await teardownContext(context, { waitForSettled: false }); - assert.step('after teardown'); + assert.equal(isSettled(), false, 'should not be settled'); + this.shouldWait = false; await settled(); - assert.verifySteps(['after teardown', 'later']); + assert.equal(isSettled(), true, 'should be settled'); }); }); diff --git a/tests/unit/teardown-rendering-context-test.js b/tests/unit/teardown-rendering-context-test.js index 8ce8fc833..3163a098c 100644 --- a/tests/unit/teardown-rendering-context-test.js +++ b/tests/unit/teardown-rendering-context-test.js @@ -5,14 +5,16 @@ import { teardownContext, teardownRenderingContext, settled, + isSettled, } from '@ember/test-helpers'; import hasEmberVersion from '@ember/test-helpers/has-ember-version'; -import { later } from '@ember/runloop'; +import setupManualTestWaiter from '../helpers/manual-test-waiter'; module('teardownRenderingContext', function(hooks) { if (!hasEmberVersion(2, 4)) { return; } + setupManualTestWaiter(hooks); hooks.beforeEach(async function() { await setupContext(this); @@ -58,15 +60,18 @@ module('teardownRenderingContext', function(hooks) { }); test('can opt out of waiting for settledness', async function(assert) { - later(() => assert.step('later'), 200); + this.shouldWait = true; + + assert.equal(isSettled(), false, 'should not be settled'); await teardownRenderingContext(this, { waitForSettled: false }); await teardownContext(this, { waitForSettled: false }); - assert.step('after teardown'); + assert.equal(isSettled(), false, 'should not be settled'); + this.shouldWait = false; await settled(); - assert.verifySteps(['after teardown', 'later']); + assert.equal(isSettled(), true, 'should be settled'); }); });