From 72ca86a5dbbe078cb8a552a7b7501407c6518fd5 Mon Sep 17 00:00:00 2001 From: Khafra Date: Mon, 1 May 2023 00:19:08 -0400 Subject: [PATCH] test: fix failing tests (#2097) * test: increase timeout for some redirect- tests * test: fixes * test: fixes * test: fixes * test: fixes * test: fixes * test: fixes * test: fixes * test: fixes --- package.json | 2 +- test/agent.js | 4 +- test/connect-timeout.js | 6 +- test/fetch/client-fetch.js | 4 +- .../jsdom-abortcontroller-1910-1464495619.js | 12 +- test/fetch/request.js | 11 +- test/proxy-agent.js | 4 +- test/redirect-request.js | 164 ++++-------------- test/redirect-stream.js | 2 + test/tls-session-reuse.js | 4 +- 10 files changed, 61 insertions(+), 152 deletions(-) diff --git a/package.json b/package.json index da9bd36d21e..6c325d75cdf 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "lint:fix": "standard --fix | snazzy", "test": "npm run test:tap && npm run test:node-fetch && npm run test:fetch && npm run test:cookies && npm run test:wpt && npm run test:websocket && npm run test:jest && npm run test:typescript", "test:cookies": "node scripts/verifyVersion 16 || tap test/cookie/*.js", - "test:node-fetch": "node scripts/verifyVersion.js 16 || mocha test/node-fetch", + "test:node-fetch": "node scripts/verifyVersion.js 16 || mocha --exit test/node-fetch", "test:fetch": "node scripts/verifyVersion.js 16 || (npm run build:node && tap --expose-gc test/fetch/*.js && tap test/webidl/*.js)", "test:jest": "node scripts/verifyVersion.js 14 || jest", "test:tap": "tap test/*.js test/diagnostics-channel/*.js", diff --git a/test/agent.js b/test/agent.js index 0d3c84d9d53..ecdbdd2f91d 100644 --- a/test/agent.js +++ b/test/agent.js @@ -1,6 +1,6 @@ 'use strict' -const { test } = require('tap') +const { test, teardown } = require('tap') const http = require('http') const { PassThrough } = require('stream') const { kRunning } = require('../lib/core/symbols') @@ -705,3 +705,5 @@ test('the dispatcher is truly global', t => { t.equal(agent, undiciFresh.getGlobalDispatcher()) t.end() }) + +teardown(() => process.exit()) diff --git a/test/connect-timeout.js b/test/connect-timeout.js index 98ed1922979..c904a7335a2 100644 --- a/test/connect-timeout.js +++ b/test/connect-timeout.js @@ -8,7 +8,7 @@ const sleep = require('atomic-sleep') test('priotorise socket errors over timeouts', (t) => { t.plan(1) const connectTimeout = 1000 - const client = new Pool('http://foobar.bar:1234', { connectTimeout }) + const client = new Pool('http://foobar.bar:1234', { connectTimeout: 1 }) client.request({ method: 'GET', path: '/foobar' }) .then(() => t.fail()) @@ -16,8 +16,8 @@ test('priotorise socket errors over timeouts', (t) => { t.equal(err.code, 'ENOTFOUND') }) - // block for 2s which is enough for the dns lookup to complete and TO to fire - sleep(connectTimeout * 2) + // block for 1s which is enough for the dns lookup to complete and TO to fire + sleep(connectTimeout) }) // never connect diff --git a/test/fetch/client-fetch.js b/test/fetch/client-fetch.js index 3ce058ee9ae..048b76a9f78 100644 --- a/test/fetch/client-fetch.js +++ b/test/fetch/client-fetch.js @@ -2,7 +2,7 @@ 'use strict' -const { test } = require('tap') +const { test, teardown } = require('tap') const { createServer } = require('http') const { ReadableStream } = require('stream/web') const { Blob } = require('buffer') @@ -672,3 +672,5 @@ test('Receiving non-Latin1 headers', async (t) => { t.same(lengths, [30, 34, 94, 104, 90]) t.end() }) + +teardown(() => process.exit()) diff --git a/test/fetch/jsdom-abortcontroller-1910-1464495619.js b/test/fetch/jsdom-abortcontroller-1910-1464495619.js index fad1e77c12b..e5a86abeaf3 100644 --- a/test/fetch/jsdom-abortcontroller-1910-1464495619.js +++ b/test/fetch/jsdom-abortcontroller-1910-1464495619.js @@ -10,11 +10,15 @@ const { JSDOM } = require('jsdom') test('third party AbortControllers', async (t) => { const server = createServer((_, res) => res.end()).listen(0) - t.teardown(server.close.bind(server)) - await once(server, 'listening') - const { AbortController } = new JSDOM().window - const controller = new AbortController() + let controller = new AbortController() + + t.teardown(() => { + controller.abort() + controller = null + return server.close() + }) + await once(server, 'listening') await t.resolves(fetch(`http://localhost:${server.address().port}`, { signal: controller.signal diff --git a/test/fetch/request.js b/test/fetch/request.js index cd32adc7d6f..e6ab49b7808 100644 --- a/test/fetch/request.js +++ b/test/fetch/request.js @@ -2,13 +2,12 @@ 'use strict' -const { test } = require('tap') +const { test, teardown } = require('tap') const { Request, Headers, fetch } = require('../../') -const { kState } = require('../../lib/fetch/symbols.js') const { Blob: ThirdPartyBlob, FormData: ThirdPartyFormData @@ -199,7 +198,7 @@ test('undefined window', t => { test('undefined body', t => { const req = new Request('http://asd', { body: undefined }) - t.equal(req[kState].body, null) + t.equal(req.body, null) t.end() }) @@ -298,7 +297,7 @@ test('post aborted signal', t => { } else { t.pass() } - }) + }, { once: true }) ac.abort('gwak') }) @@ -346,7 +345,7 @@ test('post aborted signal cloned', t => { } else { t.pass() } - }) + }, { once: true }) ac.abort('gwak') }) @@ -476,3 +475,5 @@ test('set-cookie headers get cleared when passing a Request as first param', (t) t.same(req2.headers.getSetCookie(), []) t.end() }) + +teardown(() => process.exit()) diff --git a/test/proxy-agent.js b/test/proxy-agent.js index 3d8f9903bde..a35101234c6 100644 --- a/test/proxy-agent.js +++ b/test/proxy-agent.js @@ -1,6 +1,6 @@ 'use strict' -const { test } = require('tap') +const { test, teardown } = require('tap') const { request, fetch, setGlobalDispatcher, getGlobalDispatcher } = require('..') const { InvalidArgumentError } = require('../lib/core/errors') const { nodeMajor } = require('../lib/core/util') @@ -692,3 +692,5 @@ function buildSSLProxy () { server.listen(0, () => resolve(server)) }) } + +teardown(() => process.exit()) diff --git a/test/redirect-request.js b/test/redirect-request.js index f996bfa8231..46e60cd5471 100644 --- a/test/redirect-request.js +++ b/test/redirect-request.js @@ -22,21 +22,17 @@ for (const factory of [ const request = (server, opts, ...args) => { const dispatcher = factory(server, opts) return undici.request(args[0], { ...args[1], dispatcher }, args[2]) + .finally(() => dispatcher.destroy()) } t.test('should always have a history with the final URL even if no redirections were followed', async t => { - t.plan(4) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream, context: { history } } = await request(server, undefined, `http://${server}/200?key=value`, { maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -45,15 +41,10 @@ for (const factory of [ }) t.test('should not follow redirection by default if not using RedirectAgent', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}`) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 302) t.equal(headers.location, `http://${server}/302/1`) @@ -61,18 +52,13 @@ for (const factory of [ }) t.test('should follow redirection after a HTTP 300', async t => { - t.plan(4) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream, context: { history } } = await request(server, undefined, `http://${server}/300?key=value`, { maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -88,16 +74,10 @@ for (const factory of [ }) t.test('should follow redirection after a HTTP 300 default', async t => { - t.plan(4) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream, context: { history } } = await request(server, { maxRedirections: 10 }, `http://${server}/300?key=value`) - - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -113,9 +93,6 @@ for (const factory of [ }) t.test('should follow redirection after a HTTP 301', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/301`, { @@ -124,9 +101,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -134,9 +109,6 @@ for (const factory of [ }) t.test('should follow redirection after a HTTP 302', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/302`, { @@ -145,9 +117,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -155,9 +125,6 @@ for (const factory of [ }) t.test('should follow redirection after a HTTP 303 changing method to GET', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/303`, { @@ -166,9 +133,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -176,9 +141,6 @@ for (const factory of [ }) t.test('should remove Host and request body related headers when following HTTP 303 (array)', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/303`, { @@ -202,9 +164,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -212,9 +172,6 @@ for (const factory of [ }) t.test('should remove Host and request body related headers when following HTTP 303 (object)', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/303`, { @@ -231,9 +188,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -241,9 +196,6 @@ for (const factory of [ }) t.test('should follow redirection after a HTTP 307', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/307`, { @@ -251,9 +203,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -261,9 +211,6 @@ for (const factory of [ }) t.test('should follow redirection after a HTTP 308', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/308`, { @@ -271,9 +218,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -281,18 +226,13 @@ for (const factory of [ }) t.test('should ignore HTTP 3xx response bodies', async t => { - t.plan(4) - - let body = '' const server = await startRedirectingWithBodyServer(t) const { statusCode, headers, body: bodyStream, context: { history } } = await request(server, undefined, `http://${server}/`, { maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -301,8 +241,6 @@ for (const factory of [ }) t.test('should ignore query after redirection', async t => { - t.plan(3) - const server = await startRedirectingWithQueryParams(t) const { statusCode, headers, context: { history } } = await request(server, undefined, `http://${server}/`, { @@ -316,18 +254,13 @@ for (const factory of [ }) t.test('should follow a redirect chain up to the allowed number of times', async t => { - t.plan(4) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream, context: { history } } = await request(server, undefined, `http://${server}/300`, { maxRedirections: 2 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 300) t.equal(headers.location, `http://${server}/300/3`) @@ -340,44 +273,32 @@ for (const factory of [ const server = await startRedirectingWithoutLocationServer(t) for (const code of redirectCodes) { - t.test(`should return the original response after a HTTP ${code}`, async t => { - t.plan(3) - - let body = '' - - const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/${code}`, { - maxRedirections: 10 - }) + const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/${code}`, { + maxRedirections: 10 + }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() - t.equal(statusCode, code) - t.notOk(headers.location) - t.equal(body.length, 0) - }) + t.equal(statusCode, code) + t.notOk(headers.location) + t.equal(body.length, 0) } }) t.test('should not allow invalid maxRedirections arguments', async t => { - t.plan(1) - try { await request('localhost', undefined, 'http://localhost', { method: 'GET', maxRedirections: 'INVALID' }) - throw new Error('Did not throw') + t.fail('Did not throw') } catch (err) { t.equal(err.message, 'maxRedirections must be a positive number') } }) t.test('should not allow invalid maxRedirections arguments default', async t => { - t.plan(1) - try { await request('localhost', { maxRedirections: 'INVALID' @@ -385,16 +306,13 @@ for (const factory of [ method: 'GET' }) - throw new Error('Did not throw') + t.fail('Did not throw') } catch (err) { t.equal(err.message, 'maxRedirections must be a positive number') } }) t.test('should not follow redirects when using ReadableStream request bodies', { skip: nodeMajor < 16 }, async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/301`, { @@ -403,9 +321,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 301) t.equal(headers.location, `http://${server}/301/2`) @@ -413,9 +329,6 @@ for (const factory of [ }) t.test('should not follow redirects when using Readable request bodies', async t => { - t.plan(3) - - let body = '' const server = await startRedirectingServer(t) const { statusCode, headers, body: bodyStream } = await request(server, undefined, `http://${server}/301`, { @@ -424,9 +337,7 @@ for (const factory of [ maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 301) t.equal(headers.location, `http://${server}/301/1`) @@ -435,19 +346,14 @@ for (const factory of [ } t.test('should follow redirections when going cross origin', async t => { - t.plan(4) - const [server1, server2, server3] = await startRedirectingChainServers(t) - let body = '' const { statusCode, headers, body: bodyStream, context: { history } } = await undici.request(`http://${server1}`, { method: 'POST', maxRedirections: 10 }) - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(statusCode, 200) t.notOk(headers.location) @@ -477,19 +383,15 @@ t.test('should handle errors (callback)', t => { }) t.test('should handle errors (promise)', async t => { - t.plan(1) - try { await undici.request('http://localhost:0', { maxRedirections: 10 }) - throw new Error('Did not throw') + t.fail('Did not throw') } catch (error) { t.match(error.code, /EADDRNOTAVAIL|ECONNREFUSED/) } }) t.test('removes authorization header on third party origin', async t => { - t.plan(1) - const [server1] = await startRedirectingWithAuthorization(t, 'secret') const { body: bodyStream } = await undici.request(`http://${server1}`, { maxRedirections: 10, @@ -498,17 +400,12 @@ t.test('removes authorization header on third party origin', async t => { } }) - let body = '' - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(body, '') }) t.test('removes cookie header on third party origin', async t => { - t.plan(1) - const [server1] = await startRedirectingWithCookie(t, 'a=b') const { body: bodyStream } = await undici.request(`http://${server1}`, { maxRedirections: 10, @@ -517,10 +414,7 @@ t.test('removes cookie header on third party origin', async t => { } }) - let body = '' - for await (const b of bodyStream) { - body += b - } + const body = await bodyStream.text() t.equal(body, '') }) diff --git a/test/redirect-stream.js b/test/redirect-stream.js index abb778c2c8f..55dd97beb49 100644 --- a/test/redirect-stream.js +++ b/test/redirect-stream.js @@ -419,3 +419,5 @@ t.test('removes cookie header on third party origin', async t => { t.equal(body.length, 0) }) + +t.teardown(() => process.exit()) diff --git a/test/tls-session-reuse.js b/test/tls-session-reuse.js index 147e92fd76b..ab012f16756 100644 --- a/test/tls-session-reuse.js +++ b/test/tls-session-reuse.js @@ -4,7 +4,7 @@ const { readFileSync } = require('fs') const { join } = require('path') const https = require('https') const crypto = require('crypto') -const { test } = require('tap') +const { test, teardown } = require('tap') const { Client, Pool } = require('..') const { kSocket } = require('../lib/core/symbols') const { nodeMajor } = require('../lib/core/util') @@ -181,3 +181,5 @@ test('A pool should be able to reuse TLS sessions between clients', { t.end() }) + +teardown(() => process.exit())