From 40b851742cbbd7ab3db10a28c407f58ae9d77a9e Mon Sep 17 00:00:00 2001 From: Mathy Vanvoorden Date: Wed, 29 Jul 2026 23:37:20 +0200 Subject: [PATCH] fix: try to fix two flaky tests These tests don't fail every time, heavy CPU usage causes them to fail more often since it is more a timing issue then anything else. The keep-alive test used the same Response object for all fetch calls, so depending on the timing, a second call either failed or not. Now each call gets its own Response object. The config route test wrote the session-secret file into the working directory but didn't wait for the unlink to complete, so the file could outlive the test. It is put into the temp directory now and properly removed. --- lib/__tests__/config-route.test.ts | 28 ++++++++++---------- lib/__tests__/jmap-client-resilience.test.ts | 20 ++++++++++++-- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/lib/__tests__/config-route.test.ts b/lib/__tests__/config-route.test.ts index 7c62ac59f..635dcf2f4 100644 --- a/lib/__tests__/config-route.test.ts +++ b/lib/__tests__/config-route.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, unlink, writeFileSync } from "fs"; +import { mkdtempSync, rmSync, writeFileSync } from "fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; @@ -8,7 +8,14 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; // state on the developer's machine can't leak into these env-driven // assertions. Must happen before the first GET, because the config manager // singleton loads the directory once and caches it. -process.env.ADMIN_CONFIG_DIR = mkdtempSync(path.join(tmpdir(), 'bw-config-route-')); +const TEMP_DIR = mkdtempSync(path.join(tmpdir(), 'bw-config-route-')); +process.env.ADMIN_CONFIG_DIR = TEMP_DIR; + +/** + * Backing file for the SESSION_SECRET_FILE tests. Kept in the temp dir rather + * than the working directory, which test runs share. + */ +const SECRET_FILE = path.join(TEMP_DIR, 'session-secret'); // Mock NextResponse before importing the route vi.mock('next/server', () => ({ @@ -50,6 +57,7 @@ describe('config API route', () => { afterEach(() => { process.env = { ...originalEnv }; + rmSync(SECRET_FILE, { force: true }); }); function mockRequest(headers: Record = {}): unknown { @@ -156,15 +164,11 @@ describe('config API route', () => { }); it('should enable rememberMe when SESSION_SECRET_FILE is set', async () => { - writeFileSync('./session-secret', 'test-secret'); - process.env.SESSION_SECRET_FILE = './session-secret'; + writeFileSync(SECRET_FILE, 'test-secret'); + process.env.SESSION_SECRET_FILE = SECRET_FILE; const config = await getConfig(); - unlink('./session-secret', (err) => { - if (err) throw err; - }); - expect(config.rememberMeEnabled).toBe(true); }); @@ -183,15 +187,11 @@ describe('config API route', () => { const config1 = await getConfig(); expect(config1.settingsSyncEnabled).toBe(false); - writeFileSync('./session-secret', 'test-secret'); - process.env.SESSION_SECRET_FILE = './session-secret'; + writeFileSync(SECRET_FILE, 'test-secret'); + process.env.SESSION_SECRET_FILE = SECRET_FILE; const config2 = await getConfig(); - unlink('./session-secret', (err) => { - if (err) throw err; - }); - expect(config2.settingsSyncEnabled).toBe(true); }); diff --git a/lib/__tests__/jmap-client-resilience.test.ts b/lib/__tests__/jmap-client-resilience.test.ts index a213a0490..5ea56c75c 100644 --- a/lib/__tests__/jmap-client-resilience.test.ts +++ b/lib/__tests__/jmap-client-resilience.test.ts @@ -32,8 +32,18 @@ function mockFetchResponseWithHeaders(status: number, headers: Record Promise { + return () => Promise.resolve(mockFetchResponse(status, body)); +} + describe('JMAPClient resilience', () => { let fetchSpy: ReturnType; + const connectedClients: JMAPClient[] = []; beforeEach(() => { fetchSpy = vi.spyOn(globalThis, 'fetch'); @@ -41,6 +51,10 @@ describe('JMAPClient resilience', () => { }); afterEach(() => { + // Connecting starts a keep-alive interval that outlives the test unless it + // is stopped here, and its ping then consumes a later test's mocked fetch. + connectedClients.forEach((client) => client.disconnect()); + connectedClients.length = 0; fetchSpy.mockRestore(); vi.useRealTimers(); }); @@ -57,6 +71,7 @@ describe('JMAPClient resilience', () => { const client = new JMAPClient('https://mail.example.com', 'user@test.com', 'pass123'); await client.connect(); fetchSpy.mockReset(); + connectedClients.push(client); return client; } @@ -65,6 +80,7 @@ describe('JMAPClient resilience', () => { const client = JMAPClient.withBearer('https://mail.example.com', 'token123', 'user@test.com'); await client.connect(); fetchSpy.mockReset(); + connectedClients.push(client); return client; } @@ -238,7 +254,7 @@ describe('JMAPClient resilience', () => { client.onConnectionChange(callback); const echoResponse = { methodResponses: [['Core/echo', { ping: 'pong' }, '0']] }; - fetchSpy.mockResolvedValue(mockFetchResponse(200, echoResponse)); + fetchSpy.mockImplementation(respondEveryTime(200, echoResponse)); // Advance past keep-alive interval (30s) await vi.advanceTimersByTimeAsync(30_000); @@ -342,7 +358,7 @@ describe('JMAPClient resilience', () => { client.disconnect(); // Advancing timers should not trigger any ping - fetchSpy.mockResolvedValue(mockFetchResponse(200, { methodResponses: [['Core/echo', { ping: 'pong' }, '0']] })); + fetchSpy.mockImplementation(respondEveryTime(200, { methodResponses: [['Core/echo', { ping: 'pong' }, '0']] })); await vi.advanceTimersByTimeAsync(60_000); expect(callback).not.toHaveBeenCalled();