From 4eeb35c1dea86e36f2a4e1b66d99a02a9ea66ed6 Mon Sep 17 00:00:00 2001 From: Usman Abeeb <136495186+therealbibson@users.noreply.github.com> Date: Thu, 24 Sep 2026 04:29:06 +0100 Subject: [PATCH 1/2] fix(test): isolate process.env to stop flaky auth/pairs suite (#151) Root cause: Vitest threads pool shares one process across parallel files, so process.env mutations (ADMIN_API_KEY, ADMIN_TOKEN, venue flags, etc.) race mid-assertion. Switch to forks, enforce env snapshot/restore setup, and keep ADMIN_API_KEY set through pairs inject. --- src/__tests__/auth.test.ts | 6 +++++- src/__tests__/pairs.test.ts | 12 +++++++----- tests/aggregator.property.test.ts | 7 +++++-- vitest.config.ts | 10 +++++++++- vitest.setup.ts | 31 +++++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 vitest.setup.ts diff --git a/src/__tests__/auth.test.ts b/src/__tests__/auth.test.ts index 072c2f3..ebd5c95 100644 --- a/src/__tests__/auth.test.ts +++ b/src/__tests__/auth.test.ts @@ -188,7 +188,11 @@ describe('per-key rate quotas', () => { describe('admin endpoints', () => { const ORIGINAL = process.env.ADMIN_TOKEN beforeEach(() => { process.env.ADMIN_TOKEN = 'admin-secret' }) - afterEach(() => { process.env.ADMIN_TOKEN = ORIGINAL }) + afterEach(() => { + // Node stringifies undefined → "undefined"; delete to truly clear. + if (ORIGINAL === undefined) delete process.env.ADMIN_TOKEN + else process.env.ADMIN_TOKEN = ORIGINAL + }) async function buildAdminApp() { const app = Fastify() diff --git a/src/__tests__/pairs.test.ts b/src/__tests__/pairs.test.ts index c164fce..1265e16 100644 --- a/src/__tests__/pairs.test.ts +++ b/src/__tests__/pairs.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import Fastify from 'fastify' // vi.mock factories are hoisted — declare all mock fns with vi.hoisted() @@ -41,14 +41,12 @@ const ADMIN_KEY = 'test-admin-key-abc' const VALID_ISSUER = 'GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5' async function buildApp() { - const savedKey = process.env.ADMIN_API_KEY + // Keep ADMIN_API_KEY set through inject — the route reads it at request time. + // Restoring here (before inject) raced with parallel files mutating process.env. process.env.ADMIN_API_KEY = ADMIN_KEY const app = Fastify({ logger: false }) await registerPairsRoutes(app) await app.ready() - // restore after app init - if (savedKey === undefined) delete process.env.ADMIN_API_KEY - else process.env.ADMIN_API_KEY = savedKey return app } @@ -61,6 +59,10 @@ beforeEach(() => { mockQuery.mockReset().mockResolvedValue({ rows: [] }) }) +afterEach(() => { + delete process.env.ADMIN_API_KEY +}) + describe('POST /pairs', () => { it('adds a new pair and returns 201 with pairKey', async () => { const app = await buildApp() diff --git a/tests/aggregator.property.test.ts b/tests/aggregator.property.test.ts index 1e595d5..e12c96a 100644 --- a/tests/aggregator.property.test.ts +++ b/tests/aggregator.property.test.ts @@ -55,7 +55,10 @@ describe('Price aggregator property tests', () => { // this test failed before running a single iteration — the mocked // @stellar/stellar-sdk had no Networks export, which getNetworkConfig() // needs); that volume of real work needs more than the 5s default. - it('produces valid route results for random venue prices', { timeout: 30000 }, async () => { + // Under pool:'forks' (required for process.env isolation) a full parallel + // suite contends for CPU; 10k async property iterations need headroom + // beyond the prior 30s ceiling that timed out ~1/10 solo runs. + it('produces valid route results for random venue prices', { timeout: 120_000 }, async () => { await fc.assert( fc.asyncProperty( fc.float({ min: 0, max: 2000, noNaN: true, noDefaultInfinity: true, noNegativeZero: true }), @@ -119,5 +122,5 @@ describe('Price aggregator property tests', () => { ), { numRuns: 10000 } ) - }, 15000) + }) }) diff --git a/vitest.config.ts b/vitest.config.ts index 1761797..f407847 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -6,5 +6,13 @@ export default defineConfig({ globals: true, include: ['src/**/*.test.ts', 'tests/**/*.test.ts'], exclude: ['dist/**', 'node_modules/**'], + // forks: each concurrent file gets its own process, so process.env + // mutations cannot race across files (the flake root cause). + pool: 'forks', + setupFiles: ['./vitest.setup.ts'], + // resetModules()+@stellar/stellar-sdk reimport (networkVenueConfig) and + // 10k-iteration property tests need headroom under parallel fork load; + // the 5s default was itself a source of intermittent reds. + testTimeout: 60_000, }, -}) \ No newline at end of file +}) diff --git a/vitest.setup.ts b/vitest.setup.ts new file mode 100644 index 0000000..8974d21 --- /dev/null +++ b/vitest.setup.ts @@ -0,0 +1,31 @@ +/** + * Enforce process.env isolation across the suite. + * + * Root cause of intermittent auth.test.ts / pairs.test.ts (and rotating + * victims like networkVenueConfig.test.ts) failures: Vitest's default + * `threads` pool runs multiple files concurrently inside one Node process, + * so one file's `process.env` writes are visible to another mid-assertion. + * + * This setup file snapshots `process.env` before every test and restores it + * afterwards so mutations cannot leak to the next test in the same worker. + * Combined with `pool: 'forks'` in vitest.config.ts (separate process per + * concurrent file), cross-file races are eliminated without giving up + * file parallelism. + */ +import { beforeEach, afterEach } from 'vitest' + +let envSnapshot: Record + +beforeEach(() => { + envSnapshot = { ...process.env } +}) + +afterEach(() => { + for (const key of Object.keys(process.env)) { + if (!(key in envSnapshot)) delete process.env[key] + } + for (const [key, value] of Object.entries(envSnapshot)) { + if (value === undefined) delete process.env[key] + else process.env[key] = value + } +}) From d713f17e506b7e397c757621f696d9a1854b1a72 Mon Sep 17 00:00:00 2001 From: Usman Abeeb <136495186+therealbibson@users.noreply.github.com> Date: Mon, 28 Sep 2026 08:27:48 +0100 Subject: [PATCH 2/2] test(env): correct the stated root cause and tighten the regression ceilings Review follow-up on #151. - vitest.setup.ts: the header blamed Vitest's `threads` pool running files concurrently in one process. Vitest 4's default pool is already `forks`, so files never share a process; what actually leaks is `process.env` *within* a reused fork, which runs several files sequentially. Comment corrected. - vitest.config.ts: `pool: 'forks'` pins the existing default rather than changing it, and the comment now says so. `testTimeout` 60_000 -> 20_000 so a genuine hang fails the run instead of passing slowly. - tests/aggregator.property.test.ts: property timeout 120_000 -> 30_000. - src/__tests__/usage.test.ts, src/__tests__/x402-quota.test.ts: adopt the per-file env restores from #160 (additive, compatible with the shared setup). --- src/__tests__/usage.test.ts | 6 ++++++ src/__tests__/x402-quota.test.ts | 14 +++++++++++++- tests/aggregator.property.test.ts | 7 +++---- vitest.config.ts | 13 +++++++------ vitest.setup.ts | 11 +++++++---- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/__tests__/usage.test.ts b/src/__tests__/usage.test.ts index f9a4c91..40ebf10 100644 --- a/src/__tests__/usage.test.ts +++ b/src/__tests__/usage.test.ts @@ -30,6 +30,12 @@ beforeEach(() => { vi.clearAllMocks() }) +const ORIGINAL_ADMIN_TOKEN = process.env.ADMIN_TOKEN +afterEach(() => { + if (ORIGINAL_ADMIN_TOKEN === undefined) delete process.env.ADMIN_TOKEN + else process.env.ADMIN_TOKEN = ORIGINAL_ADMIN_TOKEN +}) + function buildUsageApp() { process.env.ADMIN_TOKEN = 'admin-secret' const app = Fastify() diff --git a/src/__tests__/x402-quota.test.ts b/src/__tests__/x402-quota.test.ts index 813c450..9d95b0a 100644 --- a/src/__tests__/x402-quota.test.ts +++ b/src/__tests__/x402-quota.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' const PAYMENT_ADDRESS = 'GPAYMENTADDRESS123456789012345678901234567890123456789012' @@ -80,6 +80,18 @@ beforeEach(() => { mockInitialize.mockReset().mockResolvedValue(undefined) }) +const ENV_KEYS = ['ORACLE_PAYMENT_ADDRESS', 'STELLAR_NETWORK', 'REQUIRE_API_KEY'] as const +const originalEnv: Partial> = {} +beforeEach(() => { + for (const key of ENV_KEYS) originalEnv[key] = process.env[key] +}) +afterEach(() => { + for (const key of ENV_KEYS) { + if (originalEnv[key] === undefined) delete process.env[key] + else process.env[key] = originalEnv[key] + } +}) + async function buildAppWithAuth() { process.env.ORACLE_PAYMENT_ADDRESS = PAYMENT_ADDRESS process.env.STELLAR_NETWORK = 'testnet' diff --git a/tests/aggregator.property.test.ts b/tests/aggregator.property.test.ts index e12c96a..1206a94 100644 --- a/tests/aggregator.property.test.ts +++ b/tests/aggregator.property.test.ts @@ -55,10 +55,9 @@ describe('Price aggregator property tests', () => { // this test failed before running a single iteration — the mocked // @stellar/stellar-sdk had no Networks export, which getNetworkConfig() // needs); that volume of real work needs more than the 5s default. - // Under pool:'forks' (required for process.env isolation) a full parallel - // suite contends for CPU; 10k async property iterations need headroom - // beyond the prior 30s ceiling that timed out ~1/10 solo runs. - it('produces valid route results for random venue prices', { timeout: 120_000 }, async () => { + // 30s keeps real headroom for 10k async iterations under fork load while + // staying tight enough that a genuine hang still fails the run. + it('produces valid route results for random venue prices', { timeout: 30_000 }, async () => { await fc.assert( fc.asyncProperty( fc.float({ min: 0, max: 2000, noNaN: true, noDefaultInfinity: true, noNegativeZero: true }), diff --git a/vitest.config.ts b/vitest.config.ts index f407847..85100f4 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -6,13 +6,14 @@ export default defineConfig({ globals: true, include: ['src/**/*.test.ts', 'tests/**/*.test.ts'], exclude: ['dist/**', 'node_modules/**'], - // forks: each concurrent file gets its own process, so process.env - // mutations cannot race across files (the flake root cause). + // forks: each concurrent file gets its own process. This pins Vitest's + // existing default rather than changing it - the flake was env leakage + // within a reused fork, which setupFiles settles. pool: 'forks', setupFiles: ['./vitest.setup.ts'], - // resetModules()+@stellar/stellar-sdk reimport (networkVenueConfig) and - // 10k-iteration property tests need headroom under parallel fork load; - // the 5s default was itself a source of intermittent reds. - testTimeout: 60_000, + // resetModules()+@stellar/stellar-sdk reimport (networkVenueConfig) needs + // a little headroom over the 5s default. Kept deliberately tight: a loose + // ceiling hides a genuine hang as a slow pass. + testTimeout: 20_000, }, }) diff --git a/vitest.setup.ts b/vitest.setup.ts index 8974d21..b939bf6 100644 --- a/vitest.setup.ts +++ b/vitest.setup.ts @@ -2,14 +2,17 @@ * Enforce process.env isolation across the suite. * * Root cause of intermittent auth.test.ts / pairs.test.ts (and rotating - * victims like networkVenueConfig.test.ts) failures: Vitest's default - * `threads` pool runs multiple files concurrently inside one Node process, - * so one file's `process.env` writes are visible to another mid-assertion. + * victims like networkVenueConfig.test.ts) failures: Vitest 4's default pool + * is already `forks`, so files do not run concurrently in one process. What + * leaks instead is `process.env` *within* a fork - a reused child process runs + * several files sequentially, so a key one file sets is still set when the + * next file starts, and any suite that assumes a clean environment fails + * depending on the order the files happen to be scheduled in. * * This setup file snapshots `process.env` before every test and restores it * afterwards so mutations cannot leak to the next test in the same worker. * Combined with `pool: 'forks'` in vitest.config.ts (separate process per - * concurrent file), cross-file races are eliminated without giving up + * concurrent file), cross-file leakage is eliminated without giving up * file parallelism. */ import { beforeEach, afterEach } from 'vitest'