From 32cbcee46ff468d8e8e85eb7000e65bf2b2cc0b0 Mon Sep 17 00:00:00 2001 From: Elias Dawson <145914+eliasdawson@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:07:10 -0500 Subject: [PATCH 1/3] Make test context reusable during a test In v2.0.0 @ember/test-helpers refactored to use @ember/destroyable for teardown which also meant that the application context was destroyed during teardown, where previously it was only cleaned up and the application was destroyed. In our application tests we simulate a page refresh by calling `teardownContext`` and `setupContext`. The destroyable change broke this functionality because the context is now marked as destroyed. This commit effectively reverts the change to use @ember/destroyable and adds tests to ensure that the context is not destroyed so that it can be reset during a test. --- addon/src/setup-context.ts | 24 +------------------ addon/src/teardown-context.ts | 13 ++++++++-- .../unit/setup-application-context-test.js | 13 ++++++++++ test-app/tests/unit/setup-context-test.js | 14 +++++++++++ test-app/tests/unit/teardown-context-test.js | 19 ++++----------- 5 files changed, 43 insertions(+), 40 deletions(-) diff --git a/addon/src/setup-context.ts b/addon/src/setup-context.ts index 86945b5f3..6a434ba89 100644 --- a/addon/src/setup-context.ts +++ b/addon/src/setup-context.ts @@ -7,7 +7,7 @@ import type { Resolver } from '@ember/owner'; import { setOwner } from '@ember/application'; import buildOwner, { type Owner } from './build-owner.ts'; -import { _setupAJAXHooks, _teardownAJAXHooks } from './settled.ts'; +import { _setupAJAXHooks } from './settled.ts'; import { _prepareOnerror } from './setup-onerror.ts'; import Ember from 'ember'; import { @@ -19,10 +19,6 @@ import global from './global.ts'; import { getResolver } from './resolver.ts'; import { getApplication } from './application.ts'; import getTestMetadata from './test-metadata.ts'; -import { - registerDestructor, - associateDestroyableChild, -} from '@ember/destroyable'; import { getDeprecationsForContext, getDeprecationsDuringCallbackForContext, @@ -211,20 +207,6 @@ export function resumeTest(): void { context.resumeTest(); } -/** - @private - @param {Object} context the test context being cleaned up -*/ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -function cleanup(context: BaseContext) { - _teardownAJAXHooks(); - - // SAFETY: this is intimate API *designed* for us to override. - (Ember as any).testing = false; - - unsetContext(); -} - /** * Returns deprecations which have occurred so far for a the current test context * @@ -410,8 +392,6 @@ export default function setupContext( _backburner.DEBUG = true; - registerDestructor(context, cleanup); - _prepareOnerror(context); return Promise.resolve() @@ -438,8 +418,6 @@ export default function setupContext( return buildOwner(getApplication(), getResolver()); }) .then((owner) => { - associateDestroyableChild(context, owner); - Object.defineProperty(context, 'owner', { configurable: true, enumerable: true, diff --git a/addon/src/teardown-context.ts b/addon/src/teardown-context.ts index 329a1c6b8..cefb93a37 100644 --- a/addon/src/teardown-context.ts +++ b/addon/src/teardown-context.ts @@ -1,5 +1,7 @@ import type { TestContext } from './setup-context'; -import settled from './settled.ts'; +import Ember from 'ember'; +import { unsetContext } from './setup-context.ts'; +import settled, { _teardownAJAXHooks } from './settled.ts'; import { _cleanupOnerror } from './setup-onerror.ts'; import { destroy } from '@ember/destroyable'; @@ -30,7 +32,14 @@ export default function teardownContext( .then(() => { _cleanupOnerror(context); - destroy(context); + _teardownAJAXHooks(); + + // SAFETY: this is intimate API *designed* for us to override. + (Ember as any).testing = false; + + unsetContext(); + + destroy(context.owner); }) .finally(() => { if (waitForSettled) { diff --git a/test-app/tests/unit/setup-application-context-test.js b/test-app/tests/unit/setup-application-context-test.js index ce3b15f58..11722763f 100644 --- a/test-app/tests/unit/setup-application-context-test.js +++ b/test-app/tests/unit/setup-application-context-test.js @@ -257,4 +257,17 @@ module('setupApplicationContext', function (hooks) { 'after click resolved', ]); }); + + test('can reset context during a test', async function (assert) { + await visit('/'); + assert.equal(currentURL(), '/'); + + await teardownContext(this); + await setupContext(this); + await setupApplicationContext(this); + assert.equal(currentURL(), null); + + await visit('/'); + assert.equal(currentURL(), '/'); + }); }); diff --git a/test-app/tests/unit/setup-context-test.js b/test-app/tests/unit/setup-context-test.js index c71f97d32..c03b6aaff 100644 --- a/test-app/tests/unit/setup-context-test.js +++ b/test-app/tests/unit/setup-context-test.js @@ -943,5 +943,19 @@ module('setupContext', function (hooks) { } } }); + + test('can reset context during a test', async function (assert) { + assert.expect(1); + + let context = await setupContext({}); + try { + await teardownContext(context); + context = await setupContext(context); + } finally { + await teardownContext(context); + } + + assert.ok(true, 'No error calling setupContext after teardownContext'); + }); }); }); diff --git a/test-app/tests/unit/teardown-context-test.js b/test-app/tests/unit/teardown-context-test.js index fcf7a4bc1..bdebc373e 100644 --- a/test-app/tests/unit/teardown-context-test.js +++ b/test-app/tests/unit/teardown-context-test.js @@ -65,28 +65,17 @@ module('teardownContext', function (hooks) { assert.strictEqual(getContext(), undefined, 'context is unset'); }); - test('destroyables registered with the context are invoked', async function (assert) { - registerDestructor(context, () => { - assert.step('destructor was ran'); - }); - - assert.step('teardown started'); - + test('the owner is destroyed', async function (assert) { await teardownContext(context); - assert.step('teardown completed'); - - assert.verifySteps([ - 'teardown started', - 'destructor was ran', - 'teardown completed', - ]); + assert.ok(context.owner.isDestroyed); }); - test('the owner is destroyed', async function (assert) { + test('the context is not destroyed', async function (assert) { await teardownContext(context); assert.ok(context.owner.isDestroyed); + assert.notOk(context.isDestroyed); }); test('the application instance is destroyed and unwatched', async function (assert) { From 867e84d2db61629c507a8439d290697a52b13cf5 Mon Sep 17 00:00:00 2001 From: Elias Dawson <145914+eliasdawson@users.noreply.github.com> Date: Wed, 23 Oct 2024 12:08:24 -0500 Subject: [PATCH 2/3] Fix lint error --- test-app/tests/unit/teardown-context-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test-app/tests/unit/teardown-context-test.js b/test-app/tests/unit/teardown-context-test.js index bdebc373e..986f1007f 100644 --- a/test-app/tests/unit/teardown-context-test.js +++ b/test-app/tests/unit/teardown-context-test.js @@ -15,7 +15,6 @@ import hasjQuery from '../helpers/has-jquery'; import ajax from '../helpers/ajax'; import Pretender from 'pretender'; import setupManualTestWaiter from '../helpers/manual-test-waiter'; -import { registerDestructor } from '@ember/destroyable'; module('teardownContext', function (hooks) { if (!hasEmberVersion(2, 4)) { From af5f0b538d40358d52ff86ec7237cb4627159b78 Mon Sep 17 00:00:00 2001 From: Elias Dawson <145914+eliasdawson@users.noreply.github.com> Date: Wed, 23 Oct 2024 12:08:51 -0500 Subject: [PATCH 3/3] Fix DOM test failures --- test-app/tests/unit/dom/fill-in-test.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test-app/tests/unit/dom/fill-in-test.js b/test-app/tests/unit/dom/fill-in-test.js index 8292f0542..db5c52012 100644 --- a/test-app/tests/unit/dom/fill-in-test.js +++ b/test-app/tests/unit/dom/fill-in-test.js @@ -16,16 +16,6 @@ if (isFirefox || isChrome) { clickSteps.push('selectionchange'); } -/** - * Prior to Chrome 129 (canary), - * Chrome 127.x emits an extra selectionchange event sometimes - * - * Delete this once we don't want to test against Chrome 127 - */ -if (isChrome) { - clickSteps.push('selectionchange'); -} - module('DOM Helper: fillIn', function (hooks) { if (!hasEmberVersion(2, 4)) { return;