From 7c2f80067a108cdac8110c9229f8b2b9365ca25e Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 8 Jul 2021 16:25:16 +0300 Subject: [PATCH] test(service-worker): better align mock client implementations with actual implementations (#42736) This commit better aligns the mock client implementations used in ServiceWorker tests (and the associated typings) with the actual implementations (and the official TypeScript typings). This allows verifying the ServiceWorker behavior in a slightly more realistic environment. This is in preparation of switching from our custom typings to the official TypeScript typings (`lib.webworker.d.ts`). PR Close #42736 --- .../service-worker/test/integration_spec.ts | 2 +- packages/service-worker/worker/src/driver.ts | 4 +- .../worker/src/service-worker.d.ts | 20 +++--- .../service-worker/worker/test/happy_spec.ts | 55 ++++++++------- .../service-worker/worker/testing/clients.ts | 67 ++++++++++++++----- .../service-worker/worker/testing/scope.ts | 23 ++++--- 6 files changed, 104 insertions(+), 67 deletions(-) diff --git a/packages/service-worker/test/integration_spec.ts b/packages/service-worker/test/integration_spec.ts index 505a0e855d..6f07246906 100644 --- a/packages/service-worker/test/integration_spec.ts +++ b/packages/service-worker/test/integration_spec.ts @@ -89,7 +89,7 @@ describe('ngsw + companion lib', () => { scope = new SwTestHarnessBuilder().withServerState(server).build(); driver = new Driver(scope, scope, new CacheDatabase(scope)); - scope.clients.add('default'); + scope.clients.add('default', scope.registration.scope); scope.clients.getMock('default')!.queue.subscribe(msg => { mock.sendMessage(msg); }); diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index cca26ae408..542dea8157 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -673,7 +673,7 @@ export class Driver implements Debuggable, UpdateSource { const client = await this.scope.clients.get(clientId); - await this.updateClient(client); + await this.updateClient(client!); appVersion = this.lookupVersionByHash(this.latestHash, 'assignVersion'); } @@ -1050,7 +1050,7 @@ export class Driver implements Debuggable, UpdateSource { await Promise.all(affectedClients.map(async clientId => { const client = await this.scope.clients.get(clientId); - client.postMessage({type: 'UNRECOVERABLE_STATE', reason}); + client!.postMessage({type: 'UNRECOVERABLE_STATE', reason}); })); } diff --git a/packages/service-worker/worker/src/service-worker.d.ts b/packages/service-worker/worker/src/service-worker.d.ts index f01adabfe6..976a4a70f0 100644 --- a/packages/service-worker/worker/src/service-worker.d.ts +++ b/packages/service-worker/worker/src/service-worker.d.ts @@ -29,15 +29,16 @@ interface ExtendableEvent extends Event { // Client API declare class Client { - frameType: ClientFrameType; - id: string; - url: string; - postMessage(message: any): void; + readonly frameType: FrameType; + readonly id: string; + readonly type: ClientTypes; + readonly url: string; + postMessage(message: any): void; } interface Clients { claim(): Promise; - get(id: string): Promise; + get(id: string): Promise; matchAll( options?: T ): Promise>; @@ -46,20 +47,19 @@ interface Clients { interface ClientMatchOptions { includeUncontrolled?: boolean; - type?: ClientMatchTypes; + type?: ClientTypes; } interface WindowClient extends Client { - readonly ancestorOrigins: ReadonlyArray; readonly focused: boolean; readonly visibilityState: VisibilityState; focus(): Promise; navigate(url: string): Promise; } -type ClientFrameType = 'auxiliary'|'top-level'|'nested'|'none'; -type ClientMatchTypes = 'window'|'worker'|'sharedworker'|'all'; -type WindowClientState = 'hidden'|'visible'|'prerender'|'unloaded'; +type FrameType = 'auxiliary'|'top-level'|'nested'|'none'; +type ClientTypes = 'window'|'worker'|'sharedworker'|'all'; +type VisibilityState = 'hidden'|'visible'; // Fetch API diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index c1bc37ac1f..2280add54a 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -12,7 +12,7 @@ import {Driver, DriverReadyState} from '../src/driver'; import {AssetGroupConfig, DataGroupConfig, Manifest} from '../src/manifest'; import {sha1} from '../src/sha1'; import {clearAllCaches, MockCache} from '../testing/cache'; -import {WindowClientImpl} from '../testing/clients'; +import {MockWindowClient} from '../testing/clients'; import {MockRequest, MockResponse} from '../testing/fetch'; import {MockFileSystem, MockFileSystemBuilder, MockServerState, MockServerStateBuilder, tmpHashTableForFs} from '../testing/mock'; import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; @@ -766,7 +766,7 @@ describe('Driver', () => { await driver.initialized; await scope.handleClick( {title: 'This is a test with action', body: 'Test body with action'}, 'button'); - const message: any = scope.clients.getMock('default')!.messages[0]; + const message = scope.clients.getMock('default')!.messages[0]; expect(message.type).toEqual('NOTIFICATION_CLICK'); expect(message.data.action).toEqual('button'); @@ -781,7 +781,7 @@ describe('Driver', () => { title: 'This is a test without action', body: 'Test body without action', }); - const message: any = scope.clients.getMock('default')!.messages[0]; + const message = scope.clients.getMock('default')!.messages[0]; expect(message.type).toEqual('NOTIFICATION_CLICK'); expect(message.data.action).toBe(''); @@ -837,12 +837,14 @@ describe('Driver', () => { describe('`focusLastFocusedOrOpen` operation', () => { it('focuses last client keeping previous url', async () => { expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); - const mockClient = new WindowClientImpl('fooBar'); - spyOn(scope.clients, 'matchAll').and.returnValue(Promise.resolve([mockClient])); - spyOn(mockClient, 'focus'); - spyOn(mockClient, 'navigate'); + + scope.clients.add('fooBar', 'http://localhost/unique', 'window'); + const mockClient = scope.clients.getMock('fooBar') as MockWindowClient; const url = 'foo'; + expect(mockClient.url).toBe('http://localhost/unique'); + expect(mockClient.focused).toBeFalse(); + await driver.initialized; await scope.handleClick( { @@ -855,9 +857,8 @@ describe('Driver', () => { }, }, 'foo'); - expect(mockClient.navigate).not.toHaveBeenCalled(); - expect(mockClient.url).toEqual('http://localhost/unique'); - expect(mockClient.focus).toHaveBeenCalled(); + expect(mockClient.url).toBe('http://localhost/unique'); + expect(mockClient.focused).toBeTrue(); }); it('falls back to openWindow at url when no last client to focus', async () => { @@ -905,13 +906,15 @@ describe('Driver', () => { describe('`navigateLastFocusedOrOpen` operation', () => { it('navigates last client to `url`', async () => { - expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); - const mockClient = new WindowClientImpl('fooBar'); - spyOn(scope.clients, 'matchAll').and.returnValue(Promise.resolve([mockClient])); - spyOn(mockClient, 'focus'); - spyOn(mockClient, 'navigate').and.returnValue(Promise.resolve(mockClient)); + expect(await makeRequest(scope, '/foo.txt')).toBe('this is foo'); + + scope.clients.add('fooBar', 'http://localhost/unique', 'window'); + const mockClient = scope.clients.getMock('fooBar') as MockWindowClient; const url = 'foo'; + expect(mockClient.url).toBe('http://localhost/unique'); + expect(mockClient.focused).toBeFalse(); + await driver.initialized; await scope.handleClick( { @@ -924,16 +927,18 @@ describe('Driver', () => { }, }, 'foo'); - expect(mockClient.navigate).toHaveBeenCalledWith(`${scope.registration.scope}${url}`); - expect(mockClient.focus).toHaveBeenCalled(); + expect(mockClient.url).toBe(`${scope.registration.scope}${url}`); + expect(mockClient.focused).toBeTrue(); }); - it('navigates last client to `/` if no `url', async () => { - expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); - const mockClient = new WindowClientImpl('fooBar'); - spyOn(scope.clients, 'matchAll').and.returnValue(Promise.resolve([mockClient])); - spyOn(mockClient, 'focus'); - spyOn(mockClient, 'navigate').and.returnValue(Promise.resolve(mockClient)); + it('navigates last client to `/` if no `url`', async () => { + expect(await makeRequest(scope, '/foo.txt')).toBe('this is foo'); + + scope.clients.add('fooBar', 'http://localhost/unique', 'window'); + const mockClient = scope.clients.getMock('fooBar') as MockWindowClient; + + expect(mockClient.url).toBe('http://localhost/unique'); + expect(mockClient.focused).toBeFalse(); await driver.initialized; await scope.handleClick( @@ -947,8 +952,8 @@ describe('Driver', () => { }, }, 'foo'); - expect(mockClient.navigate).toHaveBeenCalledWith(`${scope.registration.scope}`); - expect(mockClient.focus).toHaveBeenCalled(); + expect(mockClient.url).toBe(scope.registration.scope); + expect(mockClient.focused).toBeTrue(); }); it('falls back to openWindow at url when no last client to focus', async () => { diff --git a/packages/service-worker/worker/testing/clients.ts b/packages/service-worker/worker/testing/clients.ts index a525972ab7..cbdda4963f 100644 --- a/packages/service-worker/worker/testing/clients.ts +++ b/packages/service-worker/worker/testing/clients.ts @@ -9,35 +9,39 @@ import {Subject} from 'rxjs'; -export class MockClient { - queue = new Subject(); +export class MockClient implements Client { + readonly messages: any[] = []; + readonly queue = new Subject(); + lastFocusedAt = 0; - constructor(readonly id: string) {} + constructor( + readonly id: string, readonly url: string, readonly type: ClientTypes = 'all', + readonly frameType: FrameType = 'top-level') {} - readonly messages: Object[] = []; - - postMessage(message: Object): void { + postMessage(message: any): void { this.messages.push(message); this.queue.next(message); } } -export class WindowClientImpl extends MockClient implements WindowClient { - readonly ancestorOrigins: ReadonlyArray = []; +export class MockWindowClient extends MockClient implements WindowClient { readonly focused: boolean = false; - readonly visibilityState: VisibilityState = 'hidden'; - frameType: ClientFrameType = 'top-level'; - url = 'http://localhost/unique'; + readonly visibilityState: VisibilityState = 'visible'; - constructor(readonly id: string) { - super(id); + constructor(id: string, url: string, frameType: FrameType = 'top-level') { + super(id, url, 'window', frameType); } async focus(): Promise { + // This is only used for relatively ordering clients based on focus order, so we don't need to + // use `Adapter#time`. + this.lastFocusedAt = Date.now(); + (this.focused as boolean) = true; return this; } async navigate(url: string): Promise { + (this.url as string) = url; return this; } } @@ -45,11 +49,20 @@ export class WindowClientImpl extends MockClient implements WindowClient { export class MockClients implements Clients { private clients = new Map(); - add(clientId: string): void { + add(clientId: string, url: string, type: ClientTypes = 'window'): void { if (this.clients.has(clientId)) { - return; + const existingClient = this.clients.get(clientId)!; + if (existingClient.url === url) { + return; + } + throw new Error( + `Trying to add mock client with same ID (${existingClient.id}) and different URL ` + + `(${existingClient.url} --> ${url})`); } - this.clients.set(clientId, new MockClient(clientId)); + + const client = (type === 'window') ? new MockWindowClient(clientId, url) : + new MockClient(clientId, url, type); + this.clients.set(clientId, client); } remove(clientId: string): void { @@ -57,7 +70,7 @@ export class MockClients implements Clients { } async get(id: string): Promise { - return this.clients.get(id)! as any as Client; + return this.clients.get(id)!; } getMock(id: string): MockClient|undefined { @@ -66,7 +79,25 @@ export class MockClients implements Clients { async matchAll(options?: T): Promise> { - return Array.from(this.clients.values()) as any[]; + const type = options?.type ?? 'window'; + const allClients = Array.from(this.clients.values()); + const matchedClients = + (type === 'all') ? allClients : allClients.filter(client => client.type === type); + + // Order clients according to the [spec](https://w3c.github.io/ServiceWorker/#clients-matchall): + // In most recently focused then most recently created order, with windows clients before other + // clients. + return matchedClients + // Sort in most recently created order. + .reverse() + // Sort in most recently focused order. + .sort((a, b) => b.lastFocusedAt - a.lastFocusedAt) + // Sort windows clients before other clients (otherwise leave existing order). + .sort((a, b) => { + const aScore = (a.type === 'window') ? 1 : 0; + const bScore = (b.type === 'window') ? 1 : 0; + return bScore - aScore; + }) as any; } async openWindow(url: string): Promise { diff --git a/packages/service-worker/worker/testing/scope.ts b/packages/service-worker/worker/testing/scope.ts index ddf6dd9a31..5acf62e354 100644 --- a/packages/service-worker/worker/testing/scope.ts +++ b/packages/service-worker/worker/testing/scope.ts @@ -164,14 +164,15 @@ export class SwTestHarness extends Adapter implements ServiceW } const isNavigation = req.mode === 'navigate'; + + if (clientId && !this.clients.getMock(clientId)) { + this.clients.add(clientId, isNavigation ? req.url : this.scopeUrl); + } + const event = isNavigation ? new MockFetchEvent(req, '', clientId) : new MockFetchEvent(req, clientId, ''); this.eventHandlers.get('fetch')!.call(this, event); - if (clientId) { - this.clients.add(clientId); - } - return [event.response, event.ready]; } @@ -179,15 +180,15 @@ export class SwTestHarness extends Adapter implements ServiceW if (!this.eventHandlers.has('message')) { throw new Error('No message handler registered'); } - let event: MockExtendableMessageEvent; - if (clientId === null) { - event = new MockExtendableMessageEvent(data, null); - } else { - this.clients.add(clientId); - event = new MockExtendableMessageEvent( - data, this.clients.getMock(clientId) as unknown as Client || null); + + if (clientId && !this.clients.getMock(clientId)) { + this.clients.add(clientId, this.scopeUrl); } + + const event = + new MockExtendableMessageEvent(data, clientId && this.clients.getMock(clientId) || null); this.eventHandlers.get('message')!.call(this, event); + return event.ready; }