From d98e6b507f80f95484fd9fdbe358008ff59755f3 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 22 Jul 2026 10:58:53 -0700 Subject: [PATCH] test(config): use withEnv() for TTL env mutation to satisfy check-test-isolation R1 CI verify failed: test/postgres-engine-config-cache.test.ts mutated process.env.GBRAIN_CONFIG_CACHE_TTL_MS directly in beforeEach/afterEach. Wrap each test body in withEnv() (test/helpers/with-env.ts) instead. Co-Authored-By: Claude Fable 5 --- test/postgres-engine-config-cache.test.ts | 50 ++++++++++------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/test/postgres-engine-config-cache.test.ts b/test/postgres-engine-config-cache.test.ts index 949034a10..06b62e2d7 100644 --- a/test/postgres-engine-config-cache.test.ts +++ b/test/postgres-engine-config-cache.test.ts @@ -9,8 +9,9 @@ * Pure: stubs `_sql` with a call-counting fake; no real DB. */ -import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { describe, it, expect } from 'bun:test'; import { PostgresEngine } from '../src/core/postgres-engine.ts'; +import { withEnv } from './helpers/with-env.ts'; const FAST_RETRY = { maxRetries: 3, delayMs: 1, delayMaxMs: 1, jitter: 'none' as const }; @@ -27,19 +28,12 @@ function makeEngine(rows: unknown[]) { return { engine: e, calls }; } -const savedTtl = process.env.GBRAIN_CONFIG_CACHE_TTL_MS; - -beforeEach(() => { - delete process.env.GBRAIN_CONFIG_CACHE_TTL_MS; -}); - -afterEach(() => { - if (savedTtl === undefined) delete process.env.GBRAIN_CONFIG_CACHE_TTL_MS; - else process.env.GBRAIN_CONFIG_CACHE_TTL_MS = savedTtl; -}); +/** Run `fn` with GBRAIN_CONFIG_CACHE_TTL_MS set (or cleared when undefined). */ +const withTtl = (ttl: string | undefined, fn: () => Promise) => + withEnv({ GBRAIN_CONFIG_CACHE_TTL_MS: ttl }, fn); describe('PostgresEngine config cache (#1694)', () => { - it('batch-loads once and serves repeat reads from the cache', async () => { + it('batch-loads once and serves repeat reads from the cache', () => withTtl(undefined, async () => { const { engine, calls } = makeEngine([ { key: 'search.mode', value: 'balanced' }, { key: 'embedding_multimodal', value: 'true' }, @@ -50,31 +44,31 @@ describe('PostgresEngine config cache (#1694)', () => { // One SELECT total — this is the whole point of the fix. expect(calls.length).toBe(1); expect(calls[0]).toContain('SELECT key, value FROM config'); - }); + })); - it('returns null for a known-absent key without an extra round-trip', async () => { + it('returns null for a known-absent key without an extra round-trip', () => withTtl(undefined, async () => { const { engine, calls } = makeEngine([{ key: 'a', value: '1' }]); expect(await engine.getConfig('missing.key')).toBeNull(); expect(await engine.getConfig('missing.key')).toBeNull(); expect(calls.length).toBe(1); - }); + })); - it('setConfig writes through so subsequent reads see the new value', async () => { + it('setConfig writes through so subsequent reads see the new value', () => withTtl(undefined, async () => { const { engine, calls } = makeEngine([{ key: 'k', value: 'old' }]); expect(await engine.getConfig('k')).toBe('old'); await engine.setConfig('k', 'new'); expect(await engine.getConfig('k')).toBe('new'); expect(calls.length).toBe(2); // batch load + upsert; no re-read - }); + })); - it('unsetConfig writes through so subsequent reads see absence', async () => { + it('unsetConfig writes through so subsequent reads see absence', () => withTtl(undefined, async () => { const { engine } = makeEngine([{ key: 'k', value: 'v' }]); expect(await engine.getConfig('k')).toBe('v'); await engine.unsetConfig('k'); expect(await engine.getConfig('k')).toBeNull(); - }); + })); - it('concurrent cold reads share a single batch load (single-flight)', async () => { + it('concurrent cold reads share a single batch load (single-flight)', () => withTtl(undefined, async () => { const { engine, calls } = makeEngine([{ key: 'k', value: 'v' }]); const [a, b, c] = await Promise.all([ engine.getConfig('k'), @@ -83,27 +77,25 @@ describe('PostgresEngine config cache (#1694)', () => { ]); expect([a, b, c]).toEqual(['v', 'v', null]); expect(calls.length).toBe(1); - }); + })); - it('GBRAIN_CONFIG_CACHE_TTL_MS=0 disables the cache (per-key reads)', async () => { - process.env.GBRAIN_CONFIG_CACHE_TTL_MS = '0'; + it('GBRAIN_CONFIG_CACHE_TTL_MS=0 disables the cache (per-key reads)', () => withTtl('0', async () => { const { engine, calls } = makeEngine([{ value: 'v' }]); expect(await engine.getConfig('k')).toBe('v'); expect(await engine.getConfig('k')).toBe('v'); expect(calls.length).toBe(2); expect(calls[0]).toContain('SELECT value FROM config WHERE key ='); - }); + })); - it('an expired TTL reloads from the database', async () => { - process.env.GBRAIN_CONFIG_CACHE_TTL_MS = '1'; + it('an expired TTL reloads from the database', () => withTtl('1', async () => { const { engine, calls } = makeEngine([{ key: 'k', value: 'v' }]); expect(await engine.getConfig('k')).toBe('v'); await new Promise((r) => setTimeout(r, 5)); expect(await engine.getConfig('k')).toBe('v'); expect(calls.length).toBe(2); // two batch loads - }); + })); - it('the batch load keeps the connRetry reconnect posture (#1603/#1891)', async () => { + it('the batch load keeps the connRetry reconnect posture (#1603/#1891)', () => withTtl(undefined, async () => { const e = new PostgresEngine(); (e as unknown as { _connectionStyle: string })._connectionStyle = 'instance'; (e as unknown as { _sql: unknown })._sql = null; // torn-down pool → retryable @@ -116,5 +108,5 @@ describe('PostgresEngine config cache (#1694)', () => { }; expect(await e.getConfig('k')).toBe('v'); expect(reconnects).toBe(1); - }); + })); });