From 264b1506a77fff8652457c4f95b2bc4163c74fde Mon Sep 17 00:00:00 2001 From: Chris Arderne Date: Tue, 7 Jul 2026 00:45:37 +0100 Subject: [PATCH] test: stabilise flaky CI tests Address a recurring set of flaky unit tests. Two failure modes: - Wall-clock perf microbenchmarks that assert absolute per-call timings. These carry no correctness signal and flake on shared CI runners under load. Removed or replaced with behavioural assertions. - Testcontainer-backed tests whose cold container startup on a loaded runner intermittently exceeded the per-test timeout. Raised the timeouts (several were at 60s; one had no override and ran on vitest's 5s default). --- .../services/triggerTask.server.test.ts | 4 +- .../test/api.v1.waitpoints.tokens.test.ts | 4 +- apps/webapp/test/detectbadJsonStrings.test.ts | 148 +----------------- .../test/dropTaskRunToTaskRunTagJoin.test.ts | 107 +++++++------ .../test/runsReplicationService.part2.test.ts | 5 +- .../test/runsReplicationService.part8.test.ts | 4 +- .../test/runsReplicationService.part9.test.ts | 4 +- apps/webapp/test/runsRepository.part2.test.ts | 4 +- .../distinctDbSentinel.server.test.ts | 6 +- ...ayedRunSystem.controlPlaneResolver.test.ts | 4 +- .../tests/fairQueueSelectionStrategy.test.ts | 41 ++--- 11 files changed, 100 insertions(+), 231 deletions(-) diff --git a/apps/webapp/app/runEngine/services/triggerTask.server.test.ts b/apps/webapp/app/runEngine/services/triggerTask.server.test.ts index 31c624a386..78d41fcbe1 100644 --- a/apps/webapp/app/runEngine/services/triggerTask.server.test.ts +++ b/apps/webapp/app/runEngine/services/triggerTask.server.test.ts @@ -46,7 +46,9 @@ import type { } from "~/runEngine/types"; import { RunEngineTriggerTaskService } from "./triggerTask.server"; -vi.setConfig({ testTimeout: 60_000 }); // 60 seconds timeout +// 120s: raised from 60s — cold testcontainer startup + worker shutdown on a +// loaded CI runner intermittently exceeded 60s. +vi.setConfig({ testTimeout: 120_000 }); class MockPayloadProcessor implements PayloadProcessor { async process(request: TriggerTaskRequest): Promise { diff --git a/apps/webapp/test/api.v1.waitpoints.tokens.test.ts b/apps/webapp/test/api.v1.waitpoints.tokens.test.ts index e5df494885..64a7581d77 100644 --- a/apps/webapp/test/api.v1.waitpoints.tokens.test.ts +++ b/apps/webapp/test/api.v1.waitpoints.tokens.test.ts @@ -25,7 +25,9 @@ import { import type { Prisma, PrismaClient } from "@trigger.dev/database"; import { trace } from "@opentelemetry/api"; -vi.setConfig({ testTimeout: 60_000 }); +// 120s: raised from 60s — the two-DB (hetero run-ops) cases start extra +// Postgres testcontainers whose cold startup intermittently exceeded 60s. +vi.setConfig({ testTimeout: 120_000 }); function buildEngine(opts: { prisma: any; diff --git a/apps/webapp/test/detectbadJsonStrings.test.ts b/apps/webapp/test/detectbadJsonStrings.test.ts index 74942fb09e..f2d0d025a2 100644 --- a/apps/webapp/test/detectbadJsonStrings.test.ts +++ b/apps/webapp/test/detectbadJsonStrings.test.ts @@ -38,148 +38,12 @@ describe("detectBadJsonStrings", () => { expect(result).toBe(true); }); - it("should have acceptable performance overhead", () => { - const longText = `hello world `.repeat(1_000); - const goodJson = `{"title": "hello", "text": "${longText}"}`; - const badJson = `{"title": "hello\\ud835", "text": "${longText}"}`; - - const iterations = 100_000; - - // Warm up - for (let i = 0; i < 1000; i++) { - detectBadJsonStrings(goodJson); - detectBadJsonStrings(badJson); - } - - // Measure good JSON (most common case) - const goodStart = performance.now(); - for (let i = 0; i < iterations; i++) { - detectBadJsonStrings(goodJson); - } - const goodTime = performance.now() - goodStart; - - // Measure bad JSON (edge case) - const badStart = performance.now(); - for (let i = 0; i < iterations; i++) { - detectBadJsonStrings(badJson); - } - const badTime = performance.now() - badStart; - - // Measure baseline (just function call overhead) - const baselineStart = performance.now(); - for (let i = 0; i < iterations; i++) { - // Empty function call to measure baseline - } - const baselineTime = performance.now() - baselineStart; - - const goodOverhead = goodTime - baselineTime; - const badOverhead = badTime - baselineTime; - - console.log(`Baseline (${iterations} iterations): ${baselineTime.toFixed(2)}ms`); - console.log( - `Good JSON (${iterations} iterations): ${goodTime.toFixed( - 2 - )}ms (overhead: ${goodOverhead.toFixed(2)}ms)` - ); - console.log( - `Bad JSON (${iterations} iterations): ${badTime.toFixed( - 2 - )}ms (overhead: ${badOverhead.toFixed(2)}ms)` - ); - console.log( - `Average per call - Good: ${(goodOverhead / iterations).toFixed(4)}ms, Bad: ${( - badOverhead / iterations - ).toFixed(4)}ms` - ); - - // Assertions for performance expectations - // Good JSON should be reasonably fast (most common case) - expect(goodOverhead / iterations).toBeLessThan(0.01); // Less than 10 microseconds per call - - // Bad JSON can be slower due to regex matching, but still reasonable - expect(badOverhead / iterations).toBeLessThan(0.01); // Less than 20 microseconds per call - - // Total overhead for 100k calls should be reasonable - expect(goodOverhead).toBeLessThan(1000); // Less than 1 second for 100k calls - }); - - it("should handle various JSON sizes efficiently", () => { - const sizes = [100, 1000, 10000, 100000]; - const iterations = 10_000; - - for (const size of sizes) { - const text = `hello world `.repeat(size / 11); // Approximate size - const goodJson = `{"title": "hello", "text": "${text}"}`; - - const start = performance.now(); - for (let i = 0; i < iterations; i++) { - detectBadJsonStrings(goodJson); - } - const time = performance.now() - start; - - console.log( - `Size ${size} chars (${iterations} iterations): ${time.toFixed(2)}ms (${( - time / iterations - ).toFixed(4)}ms per call)` - ); - - // Performance should scale reasonably with size - expect(time / iterations).toBeLessThan(size / 1000); // Roughly linear scaling - } - }); - - it("should show significant performance improvement with quick rejection", () => { - const longText = `hello world `.repeat(1_000); - const goodJson = `{"title": "hello", "text": "${longText}"}`; - const badJson = `{"title": "hello\\ud835", "text": "${longText}"}`; - const noUnicodeJson = `{"title": "hello", "text": "${longText}"}`; - - const iterations = 100_000; - - // Warm up - for (let i = 0; i < 1000; i++) { - detectBadJsonStrings(goodJson); - detectBadJsonStrings(badJson); - detectBadJsonStrings(noUnicodeJson); - } - - // Test strings with no Unicode escapes (99.9% case) - const noUnicodeStart = performance.now(); - for (let i = 0; i < iterations; i++) { - detectBadJsonStrings(noUnicodeJson); - } - const noUnicodeTime = performance.now() - noUnicodeStart; - - // Test strings with Unicode escapes (0.1% case) - const withUnicodeStart = performance.now(); - for (let i = 0; i < iterations; i++) { - detectBadJsonStrings(badJson); - } - const withUnicodeTime = performance.now() - withUnicodeStart; - - console.log( - `No Unicode escapes (${iterations} iterations): ${noUnicodeTime.toFixed(2)}ms (${( - noUnicodeTime / iterations - ).toFixed(4)}ms per call)` - ); - console.log( - `With Unicode escapes (${iterations} iterations): ${withUnicodeTime.toFixed(2)}ms (${( - withUnicodeTime / iterations - ).toFixed(4)}ms per call)` - ); - console.log( - `Performance ratio: ${(withUnicodeTime / noUnicodeTime).toFixed( - 2 - )}x slower for Unicode strings` - ); - - // Both cases should be extremely fast (under 1 microsecond per call) - expect(noUnicodeTime / iterations).toBeLessThan(0.001); // Less than 1 microsecond - expect(withUnicodeTime / iterations).toBeLessThan(0.001); // Less than 1 microsecond - - // The difference should be reasonable (not more than 5x) - expect(noUnicodeTime / withUnicodeTime).toBeLessThan(5); - }); + // NOTE: wall-clock microbenchmarks ("acceptable performance overhead", + // "various JSON sizes efficiently", "significant performance improvement + // with quick rejection") were removed. They asserted absolute per-call + // timings (e.g. < 1µs/call) that flake on shared CI runners under load + // while providing no correctness signal. Behavioural coverage of + // detectBadJsonStrings lives in the functional cases above and below. describe("full UTF-16 low-surrogate range coverage (U+DC00–U+DFFF)", () => { // Regression guard: a previous version of this scanner used `[cd]` to diff --git a/apps/webapp/test/dropTaskRunToTaskRunTagJoin.test.ts b/apps/webapp/test/dropTaskRunToTaskRunTagJoin.test.ts index 150ce44bf6..f96ba3d4f5 100644 --- a/apps/webapp/test/dropTaskRunToTaskRunTagJoin.test.ts +++ b/apps/webapp/test/dropTaskRunToTaskRunTagJoin.test.ts @@ -4,61 +4,68 @@ import { describe, expect } from "vitest"; import { postgresTest } from "@internal/testcontainers"; describe("drop _TaskRunToTaskRunTag implicit join", () => { - postgresTest("runTags scalar round-trips and the join table is gone", async ({ prisma }) => { - const organization = await prisma.organization.create({ - data: { - title: "test", - slug: "test", - }, - }); + // 120s timeout: this test had no override and ran on vitest's 5s default, + // which flaked whenever cold Postgres testcontainer startup on a loaded CI + // runner pushed the first query past 5s. + postgresTest( + "runTags scalar round-trips and the join table is gone", + async ({ prisma }) => { + const organization = await prisma.organization.create({ + data: { + title: "test", + slug: "test", + }, + }); - const project = await prisma.project.create({ - data: { - name: "test", - slug: "test", - organizationId: organization.id, - externalRef: "test", - }, - }); + const project = await prisma.project.create({ + data: { + name: "test", + slug: "test", + organizationId: organization.id, + externalRef: "test", + }, + }); - const runtimeEnvironment = await prisma.runtimeEnvironment.create({ - data: { - slug: "test", - type: "DEVELOPMENT", - projectId: project.id, - organizationId: organization.id, - apiKey: "test", - pkApiKey: "test", - shortcode: "test", - }, - }); + const runtimeEnvironment = await prisma.runtimeEnvironment.create({ + data: { + slug: "test", + type: "DEVELOPMENT", + projectId: project.id, + organizationId: organization.id, + apiKey: "test", + pkApiKey: "test", + shortcode: "test", + }, + }); - const taskRun = await prisma.taskRun.create({ - data: { - friendlyId: "run_1234", - taskIdentifier: "my-task", - payload: JSON.stringify({ foo: "bar" }), - payloadType: "application/json", - traceId: "1234", - spanId: "1234", - queue: "test", - runtimeEnvironmentId: runtimeEnvironment.id, - projectId: project.id, - organizationId: organization.id, - environmentType: "DEVELOPMENT", - engine: "V2", - runTags: ["alpha", "beta"], - }, - }); + const taskRun = await prisma.taskRun.create({ + data: { + friendlyId: "run_1234", + taskIdentifier: "my-task", + payload: JSON.stringify({ foo: "bar" }), + payloadType: "application/json", + traceId: "1234", + spanId: "1234", + queue: "test", + runtimeEnvironmentId: runtimeEnvironment.id, + projectId: project.id, + organizationId: organization.id, + environmentType: "DEVELOPMENT", + engine: "V2", + runTags: ["alpha", "beta"], + }, + }); - const readBack = await prisma.taskRun.findFirstOrThrow({ - where: { id: taskRun.id }, - }); - expect(readBack.runTags).toEqual(["alpha", "beta"]); + const readBack = await prisma.taskRun.findFirstOrThrow({ + where: { id: taskRun.id }, + }); + expect(readBack.runTags).toEqual(["alpha", "beta"]); - const result = await prisma.$queryRaw<{ t: string | null }[]>` + const result = await prisma.$queryRaw<{ t: string | null }[]>` SELECT to_regclass('public._TaskRunToTaskRunTag')::text as t `; - expect(result[0].t).toBeNull(); - }); + expect(result[0].t).toBeNull(); + }, + 120_000 + ); }); diff --git a/apps/webapp/test/runsReplicationService.part2.test.ts b/apps/webapp/test/runsReplicationService.part2.test.ts index dda3fe8e8d..20768c31d4 100644 --- a/apps/webapp/test/runsReplicationService.part2.test.ts +++ b/apps/webapp/test/runsReplicationService.part2.test.ts @@ -6,7 +6,10 @@ import { z } from "zod"; import { RunsReplicationService } from "~/services/runsReplicationService.server"; import { TestReplicationClickhouseFactory } from "./utils/testReplicationClickhouseFactory"; -vi.setConfig({ testTimeout: 60_000 }); +// 120s: raised from 60s — Postgres+ClickHouse replication testcontainers plus +// Redlock leader-election are slow on a loaded CI runner and intermittently +// exceeded 60s. +vi.setConfig({ testTimeout: 120_000 }); describe("RunsReplicationService (part 2/7)", () => { replicationContainerTest( diff --git a/apps/webapp/test/runsReplicationService.part8.test.ts b/apps/webapp/test/runsReplicationService.part8.test.ts index 0b4afab2d3..90d7ce525b 100644 --- a/apps/webapp/test/runsReplicationService.part8.test.ts +++ b/apps/webapp/test/runsReplicationService.part8.test.ts @@ -8,7 +8,9 @@ import { RunsReplicationService } from "~/services/runsReplicationService.server import { createInMemoryTracing } from "./utils/tracing"; import { TestReplicationClickhouseFactory } from "./utils/testReplicationClickhouseFactory"; -vi.setConfig({ testTimeout: 60_000 }); +// 120s: raised from 60s — Postgres+ClickHouse replication testcontainers are +// slow to come up on a loaded CI runner and intermittently exceeded 60s. +vi.setConfig({ testTimeout: 120_000 }); describe("RunsReplicationService (part 8/8) - dual-source dedup", () => { replicationContainerTest( diff --git a/apps/webapp/test/runsReplicationService.part9.test.ts b/apps/webapp/test/runsReplicationService.part9.test.ts index b7882d94b3..32f4955908 100644 --- a/apps/webapp/test/runsReplicationService.part9.test.ts +++ b/apps/webapp/test/runsReplicationService.part9.test.ts @@ -6,7 +6,9 @@ import { RunsReplicationService } from "~/services/runsReplicationService.server import { createInMemoryMetrics } from "./utils/tracing"; import { TestReplicationClickhouseFactory } from "./utils/testReplicationClickhouseFactory"; -vi.setConfig({ testTimeout: 90_000 }); +// 120s: raised from 90s — Postgres+ClickHouse replication testcontainers are +// slow to come up on a loaded CI runner and intermittently exceeded 90s. +vi.setConfig({ testTimeout: 120_000 }); // Copied from runsReplicationService.part4.test.ts (the only replication part-test that // injects a meter). These read metric data points out of the in-memory reader. diff --git a/apps/webapp/test/runsRepository.part2.test.ts b/apps/webapp/test/runsRepository.part2.test.ts index eca1ae619e..5d4ca06cc4 100644 --- a/apps/webapp/test/runsRepository.part2.test.ts +++ b/apps/webapp/test/runsRepository.part2.test.ts @@ -11,7 +11,9 @@ import { setTimeout } from "node:timers/promises"; import { RunsRepository } from "~/services/runsRepository/runsRepository.server"; import { setupClickhouseReplication } from "./utils/replicationUtils"; -vi.setConfig({ testTimeout: 60_000 }); +// 120s: raised from 60s — Postgres+ClickHouse replication testcontainers plus +// Redlock are slow on a loaded CI runner and intermittently exceeded 60s. +vi.setConfig({ testTimeout: 120_000 }); describe("RunsRepository (part 2/4)", () => { replicationContainerTest( diff --git a/apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts b/apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts index d2baaa6404..f701bf28f2 100644 --- a/apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts +++ b/apps/webapp/test/v3/runOpsMigration/distinctDbSentinel.server.test.ts @@ -3,8 +3,10 @@ import { PrismaClient } from "@trigger.dev/database"; import { describe, expect, vi } from "vitest"; import { probeDistinctDatabases } from "~/v3/runOpsMigration/distinctDbSentinel.server"; -// Spinning up two separate postgres clusters and probing each can exceed the 5s default. -vi.setConfig({ testTimeout: 60_000 }); +// Spinning up two separate postgres clusters and probing each can exceed the 5s +// default. 120s: raised from 60s — the two cold clusters intermittently exceeded +// 60s on a loaded CI runner. +vi.setConfig({ testTimeout: 120_000 }); function urlWithDatabase(uri: string, database: string): string { const url = new URL(uri); diff --git a/internal-packages/run-engine/src/engine/tests/delayedRunSystem.controlPlaneResolver.test.ts b/internal-packages/run-engine/src/engine/tests/delayedRunSystem.controlPlaneResolver.test.ts index 2d35d37db2..00da9167f0 100644 --- a/internal-packages/run-engine/src/engine/tests/delayedRunSystem.controlPlaneResolver.test.ts +++ b/internal-packages/run-engine/src/engine/tests/delayedRunSystem.controlPlaneResolver.test.ts @@ -14,7 +14,9 @@ import { PostgresRunStore } from "@internal/run-store"; import { RunEngine } from "../index.js"; import { setupAuthenticatedEnvironment, setupBackgroundWorker } from "./setup.js"; -vi.setConfig({ testTimeout: 60_000 }); +// 120s: the hetero PG14+PG17 case starts two Postgres testcontainers whose +// cold startup on a loaded CI runner intermittently pushed setup past 60s. +vi.setConfig({ testTimeout: 120_000 }); function createEngineOptions(redisOptions: any, prisma: any) { return { diff --git a/internal-packages/run-engine/src/run-queue/tests/fairQueueSelectionStrategy.test.ts b/internal-packages/run-engine/src/run-queue/tests/fairQueueSelectionStrategy.test.ts index 5b2bdd8bf6..c97a7aa34a 100644 --- a/internal-packages/run-engine/src/run-queue/tests/fairQueueSelectionStrategy.test.ts +++ b/internal-packages/run-engine/src/run-queue/tests/fairQueueSelectionStrategy.test.ts @@ -230,18 +230,12 @@ describe("FairDequeuingStrategy", () => { envId: "env-3", }); - const startDistribute1 = performance.now(); - const envResult = await strategy.distributeFairQueuesFromParentQueue( "parent-queue", "consumer-1" ); const result = flattenResults(envResult); - const distribute1Duration = performance.now() - startDistribute1; - - console.log("First distribution took", distribute1Duration, "ms"); - expect(result).toHaveLength(3); // Should only get the two oldest queues const queue1 = keyProducer.queueKey("org-1", "proj-1", "env-1", "queue-1"); @@ -249,33 +243,20 @@ describe("FairDequeuingStrategy", () => { const queue3 = keyProducer.queueKey("org-3", "proj-3", "env-3", "queue-3"); expect(result).toEqual([queue2, queue1, queue3]); - const startDistribute2 = performance.now(); - - const _result2 = await strategy.distributeFairQueuesFromParentQueue( - "parent-queue", - "consumer-1" + // The second call reuses the cached snapshot (reuseSnapshotCount: 1) and + // the third recomputes it. We assert the reuse/recompute path returns the + // same distribution rather than timing the calls: the previous wall-clock + // assertions (call 2 must be >2x faster than call 1, call 3 must be >2x + // slower than call 2) flaked on shared CI runners under load. + const result2 = flattenResults( + await strategy.distributeFairQueuesFromParentQueue("parent-queue", "consumer-1") ); + expect(result2).toEqual(result); - const distribute2Duration = performance.now() - startDistribute2; - - console.log("Second distribution took", distribute2Duration, "ms"); - - // Make sure the second call is more than 2 times faster than the first - expect(distribute2Duration).toBeLessThan(distribute1Duration / 2); - - const startDistribute3 = performance.now(); - - const _result3 = await strategy.distributeFairQueuesFromParentQueue( - "parent-queue", - "consumer-1" + const result3 = flattenResults( + await strategy.distributeFairQueuesFromParentQueue("parent-queue", "consumer-1") ); - - const distribute3Duration = performance.now() - startDistribute3; - - console.log("Third distribution took", distribute3Duration, "ms"); - - // Make sure the third call is more than 4 times the second - expect(distribute3Duration).toBeGreaterThan(distribute2Duration * 2); + expect(result3).toEqual(result); } );