From 794cd2e328c359c9df96b2132d471234bafc7d0c Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 16 Dec 2019 16:23:29 -0800 Subject: [PATCH 01/22] tests(driver): type check --- lighthouse-core/test/gather/driver-test.js | 139 ++++++++++++------- lighthouse-core/test/gather/fake-driver.js | 8 +- lighthouse-core/test/gather/mock-commands.js | 107 +++++++++----- tsconfig.json | 6 +- types/jest.d.ts | 19 +++ 5 files changed, 188 insertions(+), 91 deletions(-) create mode 100644 types/jest.d.ts diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 6fb7d7854c03..e326417a84c6 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -8,7 +8,6 @@ const Driver = require('../../gather/driver.js'); const Connection = require('../../gather/connections/connection.js'); const LHElement = require('../../lib/lh-element.js'); -const EventEmitter = require('events').EventEmitter; const {protocolGetVersionResponse} = require('./fake-driver.js'); const {createMockSendCommandFn, createMockOnceFn} = require('./mock-commands.js'); @@ -23,12 +22,13 @@ jest.useFakeTimers(); * * @template T * @param {Promise} promise - * @return {Promise & {isDone: () => boolean, isResolved: () => boolean, isRejected: () => boolean}} */ function makePromiseInspectable(promise) { let isResolved = false; let isRejected = false; + /** @type {T=} */ let resolvedValue = undefined; + /** @type {any=} */ let rejectionError = undefined; const inspectablePromise = promise.then(value => { isResolved = true; @@ -40,12 +40,20 @@ function makePromiseInspectable(promise) { throw err; }); - inspectablePromise.isDone = () => isResolved || isRejected; - inspectablePromise.isResolved = () => isResolved; - inspectablePromise.isRejected = () => isRejected; - inspectablePromise.getDebugValues = () => ({resolvedValue, rejectionError}); - - return inspectablePromise; + return Object.assign(inspectablePromise, { + isDone() { + return isResolved || isRejected; + }, + isResolved() { + return isResolved; + }, + isRejected() { + return isRejected; + }, + getDebugValues() { + return {resolvedValue, rejectionError}; + }, + }); } expect.extend({ @@ -53,7 +61,7 @@ expect.extend({ * Asserts that an inspectable promise created by makePromiseInspectable is currently resolved or rejected. * This is useful for situations where we want to test that we are actually waiting for a particular event. * - * @param {ReturnType} received + * @param {ReturnType} received * @param {string} failureMessage */ toBeDone(received, failureMessage) { @@ -81,14 +89,19 @@ async function flushAllTimersAndMicrotasks() { } } +/** @type {Driver & {on: ReturnType, once: ReturnType}} */ let driver; +/** @type {Connection & {sendCommand: ReturnType}} */ let connectionStub; beforeEach(() => { + // @ts-ignore connectionStub = new Connection(); + // @ts-ignore connectionStub.sendCommand = cmd => { throw new Error(`${cmd} not implemented`); }; + // @ts-ignore driver = new Driver(connectionStub); }); @@ -104,7 +117,7 @@ describe('.querySelector(All)', () => { it('returns element instance when DOM.querySelector finds a node', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('DOM.getDocument', {root: {nodeId: 249}}) + .mockResponse('DOM.getDocument', {root: {nodeId: 249}}) .mockResponse('DOM.querySelector', {nodeId: 231}); const result = await driver.querySelector('meta head'); @@ -113,7 +126,7 @@ describe('.querySelector(All)', () => { it('returns [] when DOM.querySelectorAll finds no node', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('DOM.getDocument', {root: {nodeId: 249}}) + .mockResponse('DOM.getDocument', {root: {nodeId: 249}}) .mockResponse('DOM.querySelectorAll', {nodeIds: []}); const result = await driver.querySelectorAll('#no.matches'); @@ -122,7 +135,7 @@ describe('.querySelector(All)', () => { it('returns element when DOM.querySelectorAll finds node', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('DOM.getDocument', {root: {nodeId: 249}}) + .mockResponse('DOM.getDocument', {root: {nodeId: 249}}) .mockResponse('DOM.querySelectorAll', {nodeIds: [231]}); const result = await driver.querySelectorAll('#no.matches'); @@ -226,7 +239,7 @@ describe('.evaluateAsync', () => { it('evaluates an expression in isolation', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: 1337}}}) + .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1337'}}}) .mockResponse('Page.createIsolatedWorld', {executionContextId: 1}) .mockResponse('Runtime.evaluate', {result: {value: 2}}); @@ -242,8 +255,7 @@ describe('.evaluateAsync', () => { expect(evaluateArgs).toMatchObject({contextId: 1}); // Make sure we cached the isolated context from last time - connectionStub.sendCommand = createMockSendCommandFn().mockResponse( - 'Runtime.evaluate', + connectionStub.sendCommand = createMockSendCommandFn().mockResponse('Runtime.evaluate', {result: {value: 2}} ); await driver.evaluateAsync('1 + 1', {useIsolation: true}); @@ -255,7 +267,7 @@ describe('.evaluateAsync', () => { it('recovers from isolation failures', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: 1337}}}) + .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1337'}}}) .mockResponse('Page.createIsolatedWorld', {executionContextId: 9001}) .mockResponse('Runtime.evaluate', Promise.reject(new Error('Cannot find context'))) .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: 1337}}}) @@ -291,8 +303,8 @@ describe('.beginTrace', () => { beforeEach(() => { connectionStub.sendCommand = createMockSendCommandFn() .mockResponse('Browser.getVersion', protocolGetVersionResponse) - .mockResponse('Page.enable', {}) - .mockResponse('Tracing.start', {}); + .mockResponse('Page.enable') + .mockResponse('Tracing.start'); }); it('will request default traceCategories', async () => { @@ -348,10 +360,9 @@ describe('.setExtraHTTPHeaders', () => { ); }); - it('should Network.setExtraHTTPHeaders when there are extra-headers', async () => { + it('should not call Network.setExtraHTTPHeaders when there are not extra-headers', async () => { connectionStub.sendCommand = createMockSendCommandFn(); - await driver.setExtraHTTPHeaders(); - + await driver.setExtraHTTPHeaders(null); expect(connectionStub.sendCommand).not.toHaveBeenCalled(); }); }); @@ -431,19 +442,19 @@ describe('.gotoURL', () => { beforeEach(() => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Network.enable', {}) - .mockResponse('Page.enable', {}) - .mockResponse('Page.setLifecycleEventsEnabled', {}) - .mockResponse('Emulation.setScriptExecutionDisabled', {}) - .mockResponse('Page.navigate', {}) - .mockResponse('Target.setAutoAttach', {}) - .mockResponse('Runtime.evaluate', {}); + .mockResponse('Network.enable') + .mockResponse('Page.enable') + .mockResponse('Page.setLifecycleEventsEnabled') + .mockResponse('Emulation.setScriptExecutionDisabled') + .mockResponse('Page.navigate') + .mockResponse('Target.setAutoAttach') + .mockResponse('Runtime.evaluate'); }); it('will track redirects through gotoURL load', async () => { const delay = _ => new Promise(resolve => setTimeout(resolve)); - class ReplayConnection extends EventEmitter { + class ReplayConnection extends Connection { connect() { return Promise.resolve(); } @@ -451,9 +462,14 @@ describe('.gotoURL', () => { return Promise.resolve(); } replayLog() { - redirectDevtoolsLog.forEach(msg => this.emit('protocolevent', msg)); + // @ts-ignore + redirectDevtoolsLog.forEach(msg => this.emitProtocolEvent(msg)); } - sendCommand(method) { + /** + * @param {string} method + * @param {any} _ + */ + sendCommand(method, _) { const resolve = Promise.resolve(); // If navigating, wait, then replay devtools log in parallel to resolve. @@ -524,6 +540,7 @@ describe('.gotoURL', () => { driver._waitForNetworkIdle = createMockWaitForFn(); driver._waitForCPUIdle = createMockWaitForFn(); + // @ts-ignore const waitForResult = driver[`_waitFor${name}`]; const otherWaitForResults = [ driver._waitForFCP, @@ -587,6 +604,7 @@ describe('.gotoURL', () => { driver._waitForNetworkIdle = createMockWaitForFn(); driver._waitForCPUIdle = createMockWaitForFn(); + // @ts-ignore const loadPromise = makePromiseInspectable(driver.gotoURL(url, { waitForLoad: true, passContext: { @@ -629,6 +647,7 @@ describe('.gotoURL', () => { describe('._waitForFCP', () => { it('should not resolve until FCP fires', async () => { + // @ts-ignore driver.on = driver.once = createMockOnceFn(); const waitPromise = makePromiseInspectable(driver._waitForFCP(60 * 1000).promise); @@ -648,6 +667,7 @@ describe('._waitForFCP', () => { }); it('should timeout', async () => { + // @ts-ignore driver.on = driver.once = createMockOnceFn(); const waitPromise = makePromiseInspectable(driver._waitForFCP(5000).promise); @@ -662,6 +682,7 @@ describe('._waitForFCP', () => { }); it('should be cancellable', async () => { + // @ts-ignore driver.on = driver.once = createMockOnceFn(); driver.off = jest.fn(); @@ -682,23 +703,34 @@ describe('._waitForFCP', () => { describe('.assertNoSameOriginServiceWorkerClients', () => { beforeEach(() => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('ServiceWorker.enable', {}) - .mockResponse('ServiceWorker.disable', {}) - .mockResponse('ServiceWorker.enable', {}) - .mockResponse('ServiceWorker.disable', {}); + .mockResponse('ServiceWorker.enable') + .mockResponse('ServiceWorker.disable') + .mockResponse('ServiceWorker.enable') + .mockResponse('ServiceWorker.disable'); }); + /** + * @param {number} id + * @param {string} url + * @param {boolean=} isDeleted + */ function createSWRegistration(id, url, isDeleted) { return { isDeleted: !!isDeleted, - registrationId: id, + registrationId: String(id), scopeURL: url, }; } + /** + * @param {number} id + * @param {string} url + * @param {string[]} controlledClients + * @param {LH.Crdp.ServiceWorker.ServiceWorkerVersionStatus=} status + */ function createActiveWorker(id, url, controlledClients, status = 'activated') { return { - registrationId: id, + registrationId: String(id), scriptURL: url, controlledClients, status, @@ -801,9 +833,9 @@ describe('.assertNoSameOriginServiceWorkerClients', () => { describe('.goOnline', () => { beforeEach(() => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Network.enable', {}) - .mockResponse('Emulation.setCPUThrottlingRate', {}) - .mockResponse('Network.emulateNetworkConditions', {}); + .mockResponse('Network.enable') + .mockResponse('Emulation.setCPUThrottlingRate') + .mockResponse('Network.emulateNetworkConditions'); }); it('re-establishes previous throttling settings', async () => { @@ -876,24 +908,27 @@ describe('Domain.enable/disable State', () => { connectionStub.sendCommand = createMockSendCommandFn() .mockResponse('Network.enable') .mockResponse('Network.disable') + // @ts-ignore: Fetch is not in protocol typings yet. .mockResponse('Fetch.enable') .mockResponse('Fetch.disable'); - await driver.sendCommand('Network.enable', {}); - await driver.sendCommand('Network.enable', {}); + await driver.sendCommand('Network.enable'); + await driver.sendCommand('Network.enable'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(1); - await driver.sendCommand('Network.disable', {}); + await driver.sendCommand('Network.disable'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(1); // Network still has one enable. + // @ts-ignore: Fetch is not in protocol typings yet. await driver.sendCommand('Fetch.enable', {}); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); - await driver.sendCommand('Network.disable', {}); + await driver.sendCommand('Network.disable'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(3); // Network is now disabled. + // @ts-ignore: Fetch is not in protocol typings yet. await driver.sendCommand('Fetch.disable', {}); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(4); }); @@ -905,24 +940,24 @@ describe('Domain.enable/disable State', () => { .mockResponse('Network.disable') .mockResponseToSession('Network.disable', '123'); - await driver.sendCommand('Network.enable', {}); - await driver.sendCommandToSession('Network.enable', '123', {}); + await driver.sendCommand('Network.enable'); + await driver.sendCommandToSession('Network.enable', '123'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); - await driver.sendCommand('Network.enable', {}); - await driver.sendCommandToSession('Network.enable', '123', {}); + await driver.sendCommand('Network.enable'); + await driver.sendCommandToSession('Network.enable', '123'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); - await driver.sendCommandToSession('Network.disable', '123', {}); + await driver.sendCommandToSession('Network.disable', '123'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); - await driver.sendCommand('Network.disable', {}); + await driver.sendCommand('Network.disable'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); - await driver.sendCommandToSession('Network.disable', '123', {}); + await driver.sendCommandToSession('Network.disable', '123'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(3); - await driver.sendCommand('Network.disable', {}); + await driver.sendCommand('Network.disable'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(4); }); }); diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js index 108a40aa6d78..e7720923aa85 100644 --- a/lighthouse-core/test/gather/fake-driver.js +++ b/lighthouse-core/test/gather/fake-driver.js @@ -109,6 +109,8 @@ const fakeDriverUsingRealMobileDevice = makeFakeDriver({ }, }); -module.exports = fakeDriver; -module.exports.fakeDriverUsingRealMobileDevice = fakeDriverUsingRealMobileDevice; -module.exports.protocolGetVersionResponse = protocolGetVersionResponse; +module.exports = { + ...fakeDriver, + fakeDriverUsingRealMobileDevice, + protocolGetVersionResponse, +}; diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index 773935e04222..0881770cf067 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -22,7 +22,7 @@ */ function createMockSendCommandFn() { const mockResponses = []; - const mockFn = jest.fn().mockImplementation((command, sessionId, ...args) => { + const mockFnImpl = jest.fn().mockImplementation((command, sessionId, ...args) => { const indexOfResponse = mockResponses .findIndex(entry => entry.command === command && entry.sessionId === sessionId); if (indexOfResponse === -1) throw new Error(`${command} unimplemented`); @@ -33,20 +33,37 @@ function createMockSendCommandFn() { return Promise.resolve(returnValue); }); - mockFn.mockResponse = (command, response, delay) => { - mockResponses.push({command, response, delay}); - return mockFn; - }; - - mockFn.mockResponseToSession = (command, sessionId, response, delay) => { - mockResponses.push({command, sessionId, response, delay}); - return mockFn; - }; - - mockFn.findInvocation = (command, sessionId) => { - expect(mockFn).toHaveBeenCalledWith(command, sessionId, expect.anything()); - return mockFn.mock.calls.find(call => call[0] === command && call[1] === sessionId)[2]; - }; + const mockFn = Object.assign(mockFnImpl, { + /** + * @template {keyof LH.CrdpCommands} C + * @param {C} command + * @param {RecursivePartial=} response + * @param {number=} delay + */ + mockResponse(command, response, delay) { + mockResponses.push({command, response, delay}); + return mockFn; + }, + /** + * @template {keyof LH.CrdpCommands} C + * @param {C} command + * @param {string} sessionId + * @param {RecursivePartial=} response + * @param {number=} delay + */ + mockResponseToSession(command, sessionId, response, delay) { + mockResponses.push({command, sessionId, response, delay}); + return mockFn; + }, + /** + * @param {keyof LH.CrdpCommands} command + * @param {string=} sessionId + */ + findInvocation(command, sessionId) { + expect(mockFn).toHaveBeenCalledWith(command, sessionId, expect.anything()); + return mockFn.mock.calls.find(call => call[0] === command && call[1] === sessionId)[2]; + }, + }); return mockFn; } @@ -62,7 +79,7 @@ function createMockSendCommandFn() { */ function createMockOnceFn() { const mockEvents = []; - const mockFn = jest.fn().mockImplementation((eventName, listener) => { + const mockFnImpl = jest.fn().mockImplementation((eventName, listener) => { const indexOfResponse = mockEvents.findIndex(entry => entry.event === eventName); if (indexOfResponse === -1) return; const {response} = mockEvents[indexOfResponse]; @@ -71,15 +88,25 @@ function createMockOnceFn() { setTimeout(() => listener(response), 0); }); - mockFn.mockEvent = (event, response) => { - mockEvents.push({event, response}); - return mockFn; - }; - - mockFn.findListener = event => { - expect(mockFn).toHaveBeenCalledWith(event, expect.anything()); - return mockFn.mock.calls.find(call => call[0] === event)[1]; - }; + const mockFn = Object.assign(mockFnImpl, { + ...mockFnImpl, + /** + * @template {keyof LH.CrdpEvents} E + * @param {E} event + * @param {RecursivePartial} response + */ + mockEvent(event, response) { + mockEvents.push({event, response}); + return mockFn; + }, + /** + * @param {keyof LH.CrdpEvents} event + */ + findListener(event) { + expect(mockFn).toHaveBeenCalledWith(event, expect.anything()); + return mockFn.mock.calls.find(call => call[0] === event)[1]; + }, + }); return mockFn; } @@ -90,7 +117,7 @@ function createMockOnceFn() { */ function createMockOnFn() { const mockEvents = []; - const mockFn = jest.fn().mockImplementation((eventName, listener) => { + const mockFnImpl = jest.fn().mockImplementation((eventName, listener) => { const events = mockEvents.filter(entry => entry.event === eventName); if (!events.length) return; for (const event of events) { @@ -105,15 +132,25 @@ function createMockOnFn() { }, 0); }); - mockFn.mockEvent = (event, response) => { - mockEvents.push({event, response}); - return mockFn; - }; - - mockFn.findListener = event => { - expect(mockFn).toHaveBeenCalledWith(event, expect.anything()); - return mockFn.mock.calls.find(call => call[0] === event)[1]; - }; + const mockFn = Object.assign(mockFnImpl, { + ...mockFnImpl, + /** + * @template {keyof LH.CrdpEvents} E + * @param {E} event + * @param {RecursivePartial} response + */ + mockEvent(event, response) { + mockEvents.push({event, response}); + return mockFn; + }, + /** + * @param {keyof LH.CrdpEvents} event + */ + findListener(event) { + expect(mockFn).toHaveBeenCalledWith(event, expect.anything()); + return mockFn.mock.calls.find(call => call[0] === event)[1]; + }, + }); return mockFn; } diff --git a/tsconfig.json b/tsconfig.json index f1e90c03c392..fe0eecb3da96 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -24,5 +24,9 @@ "lighthouse-core/test/**/*.js", "clients/test/**/*.js", "lighthouse-cli/test/fixtures/**/*.js", - ] + ], + "files": [ + // Opt-in to typechecking for some core tests. + "lighthouse-core/test/gather/driver-test.js", + ], } diff --git a/types/jest.d.ts b/types/jest.d.ts new file mode 100644 index 000000000000..8476a69d29d3 --- /dev/null +++ b/types/jest.d.ts @@ -0,0 +1,19 @@ +/** + * @license Copyright 2019 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ + +declare namespace jest { + interface Matchers { + /** + * Asserts that an inspectable promise created by makePromiseInspectable is currently resolved or rejected. + * This is useful for situations where we want to test that we are actually waiting for a particular event. + */ + toBeDone: (failureMessage?: string) => object; + /** + * Asserts that an i18n string (using en-US) matches an expected pattern. + */ + toBeDisplayString: (pattern: RegExp) => object; + } +} From 3c0cab4f3596e2fb1af1b8e13a5caf874160d589 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 16 Dec 2019 16:28:08 -0800 Subject: [PATCH 02/22] remove some ts ignores --- lighthouse-core/test/gather/driver-test.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index e326417a84c6..7000b455e0aa 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -647,7 +647,6 @@ describe('.gotoURL', () => { describe('._waitForFCP', () => { it('should not resolve until FCP fires', async () => { - // @ts-ignore driver.on = driver.once = createMockOnceFn(); const waitPromise = makePromiseInspectable(driver._waitForFCP(60 * 1000).promise); @@ -667,7 +666,6 @@ describe('._waitForFCP', () => { }); it('should timeout', async () => { - // @ts-ignore driver.on = driver.once = createMockOnceFn(); const waitPromise = makePromiseInspectable(driver._waitForFCP(5000).promise); @@ -682,7 +680,6 @@ describe('._waitForFCP', () => { }); it('should be cancellable', async () => { - // @ts-ignore driver.on = driver.once = createMockOnceFn(); driver.off = jest.fn(); From 88cff416cf9fd286ce84e6b83c5dc41b6bef5def Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 16 Dec 2019 16:47:07 -0800 Subject: [PATCH 03/22] RecursivePartial for array types --- lighthouse-core/test/gather/driver-test.js | 4 ++-- types/externs.d.ts | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 7000b455e0aa..8a42447e3741 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -329,8 +329,8 @@ describe('.beginTrace', () => { it('will adjust traceCategories based on chrome version', async () => { connectionStub.sendCommand = createMockSendCommandFn() .mockResponse('Browser.getVersion', {product: 'Chrome/70.0.3577.0'}) - .mockResponse('Page.enable', {}) - .mockResponse('Tracing.start', {}); + .mockResponse('Page.enable') + .mockResponse('Tracing.start'); await driver.beginTrace(); diff --git a/types/externs.d.ts b/types/externs.d.ts index 9faa358c42c1..229b5fb634ac 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -28,8 +28,9 @@ declare global { /** Make optional all properties on T and any properties on object properties of T. */ type RecursivePartial = { - [P in keyof T]+?: T[P] extends object ? - RecursivePartial : + [P in keyof T]+?: + T[P] extends (infer U)[] ? RecursivePartial[] : + T[P] extends (object|undefined) ? RecursivePartial : T[P]; }; From eaf7730922aac5bf0ba12f16e9f963e30062a704 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 19 Dec 2019 15:09:24 -0800 Subject: [PATCH 04/22] Apply suggestions from code review Co-Authored-By: Patrick Hulce --- lighthouse-core/test/gather/driver-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 8a42447e3741..e8896dc7e64f 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -95,13 +95,13 @@ let driver; let connectionStub; beforeEach(() => { - // @ts-ignore + // @ts-ignore - connectionStub has a mocked version of sendCommand implemented in each test connectionStub = new Connection(); // @ts-ignore connectionStub.sendCommand = cmd => { throw new Error(`${cmd} not implemented`); }; - // @ts-ignore + // @ts-ignore - driver has a mocked version of on/once implemented in each test driver = new Driver(connectionStub); }); @@ -540,7 +540,7 @@ describe('.gotoURL', () => { driver._waitForNetworkIdle = createMockWaitForFn(); driver._waitForCPUIdle = createMockWaitForFn(); - // @ts-ignore + // @ts-ignore - dynamic property access, tests will definitely fail if the property were to change const waitForResult = driver[`_waitFor${name}`]; const otherWaitForResults = [ driver._waitForFCP, From f377e1a6b245bd36d869ef528a4f2d640d5f45ca Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 19 Dec 2019 15:19:05 -0800 Subject: [PATCH 05/22] pr --- lighthouse-core/test/gather/driver-test.js | 15 ++++++--------- package.json | 2 +- yarn.lock | 8 ++++---- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index e8896dc7e64f..255b9728d2d7 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -11,7 +11,8 @@ const LHElement = require('../../lib/lh-element.js'); const {protocolGetVersionResponse} = require('./fake-driver.js'); const {createMockSendCommandFn, createMockOnceFn} = require('./mock-commands.js'); -const redirectDevtoolsLog = require('../fixtures/wikipedia-redirect.devtoolslog.json'); +const redirectDevtoolsLog = /** @type {LH.Protocol.RawEventMessage[]} */ ( + require('../fixtures/wikipedia-redirect.devtoolslog.json')); /* eslint-env jest */ @@ -248,7 +249,7 @@ describe('.evaluateAsync', () => { // Check that we used the correct frame when creating the isolated context const createWorldArgs = connectionStub.sendCommand.findInvocation('Page.createIsolatedWorld'); - expect(createWorldArgs).toMatchObject({frameId: 1337}); + expect(createWorldArgs).toMatchObject({frameId: '1337'}); // Check that we used the isolated context when evaluating const evaluateArgs = connectionStub.sendCommand.findInvocation('Runtime.evaluate'); @@ -462,7 +463,6 @@ describe('.gotoURL', () => { return Promise.resolve(); } replayLog() { - // @ts-ignore redirectDevtoolsLog.forEach(msg => this.emitProtocolEvent(msg)); } /** @@ -604,7 +604,7 @@ describe('.gotoURL', () => { driver._waitForNetworkIdle = createMockWaitForFn(); driver._waitForCPUIdle = createMockWaitForFn(); - // @ts-ignore + // @ts-ignore: Allow partial passContext. const loadPromise = makePromiseInspectable(driver.gotoURL(url, { waitForLoad: true, passContext: { @@ -905,7 +905,6 @@ describe('Domain.enable/disable State', () => { connectionStub.sendCommand = createMockSendCommandFn() .mockResponse('Network.enable') .mockResponse('Network.disable') - // @ts-ignore: Fetch is not in protocol typings yet. .mockResponse('Fetch.enable') .mockResponse('Fetch.disable'); @@ -917,16 +916,14 @@ describe('Domain.enable/disable State', () => { expect(connectionStub.sendCommand).toHaveBeenCalledTimes(1); // Network still has one enable. - // @ts-ignore: Fetch is not in protocol typings yet. - await driver.sendCommand('Fetch.enable', {}); + await driver.sendCommand('Fetch.enable'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(2); await driver.sendCommand('Network.disable'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(3); // Network is now disabled. - // @ts-ignore: Fetch is not in protocol typings yet. - await driver.sendCommand('Fetch.disable', {}); + await driver.sendCommand('Fetch.disable'); expect(connectionStub.sendCommand).toHaveBeenCalledTimes(4); }); diff --git a/package.json b/package.json index a5230ad0bbdd..04d6d762a59b 100644 --- a/package.json +++ b/package.json @@ -109,7 +109,7 @@ "cpy": "^7.0.1", "csv-validator": "^0.0.3", "cz-customizable": "^5.2.0", - "devtools-protocol": "0.0.588129", + "devtools-protocol": "0.0.726364", "eslint": "^4.19.1", "eslint-config-google": "^0.9.1", "eslint-plugin-local-rules": "0.1.0", diff --git a/yarn.lock b/yarn.lock index 436b3dd10518..fb9729cf6535 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2629,10 +2629,10 @@ detective@^5.0.2: defined "^1.0.0" minimist "^1.1.1" -devtools-protocol@0.0.588129: - version "0.0.588129" - resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.588129.tgz#b93d05b01679471639f2882077f77d4e96eecc18" - integrity sha512-5ID10jYNewQpBFo1IhKtYkH1b+EF9DmrnWidSdAnBq8hdJyZKxHvjHPbwTlWeqhzVzUKHSdn5EUj9BZq31U+QQ== +devtools-protocol@0.0.726364: + version "0.0.726364" + resolved "https://registry.yarnpkg.com/devtools-protocol/-/devtools-protocol-0.0.726364.tgz#1cb6b26c895fe642187e9c9a85b3fc20ebd8a9fe" + integrity sha512-PFKlRrTyKvdFF5OdHeQMjzFSC/d++eM99tyUpnHdoTKwtE3yaKg8Zvav0R+i4Dbf2Hw2i/L9uAzEDGIxCkAwZQ== diff-sequences@^24.9.0: version "24.9.0" From acdb2aeab81ddba0e880d8b187abcecb2171a6fb Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 19 Dec 2019 16:07:38 -0800 Subject: [PATCH 06/22] fix --- lighthouse-core/test/gather/driver-test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 255b9728d2d7..a1887abbfc2d 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -180,8 +180,9 @@ describe('.getObjectProperty', () => { describe('.getRequestContent', () => { it('throws if getRequestContent takes too long', async () => { + // @ts-ignore connectionStub.sendCommand = jest.fn() - .mockImplementationOnce(() => new Promise(r => setTimeout(r), 5000)); + .mockImplementationOnce(() => new Promise(r => setTimeout(r, 5000))); // Fail if we don't reach our two assertions in the catch block expect.assertions(2); @@ -282,8 +283,9 @@ describe('.evaluateAsync', () => { describe('.sendCommand', () => { it('.sendCommand timesout when commands take too long', async () => { + // @ts-ignore connectionStub.sendCommand = jest.fn() - .mockImplementationOnce(() => new Promise(r => setTimeout(r), 5000)); + .mockImplementationOnce(() => new Promise(r => setTimeout(r, 500))); driver.setNextProtocolTimeout(10000); const pageEnablePromise = driver.sendCommand('Page.enable'); From b5dd6132e8d4fce60b623146ae612e69cc8f21f3 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 19 Dec 2019 16:24:50 -0800 Subject: [PATCH 07/22] more mock types yay --- lighthouse-core/test/gather/driver-test.js | 69 +++++++++++++------- lighthouse-core/test/gather/mock-commands.js | 2 +- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index a1887abbfc2d..3e88908ea27e 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -57,6 +57,37 @@ function makePromiseInspectable(promise) { }); } +function createDecomposedPromise() { + /** @type {Function} */ + let resolve; + /** @type {Function} */ + let reject; + const promise = new Promise((r1, r2) => { + resolve = r1; + reject = r2; + }); + // @ts-ignore: Ignore 'unused' error. + return {promise, resolve, reject}; +} + +function createMockWaitForFn() { + const {promise, resolve, reject} = createDecomposedPromise(); + + const mockCancelFn = jest.fn(); + const mockFn = jest.fn().mockReturnValue({promise, cancel: mockCancelFn}); + + return Object.assign(mockFn, { + mockResolve: resolve, + /** @param {Error=} err */ + mockReject(err) { + reject(err || new Error('Rejected')); + }, + getMockCancelFn() { + return mockCancelFn; + }, + }); +} + expect.extend({ /** * Asserts that an inspectable promise created by makePromiseInspectable is currently resolved or rejected. @@ -90,7 +121,17 @@ async function flushAllTimersAndMicrotasks() { } } -/** @type {Driver & {on: ReturnType, once: ReturnType}} */ +/** + * @typedef DriverMockMethods + * @property {ReturnType} on + * @property {ReturnType} once + * @property {ReturnType} _waitForFCP + * @property {ReturnType} _waitForLoadEvent + * @property {ReturnType} _waitForNetworkIdle + * @property {ReturnType} _waitForCPUIdle +*/ + +/** @type {Driver & DriverMockMethods} */ let driver; /** @type {Connection & {sendCommand: ReturnType}} */ let connectionStub; @@ -272,7 +313,7 @@ describe('.evaluateAsync', () => { .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1337'}}}) .mockResponse('Page.createIsolatedWorld', {executionContextId: 9001}) .mockResponse('Runtime.evaluate', Promise.reject(new Error('Cannot find context'))) - .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: 1337}}}) + .mockResponse('Page.getResourceTree', {frameTree: {frame: {id: '1337'}}}) .mockResponse('Page.createIsolatedWorld', {executionContextId: 9002}) .mockResponse('Runtime.evaluate', {result: {value: 'mocked value'}}); @@ -346,10 +387,8 @@ describe('.beginTrace', () => { describe('.setExtraHTTPHeaders', () => { it('should Network.setExtraHTTPHeaders when there are extra-headers', async () => { - connectionStub.sendCommand = createMockSendCommandFn().mockResponse( - 'Network.setExtraHTTPHeaders', - {} - ); + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Network.setExtraHTTPHeaders'); await driver.setExtraHTTPHeaders({ 'Cookie': 'monster', @@ -425,24 +464,6 @@ describe('.goOffline', () => { }); describe('.gotoURL', () => { - function createMockWaitForFn() { - let resolve; - let reject; - const promise = new Promise((r1, r2) => { - resolve = r1; - reject = r2; - }); - - const mockCancelFn = jest.fn(); - const mockFn = jest.fn().mockReturnValue({promise, cancel: mockCancelFn}); - - mockFn.mockResolve = () => resolve(); - mockFn.mockReject = err => reject(err || new Error('Rejected')); - mockFn.getMockCancelFn = () => mockCancelFn; - - return mockFn; - } - beforeEach(() => { connectionStub.sendCommand = createMockSendCommandFn() .mockResponse('Network.enable') diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index 0881770cf067..ffb70ddb9f83 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -37,7 +37,7 @@ function createMockSendCommandFn() { /** * @template {keyof LH.CrdpCommands} C * @param {C} command - * @param {RecursivePartial=} response + * @param {RecursivePartial | Promise=} response * @param {number=} delay */ mockResponse(command, response, delay) { From 750f839ab68fa0ce553d87af1b9938a315e3f1a0 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 19 Dec 2019 17:09:17 -0800 Subject: [PATCH 08/22] big mood --- .../byte-efficiency/byte-efficiency-audit.js | 2 +- .../byte-efficiency/uses-long-cache-ttl.js | 2 +- .../computed/critical-request-chains.js | 2 +- lighthouse-core/computed/resource-summary.js | 2 +- .../dobetterweb/response-compression.js | 2 +- .../simulator/network-analyzer.js | 2 +- lighthouse-core/lib/network-request.js | 4 +- lighthouse-core/test/gather/driver-test.js | 29 +++++---- lighthouse-core/test/gather/mock-commands.js | 61 ++++++++++++++----- 9 files changed, 72 insertions(+), 34 deletions(-) diff --git a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js index 333fdd05e4db..03832f1db2fa 100644 --- a/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js +++ b/lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js @@ -65,7 +65,7 @@ class UnusedBytes extends Audit { * * @param {LH.Artifacts.NetworkRequest=} networkRecord * @param {number} totalBytes Uncompressed size of the resource - * @param {LH.Crdp.Page.ResourceType=} resourceType + * @param {LH.Crdp.Network.ResourceType=} resourceType * @return {number} */ static estimateTransferSize(networkRecord, totalBytes, resourceType) { diff --git a/lighthouse-core/audits/byte-efficiency/uses-long-cache-ttl.js b/lighthouse-core/audits/byte-efficiency/uses-long-cache-ttl.js index ba3f77c06dd5..3d46ac51e5c6 100644 --- a/lighthouse-core/audits/byte-efficiency/uses-long-cache-ttl.js +++ b/lighthouse-core/audits/byte-efficiency/uses-long-cache-ttl.js @@ -143,7 +143,7 @@ class CacheHeaders extends Audit { static isCacheableAsset(record) { const CACHEABLE_STATUS_CODES = new Set([200, 203, 206]); - /** @type {Set} */ + /** @type {Set} */ const STATIC_RESOURCE_TYPES = new Set([ NetworkRequest.TYPES.Font, NetworkRequest.TYPES.Image, diff --git a/lighthouse-core/computed/critical-request-chains.js b/lighthouse-core/computed/critical-request-chains.js index 94bf25f4bcc0..eb1b4fcb1870 100644 --- a/lighthouse-core/computed/critical-request-chains.js +++ b/lighthouse-core/computed/critical-request-chains.js @@ -42,7 +42,7 @@ class CriticalRequestChains { // XHRs are fetched at High priority, but we exclude them, as they are unlikely to be critical // Images are also non-critical. // Treat any missed images, primarily favicons, as non-critical resources - /** @type {Array} */ + /** @type {Array} */ const nonCriticalResourceTypes = [ NetworkRequest.TYPES.Image, NetworkRequest.TYPES.XHR, diff --git a/lighthouse-core/computed/resource-summary.js b/lighthouse-core/computed/resource-summary.js index a2c9b6b59488..b1d943bdc0da 100644 --- a/lighthouse-core/computed/resource-summary.js +++ b/lighthouse-core/computed/resource-summary.js @@ -18,7 +18,7 @@ class ResourceSummary { */ static determineResourceType(record) { if (!record.resourceType) return 'other'; - /** @type {Partial>} */ + /** @type {Partial>} */ const requestToResourceType = { 'Stylesheet': 'stylesheet', 'Image': 'image', diff --git a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js index beb2eb20bb21..36bdaeabc9da 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/response-compression.js @@ -20,7 +20,7 @@ const CHROME_EXTENSION_PROTOCOL = 'chrome-extension:'; const compressionHeaders = ['content-encoding', 'x-original-content-encoding']; const compressionTypes = ['gzip', 'br', 'deflate']; const binaryMimeTypes = ['image', 'audio', 'video']; -/** @type {Array} */ +/** @type {Array} */ const textResourceTypes = [ NetworkRequest.TYPES.Document, NetworkRequest.TYPES.Script, diff --git a/lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js b/lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js index c690c17ce3c1..e2d16f97febd 100644 --- a/lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js +++ b/lighthouse-core/lib/dependency-graph/simulator/network-analyzer.js @@ -15,7 +15,7 @@ const DEFAULT_SERVER_RESPONSE_PERCENTAGE = 0.4; /** * For certain resource types, server response time takes up a greater percentage of TTFB (dynamic * assets like HTML documents, XHR/API calls, etc) - * @type {Partial>} + * @type {Partial>} */ const SERVER_RESPONSE_PERCENTAGE_OF_TTFB = { Document: 0.9, diff --git a/lighthouse-core/lib/network-request.js b/lighthouse-core/lib/network-request.js index cce715d7f437..6b05aa36801f 100644 --- a/lighthouse-core/lib/network-request.js +++ b/lighthouse-core/lib/network-request.js @@ -49,7 +49,7 @@ const HEADER_FETCHED_SIZE = 'X-TotalFetchedSize'; * @property {number} responseMs */ -/** @type {SelfMap} */ +/** @type {SelfMap} */ const RESOURCE_TYPES = { XHR: 'XHR', Fetch: 'Fetch', @@ -112,7 +112,7 @@ class NetworkRequest { this.initiator = /** @type {LH.Crdp.Network.Initiator} */ ({type: 'other'}); /** @type {LH.Crdp.Network.ResourceTiming|undefined} */ this.timing = undefined; - /** @type {LH.Crdp.Page.ResourceType|undefined} */ + /** @type {LH.Crdp.Network.ResourceType|undefined} */ this.resourceType = undefined; this.mimeType = ''; /** @type {LH.Crdp.Network.ResourcePriority} */ diff --git a/lighthouse-core/test/gather/driver-test.js b/lighthouse-core/test/gather/driver-test.js index 3e88908ea27e..8cffcffd2b95 100644 --- a/lighthouse-core/test/gather/driver-test.js +++ b/lighthouse-core/test/gather/driver-test.js @@ -129,9 +129,13 @@ async function flushAllTimersAndMicrotasks() { * @property {ReturnType} _waitForLoadEvent * @property {ReturnType} _waitForNetworkIdle * @property {ReturnType} _waitForCPUIdle + * @property {(...args: RecursivePartial>) => ReturnType} gotoURL + * @property {(...args: RecursivePartial>) => ReturnType} goOnline */ -/** @type {Driver & DriverMockMethods} */ +/** @typedef {Driver & DriverMockMethods} TestDriver */ + +/** @type {TestDriver} */ let driver; /** @type {Connection & {sendCommand: ReturnType}} */ let connectionStub; @@ -143,7 +147,7 @@ beforeEach(() => { connectionStub.sendCommand = cmd => { throw new Error(`${cmd} not implemented`); }; - // @ts-ignore - driver has a mocked version of on/once implemented in each test + // @ts-ignore - driver has a mocked version of on/once implemented in each test driver = new Driver(connectionStub); }); @@ -448,8 +452,8 @@ describe('.getAppManifest', () => { describe('.goOffline', () => { it('should send offline emulation', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Network.enable', {}) - .mockResponse('Network.emulateNetworkConditions', {}); + .mockResponse('Network.enable') + .mockResponse('Network.emulateNetworkConditions'); await driver.goOffline(); const emulateArgs = connectionStub.sendCommand @@ -476,7 +480,7 @@ describe('.gotoURL', () => { }); it('will track redirects through gotoURL load', async () => { - const delay = _ => new Promise(resolve => setTimeout(resolve)); + const delay = () => new Promise(resolve => setTimeout(resolve)); class ReplayConnection extends Connection { connect() { @@ -504,7 +508,8 @@ describe('.gotoURL', () => { } } const replayConnection = new ReplayConnection(); - const driver = new Driver(replayConnection); + + const driver = /** @type {TestDriver} */ (new Driver(replayConnection)); // Redirect in log will go through const startUrl = 'http://en.wikipedia.org/'; @@ -522,7 +527,6 @@ describe('.gotoURL', () => { }, }, }; - const loadPromise = driver.gotoURL(startUrl, loadOptions); await flushAllTimersAndMicrotasks(); @@ -627,7 +631,6 @@ describe('.gotoURL', () => { driver._waitForNetworkIdle = createMockWaitForFn(); driver._waitForCPUIdle = createMockWaitForFn(); - // @ts-ignore: Allow partial passContext. const loadPromise = makePromiseInspectable(driver.gotoURL(url, { waitForLoad: true, passContext: { @@ -982,12 +985,13 @@ describe('Domain.enable/disable State', () => { describe('Multi-target management', () => { it('enables the Network domain for iframes', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponseToSession('Network.enable', '123', {}) - .mockResponseToSession('Target.setAutoAttach', '123', {}) - .mockResponseToSession('Runtime.runIfWaitingForDebugger', '123', {}); + .mockResponseToSession('Network.enable', '123') + .mockResponseToSession('Target.setAutoAttach', '123') + .mockResponseToSession('Runtime.runIfWaitingForDebugger', '123'); driver._eventEmitter.emit('Target.attachedToTarget', { sessionId: '123', + // @ts-ignore: Ignore partial targetInfo. targetInfo: {type: 'iframe'}, }); await flushAllTimersAndMicrotasks(); @@ -1001,10 +1005,11 @@ describe('Multi-target management', () => { it('ignores other target types, but still resumes them', async () => { connectionStub.sendCommand = createMockSendCommandFn() - .mockResponse('Target.sendMessageToTarget', {}); + .mockResponse('Target.sendMessageToTarget'); driver._eventEmitter.emit('Target.attachedToTarget', { sessionId: 'SW1', + // @ts-ignore: Ignore partial targetInfo. targetInfo: {type: 'service_worker'}, }); await flushAllTimersAndMicrotasks(); diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index ffb70ddb9f83..2941d8d5e88b 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -11,6 +11,16 @@ /* eslint-env jest */ +/** + * @template {keyof LH.CrdpCommands} C + * @typedef {((...args: LH.CrdpCommands[C]['paramsType']) => MockResponse) | RecursivePartial | Promise} MockResponse + */ + +/** + * @template {keyof LH.CrdpEvents} E + * @typedef {RecursivePartial} MockEvent + */ + /** * Creates a jest mock function whose implementation consumes mocked protocol responses matching the * requested command in the order they were mocked. @@ -21,23 +31,38 @@ * returns the protocol message argument. */ function createMockSendCommandFn() { + /** + * Typescript fails to equate template type `C` here with `C` when pushing to this array. + * Instead of sprinkling a couple ts-ignores, make `command` be any, but leave `C` for just + * documentation purposes. This is an internal type, so it doesn't matter much. + * @template {keyof LH.CrdpCommands} C + * @type {Array<{command: C|any, sessionId?: string, response?: MockResponse, delay?: number}>} + */ const mockResponses = []; - const mockFnImpl = jest.fn().mockImplementation((command, sessionId, ...args) => { - const indexOfResponse = mockResponses - .findIndex(entry => entry.command === command && entry.sessionId === sessionId); - if (indexOfResponse === -1) throw new Error(`${command} unimplemented`); - const {response, delay} = mockResponses[indexOfResponse]; - mockResponses.splice(indexOfResponse, 1); - const returnValue = typeof response === 'function' ? response(...args) : response; - if (delay) return new Promise(resolve => setTimeout(() => resolve(returnValue), delay)); - return Promise.resolve(returnValue); - }); + const mockFnImpl = jest.fn().mockImplementation( + /** + * @template {keyof LH.CrdpCommands} C + * @param {C} command + * @param {string|undefined=} sessionId + * @param {LH.CrdpCommands[C]['paramsType']} args + */ + (command, sessionId, ...args) => { + const indexOfResponse = mockResponses + .findIndex(entry => entry.command === command && entry.sessionId === sessionId); + if (indexOfResponse === -1) throw new Error(`${command} unimplemented`); + const {response, delay} = mockResponses[indexOfResponse]; + mockResponses.splice(indexOfResponse, 1); + const returnValue = typeof response === 'function' ? response(...args) : response; + if (delay) return new Promise(resolve => setTimeout(() => resolve(returnValue), delay)); + // @ts-ignore: Some covariant type stuff doesn't work here. idk, I'm not a type scientist. + return Promise.resolve(returnValue); + }); const mockFn = Object.assign(mockFnImpl, { /** * @template {keyof LH.CrdpCommands} C * @param {C} command - * @param {RecursivePartial | Promise=} response + * @param {MockResponse=} response * @param {number=} delay */ mockResponse(command, response, delay) { @@ -48,7 +73,7 @@ function createMockSendCommandFn() { * @template {keyof LH.CrdpCommands} C * @param {C} command * @param {string} sessionId - * @param {RecursivePartial=} response + * @param {MockResponse=} response * @param {number=} delay */ mockResponseToSession(command, sessionId, response, delay) { @@ -78,6 +103,10 @@ function createMockSendCommandFn() { * returns the listener . */ function createMockOnceFn() { + /** + * @template {keyof LH.CrdpEvents} E + * @type {Array<{event: E|any, response?: MockEvent}>} + */ const mockEvents = []; const mockFnImpl = jest.fn().mockImplementation((eventName, listener) => { const indexOfResponse = mockEvents.findIndex(entry => entry.event === eventName); @@ -93,7 +122,7 @@ function createMockOnceFn() { /** * @template {keyof LH.CrdpEvents} E * @param {E} event - * @param {RecursivePartial} response + * @param {MockEvent} response */ mockEvent(event, response) { mockEvents.push({event, response}); @@ -116,6 +145,10 @@ function createMockOnceFn() { * So it's good for .on w/ many events. */ function createMockOnFn() { + /** + * @template {keyof LH.CrdpEvents} E + * @type {Array<{event: E, response?: MockEvent}>} + */ const mockEvents = []; const mockFnImpl = jest.fn().mockImplementation((eventName, listener) => { const events = mockEvents.filter(entry => entry.event === eventName); @@ -137,7 +170,7 @@ function createMockOnFn() { /** * @template {keyof LH.CrdpEvents} E * @param {E} event - * @param {RecursivePartial} response + * @param {MockEvent} response */ mockEvent(event, response) { mockEvents.push({event, response}); From 50da0bdb7c7a87393f26f75de43fe46f91adf9d5 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 19 Dec 2019 17:16:12 -0800 Subject: [PATCH 09/22] bigger mood --- lighthouse-core/test/gather/fake-driver.js | 4 ++++ lighthouse-core/test/gather/mock-commands.js | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js index e7720923aa85..d65046060c44 100644 --- a/lighthouse-core/test/gather/fake-driver.js +++ b/lighthouse-core/test/gather/fake-driver.js @@ -5,6 +5,9 @@ */ 'use strict'; +/** + * @param {{protocolGetVersionResponse: LH.CrdpCommands['Browser.getVersion']['returnType']}} param0 + */ function makeFakeDriver({protocolGetVersionResponse}) { let scrollPosition = {x: 0, y: 0}; @@ -59,6 +62,7 @@ function makeFakeDriver({protocolGetVersionResponse}) { evaluateAsync() { return Promise.resolve({}); }, + /** @param {{x: number, y: number}} position */ scrollTo(position) { scrollPosition = position; return Promise.resolve(); diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index 2941d8d5e88b..7f74b2440ea6 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -147,7 +147,7 @@ function createMockOnceFn() { function createMockOnFn() { /** * @template {keyof LH.CrdpEvents} E - * @type {Array<{event: E, response?: MockEvent}>} + * @type {Array<{event: E|any, response?: MockEvent}>} */ const mockEvents = []; const mockFnImpl = jest.fn().mockImplementation((eventName, listener) => { From 245f3eddcdde35538573df10a223af1c7cbce4be Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 19 Dec 2019 17:21:57 -0800 Subject: [PATCH 10/22] done --- lighthouse-core/test/gather/fake-driver.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js index d65046060c44..f41b055ca989 100644 --- a/lighthouse-core/test/gather/fake-driver.js +++ b/lighthouse-core/test/gather/fake-driver.js @@ -77,13 +77,15 @@ function makeFakeDriver({protocolGetVersionResponse}) { return Promise.resolve(); }, endTrace() { - return Promise.resolve( - require('../fixtures/traces/progressive-app.json') - ); + // Minimal indirection so TypeScript doesn't crash trying to infer a type. + const modulePath = '../fixtures/traces/progressive-app.json'; + return Promise.resolve(require(modulePath)); }, beginDevtoolsLog() {}, endDevtoolsLog() { - return require('../fixtures/artifacts/perflog/defaultPass.devtoolslog.json'); + // Minimal indirection so TypeScript doesn't crash trying to infer a type. + const modulePath = '../fixtures/artifacts/perflog/defaultPass.devtoolslog.json'; + return require(modulePath); }, blockUrlPatterns() { return Promise.resolve(); From a5113d48b0ed9015826d993c6a641ab86192220e Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 20 Dec 2019 16:40:05 -0800 Subject: [PATCH 11/22] rm getMockedEmulationDriver --- .../test/gather/gather-runner-test.js | 263 ++++++++---------- 1 file changed, 117 insertions(+), 146 deletions(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 66095f8b1158..3876a9a1ecf2 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -15,6 +15,9 @@ const unresolvedPerfLog = require('./../fixtures/unresolved-perflog.json'); const NetworkRequest = require('../../lib/network-request.js'); const LHError = require('../../lib/lh-error.js'); const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log.js'); +const Driver = require('../../gather/driver.js'); +const Connection = require('../../gather/connections/connection.js'); +const {createMockSendCommandFn} = require('./mock-commands.js'); jest.mock('../../lib/stack-collector.js', () => () => Promise.resolve([])); @@ -39,11 +42,24 @@ class TestGathererNoArtifact extends Gatherer { const fakeDriver = require('./fake-driver.js'); const fakeDriverUsingRealMobileDevice = fakeDriver.fakeDriverUsingRealMobileDevice; -// TODO: Refactor this to use gather/mock-commands.js -function getMockedEmulationDriver(emulationFn, netThrottleFn, cpuThrottleFn, - blockUrlFn, extraHeadersFn) { - const Driver = require('../../gather/driver.js'); - const Connection = require('../../gather/connections/connection.js'); +let driver; +let connectionStub; + +function resetDefaultMockResponses() { + connectionStub.sendCommand = createMockSendCommandFn() + .mockResponse('Emulation.setCPUThrottlingRate') + .mockResponse('Emulation.setDeviceMetricsOverride') + .mockResponse('Emulation.setTouchEmulationEnabled') + .mockResponse('Network.emulateNetworkConditions') + .mockResponse('Network.enable') + .mockResponse('Network.setBlockedURLs') + .mockResponse('Network.setExtraHTTPHeaders') + .mockResponse('Network.setUserAgentOverride') + .mockResponse('Page.enable') + .mockResponse('ServiceWorker.enable'); +} + +beforeEach(() => { const EmulationDriver = class extends Driver { enableRuntimeEvents() { return Promise.resolve(); @@ -63,34 +79,14 @@ function getMockedEmulationDriver(emulationFn, netThrottleFn, cpuThrottleFn, cleanBrowserCaches() {} clearDataForOrigin() {} }; - const EmulationMock = class extends Connection { - sendCommand(command, sessionId, params) { - let fn = null; - switch (command) { - case 'Network.emulateNetworkConditions': - fn = netThrottleFn; - break; - case 'Emulation.setCPUThrottlingRate': - fn = cpuThrottleFn; - break; - case 'Emulation.setDeviceMetricsOverride': - fn = emulationFn; - break; - case 'Network.setBlockedURLs': - fn = blockUrlFn; - break; - case 'Network.setExtraHTTPHeaders': - fn = extraHeadersFn; - break; - default: - fn = null; - break; - } - return Promise.resolve(fn && fn(params)); - } + + connectionStub = new Connection(); + connectionStub.sendCommand = cmd => { + throw new Error(`${cmd} not implemented`); }; - return new EmulationDriver(new EmulationMock()); -} + driver = new EmulationDriver(connectionStub); + resetDefaultMockResponses(); +}); describe('GatherRunner', function() { it('loads a page and updates passContext.URL on redirect', () => { @@ -267,110 +263,93 @@ describe('GatherRunner', function() { test('works when running on desktop device', DESKTOP_UA, 'desktop'); }); - it('sets up the driver to begin emulation when all emulation flags are undefined', () => { - const tests = { - calledDeviceEmulation: false, - calledNetworkEmulation: false, - calledCpuEmulation: false, - }; - const createEmulationCheck = variable => (arg) => { - tests[variable] = arg; - - return true; - }; - const driver = getMockedEmulationDriver( - createEmulationCheck('calledDeviceEmulation'), - createEmulationCheck('calledNetworkEmulation'), - createEmulationCheck('calledCpuEmulation') - ); - - return GatherRunner.setupDriver(driver, { + it('sets up the driver to begin emulation when all flags are undefined', async () => { + await GatherRunner.setupDriver(driver, { settings: { emulatedFormFactor: 'mobile', internalDisableDeviceScreenEmulation: false, }, - }).then(_ => { - assert.ok(tests.calledDeviceEmulation, 'did not call device emulation'); - assert.deepEqual(tests.calledNetworkEmulation, { - latency: 0, downloadThroughput: 0, uploadThroughput: 0, offline: false, - }); - assert.ok(!tests.calledCpuEmulation, 'called cpu emulation'); }); + + connectionStub.sendCommand.findInvocation('Emulation.setDeviceMetricsOverride'); + expect(connectionStub.sendCommand.findInvocation('Network.emulateNetworkConditions')).toEqual({ + latency: 0, downloadThroughput: 0, uploadThroughput: 0, offline: false, + }); + expect(() => + connectionStub.sendCommand.findInvocation('Emulation.setCPUThrottlingRate')).toThrow(); }); it('applies the correct emulation given a particular emulationFormFactor', async () => { - const emulationFn = jest.fn(); - const driver = getMockedEmulationDriver( - emulationFn, - () => true, - () => true - ); - const getSettings = formFactor => ({ emulatedFormFactor: formFactor, internalDisableDeviceScreenEmulation: false, }); await GatherRunner.setupDriver(driver, {settings: getSettings('mobile')}); - expect(emulationFn.mock.calls.slice(-1)[0][0]).toMatchObject({mobile: true}); + expect(connectionStub.sendCommand.findInvocation('Emulation.setDeviceMetricsOverride')) + .toMatchObject({mobile: true}); + resetDefaultMockResponses(); await GatherRunner.setupDriver(driver, {settings: getSettings('desktop')}); - expect(emulationFn.mock.calls.slice(-1)[0][0]).toMatchObject({mobile: false}); + expect(connectionStub.sendCommand.findInvocation('Emulation.setDeviceMetricsOverride')) + .toMatchObject({mobile: false}); - emulationFn.mockClear(); + resetDefaultMockResponses(); await GatherRunner.setupDriver(driver, {settings: getSettings('none')}); - expect(emulationFn).not.toHaveBeenCalled(); + expect(() => + connectionStub.sendCommand.findInvocation('Emulation.setDeviceMetricsOverride')).toThrow(); }); - it('stops throttling when not devtools', () => { - const tests = { - calledDeviceEmulation: false, - calledNetworkEmulation: false, - calledCpuEmulation: false, - }; - const createEmulationCheck = variable => (...args) => { - tests[variable] = args; - return true; - }; - const driver = getMockedEmulationDriver( - createEmulationCheck('calledDeviceEmulation'), - createEmulationCheck('calledNetworkEmulation'), - createEmulationCheck('calledCpuEmulation') - ); - - return GatherRunner.setupDriver(driver, { + it('stops throttling when not devtools', async () => { + // const tests = { + // calledDeviceEmulation: false, + // calledNetworkEmulation: false, + // calledCpuEmulation: false, + // }; + // const createEmulationCheck = variable => (...args) => { + // tests[variable] = args; + // return true; + // }; + // const driver = getMockedEmulationDriver( + // createEmulationCheck('calledDeviceEmulation'), + // createEmulationCheck('calledNetworkEmulation'), + // createEmulationCheck('calledCpuEmulation') + // ); + + // return GatherRunner.setupDriver(driver, { + // settings: { + // emulatedFormFactor: 'mobile', + // throttlingMethod: 'provided', + // internalDisableDeviceScreenEmulation: false, + // }, + // }).then(_ => { + // assert.ok(tests.calledDeviceEmulation, 'did not call device emulation'); + // assert.deepEqual(tests.calledNetworkEmulation, [{ + // latency: 0, downloadThroughput: 0, uploadThroughput: 0, offline: false, + // }]); + // assert.ok(!tests.calledCpuEmulation, 'called CPU emulation'); + // }); + + // ? ... this test is the same as above.. + + await GatherRunner.setupDriver(driver, { settings: { emulatedFormFactor: 'mobile', throttlingMethod: 'provided', internalDisableDeviceScreenEmulation: false, }, - }).then(_ => { - assert.ok(tests.calledDeviceEmulation, 'did not call device emulation'); - assert.deepEqual(tests.calledNetworkEmulation, [{ - latency: 0, downloadThroughput: 0, uploadThroughput: 0, offline: false, - }]); - assert.ok(!tests.calledCpuEmulation, 'called CPU emulation'); }); - }); - - it('sets throttling according to settings', () => { - const tests = { - calledDeviceEmulation: false, - calledNetworkEmulation: false, - calledCpuEmulation: false, - }; - const createEmulationCheck = variable => (...args) => { - tests[variable] = args; - return true; - }; - const driver = getMockedEmulationDriver( - createEmulationCheck('calledDeviceEmulation'), - createEmulationCheck('calledNetworkEmulation'), - createEmulationCheck('calledCpuEmulation') - ); + connectionStub.sendCommand.findInvocation('Emulation.setDeviceMetricsOverride'); + expect(connectionStub.sendCommand.findInvocation('Network.emulateNetworkConditions')).toEqual({ + latency: 0, downloadThroughput: 0, uploadThroughput: 0, offline: false, + }); + expect(() => + connectionStub.sendCommand.findInvocation('Emulation.setCPUThrottlingRate')).toThrow(); + }); - return GatherRunner.setupDriver(driver, { + it('sets throttling according to settings', async () => { + await GatherRunner.setupDriver(driver, { settings: { emulatedFormFactor: 'mobile', internalDisableDeviceScreenEmulation: false, throttlingMethod: 'devtools', @@ -381,12 +360,14 @@ describe('GatherRunner', function() { cpuSlowdownMultiplier: 1, }, }, - }).then(_ => { - assert.ok(tests.calledDeviceEmulation, 'did not call device emulation'); - assert.deepEqual(tests.calledNetworkEmulation, [{ - latency: 100, downloadThroughput: 1024, uploadThroughput: 1024, offline: false, - }]); - assert.deepEqual(tests.calledCpuEmulation, [{rate: 1}]); + }); + + connectionStub.sendCommand.findInvocation('Emulation.setDeviceMetricsOverride'); + expect(connectionStub.sendCommand.findInvocation('Network.emulateNetworkConditions')).toEqual({ + latency: 100, downloadThroughput: 1024, uploadThroughput: 1024, offline: false, + }); + expect(connectionStub.sendCommand.findInvocation('Emulation.setCPUThrottlingRate')).toEqual({ + rate: 1, }); }); @@ -565,13 +546,8 @@ describe('GatherRunner', function() { }); }); - it('tells the driver to block given URL patterns when blockedUrlPatterns is given', () => { - let receivedUrlPatterns = null; - const driver = getMockedEmulationDriver(null, null, null, params => { - receivedUrlPatterns = params.urls; - }); - - return GatherRunner.setupPassNetwork({ + it('tells the driver to block given URL patterns when blockedUrlPatterns is given', async () => { + await GatherRunner.setupPassNetwork({ driver, settings: { blockedUrlPatterns: ['http://*.evil.com', '.jpg', '.woff2'], @@ -580,46 +556,41 @@ describe('GatherRunner', function() { blockedUrlPatterns: ['*.jpeg'], gatherers: [], }, - }).then(() => assert.deepStrictEqual( - receivedUrlPatterns.sort(), - ['*.jpeg', '.jpg', '.woff2', 'http://*.evil.com'] - )); - }); - - it('does not throw when blockedUrlPatterns is not given', () => { - let receivedUrlPatterns = null; - const driver = getMockedEmulationDriver(null, null, null, params => { - receivedUrlPatterns = params.urls; }); - return GatherRunner.setupPassNetwork({ + const blockedUrlsResult = connectionStub.sendCommand.findInvocation('Network.setBlockedURLs'); + blockedUrlsResult.urls.sort(); + expect(blockedUrlsResult) + .toEqual({urls: ['*.jpeg', '.jpg', '.woff2', 'http://*.evil.com']}); + }); + + it('does not throw when blockedUrlPatterns is not given', async () => { + await GatherRunner.setupPassNetwork({ driver, settings: {}, passConfig: {gatherers: []}, - }).then(() => assert.deepStrictEqual(receivedUrlPatterns, [])); - }); + }); + expect(connectionStub.sendCommand.findInvocation('Network.setBlockedURLs')) + .toEqual({urls: []}); + }); - it('tells the driver to set additional http headers when extraHeaders flag is given', () => { - let receivedHeaders = null; - const driver = getMockedEmulationDriver(null, null, null, null, params => { - receivedHeaders = params.headers; - }); - const headers = { + it('tells the driver to set additional http when extraHeaders flag is given', async () => { + const extraHeaders = { 'Cookie': 'monster', 'x-men': 'wolverine', }; - return GatherRunner.setupPassNetwork({ + await GatherRunner.setupPassNetwork({ driver, settings: { - extraHeaders: headers, + extraHeaders, }, passConfig: {gatherers: []}, - }).then(() => assert.deepStrictEqual( - receivedHeaders, - headers - )); + }); + + expect(connectionStub.sendCommand.findInvocation('Network.setExtraHTTPHeaders')) + .toEqual({headers: extraHeaders}); }); it('tells the driver to begin tracing', async () => { From 56cbc04d83d3e1bf09fabb579462afc7aa6dc665 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 20 Dec 2019 16:49:53 -0800 Subject: [PATCH 12/22] move EmulationDriver up --- .../test/gather/gather-runner-test.js | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 3876a9a1ecf2..ab324ec261bd 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -39,6 +39,26 @@ class TestGathererNoArtifact extends Gatherer { afterPass() {} } +class EmulationDriver extends Driver { + enableRuntimeEvents() { + return Promise.resolve(); + } + enableAsyncStacks() { + return Promise.resolve(); + } + assertNoSameOriginServiceWorkerClients() { + return Promise.resolve(); + } + cacheNatives() { + return Promise.resolve(); + } + registerPerformanceObserver() { + return Promise.resolve(); + } + cleanBrowserCaches() {} + clearDataForOrigin() {} +} + const fakeDriver = require('./fake-driver.js'); const fakeDriverUsingRealMobileDevice = fakeDriver.fakeDriverUsingRealMobileDevice; @@ -60,26 +80,6 @@ function resetDefaultMockResponses() { } beforeEach(() => { - const EmulationDriver = class extends Driver { - enableRuntimeEvents() { - return Promise.resolve(); - } - enableAsyncStacks() { - return Promise.resolve(); - } - assertNoSameOriginServiceWorkerClients() { - return Promise.resolve(); - } - cacheNatives() { - return Promise.resolve(); - } - registerPerformanceObserver() { - return Promise.resolve(); - } - cleanBrowserCaches() {} - clearDataForOrigin() {} - }; - connectionStub = new Connection(); connectionStub.sendCommand = cmd => { throw new Error(`${cmd} not implemented`); From a3ad54f6f7d3d986db61ebc7e40264c10b473af2 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 9 Jan 2020 17:00:49 -0800 Subject: [PATCH 13/22] delete test --- .../test/gather/gather-runner-test.js | 49 +------------------ 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index ab324ec261bd..93ca8b6cc3c7 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -267,6 +267,7 @@ describe('GatherRunner', function() { await GatherRunner.setupDriver(driver, { settings: { emulatedFormFactor: 'mobile', + throttlingMethod: 'provided', internalDisableDeviceScreenEmulation: false, }, }); @@ -300,54 +301,6 @@ describe('GatherRunner', function() { connectionStub.sendCommand.findInvocation('Emulation.setDeviceMetricsOverride')).toThrow(); }); - it('stops throttling when not devtools', async () => { - // const tests = { - // calledDeviceEmulation: false, - // calledNetworkEmulation: false, - // calledCpuEmulation: false, - // }; - // const createEmulationCheck = variable => (...args) => { - // tests[variable] = args; - // return true; - // }; - // const driver = getMockedEmulationDriver( - // createEmulationCheck('calledDeviceEmulation'), - // createEmulationCheck('calledNetworkEmulation'), - // createEmulationCheck('calledCpuEmulation') - // ); - - // return GatherRunner.setupDriver(driver, { - // settings: { - // emulatedFormFactor: 'mobile', - // throttlingMethod: 'provided', - // internalDisableDeviceScreenEmulation: false, - // }, - // }).then(_ => { - // assert.ok(tests.calledDeviceEmulation, 'did not call device emulation'); - // assert.deepEqual(tests.calledNetworkEmulation, [{ - // latency: 0, downloadThroughput: 0, uploadThroughput: 0, offline: false, - // }]); - // assert.ok(!tests.calledCpuEmulation, 'called CPU emulation'); - // }); - - // ? ... this test is the same as above.. - - await GatherRunner.setupDriver(driver, { - settings: { - emulatedFormFactor: 'mobile', - throttlingMethod: 'provided', - internalDisableDeviceScreenEmulation: false, - }, - }); - - connectionStub.sendCommand.findInvocation('Emulation.setDeviceMetricsOverride'); - expect(connectionStub.sendCommand.findInvocation('Network.emulateNetworkConditions')).toEqual({ - latency: 0, downloadThroughput: 0, uploadThroughput: 0, offline: false, - }); - expect(() => - connectionStub.sendCommand.findInvocation('Emulation.setCPUThrottlingRate')).toThrow(); - }); - it('sets throttling according to settings', async () => { await GatherRunner.setupDriver(driver, { settings: { From 0d3379c797c305ef1b1e2cbffcc83ab982e7cd68 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 10 Jan 2020 13:04:27 -0800 Subject: [PATCH 14/22] fix sillyness --- lighthouse-core/test/gather/mock-commands.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lighthouse-core/test/gather/mock-commands.js b/lighthouse-core/test/gather/mock-commands.js index 7f74b2440ea6..80770e4ac4e8 100644 --- a/lighthouse-core/test/gather/mock-commands.js +++ b/lighthouse-core/test/gather/mock-commands.js @@ -118,7 +118,6 @@ function createMockOnceFn() { }); const mockFn = Object.assign(mockFnImpl, { - ...mockFnImpl, /** * @template {keyof LH.CrdpEvents} E * @param {E} event @@ -166,7 +165,6 @@ function createMockOnFn() { }); const mockFn = Object.assign(mockFnImpl, { - ...mockFnImpl, /** * @template {keyof LH.CrdpEvents} E * @param {E} event From e14bcdec972112fc84e2c086ec0c7ac540480079 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 10 Jan 2020 16:10:37 -0800 Subject: [PATCH 15/22] wip --- .../test/gather/gather-runner-test.js | 164 +++++++++++++++--- tsconfig.json | 1 + types/externs.d.ts | 44 +++++ 3 files changed, 181 insertions(+), 28 deletions(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 78dd8380b338..b6bcd42d3448 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -8,7 +8,7 @@ /* eslint-env jest */ const Gatherer = require('../../gather/gatherers/gatherer.js'); -const GatherRunner = require('../../gather/gather-runner.js'); +const GatherRunner_ = require('../../gather/gather-runner.js'); const assert = require('assert'); const Config = require('../../config/config.js'); const unresolvedPerfLog = require('./../fixtures/unresolved-perflog.json'); @@ -21,6 +21,48 @@ const {createMockSendCommandFn} = require('./mock-commands.js'); jest.mock('../../lib/stack-collector.js', () => () => Promise.resolve([])); +/** + * @template {Array} TParams + * @template TReturn + * @param {(...args: TParams) => TReturn} fn + */ +function makeParamsOptional(fn) { + return /** @type {(...args: RecursivePartial) => TReturn} */ (fn); +} + +const GatherRunner = { + afterPass: makeParamsOptional(GatherRunner_.afterPass), + beginRecording: makeParamsOptional(GatherRunner_.beginRecording), + collectArtifacts: makeParamsOptional(GatherRunner_.collectArtifacts), + endRecording: makeParamsOptional(GatherRunner_.endRecording), + getInterstitialError: makeParamsOptional(GatherRunner_.getInterstitialError), + getNetworkError: makeParamsOptional(GatherRunner_.getNetworkError), + getPageLoadError: makeParamsOptional(GatherRunner_.getPageLoadError), + getWebAppManifest: makeParamsOptional(GatherRunner_.getWebAppManifest), + initializeBaseArtifacts: makeParamsOptional(GatherRunner_.initializeBaseArtifacts), + loadPage: makeParamsOptional(GatherRunner_.loadPage), + run: makeParamsOptional(GatherRunner_.run), + runPass: makeParamsOptional(GatherRunner_.runPass), + setupDriver: makeParamsOptional(GatherRunner_.setupDriver), + setupPassNetwork: makeParamsOptional(GatherRunner_.setupPassNetwork), +}; + +/** + * @param {RecursivePartial} json + */ +function makeConfig(json) { + // @ts-ignore: allow recursive partial. + const config = new Config(json); + if (!config.passes) throw new Error('expected config.passes to exist'); + return {passes: config.passes, settings: config.settings}; +} + +const t = /** @type {RecursivePartial} */ ({}); +const t2 = t.passes && t.passes[0]; +if (t2) { + t2; +} + class TestGatherer extends Gatherer { constructor() { super(); @@ -55,14 +97,20 @@ class EmulationDriver extends Driver { registerPerformanceObserver() { return Promise.resolve(); } - cleanBrowserCaches() {} - clearDataForOrigin() {} + cleanBrowserCaches() { + return Promise.resolve(); + } + clearDataForOrigin() { + return Promise.resolve(); + } } const fakeDriver = require('./fake-driver.js'); const fakeDriverUsingRealMobileDevice = fakeDriver.fakeDriverUsingRealMobileDevice; +/** @type {EmulationDriver} */ let driver; +/** @type {Connection & {sendCommand: ReturnType}} */ let connectionStub; function resetDefaultMockResponses() { @@ -80,7 +128,9 @@ function resetDefaultMockResponses() { } beforeEach(() => { + // @ts-ignore - connectionStub has a mocked version of sendCommand implemented in each test connectionStub = new Connection(); + // @ts-ignore connectionStub.sendCommand = cmd => { throw new Error(`${cmd} not implemented`); }; @@ -134,7 +184,7 @@ describe('GatherRunner', function() { it('collects benchmark as an artifact', async () => { const requestedUrl = 'https://example.com'; const driver = fakeDriver; - const config = new Config({passes: []}); + const config = makeConfig({passes: []}); const options = {requestedUrl, driver, settings: config.settings}; const results = await GatherRunner.run(config.passes, options); @@ -144,7 +194,7 @@ describe('GatherRunner', function() { it('collects host user agent as an artifact', async () => { const requestedUrl = 'https://example.com'; const driver = fakeDriver; - const config = new Config({passes: []}); + const config = makeConfig({passes: []}); const options = {requestedUrl, driver, settings: config.settings}; const results = await GatherRunner.run(config.passes, options); @@ -155,7 +205,7 @@ describe('GatherRunner', function() { it('collects network user agent as an artifact', async () => { const requestedUrl = 'https://example.com'; const driver = fakeDriver; - const config = new Config({passes: [{}]}); + const config = makeConfig({passes: [{}]}); const options = {requestedUrl, driver, settings: config.settings}; const results = await GatherRunner.run(config.passes, options); @@ -170,7 +220,7 @@ describe('GatherRunner', function() { return Promise.resolve(finalUrl); }, }); - const config = new Config({passes: [{}]}); + const config = makeConfig({passes: [{}]}); const options = {requestedUrl, driver, settings: config.settings}; return GatherRunner.run(config.passes, options).then(artifacts => { @@ -184,7 +234,7 @@ describe('GatherRunner', function() { it('works when running on desktop device without emulation', async () => { const driver = fakeDriver; - const config = new Config({ + const config = makeConfig({ passes: [], settings: {emulatedFormFactor: 'none', internalDisableDeviceScreenEmulation: false}, }); @@ -281,6 +331,7 @@ describe('GatherRunner', function() { }); it('applies the correct emulation given a particular emulationFormFactor', async () => { + /** @param {'mobile'|'desktop'|'none'} formFactor */ const getSettings = formFactor => ({ emulatedFormFactor: formFactor, internalDisableDeviceScreenEmulation: false, @@ -876,6 +927,15 @@ describe('GatherRunner', function() { }); describe('#getNetworkError', () => { + /** + * @param {NetworkRequest=} mainRecord + */ + function getAndExpectError(mainRecord) { + const error = GatherRunner.getNetworkError(mainRecord); + if (!error) throw new Error('expected a network error'); + return error; + } + it('passes when the page is loaded', () => { const url = 'http://the-page.com'; const mainRecord = new NetworkRequest(); @@ -889,7 +949,7 @@ describe('GatherRunner', function() { mainRecord.url = url; mainRecord.failed = true; mainRecord.localizedFailDescription = 'foobar'; - const error = GatherRunner.getNetworkError(mainRecord); + const error = getAndExpectError(mainRecord); assert.equal(error.message, 'FAILED_DOCUMENT_REQUEST'); assert.equal(error.code, 'FAILED_DOCUMENT_REQUEST'); expect(error.friendlyMessage) @@ -897,7 +957,7 @@ describe('GatherRunner', function() { }); it('fails when page times out', () => { - const error = GatherRunner.getNetworkError(undefined); + const error = getAndExpectError(undefined); assert.equal(error.message, 'NO_DOCUMENT_REQUEST'); assert.equal(error.code, 'NO_DOCUMENT_REQUEST'); expect(error.friendlyMessage).toBeDisplayString(/^Lighthouse was unable to reliably load/); @@ -908,7 +968,7 @@ describe('GatherRunner', function() { const mainRecord = new NetworkRequest(); mainRecord.url = url; mainRecord.statusCode = 404; - const error = GatherRunner.getNetworkError(mainRecord); + const error = getAndExpectError(mainRecord); assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST'); assert.equal(error.code, 'ERRORED_DOCUMENT_REQUEST'); expect(error.friendlyMessage) @@ -920,7 +980,7 @@ describe('GatherRunner', function() { const mainRecord = new NetworkRequest(); mainRecord.url = url; mainRecord.statusCode = 500; - const error = GatherRunner.getNetworkError(mainRecord); + const error = getAndExpectError(mainRecord); assert.equal(error.message, 'ERRORED_DOCUMENT_REQUEST'); assert.equal(error.code, 'ERRORED_DOCUMENT_REQUEST'); expect(error.friendlyMessage) @@ -933,7 +993,7 @@ describe('GatherRunner', function() { mainRecord.url = url; mainRecord.failed = true; mainRecord.localizedFailDescription = 'net::ERR_NAME_NOT_RESOLVED'; - const error = GatherRunner.getNetworkError(mainRecord); + const error = getAndExpectError(mainRecord); assert.equal(error.message, 'DNS_FAILURE'); assert.equal(error.code, 'DNS_FAILURE'); expect(error.friendlyMessage).toBeDisplayString(/^DNS servers could not resolve/); @@ -941,6 +1001,16 @@ describe('GatherRunner', function() { }); describe('#getInterstitialError', () => { + /** + * @param {NetworkRequest} mainRecord + * @param {NetworkRequest[]} networkRecords + */ + function getAndExpectError(mainRecord, networkRecords) { + const error = GatherRunner.getInterstitialError(mainRecord, networkRecords); + if (!error) throw new Error('expected an interstitial error'); + return error; + } + it('passes when the page was not requested', () => { expect(GatherRunner.getInterstitialError(undefined, [])).toBeUndefined(); }); @@ -983,7 +1053,7 @@ describe('GatherRunner', function() { interstitialRecord.url = 'data:text/html;base64,abcdef'; interstitialRecord.documentURL = 'chrome-error://chromewebdata/'; const records = [mainRecord, interstitialRecord]; - const error = GatherRunner.getInterstitialError(mainRecord, records); + const error = getAndExpectError(mainRecord, records); expect(error.message).toEqual('CHROME_INTERSTITIAL_ERROR'); expect(error.code).toEqual('CHROME_INTERSTITIAL_ERROR'); expect(error.friendlyMessage).toBeDisplayString(/^Chrome prevented/); @@ -999,7 +1069,7 @@ describe('GatherRunner', function() { interstitialRecord.url = 'data:text/html;base64,abcdef'; interstitialRecord.documentURL = 'chrome-error://chromewebdata/'; const records = [mainRecord, interstitialRecord]; - const error = GatherRunner.getInterstitialError(mainRecord, records); + const error = getAndExpectError(mainRecord, records); expect(error.message).toEqual('INSECURE_DOCUMENT_REQUEST'); expect(error.code).toEqual('INSECURE_DOCUMENT_REQUEST'); expect(error.friendlyMessage).toBeDisplayString(/valid security certificate/); @@ -1025,14 +1095,35 @@ describe('GatherRunner', function() { }); describe('#getPageLoadError', () => { + const LoadFailureMode = { + fatal: /** @type {'fatal'} */ ('fatal'), + ignore: /** @type {'ignore'} */ ('ignore'), + warn: /** @type {'warn'} */ ('warn'), + }; + + /** + * @param {RecursivePartial} passContext + * @param {RecursivePartial} loadData + * @param {LH.LighthouseError|undefined} navigationError + */ + function getAndExpectError(passContext, loadData, navigationError) { + const error = GatherRunner.getPageLoadError(passContext, loadData, navigationError); + if (!error) throw new Error('expected a page load error'); + return error; + } + + /** @type {LH.LighthouseError} */ let navigationError; beforeEach(() => { - navigationError = new Error('NAVIGATION_ERROR'); + navigationError = /** @type {LH.LighthouseError} */ (new Error('NAVIGATION_ERROR')); }); it('passes when the page is loaded', () => { - const passContext = {url: 'http://the-page.com', passConfig: {loadFailureMode: 'fatal'}}; + const passContext = { + url: 'http://the-page.com', + passConfig: {loadFailureMode: LoadFailureMode.fatal}, + }; const mainRecord = new NetworkRequest(); const loadData = {networkRecords: [mainRecord]}; mainRecord.url = passContext.url; @@ -1041,9 +1132,11 @@ describe('GatherRunner', function() { }); it('passes when the page is loaded, ignoring any fragment', () => { - const url = 'http://example.com/#/page/list'; + const passContext = { + url: 'http://example.com/#/page/list', + passConfig: {loadFailureMode: LoadFailureMode.fatal}, + }; const mainRecord = new NetworkRequest(); - const passContext = {url, passConfig: {loadFailureMode: 'fatal'}}; const loadData = {networkRecords: [mainRecord]}; mainRecord.url = 'http://example.com'; const error = GatherRunner.getPageLoadError(passContext, loadData, undefined); @@ -1051,7 +1144,10 @@ describe('GatherRunner', function() { }); it('passes when the page is expected to fail', () => { - const passContext = {url: 'http://the-page.com', passConfig: {loadFailureMode: 'ignore'}}; + const passContext = { + url: 'http://the-page.com', + passConfig: {loadFailureMode: LoadFailureMode.ignore}, + }; const mainRecord = new NetworkRequest(); const loadData = {networkRecords: [mainRecord]}; mainRecord.url = passContext.url; @@ -1062,7 +1158,10 @@ describe('GatherRunner', function() { }); it('fails with interstitial error first', () => { - const passContext = {url: 'http://the-page.com', passConfig: {loadFailureMode: 'fatal'}}; + const passContext = { + url: 'http://the-page.com', + passConfig: {loadFailureMode: LoadFailureMode.fatal}, + }; const mainRecord = new NetworkRequest(); const interstitialRecord = new NetworkRequest(); const loadData = {networkRecords: [mainRecord, interstitialRecord]}; @@ -1072,41 +1171,50 @@ describe('GatherRunner', function() { interstitialRecord.url = 'data:text/html;base64,abcdef'; interstitialRecord.documentURL = 'chrome-error://chromewebdata/'; - const error = GatherRunner.getPageLoadError(passContext, loadData, navigationError); + const error = getAndExpectError(passContext, loadData, navigationError); expect(error.message).toEqual('CHROME_INTERSTITIAL_ERROR'); }); it('fails with network error next', () => { - const passContext = {url: 'http://the-page.com', passConfig: {loadFailureMode: 'fatal'}}; + const passContext = { + url: 'http://the-page.com', + passConfig: {loadFailureMode: LoadFailureMode.fatal}, + }; const mainRecord = new NetworkRequest(); const loadData = {networkRecords: [mainRecord]}; mainRecord.url = passContext.url; mainRecord.failed = true; - const error = GatherRunner.getPageLoadError(passContext, loadData, navigationError); + const error = getAndExpectError(passContext, loadData, navigationError); expect(error.message).toEqual('FAILED_DOCUMENT_REQUEST'); }); it('fails with nav error last', () => { - const passContext = {url: 'http://the-page.com', passConfig: {loadFailureMode: 'fatal'}}; + const passContext = { + url: 'http://the-page.com', + passConfig: {loadFailureMode: LoadFailureMode.fatal}, + }; const mainRecord = new NetworkRequest(); const loadData = {networkRecords: [mainRecord]}; mainRecord.url = passContext.url; - const error = GatherRunner.getPageLoadError(passContext, loadData, navigationError); + const error = getAndExpectError(passContext, loadData, navigationError); expect(error.message).toEqual('NAVIGATION_ERROR'); }); it('fails when loadFailureMode is warn', () => { - const passContext = {url: 'http://the-page.com', passConfig: {loadFailureMode: 'warn'}}; + const passContext = { + url: 'http://the-page.com', + passConfig: {loadFailureMode: LoadFailureMode.warn}, + }; const mainRecord = new NetworkRequest(); const loadData = {networkRecords: [mainRecord]}; mainRecord.url = passContext.url; - const error = GatherRunner.getPageLoadError(passContext, loadData, navigationError); + const error = getAndExpectError(passContext, loadData, navigationError); expect(error.message).toEqual('NAVIGATION_ERROR'); }); }); diff --git a/tsconfig.json b/tsconfig.json index fe0eecb3da96..dfdb8067addb 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -28,5 +28,6 @@ "files": [ // Opt-in to typechecking for some core tests. "lighthouse-core/test/gather/driver-test.js", + "lighthouse-core/test/gather/gather-runner-test.js", ], } diff --git a/types/externs.d.ts b/types/externs.d.ts index 2c210b209e38..75fcd6060624 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -7,6 +7,44 @@ import _Crdp from 'devtools-protocol/types/protocol'; import _CrdpMappings from 'devtools-protocol/types/protocol-mapping' +// Convert unions (T1 | T2 | T3) into tuples ([T1, T2, T3]). +// https://stackoverflow.com/a/52933137/2788187 https://stackoverflow.com/a/50375286 +type UnionToIntersection = +(U extends any ? (k: U)=>void : never) extends ((k: infer I)=>void) ? I : never + +type UnionToFunctions = + U extends unknown ? (k: U) => void : never; + +type IntersectionOfFunctionsToType = + F extends { (a: infer A): void; (b: infer B): void; (c: infer C): void; } ? [A, B, C] : + F extends { (a: infer A): void; (b: infer B): void; } ? [A, B] : + F extends { (a: infer A): void } ? [A] : + never; + +type SplitType = + IntersectionOfFunctionsToType>>; + +// (T1 | T2 | T3) -> [RecursivePartial(T1), RecursivePartial(T2), RecursivePartial(T3)] +type RecursivePartialUnion> = {[P in keyof S]: RecursivePartial}; + +// Returns length of tuple. +type GetLength = T extends { length: infer L } ? L : never + +// type Test2> = {[P in keyof S]: RecursivePartial}; +// type C = Test2<{ +// a: Array<{a: number} | null; +// }>[number]; + +// type E = RecursivePartial<{ +// a: Array<{a: number}> | Array<{b: number}> | Array<{c: number}> | null; +// }>; +// type E1 = SplitType<{ +// a: Array<{a: number}> | Array<{b: number}> | Array<{c: number}> | null; +// }['a']>; + +// type Test3 = T extends (infer U1 extends any ? any : never) | (infer U1 extends any ? any : never) ? 1 : 2; +// type D = Test3; + declare global { // Augment Intl to include // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/getCanonicalLocales @@ -29,6 +67,12 @@ declare global { /** Make optional all properties on T and any properties on object properties of T. */ type RecursivePartial = { [P in keyof T]+?: + // If type is a union, map each individual component and transform the resultant tuple back into a union. + // Only 3 components are supported. For more, modify the following line and `IntersectionOfFunctionsToType`. + // Guard against large string unions, which would be unreasonable to support (much more than 3 components is common). + SplitType extends string[] ? T[P] : + GetLength> extends 2|3 ? RecursivePartialUnion[number] : + // SplitType extends [any, any] | [undefined, any, any] ? RecursivePartialUnion[number] : T[P] extends (infer U)[] ? RecursivePartial[] : T[P] extends (object|undefined) ? RecursivePartial : T[P]; From 00e7b6bda00f8374e5157ffc18867d8e0a9a6c19 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 10 Jan 2020 16:33:34 -0800 Subject: [PATCH 16/22] more wip --- .../test/gather/gather-runner-test.js | 106 +++++++++++------- 1 file changed, 67 insertions(+), 39 deletions(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index b6bcd42d3448..31ce2d4d1957 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -246,7 +246,7 @@ describe('GatherRunner', function() { it('works when running on desktop device with mobile emulation', async () => { const driver = fakeDriver; - const config = new Config({ + const config = makeConfig({ passes: [], settings: {emulatedFormFactor: 'mobile', internalDisableDeviceScreenEmulation: false}, }); @@ -258,7 +258,7 @@ describe('GatherRunner', function() { it('works when running on mobile device without emulation', async () => { const driver = fakeDriverUsingRealMobileDevice; - const config = new Config({ + const config = makeConfig({ passes: [], settings: {emulatedFormFactor: 'none', internalDisableDeviceScreenEmulation: false}, }); @@ -270,7 +270,7 @@ describe('GatherRunner', function() { it('works when running on mobile device with desktop emulation', async () => { const driver = fakeDriverUsingRealMobileDevice; - const config = new Config({ + const config = makeConfig({ passes: [], settings: {emulatedFormFactor: 'desktop', internalDisableDeviceScreenEmulation: false}, }); @@ -284,6 +284,11 @@ describe('GatherRunner', function() { describe('collects HostFormFactor as an artifact', () => { const requestedUrl = 'https://example.com'; + /** + * @param {string} name + * @param {string} userAgent + * @param {string} expectedValue + */ function test(name, userAgent, expectedValue) { it(name, async () => { const driver = Object.assign({}, fakeDriver, { @@ -291,7 +296,7 @@ describe('GatherRunner', function() { return Promise.resolve({userAgent: userAgent}); }, }); - const config = new Config({ + const config = makeConfig({ passes: [], settings: {}, }); @@ -377,10 +382,12 @@ describe('GatherRunner', function() { it('clears origin storage', () => { const asyncFunc = () => Promise.resolve(); + /** @type {Record} */ const tests = { calledCleanBrowserCaches: false, calledClearStorage: false, }; + /** @param {string} variable */ const createCheck = variable => () => { tests[variable] = true; return Promise.resolve(); @@ -409,9 +416,11 @@ describe('GatherRunner', function() { it('clears the disk & memory cache on a perf run', async () => { const asyncFunc = () => Promise.resolve(); + /** @type {Record} */ const tests = { calledCleanBrowserCaches: false, }; + /** @param {string} variable */ const createCheck = variable => () => { tests[variable] = true; return Promise.resolve(); @@ -491,13 +500,14 @@ describe('GatherRunner', function() { const navigationError = new LHError(LHError.errors.NO_FCP); const driver = Object.assign({}, fakeDriver, { online: true, + /** @param {string} url */ gotoURL: url => url.includes('blank') ? null : Promise.reject(navigationError), endDevtoolsLog() { return networkRecordsToDevtoolsLog([{url: requestedUrl}]); }, }); - const config = new Config({ + const config = makeConfig({ passes: [{ recordTrace: true, passName: 'firstPass', @@ -528,6 +538,7 @@ describe('GatherRunner', function() { .mockRejectedValueOnce(navigationError); const driver = Object.assign({}, fakeDriver, { online: true, + /** @param {string} url */ gotoURL: url => url.includes('blank') ? gotoUrlForAboutBlank() : gotoUrlForRealUrl(), endDevtoolsLog() { return networkRecordsToDevtoolsLog([{url: requestedUrl}]); @@ -557,10 +568,12 @@ describe('GatherRunner', function() { it('does not clear origin storage with flag --disable-storage-reset', () => { const asyncFunc = () => Promise.resolve(); + /** @type {Record () => { tests[variable] = true; return Promise.resolve(); @@ -766,7 +779,7 @@ describe('GatherRunner', function() { const t1 = new TestGatherer(); const t2 = new TestGatherer(); - const config = new Config({ + const config = makeConfig({ passes: [{ recordTrace: true, passName: 'firstPass', @@ -792,7 +805,7 @@ describe('GatherRunner', function() { }); it('respects trace names', () => { - const config = new Config({ + const config = makeConfig({ passes: [{ recordTrace: true, passName: 'firstPass', @@ -819,7 +832,7 @@ describe('GatherRunner', function() { }); it('doesn\'t leave networkRecords as an artifact', () => { - const config = new Config({ + const config = makeConfig({ passes: [{ recordTrace: true, passName: 'firstPass', @@ -851,7 +864,7 @@ describe('GatherRunner', function() { endDevtoolsLog: () => [], }); - const config = new Config({ + const config = makeConfig({ passes: [{ passName: 'firstPass', recordTrace: true, @@ -873,7 +886,7 @@ describe('GatherRunner', function() { const t1 = new (class Test1 extends TestGatherer {})(); const t2 = new (class Test2 extends TestGatherer {})(); const t3 = new (class Test3 extends TestGatherer {})(); - const config = new Config({ + const config = makeConfig({ passes: [{ passName: 'firstPass', recordTrace: true, @@ -1222,23 +1235,36 @@ describe('GatherRunner', function() { describe('artifact collection', () => { // Make sure our gatherers never execute in parallel it('runs gatherer lifecycle methods strictly in sequence', async () => { + /** @type {Record} */ const counter = { beforePass: 0, pass: 0, afterPass: 0, }; const shortPause = () => new Promise(resolve => setTimeout(resolve, 50)); + /** + * @param {string} counterName + * @param {number} value + */ async function fastish(counterName, value) { assert.strictEqual(counter[counterName], value - 1); counter[counterName] = value; await shortPause(); assert.strictEqual(counter[counterName], value); } + /** + * @param {string} counterName + * @param {number} value + */ async function medium(counterName, value) { await Promise.resolve(); await Promise.resolve(); await fastish(counterName, value); } + /** + * @param {string} counterName + * @param {number} value + */ async function slowwwww(counterName, value) { await shortPause(); await shortPause(); @@ -1283,7 +1309,7 @@ describe('GatherRunner', function() { } }, ]; - const config = new Config({ + const config = makeConfig({ passes: [{ gatherers: gatherers.map(G => ({instance: new G()})), }], @@ -1338,7 +1364,7 @@ describe('GatherRunner', function() { }(), ].map(instance => ({instance})); const gathererNames = gatherers.map(gatherer => gatherer.instance.name); - const config = new Config({ + const config = makeConfig({ passes: [{ gatherers, }], @@ -1355,7 +1381,7 @@ describe('GatherRunner', function() { }); }); - it('passes gatherer options', () => { + it('passes gatherer options', async () => { const calls = {beforePass: [], pass: [], afterPass: []}; class EavesdropGatherer extends Gatherer { beforePass(context) { @@ -1376,34 +1402,36 @@ describe('GatherRunner', function() { {instance: new class EavesdropGatherer3 extends EavesdropGatherer {}()}, ]; - const config = new Config({ + const config = makeConfig({ passes: [{gatherers}], }); - return GatherRunner.run(config.passes, { + /** @type {any} */ + const artifacts = await GatherRunner.run(config.passes, { driver: fakeDriver, requestedUrl: 'https://example.com', settings: config.settings, - }).then(artifacts => { - assert.equal(artifacts.EavesdropGatherer1, 1); - assert.equal(artifacts.EavesdropGatherer2, 2); - assert.equal(artifacts.EavesdropGatherer3, 'none'); - - // assert that all three phases received the gatherer options expected - const expectedOptions = [{x: 1}, {x: 2}, {}]; - for (let i = 0; i < 3; i++) { - assert.deepEqual(calls.beforePass[i], expectedOptions[i]); - assert.deepEqual(calls.pass[i], expectedOptions[i]); - assert.deepEqual(calls.afterPass[i], expectedOptions[i]); - } }); + + assert.equal(artifacts.EavesdropGatherer1, 1); + assert.equal(artifacts.EavesdropGatherer2, 2); + assert.equal(artifacts.EavesdropGatherer3, 'none'); + + // assert that all three phases received the gatherer options expected + const expectedOptions = [{x: 1}, {x: 2}, {}]; + for (let i = 0; i < 3; i++) { + assert.deepEqual(calls.beforePass[i], expectedOptions[i]); + assert.deepEqual(calls.pass[i], expectedOptions[i]); + assert.deepEqual(calls.afterPass[i], expectedOptions[i]); + } }); - it('uses the last not-undefined phase result as artifact', () => { + it('uses the last not-undefined phase result as artifact', async () => { const recoverableError = new Error('My recoverable error'); const someOtherError = new Error('Bad, bad error.'); // Gatherer results are all expected to be arrays of promises + /** @type {any} Using fake gatherers. */ const gathererResults = { // 97 wins. AfterGatherer: [ @@ -1434,12 +1462,12 @@ describe('GatherRunner', function() { ], }; - return GatherRunner.collectArtifacts(gathererResults).then(({artifacts}) => { - assert.strictEqual(artifacts.AfterGatherer, 97); - assert.strictEqual(artifacts.PassGatherer, 284); - assert.strictEqual(artifacts.SingleErrorGatherer, recoverableError); - assert.strictEqual(artifacts.TwoErrorGatherer, recoverableError); - }); + /** @type {any} Using fake gatherers. */ + const {artifacts} = await GatherRunner.collectArtifacts(gathererResults); + assert.strictEqual(artifacts.AfterGatherer, 97); + assert.strictEqual(artifacts.PassGatherer, 284); + assert.strictEqual(artifacts.SingleErrorGatherer, recoverableError); + assert.strictEqual(artifacts.TwoErrorGatherer, recoverableError); }); it('produces a deduped LighthouseRunWarnings artifact from array of warnings', async () => { @@ -1458,7 +1486,7 @@ describe('GatherRunner', function() { } } - const config = new Config({ + const config = makeConfig({ passes: [{ gatherers: [{instance: new WarningGatherer()}], }], @@ -1511,7 +1539,7 @@ describe('GatherRunner', function() { }(), ].map(instance => ({instance})); const gathererNames = gatherers.map(gatherer => gatherer.instance.name); - const config = new Config({ + const config = makeConfig({ passes: [{ gatherers, }], @@ -1531,7 +1559,7 @@ describe('GatherRunner', function() { }); it('rejects if a gatherer does not provide an artifact', () => { - const config = new Config({ + const config = makeConfig({ passes: [{ recordTrace: true, passName: 'firstPass', @@ -1549,7 +1577,7 @@ describe('GatherRunner', function() { }); it('rejects when domain name can\'t be resolved', () => { - const config = new Config({ + const config = makeConfig({ passes: [{ recordTrace: true, passName: 'firstPass', @@ -1581,7 +1609,7 @@ describe('GatherRunner', function() { }); it('resolves when domain name can\'t be resolved but is offline', () => { - const config = new Config({ + const config = makeConfig({ passes: [{ recordTrace: true, passName: 'firstPass', From 36ab69963f8d3458ae28704a2a86e1cec6e0ee8d Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 10 Jan 2020 17:28:59 -0800 Subject: [PATCH 17/22] done --- .../test/gather/gather-runner-test.js | 104 +++++++++++------- .../test/network-records-to-devtools-log.js | 3 +- types/externs.d.ts | 5 +- 3 files changed, 68 insertions(+), 44 deletions(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 31ce2d4d1957..7f6d9ad11987 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -57,11 +57,11 @@ function makeConfig(json) { return {passes: config.passes, settings: config.settings}; } -const t = /** @type {RecursivePartial} */ ({}); -const t2 = t.passes && t.passes[0]; -if (t2) { - t2; -} +const LoadFailureMode = { + fatal: /** @type {'fatal'} */ ('fatal'), + ignore: /** @type {'ignore'} */ ('ignore'), + warn: /** @type {'warn'} */ ('warn'), +}; class TestGatherer extends Gatherer { constructor() { @@ -441,7 +441,7 @@ describe('GatherRunner', function() { }; const passConfig = { passName: 'default', - loadFailureMode: 'ignore', + loadFailureMode: LoadFailureMode.ignore, recordTrace: true, useThrottling: true, gatherers: [], @@ -468,13 +468,14 @@ describe('GatherRunner', function() { const navigationError = new LHError(LHError.errors.NO_FCP); const driver = Object.assign({}, fakeDriver, { online: true, + /** @param {string} url */ gotoURL: url => url.includes('blank') ? null : Promise.reject(navigationError), endDevtoolsLog() { return networkRecordsToDevtoolsLog([{url: requestedUrl, statusCode: 500}]); }, }); - const config = new Config({ + const config = makeConfig({ passes: [{ recordTrace: true, passName: 'firstPass', @@ -490,7 +491,8 @@ describe('GatherRunner', function() { const artifacts = await GatherRunner.run(config.passes, options); expect(artifacts.LighthouseRunWarnings).toHaveLength(1); expect(artifacts.PageLoadError).toBeInstanceOf(Error); - expect(artifacts.PageLoadError.code).toEqual('ERRORED_DOCUMENT_REQUEST'); + expect(artifacts.PageLoadError).toMatchObject({code: 'ERRORED_DOCUMENT_REQUEST'}); + // @ts-ignore: Fake gatherer. expect(artifacts.TestGatherer).toBeUndefined(); }); @@ -523,7 +525,8 @@ describe('GatherRunner', function() { const artifacts = await GatherRunner.run(config.passes, options); expect(artifacts.LighthouseRunWarnings).toHaveLength(1); expect(artifacts.PageLoadError).toBeInstanceOf(Error); - expect(artifacts.PageLoadError.code).toEqual('NO_FCP'); + expect(artifacts.PageLoadError).toMatchObject({code: 'NO_FCP'}); + // @ts-ignore: Fake gatherer. expect(artifacts.TestGatherer).toBeUndefined(); }); @@ -545,7 +548,7 @@ describe('GatherRunner', function() { }, }); - const config = new Config({ + const config = makeConfig({ passes: [{passName: 'defaultPass', recordTrace: true}, { loadFailureMode: 'warn', recordTrace: true, @@ -562,13 +565,14 @@ describe('GatherRunner', function() { const artifacts = await GatherRunner.run(config.passes, options); expect(artifacts.LighthouseRunWarnings).toHaveLength(1); expect(artifacts.PageLoadError).toEqual(null); + // @ts-ignore: Fake gatherer. expect(artifacts.TestGatherer).toBeUndefined(); expect(artifacts.devtoolsLogs).toHaveProperty('pageLoadError-nextPass'); }); it('does not clear origin storage with flag --disable-storage-reset', () => { const asyncFunc = () => Promise.resolve(); - /** @type {Record} */ const tests = { calledCleanBrowserCaches: false, calledClearStorage: false, @@ -749,6 +753,7 @@ describe('GatherRunner', function() { it('resets scroll position between every gatherer', async () => { class ScrollMcScrollyGatherer extends TestGatherer { + /** @param {{driver: Driver}} context */ afterPass(context) { context.driver.scrollTo({x: 1000, y: 1000}); } @@ -766,7 +771,11 @@ describe('GatherRunner', function() { ], }; - await GatherRunner.afterPass({url, driver, passConfig}, {}, {TestGatherer: []}); + /** @type {any} Using fake gatherer. */ + const gathererResults = { + TestGatherer: [], + }; + await GatherRunner.afterPass({url, driver, passConfig}, {}, gathererResults); // One time for the afterPass of ScrollMcScrolly, two times for the resets of the two gatherers. expect(scrollToSpy.mock.calls).toEqual([ [{x: 1000, y: 1000}], @@ -851,6 +860,7 @@ describe('GatherRunner', function() { return GatherRunner.run(config.passes, options) .then(artifacts => { + // @ts-ignore assert.equal(artifacts.networkRecords, undefined); }); }); @@ -858,7 +868,7 @@ describe('GatherRunner', function() { it('saves trace and devtoolsLog with error prefix when there was a runtime error', async () => { const requestedUrl = 'https://example.com'; const driver = Object.assign({}, fakeDriver, { - // resolved URL here does not match any request in the network records, causing a runtime error. + /** @param {string} _ Resolved URL here does not match any request in the network records, causing a runtime error. */ gotoURL: async _ => requestedUrl, online: true, endDevtoolsLog: () => [], @@ -874,7 +884,8 @@ describe('GatherRunner', function() { const options = {driver, requestedUrl, settings: config.settings}; const artifacts = await GatherRunner.run(config.passes, options); - expect(artifacts.PageLoadError.code).toEqual('NO_DOCUMENT_REQUEST'); + expect(artifacts.PageLoadError).toMatchObject({code: 'NO_DOCUMENT_REQUEST'}); + // @ts-ignore: Fake gatherer. expect(artifacts.TestGatherer).toBeUndefined(); // The only loadData available should be prefixed with `pageLoadError-`. @@ -905,7 +916,7 @@ describe('GatherRunner', function() { const requestedUrl = 'https://www.reddit.com/r/nba'; let firstLoad = true; const driver = Object.assign({}, fakeDriver, { - // Loads the page successfully in the first pass, fails with NO_FCP in the second. + /** @param {string} url Loads the page successfully in the first pass, fails with NO_FCP in the second. */ async gotoURL(url) { if (url.includes('blank')) return null; if (firstLoad) { @@ -926,13 +937,16 @@ describe('GatherRunner', function() { expect(t3.called).toBe(false); // But only t1 has a valid artifact; t2 and t3 aren't defined. + // @ts-ignore: Fake gatherer. expect(artifacts.Test1).toBe('MyArtifact'); + // @ts-ignore: Fake gatherer. expect(artifacts.Test2).toBeUndefined(); + // @ts-ignore: Fake gatherer. expect(artifacts.Test3).toBeUndefined(); // PageLoadError artifact has the error. expect(artifacts.PageLoadError).toBeInstanceOf(LHError); - expect(artifacts.PageLoadError.code).toEqual('NO_FCP'); + expect(artifacts.PageLoadError).toMatchObject({code: 'NO_FCP'}); // firstPass has a saved trace and devtoolsLog, secondPass has an error trace and log. expect(Object.keys(artifacts.traces)).toEqual(['firstPass', 'pageLoadError-secondPass']); @@ -1108,12 +1122,6 @@ describe('GatherRunner', function() { }); describe('#getPageLoadError', () => { - const LoadFailureMode = { - fatal: /** @type {'fatal'} */ ('fatal'), - ignore: /** @type {'ignore'} */ ('ignore'), - warn: /** @type {'warn'} */ ('warn'), - }; - /** * @param {RecursivePartial} passContext * @param {RecursivePartial} loadData @@ -1315,6 +1323,7 @@ describe('GatherRunner', function() { }], }); + /** @type {any} Using fake gatherers. */ const artifacts = await GatherRunner.run(config.passes, { driver: fakeDriver, requestedUrl: 'https://example.com', @@ -1382,37 +1391,47 @@ describe('GatherRunner', function() { }); it('passes gatherer options', async () => { + /** @type {Record} */ const calls = {beforePass: [], pass: [], afterPass: []}; - class EavesdropGatherer extends Gatherer { - beforePass(context) { - calls.beforePass.push(context.options); - } - pass(context) { - calls.pass.push(context.options); - } - afterPass(context) { - calls.afterPass.push(context.options); - return context.options.x || 'none'; - } - } + /** @param {string} name */ + const makeEavesdropGatherer = name => { + const C = class extends Gatherer {}; + Object.defineProperty(C, 'name', {value: name}); + return Object.assign(new C, { + /** @param {LH.Gatherer.PassContext} context */ + beforePass(context) { + calls.beforePass.push(context.options); + }, + /** @param {LH.Gatherer.PassContext} context */ + pass(context) { + calls.pass.push(context.options); + }, + /** @param {LH.Gatherer.PassContext} context */ + afterPass(context) { + calls.afterPass.push(context.options); + // @ts-ignore + return context.options.x || 'none'; + }, + }); + }; const gatherers = [ - {instance: new class EavesdropGatherer1 extends EavesdropGatherer {}(), options: {x: 1}}, - {instance: new class EavesdropGatherer2 extends EavesdropGatherer {}(), options: {x: 2}}, - {instance: new class EavesdropGatherer3 extends EavesdropGatherer {}()}, + {instance: makeEavesdropGatherer('EavesdropGatherer1'), options: {x: 1}}, + {instance: makeEavesdropGatherer('EavesdropGatherer2'), options: {x: 2}}, + {instance: makeEavesdropGatherer('EavesdropGatherer3')}, ]; const config = makeConfig({ passes: [{gatherers}], }); - /** @type {any} */ + /** @type {any} Using fake gatherers. */ const artifacts = await GatherRunner.run(config.passes, { driver: fakeDriver, requestedUrl: 'https://example.com', settings: config.settings, }); - + assert.equal(artifacts.EavesdropGatherer1, 1); assert.equal(artifacts.EavesdropGatherer2, 2); assert.equal(artifacts.EavesdropGatherer3, 'none'); @@ -1478,6 +1497,7 @@ describe('GatherRunner', function() { ]; class WarningGatherer extends Gatherer { + /** @param {LH.Gatherer.PassContext} passContext */ afterPass(passContext) { passContext.LighthouseRunWarnings.push(...runWarnings, ...runWarnings); assert.strictEqual(passContext.LighthouseRunWarnings.length, runWarnings.length * 2); @@ -1553,7 +1573,7 @@ describe('GatherRunner', function() { gathererNames.forEach(gathererName => { const errorArtifact = artifacts[gathererName]; assert.ok(errorArtifact instanceof Error); - assert.strictEqual(errorArtifact.message, gathererName); + expect(errorArtifact).toMatchObject({message: gathererName}); }); }); }); @@ -1642,6 +1662,7 @@ describe('GatherRunner', function() { describe('.getWebAppManifest', () => { const MANIFEST_URL = 'https://example.com/manifest.json'; + /** @type {RecursivePartial} */ let passContext; beforeEach(() => { @@ -1662,10 +1683,11 @@ describe('GatherRunner', function() { it('should parse the manifest when found', async () => { const manifest = {name: 'App'}; const getAppManifest = jest.spyOn(fakeDriver, 'getAppManifest'); + // @ts-ignore: Some terrible @types/jest bug lies here. getAppManifest.mockResolvedValueOnce({data: JSON.stringify(manifest), url: MANIFEST_URL}); const result = await GatherRunner.getWebAppManifest(passContext); expect(result).toHaveProperty('raw', JSON.stringify(manifest)); - expect(result.value).toMatchObject({ + expect(result && result.value).toMatchObject({ name: {value: 'App', raw: 'App'}, start_url: {value: passContext.url, raw: undefined}, }); diff --git a/lighthouse-core/test/network-records-to-devtools-log.js b/lighthouse-core/test/network-records-to-devtools-log.js index 50c380345395..5e025135ba4f 100644 --- a/lighthouse-core/test/network-records-to-devtools-log.js +++ b/lighthouse-core/test/network-records-to-devtools-log.js @@ -1,3 +1,4 @@ +// @ts-nocheck /** * @license Copyright 2018 Google Inc. All Rights Reserved. * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 @@ -16,7 +17,7 @@ const redirectSuffix = ':redirect'; /** * Extract requestId without any `:redirect` strings. - * @param {Partial} requestId + * @param {Partial} record */ function getBaseRequestId(record) { if (!record.requestId) return; diff --git a/types/externs.d.ts b/types/externs.d.ts index e5cfa3f5a000..dc7ca97cf0de 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -16,6 +16,7 @@ type UnionToFunctions = U extends unknown ? (k: U) => void : never; type IntersectionOfFunctionsToType = + F extends { (a: infer A): void; (b: infer B): void; (c: infer C): void; (d: infer D): void; } ? [A, B, C, D] : F extends { (a: infer A): void; (b: infer B): void; (c: infer C): void; } ? [A, B, C] : F extends { (a: infer A): void; (b: infer B): void; } ? [A, B] : F extends { (a: infer A): void } ? [A] : @@ -68,10 +69,10 @@ declare global { type RecursivePartial = { [P in keyof T]+?: // If type is a union, map each individual component and transform the resultant tuple back into a union. - // Only 3 components are supported. For more, modify the following line and `IntersectionOfFunctionsToType`. + // Only up to 4 components is supported. For more, modify the following line and `IntersectionOfFunctionsToType`. // Guard against large string unions, which would be unreasonable to support (much more than 3 components is common). SplitType extends string[] ? T[P] : - GetLength> extends 2|3 ? RecursivePartialUnion[number] : + GetLength> extends 2|3|4 ? RecursivePartialUnion[number] : // Recurse into arrays. T[P] extends (infer U)[] ? RecursivePartial[] : // Recurse into objects. From 5739f2f23eeec434eceb72121950ef048c27bd2d Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 10 Jan 2020 17:30:52 -0800 Subject: [PATCH 18/22] rm test code --- types/externs.d.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/types/externs.d.ts b/types/externs.d.ts index dc7ca97cf0de..3f917a045b24 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -28,24 +28,9 @@ type SplitType = // (T1 | T2 | T3) -> [RecursivePartial(T1), RecursivePartial(T2), RecursivePartial(T3)] type RecursivePartialUnion> = {[P in keyof S]: RecursivePartial}; -// Returns length of tuple. +// Return length of a tuple. type GetLength = T extends { length: infer L } ? L : never -// type Test2> = {[P in keyof S]: RecursivePartial}; -// type C = Test2<{ -// a: Array<{a: number} | null; -// }>[number]; - -// type E = RecursivePartial<{ -// a: Array<{a: number}> | Array<{b: number}> | Array<{c: number}> | null; -// }>; -// type E1 = SplitType<{ -// a: Array<{a: number}> | Array<{b: number}> | Array<{c: number}> | null; -// }['a']>; - -// type Test3 = T extends (infer U1 extends any ? any : never) | (infer U1 extends any ? any : never) ? 1 : 2; -// type D = Test3; - declare global { // Augment Intl to include // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/getCanonicalLocales From 9b1aa2e130656dac1500403636e524b2111fe4cb Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 13 Jan 2020 10:43:36 -0800 Subject: [PATCH 19/22] remove config null checks --- lighthouse-core/test/gather/gather-runner-test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 7f6d9ad11987..8798936cf072 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -52,9 +52,7 @@ const GatherRunner = { */ function makeConfig(json) { // @ts-ignore: allow recursive partial. - const config = new Config(json); - if (!config.passes) throw new Error('expected config.passes to exist'); - return {passes: config.passes, settings: config.settings}; + return new Config(json); } const LoadFailureMode = { From 0fbec8fa9e22ca9b7cb1eab66a3bc2dd065ff75c Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 13 Jan 2020 16:20:04 -0800 Subject: [PATCH 20/22] pr --- .../test/gather/gather-runner-test.js | 49 +++++-------------- types/externs.d.ts | 11 ++++- 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js index 8798936cf072..f324e9889c82 100644 --- a/lighthouse-core/test/gather/gather-runner-test.js +++ b/lighthouse-core/test/gather/gather-runner-test.js @@ -490,7 +490,7 @@ describe('GatherRunner', function() { expect(artifacts.LighthouseRunWarnings).toHaveLength(1); expect(artifacts.PageLoadError).toBeInstanceOf(Error); expect(artifacts.PageLoadError).toMatchObject({code: 'ERRORED_DOCUMENT_REQUEST'}); - // @ts-ignore: Fake gatherer. + // @ts-ignore: Test-only gatherer. expect(artifacts.TestGatherer).toBeUndefined(); }); @@ -524,7 +524,7 @@ describe('GatherRunner', function() { expect(artifacts.LighthouseRunWarnings).toHaveLength(1); expect(artifacts.PageLoadError).toBeInstanceOf(Error); expect(artifacts.PageLoadError).toMatchObject({code: 'NO_FCP'}); - // @ts-ignore: Fake gatherer. + // @ts-ignore: Test-only gatherer. expect(artifacts.TestGatherer).toBeUndefined(); }); @@ -563,7 +563,7 @@ describe('GatherRunner', function() { const artifacts = await GatherRunner.run(config.passes, options); expect(artifacts.LighthouseRunWarnings).toHaveLength(1); expect(artifacts.PageLoadError).toEqual(null); - // @ts-ignore: Fake gatherer. + // @ts-ignore: Test-only gatherer. expect(artifacts.TestGatherer).toBeUndefined(); expect(artifacts.devtoolsLogs).toHaveProperty('pageLoadError-nextPass'); }); @@ -769,7 +769,7 @@ describe('GatherRunner', function() { ], }; - /** @type {any} Using fake gatherer. */ + /** @type {any} Using Test-only gatherer. */ const gathererResults = { TestGatherer: [], }; @@ -838,31 +838,6 @@ describe('GatherRunner', function() { }); }); - it('doesn\'t leave networkRecords as an artifact', () => { - const config = makeConfig({ - passes: [{ - recordTrace: true, - passName: 'firstPass', - gatherers: [{instance: new TestGatherer()}], - }, { - recordTrace: true, - passName: 'secondPass', - gatherers: [{instance: new TestGatherer()}], - }], - }); - const options = { - driver: fakeDriver, - requestedUrl: 'https://example.com', - settings: config.settings, - }; - - return GatherRunner.run(config.passes, options) - .then(artifacts => { - // @ts-ignore - assert.equal(artifacts.networkRecords, undefined); - }); - }); - it('saves trace and devtoolsLog with error prefix when there was a runtime error', async () => { const requestedUrl = 'https://example.com'; const driver = Object.assign({}, fakeDriver, { @@ -883,7 +858,7 @@ describe('GatherRunner', function() { const artifacts = await GatherRunner.run(config.passes, options); expect(artifacts.PageLoadError).toMatchObject({code: 'NO_DOCUMENT_REQUEST'}); - // @ts-ignore: Fake gatherer. + // @ts-ignore: Test-only gatherer. expect(artifacts.TestGatherer).toBeUndefined(); // The only loadData available should be prefixed with `pageLoadError-`. @@ -935,11 +910,11 @@ describe('GatherRunner', function() { expect(t3.called).toBe(false); // But only t1 has a valid artifact; t2 and t3 aren't defined. - // @ts-ignore: Fake gatherer. + // @ts-ignore: Test-only gatherer. expect(artifacts.Test1).toBe('MyArtifact'); - // @ts-ignore: Fake gatherer. + // @ts-ignore: Test-only gatherer. expect(artifacts.Test2).toBeUndefined(); - // @ts-ignore: Fake gatherer. + // @ts-ignore: Test-only gatherer. expect(artifacts.Test3).toBeUndefined(); // PageLoadError artifact has the error. @@ -1321,7 +1296,7 @@ describe('GatherRunner', function() { }], }); - /** @type {any} Using fake gatherers. */ + /** @type {any} Using Test-only gatherers. */ const artifacts = await GatherRunner.run(config.passes, { driver: fakeDriver, requestedUrl: 'https://example.com', @@ -1423,7 +1398,7 @@ describe('GatherRunner', function() { passes: [{gatherers}], }); - /** @type {any} Using fake gatherers. */ + /** @type {any} Using Test-only gatherers. */ const artifacts = await GatherRunner.run(config.passes, { driver: fakeDriver, requestedUrl: 'https://example.com', @@ -1448,7 +1423,7 @@ describe('GatherRunner', function() { const someOtherError = new Error('Bad, bad error.'); // Gatherer results are all expected to be arrays of promises - /** @type {any} Using fake gatherers. */ + /** @type {any} Using Test-only gatherers. */ const gathererResults = { // 97 wins. AfterGatherer: [ @@ -1479,7 +1454,7 @@ describe('GatherRunner', function() { ], }; - /** @type {any} Using fake gatherers. */ + /** @type {any} Using Test-only gatherers. */ const {artifacts} = await GatherRunner.collectArtifacts(gathererResults); assert.strictEqual(artifacts.AfterGatherer, 97); assert.strictEqual(artifacts.PassGatherer, 284); diff --git a/types/externs.d.ts b/types/externs.d.ts index 3f917a045b24..ee9e8c53f3b8 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -29,7 +29,7 @@ type SplitType = type RecursivePartialUnion> = {[P in keyof S]: RecursivePartial}; // Return length of a tuple. -type GetLength = T extends { length: infer L } ? L : never +type GetLength = T extends { length: infer L } ? L : never; declare global { // Augment Intl to include @@ -53,15 +53,24 @@ declare global { /** Make optional all properties on T and any properties on object properties of T. */ type RecursivePartial = { [P in keyof T]+?: + // RE: First two conditions. // If type is a union, map each individual component and transform the resultant tuple back into a union. // Only up to 4 components is supported. For more, modify the following line and `IntersectionOfFunctionsToType`. + // Ex: `{passes: PassJson[] | null}` - T[P] doesn't exactly match the array-recursing condition, so without these first couple + // conditions, it would fall through to the last condition (would just return T[P]). + + // RE: First condition. // Guard against large string unions, which would be unreasonable to support (much more than 3 components is common). + SplitType extends string[] ? T[P] : GetLength> extends 2|3|4 ? RecursivePartialUnion[number] : + // Recurse into arrays. T[P] extends (infer U)[] ? RecursivePartial[] : + // Recurse into objects. T[P] extends (object|undefined) ? RecursivePartial : + // Strings, numbers, etc. (terminal types) end here. T[P]; }; From 652433e9b478bd4ffe0e687214a57045ec3a4609 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 13 Jan 2020 16:21:22 -0800 Subject: [PATCH 21/22] comment --- types/externs.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/externs.d.ts b/types/externs.d.ts index ee9e8c53f3b8..4656874eb5f9 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -60,7 +60,7 @@ declare global { // conditions, it would fall through to the last condition (would just return T[P]). // RE: First condition. - // Guard against large string unions, which would be unreasonable to support (much more than 3 components is common). + // Guard against large string unions, which would be unreasonable to support (much more than 4 components is common). SplitType extends string[] ? T[P] : GetLength> extends 2|3|4 ? RecursivePartialUnion[number] : From 4d9beb8d590b94c82b53dc1964766abd8baf6c85 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 13 Jan 2020 17:04:36 -0800 Subject: [PATCH 22/22] comment --- types/externs.d.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/types/externs.d.ts b/types/externs.d.ts index 4656874eb5f9..6c878c7b53dc 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -55,7 +55,8 @@ declare global { [P in keyof T]+?: // RE: First two conditions. // If type is a union, map each individual component and transform the resultant tuple back into a union. - // Only up to 4 components is supported. For more, modify the following line and `IntersectionOfFunctionsToType`. + // Only up to 4 components of a union is supported (all but the last few are dropped). For more, modify the second condition + // and `IntersectionOfFunctionsToType`. // Ex: `{passes: PassJson[] | null}` - T[P] doesn't exactly match the array-recursing condition, so without these first couple // conditions, it would fall through to the last condition (would just return T[P]).