Skip to content

Commit 82bd4fa

Browse files
committed
fix(testcontainers): per-test redis for background-worker tests + faster redis boot
1 parent 454b5c7 commit 82bd4fa

5 files changed

Lines changed: 52 additions & 29 deletions

File tree

internal-packages/run-engine/src/engine/tests/batchTriggerAndWait.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { assertNonNullable, containerTest } from "@internal/testcontainers";
1+
import { assertNonNullable, containerTestWithIsolatedRedis as containerTest } from "@internal/testcontainers";
22
import { trace } from "@internal/tracing";
33
import { expect, describe } from "vitest";
44
import { RunEngine } from "../index.js";

internal-packages/run-engine/src/engine/tests/batchTwoPhase.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { assertNonNullable, containerTest } from "@internal/testcontainers";
1+
import { assertNonNullable, containerTestWithIsolatedRedis as containerTest } from "@internal/testcontainers";
22
import { trace } from "@internal/tracing";
33
import { expect, describe, vi } from "vitest";
44
import { RunEngine } from "../index.js";

internal-packages/testcontainers/src/index.ts

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -260,13 +260,9 @@ export const redisOptions = async (
260260
await use(options);
261261
};
262262

263-
type RedisTestContext = {
264-
redisContainer: StartedRedisContainer;
265-
resetRedis: void;
266-
redisOptions: RedisOptions;
267-
};
268-
269-
// Boot redis once per worker.
263+
// Worker-scoped redis: booted once per worker, FLUSHALL per test. Big win for redis-heavy files
264+
// (buffer.test.ts: 88 boots -> 1). Safe ONLY for tests that don't leave background redis work
265+
// (a Worker loop, BatchQueue) running past the test body - use isolatedRedisTest for those.
270266
const bootWorkerRedis = async ({}, use: Use<StartedRedisContainer>) => {
271267
const { container } = await createRedisContainer({ port: 6379 });
272268
try {
@@ -276,7 +272,6 @@ const bootWorkerRedis = async ({}, use: Use<StartedRedisContainer>) => {
276272
}
277273
};
278274

279-
// Per test: FLUSHALL the shared redis (auto fixture so it runs for every test).
280275
const flushRedis = async (
281276
{ redisContainer }: { redisContainer: StartedRedisContainer },
282277
use: Use<void>
@@ -295,12 +290,23 @@ const flushRedis = async (
295290
await use();
296291
};
297292

293+
type RedisTestContext = {
294+
redisContainer: StartedRedisContainer;
295+
resetRedis: void;
296+
redisOptions: RedisOptions;
297+
};
298+
298299
export const redisTest = test.extend<RedisTestContext>({
299300
redisContainer: [bootWorkerRedis, { scope: "worker" }],
300301
resetRedis: [flushRedis, { auto: true }],
301302
redisOptions,
302303
});
303304

305+
// Per-test redis for tests with background redis work (redis-worker Workers, BatchQueue) that can
306+
// outlive the test body - a shared redis would let leaked work hit a closed connection / next test
307+
// ("Connection is closed"). Boot is kept fast (see createRedisContainer).
308+
export const isolatedRedisTest = test.extend<RedisContext>({ network, redisContainer, redisOptions });
309+
304310
const electricOrigin = async (
305311
{
306312
postgresContainer,
@@ -424,9 +430,9 @@ type ContainerTestContext = {
424430
clickhouseClient: ClickHouseClient;
425431
};
426432

427-
// The workhorse fixture (~36 files). Boots postgres+redis+clickhouse ONCE per worker and isolates
428-
// per test (postgres template-clone, redis FLUSHALL, clickhouse TRUNCATE) - no per-test boots, no
429-
// shared docker network needed.
433+
// The workhorse fixture (~36 files). Postgres (template-clone), Redis (FLUSHALL) and ClickHouse
434+
// (truncate) all boot once per worker - no per-test container boots. Use containerTestWithIsolatedRedis
435+
// for tests that run background redis work (BatchQueue, redis-worker Workers) past the test body.
430436
export const containerTest = test.extend<ContainerTestContext>({
431437
postgresContainer: clonedPostgresContainer,
432438
prisma: prismaFromContainer,
@@ -438,18 +444,41 @@ export const containerTest = test.extend<ContainerTestContext>({
438444
clickhouseClient: scopedClickhouseClient,
439445
});
440446

447+
type ContainerWithIsolatedRedisContext = {
448+
network: StartedNetwork;
449+
postgresContainer: StartedPostgreSqlContainer;
450+
prisma: PrismaClient;
451+
redisContainer: StartedRedisContainer;
452+
redisOptions: RedisOptions;
453+
clickhouseContainer: StartedClickHouseContainer;
454+
resetClickhouse: void;
455+
clickhouseClient: ClickHouseClient;
456+
};
457+
458+
// Same as containerTest but Redis is PER-TEST - for tests whose background redis work (BatchQueue,
459+
// Workers) outlives the test body and would otherwise hit a closed/shared connection.
460+
export const containerTestWithIsolatedRedis = test.extend<ContainerWithIsolatedRedisContext>({
461+
network,
462+
postgresContainer: clonedPostgresContainer,
463+
prisma: prismaFromContainer,
464+
redisContainer,
465+
redisOptions,
466+
clickhouseContainer: [bootWorkerClickhouse, { scope: "worker" }],
467+
resetClickhouse: [truncateClickhouseFixture, { auto: true }],
468+
clickhouseClient: scopedClickhouseClient,
469+
});
470+
441471
// For tests that exercise the Postgres -> ClickHouse logical-replication pipeline (WAL slots,
442472
// publications, REPLICA IDENTITY). These need a dedicated Postgres per test - the worker-scoped +
443473
// template-clone model used by containerTest doesn't carry logical replication across cloned dbs.
444-
// ONLY postgres needs to be per-test (the WAL slot/publication lives in the db it writes to). Redis
445-
// and ClickHouse are still worker-scoped + reset per test - the pipeline writes pg->clickhouse and a
446-
// shared+truncated clickhouse is fine - so we only pay a postgres boot per test, not all three.
474+
// Postgres is per-test (the WAL slot/publication lives in the db it writes to); ClickHouse is
475+
// worker-scoped + truncated (the pipeline writes pg->clickhouse and a shared+truncated clickhouse is
476+
// fine). Redis is per-test too (background work safety, same as containerTest).
447477
type ReplicationContainerTestContext = {
448478
network: StartedNetwork;
449479
postgresContainer: StartedPostgreSqlContainer;
450480
prisma: PrismaClient;
451481
redisContainer: StartedRedisContainer;
452-
resetRedis: void;
453482
redisOptions: RedisOptions;
454483
clickhouseContainer: StartedClickHouseContainer;
455484
resetClickhouse: void;
@@ -460,8 +489,7 @@ export const replicationContainerTest = test.extend<ReplicationContainerTestCont
460489
network,
461490
postgresContainer,
462491
prisma,
463-
redisContainer: [bootWorkerRedis, { scope: "worker" }],
464-
resetRedis: [flushRedis, { auto: true }],
492+
redisContainer,
465493
redisOptions,
466494
clickhouseContainer: [bootWorkerClickhouse, { scope: "worker" }],
467495
resetClickhouse: [truncateClickhouseFixture, { auto: true }],

internal-packages/testcontainers/src/utils.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,11 @@ export async function createRedisContainer({
114114
container = container.withNetwork(network).withNetworkAliases("redis");
115115
}
116116

117+
// Wait only on the readiness log (RedisContainer's default) - the previous Docker healthcheck added
118+
// a full poll-cycle of latency per boot, which dominates per-test redis. verifyRedisConnection
119+
// below still confirms the container actually accepts connections before we hand it to the test.
117120
const startedContainer = await container
118-
.withHealthCheck({
119-
test: ["CMD", "redis-cli", "ping"],
120-
interval: 1000,
121-
timeout: 3000,
122-
retries: 5,
123-
})
124-
.withWaitStrategy(
125-
Wait.forAll([Wait.forHealthCheck(), Wait.forLogMessage("Ready to accept connections")])
126-
)
121+
.withWaitStrategy(Wait.forLogMessage("Ready to accept connections"))
127122
.start();
128123

129124
// Add a verification step

packages/redis-worker/src/worker.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { redisTest } from "@internal/testcontainers";
1+
import { isolatedRedisTest as redisTest } from "@internal/testcontainers";
22
import { Logger } from "@trigger.dev/core/logger";
33
import { describe } from "node:test";
44
import { expect } from "vitest";

0 commit comments

Comments
 (0)