From d25b0f70bc191b00ced9fab8dbc82310cf2c4477 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Tue, 9 Nov 2021 23:11:42 +0100 Subject: [PATCH] chore: api testing test nits (#10180) --- packages/playwright-core/src/client/fetch.ts | 2 +- .../playwright-core/src/client/network.ts | 1 + packages/playwright-core/src/server/fetch.ts | 2 +- tests/browsercontext-fetch.spec.ts | 28 ++++++++----------- tests/global-fetch.spec.ts | 25 ++++++++--------- tests/page/page-network-response.spec.ts | 10 +++++++ 6 files changed, 37 insertions(+), 31 deletions(-) diff --git a/packages/playwright-core/src/client/fetch.ts b/packages/playwright-core/src/client/fetch.ts index 8213a120b2..970c22d11d 100644 --- a/packages/playwright-core/src/client/fetch.ts +++ b/packages/playwright-core/src/client/fetch.ts @@ -214,7 +214,7 @@ export class APIResponse implements api.APIResponse { } ok(): boolean { - return this._initializer.status === 0 || (this._initializer.status >= 200 && this._initializer.status <= 299); + return this._initializer.status >= 200 && this._initializer.status <= 299; } url(): string { diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index 3113c25511..98c942b896 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -360,6 +360,7 @@ export class Response extends ChannelOwner= 200 && this._initializer.status <= 299); } diff --git a/packages/playwright-core/src/server/fetch.ts b/packages/playwright-core/src/server/fetch.ts index 81d2b88f36..2d1f51dafc 100644 --- a/packages/playwright-core/src/server/fetch.ts +++ b/packages/playwright-core/src/server/fetch.ts @@ -155,7 +155,7 @@ export abstract class APIRequestContext extends SdkObject { return { error: `${fetchResponse.status} ${fetchResponse.statusText}` }; return { fetchResponse: { ...fetchResponse, fetchUid } }; } catch (e) { - return { error: String(e) }; + return { error: e instanceof Error ? e.message : String(e) }; } } diff --git a/tests/browsercontext-fetch.spec.ts b/tests/browsercontext-fetch.spec.ts index c73204798f..c4f743821f 100644 --- a/tests/browsercontext-fetch.spec.ts +++ b/tests/browsercontext-fetch.spec.ts @@ -48,7 +48,6 @@ it('get should work', async ({ context, server }) => { expect(response.status()).toBe(200); expect(response.statusText()).toBe('OK'); expect(response.ok()).toBeTruthy(); - expect(response.url()).toBe(server.PREFIX + '/simple.json'); expect(response.headers()['content-type']).toBe('application/json; charset=utf-8'); expect(response.headersArray()).toContainEqual({ name: 'Content-Type', value: 'application/json; charset=utf-8' }); expect(await response.text()).toBe('{"foo": "bar"}\n'); @@ -60,7 +59,6 @@ it('fetch should work', async ({ context, server }) => { expect(response.status()).toBe(200); expect(response.statusText()).toBe('OK'); expect(response.ok()).toBeTruthy(); - expect(response.url()).toBe(server.PREFIX + '/simple.json'); expect(response.headers()['content-type']).toBe('application/json; charset=utf-8'); expect(response.headersArray()).toContainEqual({ name: 'Content-Type', value: 'application/json; charset=utf-8' }); expect(await response.text()).toBe('{"foo": "bar"}\n'); @@ -94,7 +92,7 @@ it('should throw on network error when sending body', async ({ context, server } req.socket.destroy(); }); const error = await context.request.get(server.PREFIX + '/test').catch(e => e); - expect(error.message).toContain('Error: aborted'); + expect(error.message).toContain(': aborted'); }); it('should throw on network error when sending body after redirect', async ({ context, server }) => { @@ -109,7 +107,7 @@ it('should throw on network error when sending body after redirect', async ({ co req.socket.destroy(); }); const error = await context.request.get(server.PREFIX + '/redirect').catch(e => e); - expect(error.message).toContain('Error: aborted'); + expect(error.message).toContain(': aborted'); }); it('should add session cookies to request', async ({ context, server }) => { @@ -130,22 +128,20 @@ it('should add session cookies to request', async ({ context, server }) => { expect(req.headers.cookie).toEqual('username=John Doe'); }); -for (const method of ['fetch', 'delete', 'get', 'head', 'patch', 'post', 'put']) { +for (const method of ['fetch', 'delete', 'get', 'head', 'patch', 'post', 'put'] as const) { it(`${method} should support queryParams`, async ({ context, server }) => { - let request; const url = new URL(server.EMPTY_PAGE); url.searchParams.set('p1', 'v1'); url.searchParams.set('парам2', 'знач2'); - server.setRoute(url.pathname + url.search, (req, res) => { - request = req; - server.serveFile(req, res); - }); - await context.request[method](server.EMPTY_PAGE + '?p1=foo', { - params: { - 'p1': 'v1', - 'парам2': 'знач2', - } - }); + const [request] = await Promise.all([ + server.waitForRequest(url.pathname + url.search), + context.request[method](server.EMPTY_PAGE + '?p1=foo', { + params: { + 'p1': 'v1', + 'парам2': 'знач2', + } + }), + ]); const params = new URLSearchParams(request.url.substr(request.url.indexOf('?'))); expect(params.get('p1')).toEqual('v1'); expect(params.get('парам2')).toEqual('знач2'); diff --git a/tests/global-fetch.spec.ts b/tests/global-fetch.spec.ts index 193818ee4c..794d80ddcb 100644 --- a/tests/global-fetch.spec.ts +++ b/tests/global-fetch.spec.ts @@ -38,7 +38,7 @@ it.afterAll(() => { http.globalAgent = prevAgent; }); -for (const method of ['fetch', 'delete', 'get', 'head', 'patch', 'post', 'put']) { +for (const method of ['fetch', 'delete', 'get', 'head', 'patch', 'post', 'put'] as const) { it(`${method} should work`, async ({ playwright, server }) => { const request = await playwright.request.newContext(); const response = await request[method](server.PREFIX + '/simple.json'); @@ -46,22 +46,21 @@ for (const method of ['fetch', 'delete', 'get', 'head', 'patch', 'post', 'put']) expect(response.status()).toBe(200); expect(response.statusText()).toBe('OK'); expect(response.ok()).toBeTruthy(); - expect(response.url()).toBe(server.PREFIX + '/simple.json'); expect(response.headers()['content-type']).toBe('application/json; charset=utf-8'); expect(response.headersArray()).toContainEqual({ name: 'Content-Type', value: 'application/json; charset=utf-8' }); expect(await response.text()).toBe(method === 'head' ? '' : '{"foo": "bar"}\n'); }); - - it(`should dispose global ${method} request`, async function({ playwright, context, server }) { - const request = await playwright.request.newContext(); - const response = await request.get(server.PREFIX + '/simple.json'); - expect(await response.json()).toEqual({ foo: 'bar' }); - await request.dispose(); - const error = await response.body().catch(e => e); - expect(error.message).toContain('Response has been disposed'); - }); } +it(`should dispose global request`, async function({ playwright, server }) { + const request = await playwright.request.newContext(); + const response = await request.get(server.PREFIX + '/simple.json'); + expect(await response.json()).toEqual({ foo: 'bar' }); + await request.dispose(); + const error = await response.body().catch(e => e); + expect(error.message).toContain('Response has been disposed'); +}); + it('should support global userAgent option', async ({ playwright, server }) => { const request = await playwright.request.newContext({ userAgent: 'My Agent' }); const [serverRequest, response] = await Promise.all([ @@ -111,8 +110,8 @@ it('should support global httpCredentials option', async ({ playwright, server } it('should return error with wrong credentials', async ({ playwright, server }) => { server.setAuth('/empty.html', 'user', 'pass'); const request = await playwright.request.newContext({ httpCredentials: { username: 'user', password: 'wrong' } }); - const response2 = await request.get(server.EMPTY_PAGE); - expect(response2.status()).toBe(401); + const response = await request.get(server.EMPTY_PAGE); + expect(response.status()).toBe(401); }); it('should use socks proxy', async ({ playwright, server, socksPort }) => { diff --git a/tests/page/page-network-response.spec.ts b/tests/page/page-network-response.spec.ts index a2b25b4a15..20893b54b3 100644 --- a/tests/page/page-network-response.spec.ts +++ b/tests/page/page-network-response.spec.ts @@ -15,6 +15,7 @@ * limitations under the License. */ +import url from 'url'; import { test as it, expect } from './pageTest'; import fs from 'fs'; @@ -267,3 +268,12 @@ it('should behave the same way for headers and allHeaders', async ({ page, serve expect(allHeaders['name-b']).toEqual('v4'); }); +it('should provide a Response with a file URL', async ({ page, asset, isAndroid, browserName }) => { + it.skip(isAndroid, 'No files on Android'); + it.fixme(browserName === 'firefox', 'Firefox does return null for file:// URLs'); + + const fileurl = url.pathToFileURL(asset('frames/two-frames.html')).href; + const response = await page.goto(fileurl); + expect(response.status()).toBe(0); + expect(response.ok()).toBe(true); +});