diff --git a/lighthouse-core/gather/gatherers/offline.js b/lighthouse-core/gather/gatherers/offline.js index 673f0d455506..5b9814bc0b62 100644 --- a/lighthouse-core/gather/gatherers/offline.js +++ b/lighthouse-core/gather/gatherers/offline.js @@ -13,6 +13,9 @@ class Offline extends Gatherer { * @param {LH.Gatherer.PassContext} passContext */ beforePass(passContext) { + // This call sets up the offline state for the page navigation of the `offlinePass` in gather-runner. + // gather-runner will automatically go back online before the `afterPass` phase, so no additional + // cleanup is necessary. return passContext.driver.goOffline(); } @@ -21,14 +24,13 @@ class Offline extends Gatherer { * @param {LH.Gatherer.LoadData} loadData * @return {Promise} */ - afterPass(passContext, loadData) { + async afterPass(passContext, loadData) { const navigationRecord = loadData.networkRecords.filter(record => { return URL.equalWithExcludedFragments(record.url, passContext.url) && record.fetchedViaServiceWorker; }).pop(); // Take the last record that matches. - return passContext.driver.goOnline(passContext) - .then(_ => navigationRecord ? navigationRecord.statusCode : -1); + return navigationRecord ? navigationRecord.statusCode : -1; } } diff --git a/lighthouse-core/gather/gatherers/start-url.js b/lighthouse-core/gather/gatherers/start-url.js index e19e1effe4f6..07c387fe73c0 100644 --- a/lighthouse-core/gather/gatherers/start-url.js +++ b/lighthouse-core/gather/gatherers/start-url.js @@ -11,20 +11,36 @@ const Gatherer = require('./gatherer.js'); class StartUrl extends Gatherer { /** - * Grab the manifest, extract it's start_url, attempt to `fetch()` it while offline + * Go offline, assess the start url, go back online. * @param {LH.Gatherer.PassContext} passContext * @return {Promise} */ async afterPass(passContext) { + // `afterPass` is always online, so manually go offline to check start_url. + await passContext.driver.goOffline(); + const result = await this._determineStartUrlAvailability(passContext); + await passContext.driver.goOnline(passContext); + + return result; + } + + /** + * Grab the manifest, extract its start_url, attempt to `fetch()` it while offline + * @param {LH.Gatherer.PassContext} passContext + * @return {Promise} + */ + async _determineStartUrlAvailability(passContext) { const manifest = passContext.baseArtifacts.WebAppManifest; const startUrlInfo = this._readManifestStartUrl(manifest); if (startUrlInfo.isReadFailure) { return {statusCode: -1, explanation: startUrlInfo.reason}; } - return this._attemptStartURLFetch(passContext.driver, startUrlInfo.startUrl).catch(() => { - return {statusCode: -1, explanation: 'Unable to fetch start URL via service worker.'}; - }); + try { + return await this._attemptStartURLFetch(passContext.driver, startUrlInfo.startUrl); + } catch (err) { + return {statusCode: -1, explanation: 'Error while fetching start_url via service worker.'}; + } } /** @@ -59,7 +75,7 @@ class StartUrl extends Gatherer { // Wait up to 3s to get a matched network request from the fetch() to work const timeoutPromise = new Promise(resolve => setTimeout( - () => resolve({statusCode: -1, explanation: 'Timed out waiting for fetched start_url.'}), + () => resolve({statusCode: -1, explanation: 'Timed out waiting for start_url to respond.'}), 3000 ) ); @@ -77,7 +93,7 @@ class StartUrl extends Gatherer { if (!response.fromServiceWorker) { return resolve({ statusCode: -1, - explanation: 'Unable to fetch start URL via service worker.', + explanation: 'The start_url did respond, but not via a service worker.', }); } // Successful SW-served fetch of the start_URL diff --git a/lighthouse-core/test/gather/gatherers/start-url-test.js b/lighthouse-core/test/gather/gatherers/start-url-test.js index 3868ec62541c..113b93dea6ba 100644 --- a/lighthouse-core/test/gather/gatherers/start-url-test.js +++ b/lighthouse-core/test/gather/gatherers/start-url-test.js @@ -7,168 +7,220 @@ /* eslint-env jest */ +jest.useFakeTimers(); + const StartUrlGatherer = require('../../../gather/gatherers/start-url.js'); const parseManifest = require('../../../lib/manifest-parser.js'); -const assert = require('assert'); - -const mockDriver = { - goOffline() { - return Promise.resolve(); - }, - goOnline() { - return Promise.resolve(); - }, - off() {}, -}; - -const wrapSendCommand = (mockDriver, url, status = 200, fromServiceWorker = false) => { - mockDriver = Object.assign({}, mockDriver); - mockDriver.evaluateAsync = () => Promise.resolve(); - mockDriver.on = (name, cb) => { - cb({response: {status, url, fromServiceWorker}}); - }; - - return mockDriver; -}; - -describe('Start-url gatherer', () => { - let baseArtifacts; + +describe('StartUrl Gatherer', () => { + let mockDriver; + let gatherer; function createArtifactsWithURL(url) { return {WebAppManifest: {value: {start_url: {value: url}}}}; } + function unimplemented() { + throw new Error('Unimplemented'); + } + beforeEach(() => { - jest.useFakeTimers(); - baseArtifacts = {WebAppManifest: null}; + gatherer = new StartUrlGatherer(); + mockDriver = { + goOffline: unimplemented, + goOnline: unimplemented, + evaluateAsync: unimplemented, + on: unimplemented, + off: unimplemented, + }; }); afterEach(() => { jest.advanceTimersByTime(5000); }); - it('returns an artifact set to -1 when offline loading fails', () => { - const startUrlGatherer = new StartUrlGatherer(); - const startUrlGathererWithQueryString = new StartUrlGatherer(); - const startUrlGathererWithResponseNotFromSW = new StartUrlGatherer(); - const throwOnEvaluate = (mockDriver) => { - mockDriver.on = () => {}; - mockDriver.evaluateAsync = () => { - throw new Error({ - TypeError: 'Failed to fetch', - __failedInBrowser: true, - name: 'TypeError', - message: 'Failed to fetch', - }); - }; - - return mockDriver; - }; + it('returns a explanation when manifest cannot be found', async () => { + mockDriver.goOffline = jest.fn(); + mockDriver.goOnline = jest.fn(); const passContext = { - url: 'https://do-not-match.com/', - baseArtifacts, - driver: throwOnEvaluate(wrapSendCommand(mockDriver, 'https://do-not-match.com/', -1)), + baseArtifacts: {WebAppManifest: null}, + driver: mockDriver, }; - const passContextWithFragment = { - baseArtifacts, - url: 'https://ifixit-pwa.appspot.com/?history', - driver: throwOnEvaluate(wrapSendCommand(mockDriver, 'https://ifixit-pwa.appspot.com/?history', -1)), + + const result = await gatherer.afterPass(passContext); + expect(result).toEqual({ + statusCode: -1, + explanation: 'No usable web app manifest found on page.', + }); + }); + + it('returns a explanation when manifest cannot be parsed', async () => { + mockDriver.goOffline = jest.fn(); + mockDriver.goOnline = jest.fn(); + + const passContext = { + baseArtifacts: { + WebAppManifest: parseManifest( + 'this is invalid', + 'https://example.com/manifest.json', + 'https://example.com/' + ), + }, + driver: mockDriver, }; - const passContextWithResponseNotFromSW = { - url: 'https://do-not-match.com/', - baseArtifacts: createArtifactsWithURL('https://do-not-match.com/'), - driver: wrapSendCommand(mockDriver, 'https://do-not-match.com/', 200), + + const result = await gatherer.afterPass(passContext); + expect(result).toEqual({ + statusCode: -1, + explanation: + `Error fetching web app manifest: ERROR: file isn't valid JSON: ` + + `SyntaxError: Unexpected token h in JSON at position 1.`, + }); + }); + + it('sets the status code to -1 when navigation fails', async () => { + mockDriver.goOffline = jest.fn(); + mockDriver.goOnline = jest.fn(); + mockDriver.evaluateAsync = jest.fn().mockRejectedValue(new Error('Fetch failed')); + mockDriver.on = jest.fn(); + mockDriver.off = jest.fn(); + + const passContext = { + baseArtifacts: createArtifactsWithURL('https://example.com/'), + driver: mockDriver, }; - return Promise.all([ - startUrlGatherer.afterPass(passContext), - startUrlGathererWithQueryString.afterPass(passContextWithFragment), - startUrlGathererWithResponseNotFromSW.afterPass(passContextWithResponseNotFromSW), - ]).then(([artifact, artifactWithQueryString, artifactWithResponseNotFromSW]) => { - assert.equal(artifact.statusCode, -1); - assert.ok(artifact.explanation, 'did not set debug string'); - assert.equal(artifactWithQueryString.statusCode, -1); - assert.ok(artifactWithQueryString.explanation, 'did not set debug string'); - assert.equal(artifactWithResponseNotFromSW.statusCode, -1); - assert.equal(artifactWithResponseNotFromSW.explanation, - 'Unable to fetch start URL via service worker.'); + const result = await gatherer.afterPass(passContext); + expect(mockDriver.goOffline).toHaveBeenCalled(); + expect(mockDriver.goOnline).toHaveBeenCalled(); + expect(result).toEqual({ + statusCode: -1, + explanation: 'Error while fetching start_url via service worker.', }); }); - it('returns an artifact set to 200 when offline loading from service worker succeeds', () => { - const startUrlGatherer = new StartUrlGatherer(); - const startUrlGathererWithFragment = new StartUrlGatherer(); + it('sets the status code to page status code', async () => { + mockDriver.goOffline = jest.fn(); + mockDriver.goOnline = jest.fn(); + mockDriver.evaluateAsync = jest.fn().mockResolvedValue(); + mockDriver.on = jest.fn(); + mockDriver.off = jest.fn(); + const passContext = { - url: 'https://ifixit-pwa.appspot.com/', - baseArtifacts: createArtifactsWithURL('https://ifixit-pwa.appspot.com/'), - driver: wrapSendCommand(mockDriver, 'https://ifixit-pwa.appspot.com/', 200, true), + baseArtifacts: createArtifactsWithURL('https://example.com/'), + driver: mockDriver, }; - const passContextWithFragment = { - url: 'https://ifixit-pwa.appspot.com/#/history', - baseArtifacts: createArtifactsWithURL('https://ifixit-pwa.appspot.com/#/history'), - driver: wrapSendCommand(mockDriver, 'https://ifixit-pwa.appspot.com/#/history', 200, true), + + const response = { + url: 'https://example.com/', + status: 200, + fromServiceWorker: true, }; - return Promise.all([ - startUrlGatherer.afterPass(passContext), - startUrlGathererWithFragment.afterPass(passContextWithFragment), - ]).then(([artifact, artifactWithFragment]) => { - assert.equal(artifact.statusCode, 200); - assert.equal(artifactWithFragment.statusCode, 200); + mockDriver.on.mockImplementation((_, onResponseReceived) => onResponseReceived({response})); + + const result = await gatherer.afterPass(passContext); + expect(mockDriver.goOffline).toHaveBeenCalled(); + expect(mockDriver.goOnline).toHaveBeenCalled(); + expect(result).toEqual({ + statusCode: 200, }); }); - it('returns a explanation when manifest cannot be found', () => { - const driver = Object.assign({}, mockDriver); - const startUrlGatherer = new StartUrlGatherer(); + it('sets the status code to -1 when not from service worker', async () => { + mockDriver.goOffline = jest.fn(); + mockDriver.goOnline = jest.fn(); + mockDriver.evaluateAsync = jest.fn().mockResolvedValue(); + mockDriver.on = jest.fn(); + mockDriver.off = jest.fn(); + const passContext = { - baseArtifacts, - url: 'https://ifixit-pwa.appspot.com/', - driver, + baseArtifacts: createArtifactsWithURL('https://example.com/'), + driver: mockDriver, }; - return startUrlGatherer.afterPass(passContext) - .then(artifact => { - assert.equal(artifact.explanation, - `No usable web app manifest found on page.`); - }); + const response = { + url: 'https://example.com/', + status: 200, + fromServiceWorker: false, + }; + + mockDriver.on.mockImplementation((_, onResponseReceived) => onResponseReceived({response})); + + const result = await gatherer.afterPass(passContext); + expect(mockDriver.goOffline).toHaveBeenCalled(); + expect(mockDriver.goOnline).toHaveBeenCalled(); + expect(result).toEqual({ + statusCode: -1, + explanation: 'The start_url did respond, but not via a service worker.', + }); }); - it('returns a explanation when manifest cannot be parsed', () => { - const driver = Object.assign({}, mockDriver); - const startUrlGatherer = new StartUrlGatherer(); + it('sets the status code to -1 when times out waiting for a matching url', async () => { + mockDriver.goOffline = jest.fn(); + mockDriver.goOnline = jest.fn(); + mockDriver.evaluateAsync = jest.fn().mockResolvedValue(); + mockDriver.on = jest.fn(); + mockDriver.off = jest.fn(); + const passContext = { - baseArtifacts, - url: 'https://ifixit-pwa.appspot.com/', - driver, + baseArtifacts: createArtifactsWithURL('https://example.com/'), + driver: mockDriver, }; - baseArtifacts.WebAppManifest = parseManifest( - 'this is invalid', - 'https://ifixit-pwa.appspot.com/manifest.json', - passContext.url - ); - - return startUrlGatherer.afterPass(passContext) - .then(artifact => { - assert.equal(artifact.explanation, - `Error fetching web app manifest: ERROR: file isn't valid JSON: ` + - `SyntaxError: Unexpected token h in JSON at position 1.`); - }); + const response = { + url: 'https://random-other-url.com/', + status: 200, + fromServiceWorker: true, + }; + + mockDriver.on.mockImplementation((_, onResponseReceived) => onResponseReceived({response})); + + const resultPromise = gatherer.afterPass(passContext); + // Wait a tick for the evaluateAsync promise resolution to hold for the timer. + await Promise.resolve(); + // Skip past our timeout of 3000. + jest.advanceTimersByTime(5000); + const result = await resultPromise; + + expect(mockDriver.goOffline).toHaveBeenCalled(); + expect(mockDriver.goOnline).toHaveBeenCalled(); + expect(result).toEqual({ + statusCode: -1, + explanation: 'Timed out waiting for start_url to respond.', + }); }); - it('times out when a start_url is too slow to respond', async () => { - const startUrlGatherer = new StartUrlGatherer(); + it('navigates *while* offline', async () => { + const callsAtNavigationTime = []; + mockDriver.goOffline = jest.fn(); + mockDriver.goOnline = jest.fn(); + mockDriver.evaluateAsync = async () => { + callsAtNavigationTime.push( + ...mockDriver.goOffline.mock.calls.map(() => 'offline'), + ...mockDriver.goOnline.mock.calls.map(() => 'online') + ); + }; + mockDriver.on = jest.fn(); + mockDriver.off = jest.fn(); + const passContext = { - url: 'https://ifixit-pwa.appspot.com/', - baseArtifacts: createArtifactsWithURL('https://ifixit-pwa.appspot.com/'), - driver: wrapSendCommand(mockDriver, ''), + baseArtifacts: createArtifactsWithURL('https://example.com/'), + driver: mockDriver, }; - const resultPromise = startUrlGatherer.afterPass(passContext); - jest.advanceTimersByTime(5000); - const artifact = await resultPromise; - assert.equal(artifact.explanation, 'Timed out waiting for fetched start_url.'); + const response = { + url: 'https://example.com/', + status: 200, + fromServiceWorker: true, + }; + + mockDriver.on.mockImplementation((_, onResponseReceived) => onResponseReceived({response})); + const result = await gatherer.afterPass(passContext); + expect(callsAtNavigationTime).toEqual(['offline']); + expect(result).toEqual({ + statusCode: 200, + }); }); });