From 1eeb93def20eed94afd1ba2513f11d5490c83841 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 31 Mar 2020 14:21:47 -0700 Subject: [PATCH] Introduce `setupTestAdapter` hook (#1165) While working on #1163, we encountered an issue with the test adapter, where unmet expectations (when you set up a `respondTo` for a test that was never triggered) did not cause the offending test to fail. Instead, they became unhandled promise rejections (i.e. "global errors") and fails the _next_ test that runs. This is because we were using the `QUnit.{testStart,testDone}` hooks as if they were a global version of `{before,after}Each`. However, the global hooks are really intended for things like test reporters and runs too late to affect things like the pass or fail status of a test. This commit introduces a `setupTestAdapter` function that uses the normal (not global) APIs so that the failures are attached to the correct test. --- tests/acceptance/app-picker-test.js | 3 +- tests/acceptance/component-tree-test.js | 3 +- tests/acceptance/container-test.js | 3 +- tests/acceptance/data-test.js | 3 +- tests/acceptance/deprecation-test.js | 3 +- tests/acceptance/info-test.js | 3 +- tests/acceptance/object-inspector-test.js | 3 +- tests/acceptance/promise-test.js | 3 +- tests/acceptance/render-tree-test.js | 3 +- tests/acceptance/route-tree-test.js | 3 +- tests/acceptance/whats-new-test.js | 3 + tests/test-adapter.js | 82 +++++++++++------------ 12 files changed, 64 insertions(+), 51 deletions(-) diff --git a/tests/acceptance/app-picker-test.js b/tests/acceptance/app-picker-test.js index d7c441174a..bb22807f11 100644 --- a/tests/acceptance/app-picker-test.js +++ b/tests/acceptance/app-picker-test.js @@ -5,9 +5,10 @@ import { } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import { respondWith, disableDefaultResponseFor } from '../test-adapter'; +import { setupTestAdapter, respondWith, disableDefaultResponseFor } from '../test-adapter'; module('App Picker', function(hooks) { + setupTestAdapter(hooks); setupApplicationTest(hooks); hooks.beforeEach(function() { diff --git a/tests/acceptance/component-tree-test.js b/tests/acceptance/component-tree-test.js index f6924199c1..810deb651f 100644 --- a/tests/acceptance/component-tree-test.js +++ b/tests/acceptance/component-tree-test.js @@ -9,7 +9,7 @@ import { } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import { respondWith, sendMessage } from '../test-adapter'; +import { setupTestAdapter, respondWith, sendMessage } from '../test-adapter'; function textFor(selector, context) { return context.querySelector(selector).textContent.trim(); @@ -96,6 +96,7 @@ function getRenderTree() { } module('Component Tab', function (hooks) { + setupTestAdapter(hooks); setupApplicationTest(hooks); hooks.beforeEach(function() { diff --git a/tests/acceptance/container-test.js b/tests/acceptance/container-test.js index 2a3b229096..5440c829d5 100644 --- a/tests/acceptance/container-test.js +++ b/tests/acceptance/container-test.js @@ -8,7 +8,7 @@ import { fillIn, currentURL } from 'ember-test-helpers'; -import { respondWith } from '../test-adapter'; +import { setupTestAdapter, respondWith } from '../test-adapter'; function getTypes() { return [ @@ -39,6 +39,7 @@ function getControllers() { } module('Container Tab', function(outer) { + setupTestAdapter(outer); setupApplicationTest(outer); module('With default types', function(inner) { diff --git a/tests/acceptance/data-test.js b/tests/acceptance/data-test.js index 01893ebe13..229f8a1ad6 100644 --- a/tests/acceptance/data-test.js +++ b/tests/acceptance/data-test.js @@ -7,7 +7,7 @@ import { } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import { respondWith, sendMessage } from '../test-adapter'; +import { setupTestAdapter, respondWith, sendMessage } from '../test-adapter'; function getFilters() { return [{ name: 'isNew', desc: 'New' }]; @@ -59,6 +59,7 @@ function getRecords(type) { } module('Data Tab', function(outer) { + setupTestAdapter(outer); setupApplicationTest(outer); module('Model Types', function(inner) { diff --git a/tests/acceptance/deprecation-test.js b/tests/acceptance/deprecation-test.js index 566a7ee9d3..9de6de85a9 100644 --- a/tests/acceptance/deprecation-test.js +++ b/tests/acceptance/deprecation-test.js @@ -1,7 +1,7 @@ import { visit, findAll, fillIn, click } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import { enableOpenResource, respondWith, expectOpenResource, disableDefaultResponseFor } from '../test-adapter'; +import { setupTestAdapter, enableOpenResource, respondWith, expectOpenResource, disableDefaultResponseFor } from '../test-adapter'; /* Toggling the source can be done by clicking the @@ -53,6 +53,7 @@ function deprecationsWithSource() { } module('Deprecation Tab', function(outer) { + setupTestAdapter(outer); setupApplicationTest(outer); outer.beforeEach(function() { diff --git a/tests/acceptance/info-test.js b/tests/acceptance/info-test.js index 29f1063a2e..4aaf9a78e4 100644 --- a/tests/acceptance/info-test.js +++ b/tests/acceptance/info-test.js @@ -2,9 +2,10 @@ import { visit, findAll } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import config from 'ember-inspector/config/environment'; -import { respondWith } from '../test-adapter'; +import { setupTestAdapter, respondWith } from '../test-adapter'; module('Info Tab', function(hooks) { + setupTestAdapter(hooks); setupApplicationTest(hooks); test("Libraries are displayed correctly", async function(assert) { diff --git a/tests/acceptance/object-inspector-test.js b/tests/acceptance/object-inspector-test.js index 41b20218ab..320a856f4c 100644 --- a/tests/acceptance/object-inspector-test.js +++ b/tests/acceptance/object-inspector-test.js @@ -8,7 +8,7 @@ import { } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import { respondWith, sendMessage } from '../test-adapter'; +import { setupTestAdapter, respondWith, sendMessage } from '../test-adapter'; function objectFactory(props) { return { @@ -69,6 +69,7 @@ function objectToInspect() { } module('Object Inspector', function(hooks) { + setupTestAdapter(hooks); setupApplicationTest(hooks); test("The object displays correctly", async function (assert) { diff --git a/tests/acceptance/promise-test.js b/tests/acceptance/promise-test.js index 7642c66a9c..24693e98d8 100644 --- a/tests/acceptance/promise-test.js +++ b/tests/acceptance/promise-test.js @@ -8,7 +8,7 @@ import { } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import { respondWith, sendMessage } from '../test-adapter'; +import { setupTestAdapter, respondWith, sendMessage } from '../test-adapter'; let guids = 0; @@ -28,6 +28,7 @@ function generatePromise(props) { } module('Promise Tab', function(outer) { + setupTestAdapter(outer); setupApplicationTest(outer); outer.beforeEach(function() { diff --git a/tests/acceptance/render-tree-test.js b/tests/acceptance/render-tree-test.js index 8a3e7f7ab9..eff71f2efd 100644 --- a/tests/acceptance/render-tree-test.js +++ b/tests/acceptance/render-tree-test.js @@ -1,7 +1,7 @@ import { visit, findAll, click, fillIn } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import { respondWith } from '../test-adapter'; +import { setupTestAdapter, respondWith } from '../test-adapter'; function generateProfiles() { return [ @@ -27,6 +27,7 @@ function generateProfiles() { } module('Render Tree Tab', function(outer) { + setupTestAdapter(outer); setupApplicationTest(outer); outer.afterEach(function() { diff --git a/tests/acceptance/route-tree-test.js b/tests/acceptance/route-tree-test.js index 462951582f..8f331dfea6 100644 --- a/tests/acceptance/route-tree-test.js +++ b/tests/acceptance/route-tree-test.js @@ -8,7 +8,7 @@ import { } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; -import { respondWith, sendMessage } from '../test-adapter'; +import { setupTestAdapter, respondWith, sendMessage } from '../test-adapter'; function isObject(item) { return (item && typeof item === 'object' && !Array.isArray(item)); @@ -75,6 +75,7 @@ function routeTree() { } module('Route Tree Tab', function(outer) { + setupTestAdapter(outer); setupApplicationTest(outer); outer.beforeEach(function() { diff --git a/tests/acceptance/whats-new-test.js b/tests/acceptance/whats-new-test.js index e7614ed226..f6e6f424c4 100644 --- a/tests/acceptance/whats-new-test.js +++ b/tests/acceptance/whats-new-test.js @@ -2,6 +2,8 @@ import { visit, find } from '@ember/test-helpers'; import { module, test } from 'qunit'; import { setupApplicationTest } from 'ember-qunit'; import Pretender from 'pretender'; +import { setupTestAdapter } from '../test-adapter'; + function urlFor(ref) { return `https://raw.githubusercontent.com/emberjs/ember-inspector/${encodeURIComponent(ref)}/CHANGELOG.md`; @@ -42,6 +44,7 @@ function generateContent(master = false) { } module('Whats New', function(outer) { + setupTestAdapter(outer); setupApplicationTest(outer); outer.beforeEach(function() { diff --git a/tests/test-adapter.js b/tests/test-adapter.js index 463cac8abe..18b858563f 100644 --- a/tests/test-adapter.js +++ b/tests/test-adapter.js @@ -8,60 +8,60 @@ let resourcesEnabled = false; let resources = []; let responders = []; -// Some default responders that are part of the normal application boot cycle -QUnit.testStart(() => { - respondWith('check-version', false, { isDefault: true }); - - respondWith('general:applicationBooted', { - type: 'general:applicationBooted', - applicationId: 'my-app', - applicationName: 'My App', - booted: true - }, { isDefault: true }); - - respondWith('app-picker-loaded', { - type: 'apps-loaded', - applicationId: null, - applicationName: null, - apps: [{ +export function setupTestAdapter(hooks) { + // Some default responders that are part of the normal application boot cycle + hooks.beforeEach(function() { + respondWith('check-version', false, { isDefault: true }); + + respondWith('general:applicationBooted', { + type: 'general:applicationBooted', applicationId: 'my-app', - applicationName: 'My App' - }] - }, { isDefault: true }); - - respondWith('app-selected', false, { isDefault: true }); - - respondWith('deprecation:getCount', ({ applicationId, applicationName }) => ({ - type: 'deprecation:count', - applicationId, - applicationName, - count: 0 - }), { isDefault: true }); -}); - -// Ensure all expectations are met and reset the global states -QUnit.testDone(({ failed }) => { - if (failed === 0) { + applicationName: 'My App', + booted: true + }, { isDefault: true }); + + respondWith('app-picker-loaded', { + type: 'apps-loaded', + applicationId: null, + applicationName: null, + apps: [{ + applicationId: 'my-app', + applicationName: 'My App' + }] + }, { isDefault: true }); + + respondWith('app-selected', false, { isDefault: true }); + + respondWith('deprecation:getCount', ({ applicationId, applicationName }) => ({ + type: 'deprecation:count', + applicationId, + applicationName, + count: 0 + }), { isDefault: true }); + }); + + // Ensure all expectations are met and reset the global states + hooks.afterEach(function(assert) { for (let { file, line, actual, expected, reject } of resources) { if (!isNaN(expected) && actual !== expected) { - QUnit.assert.strictEqual(actual, expected, `Expceting resouce ${file}:${line} to be opened ${expected} time(s)`); + assert.strictEqual(actual, expected, `Expceting resouce ${file}:${line} to be opened ${expected} time(s)`); reject(`Expceting resouce ${file}:${line} to be opened ${expected} time(s), was opened ${actual} time(s)`); } } for (let { type, isDefault, actual, expected, reject } of responders) { if (!isDefault && !isNaN(expected) && actual !== expected) { - QUnit.assert.strictEqual(actual, expected, `The correct amount of ${type} messages are sent`); + assert.strictEqual(actual, expected, `The correct amount of ${type} messages are sent`); reject(`Expecting ${expected} ${type} messages, got ${actual}`); } } - } - adapter = null; - resourcesEnabled = false; - resources.length = 0; - responders.length = 0; -}); + adapter = null; + resourcesEnabled = false; + resources.length = 0; + responders.length = 0; + }); +} /** * Allow `openResouce` to be called on the adapter.