From 863e81bc01cfd4c0e763e0f23777862a920ea455 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Sat, 8 May 2021 12:08:56 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20=F0=9F=90=9B=20migrate=20IPC=20to=20?= =?UTF-8?q?service-based=20window=20for=20reliable=20testing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This resolves a longstanding issue where IPC tests rely on the real window.postMessage and MessageChannel APIs. These are evented and asynchronous, resulting in nondeterministic tests. This commit updates tests with synchronous mocked APIs. --- ui/desktop/app/services/ipc.js | 17 ++--- ui/desktop/tests/acceptance/origin-test.js | 38 ++--------- ui/desktop/tests/helpers/window-mock-ipc.js | 75 +++++++++++++++++++++ ui/desktop/tests/unit/services/ipc-test.js | 20 ++++++ 4 files changed, 105 insertions(+), 45 deletions(-) create mode 100644 ui/desktop/tests/helpers/window-mock-ipc.js diff --git a/ui/desktop/app/services/ipc.js b/ui/desktop/app/services/ipc.js index 8be63359f..e7766c302 100644 --- a/ui/desktop/app/services/ipc.js +++ b/ui/desktop/app/services/ipc.js @@ -1,5 +1,5 @@ import Service from '@ember/service'; -import { getOwner } from '@ember/application'; +import { inject as service } from '@ember/service'; import { Promise } from 'rsvp'; /** @@ -58,7 +58,7 @@ export class IPCRequest { this.method = method; this.payload = payload; this.origin = window?.location?.origin; - this.#channel = new MessageChannel(); + this.#channel = new window.MessageChannel(); this.#promise = new Promise((resolve, reject) => { this.#channel.port1.onmessage = (event) => { this.close(); @@ -124,18 +124,9 @@ export class IPCRequest { * } */ export default class IpcService extends Service { - // =attributes + // =services - /** - * Looks up the window object indirectly. - * @type {Window} - */ - get window() { - // The Ember way of accessing globals... - const document = getOwner(this).lookup('service:-document').documentElement; - // defaultView === window, but without using globals directly - return document.parentNode.defaultView; - } + @service('browser/window') window; // =methods diff --git a/ui/desktop/tests/acceptance/origin-test.js b/ui/desktop/tests/acceptance/origin-test.js index 7181d33de..eddad56bb 100644 --- a/ui/desktop/tests/acceptance/origin-test.js +++ b/ui/desktop/tests/acceptance/origin-test.js @@ -19,15 +19,17 @@ import { // authenticateSession, invalidateSession, } from 'ember-simple-auth/test-support'; +import { setupBrowserFakes } from 'ember-browser-services/test-support'; +import WindowMockIPC from '../helpers/window-mock-ipc'; import config from '../../config/environment'; module('Acceptance | origin', function (hooks) { setupApplicationTest(hooks); + setupBrowserFakes(hooks, { window: true }); setupMirage(hooks); const currentOrigin = window.location.origin; let mockIPC; - let messageHandler; const instances = { scopes: { @@ -104,40 +106,12 @@ module('Acceptance | origin', function (hooks) { urls.projects = `${urls.scopes.global}/projects`; urls.targets = `${urls.projects}/targets`; - class MockIPC { - origin = null; - - invoke(method, payload) { - return this[method](payload); - } - - getOrigin() { - return this.origin; - } - - setOrigin(origin) { - this.origin = origin; - return this.origin; - } - - resetOrigin() {} - } - - mockIPC = new MockIPC(); - messageHandler = async function (event) { - if (event.origin !== currentOrigin) return; - const { method, payload } = event.data; - if (method) { - const response = await mockIPC.invoke(method, payload); - event.ports[0].postMessage(response); - } - }; - - window.addEventListener('message', messageHandler); + // Mock the postMessage interface used by IPC. + this.owner.register('service:browser/window', WindowMockIPC); + mockIPC = this.owner.lookup('service:browser/window').mockIPC; }); hooks.afterEach(function () { - window.removeEventListener('message', messageHandler); sinon.restore(); }); diff --git a/ui/desktop/tests/helpers/window-mock-ipc.js b/ui/desktop/tests/helpers/window-mock-ipc.js new file mode 100644 index 000000000..0eb9535fc --- /dev/null +++ b/ui/desktop/tests/helpers/window-mock-ipc.js @@ -0,0 +1,75 @@ +import Service from '@ember/service'; + +class MockIPC { + origin = null; + + invoke(method, payload) { + return this[method](payload); + } + + getOrigin() { + return this.origin; + } + + setOrigin(origin) { + this.origin = origin; + return this.origin; + } + + resetOrigin() {} +} + +/** + * Mock window service with support for simplified postMessage and + * MessageChannel APIs. This mock implements just enough functionality + * to support the IPC mechanism. + * + * This mock is necessary to guarantee synchronous and deterministic + * operation. The window's own API is evented and asychronous, which + * cannot be relied on for testing purposes. + * + * @todo refactor mock postMessage and MessageChannel for reusability + * outside of tests + */ +export default class extends Service { + // =attributes + + /** + * + */ + mockIPC = new MockIPC(); + + /** + * + */ + MessageChannel = class MessageChannel { + constructor() { + this.port1.postMessage = this.port1.postMessage.bind(this); + this.port2.postMessage = this.port2.postMessage.bind(this); + } + port1 = { + postMessage() {}, + close() {}, + }; + port2 = { + postMessage(data) { + this.port1.onmessage({ data }); + }, + close() {}, + }; + }; + + // =methods + + /** + * + */ + postMessage(data, origin, ports) { + if (origin !== window.location.origin) return; + const { method, payload } = data; + if (method) { + const response = this.mockIPC.invoke(method, payload); + ports[0].postMessage(response); + } + } +} diff --git a/ui/desktop/tests/unit/services/ipc-test.js b/ui/desktop/tests/unit/services/ipc-test.js index a892ccccc..d193074c5 100644 --- a/ui/desktop/tests/unit/services/ipc-test.js +++ b/ui/desktop/tests/unit/services/ipc-test.js @@ -6,6 +6,23 @@ import sinon from 'sinon'; module('Unit | Service | ipc', function (hooks) { setupTest(hooks); + class MessageChannelMock { + constructor() { + this.port1.postMessage = this.port1.postMessage.bind(this); + this.port2.postMessage = this.port2.postMessage.bind(this); + } + port1 = { + postMessage() {}, + close() {}, + }; + port2 = { + postMessage(data) { + this.port1.onmessage({ data }); + }, + close() {}, + }; + } + hooks.beforeEach(() => { // IPC requests talk to the main process indirectly via window.postMessage // proxy layer (mediated by preload.js). @@ -31,6 +48,7 @@ module('Unit | Service | ipc', function (hooks) { assert.equal(request.method, 'hello'); assert.equal(request.payload, 'world'); }, + MessageChannel: MessageChannelMock, }; new IPCRequest('hello', 'world', myWindow); }); @@ -45,6 +63,7 @@ module('Unit | Service | ipc', function (hooks) { test: 42, }); }, + MessageChannel: MessageChannelMock, }; const request = new IPCRequest('hello', 'world', myWindow); const response = await request; @@ -65,6 +84,7 @@ module('Unit | Service | ipc', function (hooks) { // Post back an error on the response port port.postMessage(new Error(stringifiedPayload)); }, + MessageChannel: MessageChannelMock, }; try { await new IPCRequest('hello', 'world', myWindow); From 85ddd7fc4b742b656535412cb08a8ff33d490205 Mon Sep 17 00:00:00 2001 From: Randall Morey Date: Sat, 8 May 2021 12:10:41 -0400 Subject: [PATCH 2/2] chore: remove empty comments --- ui/desktop/tests/helpers/window-mock-ipc.js | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ui/desktop/tests/helpers/window-mock-ipc.js b/ui/desktop/tests/helpers/window-mock-ipc.js index 0eb9535fc..9d191d87a 100644 --- a/ui/desktop/tests/helpers/window-mock-ipc.js +++ b/ui/desktop/tests/helpers/window-mock-ipc.js @@ -34,14 +34,8 @@ class MockIPC { export default class extends Service { // =attributes - /** - * - */ mockIPC = new MockIPC(); - /** - * - */ MessageChannel = class MessageChannel { constructor() { this.port1.postMessage = this.port1.postMessage.bind(this); @@ -61,9 +55,6 @@ export default class extends Service { // =methods - /** - * - */ postMessage(data, origin, ports) { if (origin !== window.location.origin) return; const { method, payload } = data;