From 5ff660de15e7e5b4b2660d3ac52a3a6dc30bd2b0 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Wed, 4 Mar 2020 15:59:26 -0800 Subject: [PATCH] feat(navigation): waitForNavigation/goto should not wait until response finished (#1225) --- docs/api.md | 4 ++++ src/network.ts | 8 +++++--- test/network.spec.js | 23 +++++++++++++---------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/docs/api.md b/docs/api.md index 3061097813189..6f77b22c5bcbe 100644 --- a/docs/api.md +++ b/docs/api.md @@ -3185,6 +3185,7 @@ ResourceType will be one of the following: `document`, `stylesheet`, `image`, `m - [response.buffer()](#responsebuffer) +- [response.finished()](#responsefinished) - [response.frame()](#responseframe) - [response.headers()](#responseheaders) - [response.json()](#responsejson) @@ -3199,6 +3200,9 @@ ResourceType will be one of the following: `document`, `stylesheet`, `image`, `m #### response.buffer() - returns: > Promise which resolves to a buffer with response body. +#### response.finished() +- returns: Waits for this response to finish, throws when corresponding request failed. + #### response.frame() - returns: A [Frame] that initiated this response, or `null` if navigating to error pages. diff --git a/src/network.ts b/src/network.ts index e81314658062f..8feb5683c4659 100644 --- a/src/network.ts +++ b/src/network.ts @@ -165,9 +165,7 @@ export class Request { } async _waitForResponse(): Promise { - const response = await this._waitForResponsePromise; - await response._finishedPromise; - return response; + return await this._waitForResponsePromise; } _setResponse(response: Response) { @@ -271,6 +269,10 @@ export class Response { return this._headers; } + finished(): Promise { + return this._finishedPromise; + } + buffer(): Promise { if (!this._contentPromise) { this._contentPromise = this._finishedPromise.then(async error => { diff --git a/test/network.spec.js b/test/network.spec.js index 080eac72ceacf..3a379c7a8f726 100644 --- a/test/network.spec.js +++ b/test/network.spec.js @@ -258,21 +258,23 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM expect(failedRequests[0].frame()).toBeTruthy(); }); it('Page.Events.RequestFinished', async({page, server}) => { - const requests = []; - page.on('requestfinished', request => requests.push(request)); - await page.goto(server.EMPTY_PAGE); - expect(requests.length).toBe(1); - expect(requests[0].url()).toBe(server.EMPTY_PAGE); - expect(requests[0].response()).toBeTruthy(); - expect(requests[0].frame() === page.mainFrame()).toBe(true); - expect(requests[0].frame().url()).toBe(server.EMPTY_PAGE); + const [response] = await Promise.all([ + page.goto(server.EMPTY_PAGE), + page.waitForEvent('requestfinished') + ]); + const request = response.request(); + expect(request.url()).toBe(server.EMPTY_PAGE); + expect(request.response()).toBeTruthy(); + expect(request.frame() === page.mainFrame()).toBe(true); + expect(request.frame().url()).toBe(server.EMPTY_PAGE); }); it('should fire events in proper order', async({page, server}) => { const events = []; page.on('request', request => events.push('request')); page.on('response', response => events.push('response')); - page.on('requestfinished', request => events.push('requestfinished')); - await page.goto(server.EMPTY_PAGE); + const response = await page.goto(server.EMPTY_PAGE); + await response.finished(); + events.push('requestfinished') expect(events).toEqual(['request', 'response', 'requestfinished']); }); it('should support redirects', async({page, server}) => { @@ -284,6 +286,7 @@ module.exports.describe = function({testRunner, expect, MAC, WIN, FFOX, CHROMIUM server.setRedirect('/foo.html', '/empty.html'); const FOO_URL = server.PREFIX + '/foo.html'; const response = await page.goto(FOO_URL); + await response.finished(); expect(events).toEqual([ `GET ${FOO_URL}`, `302 ${FOO_URL}`,