From 381ecc120b52a5c97d585eae2ecd23d4c7d341de Mon Sep 17 00:00:00 2001 From: Seth Holladay Date: Wed, 26 Jun 2024 02:54:20 -0400 Subject: [PATCH] Fix flakey retry tests --- package.json | 2 +- test/helpers/with-performance-observer.ts | 42 ----------------------- test/helpers/with-performance.ts | 28 +++++++++++++++ test/retry.ts | 14 ++++---- 4 files changed, 35 insertions(+), 51 deletions(-) delete mode 100644 test/helpers/with-performance-observer.ts create mode 100644 test/helpers/with-performance.ts diff --git a/package.json b/package.json index 8a571e0f..2d2dc982 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "node": ">=18" }, "scripts": { - "test": "xo && npm run build && ava --timeout=10m --serial", + "test": "xo && npm run build && ava", "debug": "PWDEBUG=1 ava --timeout=2m", "release": "np", "build": "del-cli distribution && tsc --project tsconfig.dist.json", diff --git a/test/helpers/with-performance-observer.ts b/test/helpers/with-performance-observer.ts deleted file mode 100644 index c6c9e29c..00000000 --- a/test/helpers/with-performance-observer.ts +++ /dev/null @@ -1,42 +0,0 @@ -import {performance, PerformanceObserver} from 'node:perf_hooks'; -import process from 'node:process'; -import type {ExecutionContext} from 'ava'; - -type Argument = { - name: string; - expectedDuration: number; - t: ExecutionContext; - test: () => Promise; -}; - -// We allow the tests to take more time on CI than locally, to reduce flakiness -const allowedOffset = process.env.CI ? 1000 : 300; - -export async function withPerformanceObserver({ - name, - expectedDuration, - t, - test, -}: Argument) { - // Register observer that asserts on duration when a measurement is performed - const obs = new PerformanceObserver(items => { - const measurements = items.getEntries(); - - const duration = measurements[0].duration ?? Number.NaN; - - t.true( - Math.abs(duration - expectedDuration) < allowedOffset, - `Duration of ${duration}ms is not close to expected duration ${expectedDuration}ms`, - ); - - obs.disconnect(); - }); - obs.observe({entryTypes: ['measure']}); - - // Start measuring - performance.mark(`start-${name}`); - await test(); - performance.mark(`end-${name}`); - - performance.measure(name, `start-${name}`, `end-${name}`); -} diff --git a/test/helpers/with-performance.ts b/test/helpers/with-performance.ts new file mode 100644 index 00000000..10df3d50 --- /dev/null +++ b/test/helpers/with-performance.ts @@ -0,0 +1,28 @@ +import {performance} from 'node:perf_hooks'; +import process from 'node:process'; +import type {ExecutionContext} from 'ava'; + +type Argument = { + expectedDuration: number; + t: ExecutionContext; + test: () => Promise; +}; + +// We allow the tests to take more time on CI than locally, to reduce flakiness +const allowedOffset = process.env.CI ? 300 : 200; + +export async function withPerformance({ + expectedDuration, + t, + test, +}: Argument) { + const startTime = performance.now(); + await test(); + const endTime = performance.now(); + + const duration = endTime - startTime; + t.true( + Math.abs(duration - expectedDuration) < allowedOffset, + `Duration of ${duration}ms is not close to expected duration ${expectedDuration}ms`, + ); +} diff --git a/test/retry.ts b/test/retry.ts index 3f45c65e..d73e9652 100644 --- a/test/retry.ts +++ b/test/retry.ts @@ -1,7 +1,7 @@ import test from 'ava'; import ky from '../source/index.js'; import {createHttpTestServer} from './helpers/create-http-test-server.js'; -import {withPerformanceObserver} from './helpers/with-performance-observer.js'; +import {withPerformance} from './helpers/with-performance.js'; const fixture = 'fixture'; const defaultRetryCount = 2; @@ -442,7 +442,7 @@ test('throws when retry.statusCodes is not an array', async t => { await server.close(); }); -test('respect maximum backoff', async t => { +test('respect maximum backoffLimit', async t => { const retryCount = 4; let requestCount = 0; @@ -457,9 +457,8 @@ test('respect maximum backoff', async t => { } }); - await withPerformanceObserver({ + await withPerformance({ t, - name: 'default', expectedDuration: 300 + 600 + 1200 + 2400, async test() { t.is(await ky(server.url, { @@ -469,9 +468,9 @@ test('respect maximum backoff', async t => { }); requestCount = 0; - await withPerformanceObserver({ + + await withPerformance({ t, - name: 'custom', expectedDuration: 300 + 600 + 1000 + 1000, async test() { t.is(await ky(server.url, { @@ -501,9 +500,8 @@ test('respect custom retry.delay', async t => { } }); - await withPerformanceObserver({ + await withPerformance({ t, - name: 'linear', expectedDuration: 200 + 300 + 400 + 500, async test() { t.is(await ky(server.url, {