Skip to content

Commit 4c2c255

Browse files
authored
test(webapp): split triggerTask engine test into per-concern files (#4167)
The engine `triggerTask` suite was a single 2447-line file with 23 `containerTest` cases, each spinning its own Postgres + Redis. vitest shards by whole file, so all 23 container setups landed on one shard and dominated its wall-clock. The recorded entry in `test-timings.json` badly under-counts the real cost (it does not capture the per-`containerTest` container startup that dominates on CI), so the duration-sharding sequencer treated the file as light and stacked it, producing one ~21 minute shard. Splitting does not reduce the number of container setups; it lets those 23 cases distribute across shards instead of stacking on one. The webapp unit-test stage is gated by its slowest shard, so this cuts the stage's wall-clock roughly in half. ## CI timing (before vs after) Real CI wall-clock of the `Unit Tests: Webapp` shards (`--shard=i/10`). "Before" is sampled from recent runs on other branches (unsplit file, from `main`); "after" is this PR. | Shard | Before (s) | After (s) | |------:|-----------:|----------:| | 1 | 250 | 359 | | 2 | 444 | 411 | | 3 | 497 | 659 | | 4 | **1257** | 284 | | 5 | 545 | 641 | | 6 | 284 | 644 | | 7 | 244 | 214 | | 8 | 340 | 445 | | 9 | 188 | 395 | | 10 | 234 | 567 | | **Slowest shard (gates the stage)** | **~1247s (≈21m)** | **659s (≈11m)** | | Sum of all shards | 4283 | 4619 | Before: shard 4 is the long pole at 1237s / 1247s / 1257s across three sampled runs (the `triggerTask` file plus whatever else the packer put with it). After: the six pieces spread across shards, the slowest drops to 659s. The small rise in summed time is the extra per-file container startup, paid in parallel across shards, so the gating number still falls by about 10 minutes. ## Change Split into six per-concern files that share a `triggerTaskTestHelpers` module (the `vi.mock` calls stay per-file, since vitest hoists them): - `triggerTask.test.ts` (3): trigger + concurrencyKey coercion - `triggerTask.idempotency.test.ts` (4): idempotency + queue resolution - `triggerTask.debounce.test.ts` (4): retries + debounce validation - `triggerTask.mollifier.test.ts` (4): mollifier call-site behaviour - `triggerTask.metadataCache.test.ts` (4): DefaultQueueManager task metadata cache - `triggerTask.residency.test.ts` (4): child run residency inheritance All 23 cases are preserved. The file's `test-timings.json` entry is split across the new files so bin-packing stays balanced. While rewriting these files, cleanup was moved to `onTestFinished(() => engine.quit())` so an `engine`/`Redis` leaked on a failing assertion no longer persists on the worker-scoped Redis and cascades into later cases (`hookTimeout` raised to 60s so the after-cleanup gets the full budget). Prisma lookups switched from `findUnique` to `findFirst` to match the repo convention. Verified: all six files run green locally (23/23), oxlint and oxfmt clean.
1 parent e4ae8cb commit 4c2c255

8 files changed

Lines changed: 2327 additions & 2148 deletions

apps/webapp/test/engine/triggerTask.debounce.test.ts

Lines changed: 462 additions & 0 deletions
Large diffs are not rendered by default.

apps/webapp/test/engine/triggerTask.idempotency.test.ts

Lines changed: 651 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 342 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,342 @@
1+
import { describe, expect, onTestFinished, vi } from "vitest";
2+
3+
// db.server + splitMode are mocked so the idempotency dedup client resolves to
4+
// the container prisma passed into the concern (split stays off).
5+
vi.mock("~/db.server", () => ({
6+
prisma: {},
7+
$replica: {},
8+
runOpsNewPrisma: {},
9+
runOpsLegacyPrisma: {},
10+
}));
11+
12+
vi.mock("~/v3/runOpsMigration/splitMode.server", () => ({ isSplitEnabled: async () => false }));
13+
14+
vi.mock("~/services/platform.v3.server", async (importOriginal) => {
15+
const actual = (await importOriginal()) as Record<string, unknown>;
16+
return {
17+
...actual,
18+
getEntitlement: vi.fn(),
19+
};
20+
});
21+
22+
import { RunEngine } from "@internal/run-engine";
23+
import { setupAuthenticatedEnvironment, setupBackgroundWorker } from "@internal/run-engine/tests";
24+
import { assertNonNullable, containerTest } from "@internal/testcontainers";
25+
import { trace } from "@opentelemetry/api";
26+
import { Redis } from "ioredis";
27+
import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
28+
import { DefaultQueueManager } from "~/runEngine/concerns/queues.server";
29+
import { RedisTaskMetadataCache } from "~/services/taskMetadataCache.server";
30+
import { RunEngineTriggerTaskService } from "../../app/runEngine/services/triggerTask.server";
31+
import { setTimeout } from "node:timers/promises";
32+
import {
33+
MockPayloadProcessor,
34+
MockTraceEventConcern,
35+
MockTriggerTaskValidator,
36+
} from "./triggerTaskTestHelpers";
37+
38+
vi.setConfig({ testTimeout: 60_000, hookTimeout: 60_000 });
39+
40+
describe("DefaultQueueManager task metadata cache", () => {
41+
containerTest(
42+
"warm cache returns metadata without falling through to PG",
43+
async ({ prisma, redisOptions }) => {
44+
const engine = new RunEngine({
45+
prisma,
46+
worker: { redis: redisOptions, workers: 1, tasksPerWorker: 10, pollIntervalMs: 100 },
47+
queue: { redis: redisOptions },
48+
runLock: { redis: redisOptions },
49+
machines: {
50+
defaultMachine: "small-1x",
51+
machines: {
52+
"small-1x": { name: "small-1x", cpu: 0.5, memory: 0.5, centsPerMs: 0.0001 },
53+
},
54+
baseCostInCents: 0.0005,
55+
},
56+
tracer: trace.getTracer("test", "0.0.0"),
57+
});
58+
onTestFinished(() => engine.quit());
59+
60+
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
61+
const taskIdentifier = "cached-task";
62+
const setup = await setupBackgroundWorker(engine, environment, taskIdentifier);
63+
64+
const redis = new Redis(redisOptions);
65+
onTestFinished(() => redis.quit());
66+
const cache = new RedisTaskMetadataCache({ redis });
67+
68+
// Pre-populate cache with AGENT triggerSource; DB row has the default STANDARD.
69+
// If the read path hits the cache, the resulting TaskRun.taskKind reflects the
70+
// cached value. If it falls through to PG, it reflects STANDARD.
71+
await cache.populateByCurrentWorker(environment.id, setup.worker.id, [
72+
{
73+
slug: taskIdentifier,
74+
ttl: null,
75+
triggerSource: "AGENT",
76+
queueId: null,
77+
queueName: `task/${taskIdentifier}`,
78+
},
79+
]);
80+
81+
const queuesManager = new DefaultQueueManager(prisma, engine, undefined, cache);
82+
const triggerTaskService = new RunEngineTriggerTaskService({
83+
engine,
84+
prisma,
85+
payloadProcessor: new MockPayloadProcessor(),
86+
queueConcern: queuesManager,
87+
idempotencyKeyConcern: new IdempotencyKeyConcern(
88+
prisma,
89+
engine,
90+
new MockTraceEventConcern()
91+
),
92+
validator: new MockTriggerTaskValidator(),
93+
traceEventConcern: new MockTraceEventConcern(),
94+
tracer: trace.getTracer("test", "0.0.0"),
95+
metadataMaximumSize: 1024 * 1024,
96+
});
97+
98+
const result = await triggerTaskService.call({
99+
taskId: taskIdentifier,
100+
environment,
101+
body: { payload: { test: "x" } },
102+
});
103+
104+
assertNonNullable(result);
105+
expect(result.run.taskIdentifier).toBe(taskIdentifier);
106+
expect((result.run.annotations as { taskKind?: string } | null)?.taskKind).toBe("AGENT");
107+
}
108+
);
109+
110+
containerTest(
111+
"cache miss falls through to PG and back-fills the cache",
112+
async ({ prisma, redisOptions }) => {
113+
const engine = new RunEngine({
114+
prisma,
115+
worker: { redis: redisOptions, workers: 1, tasksPerWorker: 10, pollIntervalMs: 100 },
116+
queue: { redis: redisOptions },
117+
runLock: { redis: redisOptions },
118+
machines: {
119+
defaultMachine: "small-1x",
120+
machines: {
121+
"small-1x": { name: "small-1x", cpu: 0.5, memory: 0.5, centsPerMs: 0.0001 },
122+
},
123+
baseCostInCents: 0.0005,
124+
},
125+
tracer: trace.getTracer("test", "0.0.0"),
126+
});
127+
onTestFinished(() => engine.quit());
128+
129+
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
130+
const taskIdentifier = "miss-task";
131+
await setupBackgroundWorker(engine, environment, taskIdentifier);
132+
133+
const redis = new Redis(redisOptions);
134+
onTestFinished(() => redis.quit());
135+
const cache = new RedisTaskMetadataCache({ redis });
136+
137+
// Cache starts empty. Sanity-check both keyspaces.
138+
expect(await cache.getCurrent(environment.id, taskIdentifier)).toBeNull();
139+
140+
const queuesManager = new DefaultQueueManager(prisma, engine, undefined, cache);
141+
const triggerTaskService = new RunEngineTriggerTaskService({
142+
engine,
143+
prisma,
144+
payloadProcessor: new MockPayloadProcessor(),
145+
queueConcern: queuesManager,
146+
idempotencyKeyConcern: new IdempotencyKeyConcern(
147+
prisma,
148+
engine,
149+
new MockTraceEventConcern()
150+
),
151+
validator: new MockTriggerTaskValidator(),
152+
traceEventConcern: new MockTraceEventConcern(),
153+
tracer: trace.getTracer("test", "0.0.0"),
154+
metadataMaximumSize: 1024 * 1024,
155+
});
156+
157+
const result = await triggerTaskService.call({
158+
taskId: taskIdentifier,
159+
environment,
160+
body: { payload: { test: "x" } },
161+
});
162+
163+
assertNonNullable(result);
164+
expect((result.run.annotations as { taskKind?: string } | null)?.taskKind).toBe("STANDARD");
165+
166+
// Back-fill is fire-and-forget; poll with a bounded timeout to avoid CI flakes.
167+
let backfilled = await cache.getCurrent(environment.id, taskIdentifier);
168+
for (let i = 0; i < 40 && !backfilled; i++) {
169+
await setTimeout(25);
170+
backfilled = await cache.getCurrent(environment.id, taskIdentifier);
171+
}
172+
expect(backfilled).not.toBeNull();
173+
expect(backfilled?.triggerSource).toBe("STANDARD");
174+
expect(backfilled?.queueName).toBe(`task/${taskIdentifier}`);
175+
}
176+
);
177+
178+
containerTest(
179+
"queue-override + ttl path returns taskKind from cache without a BWT lookup",
180+
async ({ prisma, redisOptions }) => {
181+
const engine = new RunEngine({
182+
prisma,
183+
worker: { redis: redisOptions, workers: 1, tasksPerWorker: 10, pollIntervalMs: 100 },
184+
queue: { redis: redisOptions },
185+
runLock: { redis: redisOptions },
186+
machines: {
187+
defaultMachine: "small-1x",
188+
machines: {
189+
"small-1x": { name: "small-1x", cpu: 0.5, memory: 0.5, centsPerMs: 0.0001 },
190+
},
191+
baseCostInCents: 0.0005,
192+
},
193+
tracer: trace.getTracer("test", "0.0.0"),
194+
});
195+
onTestFinished(() => engine.quit());
196+
197+
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
198+
const taskIdentifier = "override-task";
199+
const setup = await setupBackgroundWorker(engine, environment, taskIdentifier);
200+
201+
const redis = new Redis(redisOptions);
202+
onTestFinished(() => redis.quit());
203+
const cache = new RedisTaskMetadataCache({ redis });
204+
205+
// Cache says AGENT; DB row says STANDARD. Caller provides both a queue
206+
// override and an explicit TTL — the hot path the PR regressed.
207+
await cache.populateByCurrentWorker(environment.id, setup.worker.id, [
208+
{
209+
slug: taskIdentifier,
210+
ttl: null,
211+
triggerSource: "AGENT",
212+
queueId: null,
213+
queueName: `task/${taskIdentifier}`,
214+
},
215+
]);
216+
217+
const queuesManager = new DefaultQueueManager(prisma, engine, undefined, cache);
218+
const triggerTaskService = new RunEngineTriggerTaskService({
219+
engine,
220+
prisma,
221+
payloadProcessor: new MockPayloadProcessor(),
222+
queueConcern: queuesManager,
223+
idempotencyKeyConcern: new IdempotencyKeyConcern(
224+
prisma,
225+
engine,
226+
new MockTraceEventConcern()
227+
),
228+
validator: new MockTriggerTaskValidator(),
229+
traceEventConcern: new MockTraceEventConcern(),
230+
tracer: trace.getTracer("test", "0.0.0"),
231+
metadataMaximumSize: 1024 * 1024,
232+
});
233+
234+
const result = await triggerTaskService.call({
235+
taskId: taskIdentifier,
236+
environment,
237+
body: {
238+
payload: { test: "x" },
239+
options: {
240+
queue: { name: "caller-queue" },
241+
ttl: "5m",
242+
},
243+
},
244+
});
245+
246+
assertNonNullable(result);
247+
expect(result.run.queue).toBe("caller-queue");
248+
expect((result.run.annotations as { taskKind?: string } | null)?.taskKind).toBe("AGENT");
249+
}
250+
);
251+
252+
containerTest(
253+
"locked-version trigger reads from by-worker keyspace, not env keyspace",
254+
async ({ prisma, redisOptions }) => {
255+
const engine = new RunEngine({
256+
prisma,
257+
worker: { redis: redisOptions, workers: 1, tasksPerWorker: 10, pollIntervalMs: 100 },
258+
queue: { redis: redisOptions },
259+
runLock: { redis: redisOptions },
260+
machines: {
261+
defaultMachine: "small-1x",
262+
machines: {
263+
"small-1x": { name: "small-1x", cpu: 0.5, memory: 0.5, centsPerMs: 0.0001 },
264+
},
265+
baseCostInCents: 0.0005,
266+
},
267+
tracer: trace.getTracer("test", "0.0.0"),
268+
});
269+
onTestFinished(() => engine.quit());
270+
271+
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
272+
const taskIdentifier = "keyspace-task";
273+
const worker = await setupBackgroundWorker(engine, environment, taskIdentifier);
274+
275+
const redis = new Redis(redisOptions);
276+
onTestFinished(() => redis.quit());
277+
const cache = new RedisTaskMetadataCache({ redis });
278+
279+
// Populate the two keyspaces with conflicting triggerSource values so we
280+
// can tell which keyspace the read used. The real worker's by-worker
281+
// hash gets AGENT; the env hash gets SCHEDULED (seeded via a throwaway
282+
// worker id since `populateByCurrentWorker` writes both keyspaces and
283+
// we want the real worker's by-worker hash untouched).
284+
await cache.populateByWorker(worker.worker.id, [
285+
{
286+
slug: taskIdentifier,
287+
ttl: null,
288+
triggerSource: "AGENT",
289+
queueId: null,
290+
queueName: `task/${taskIdentifier}`,
291+
},
292+
]);
293+
await cache.populateByCurrentWorker(environment.id, "dummy-worker-for-env-seed", [
294+
{
295+
slug: taskIdentifier,
296+
ttl: null,
297+
triggerSource: "SCHEDULED",
298+
queueId: null,
299+
queueName: `task/${taskIdentifier}`,
300+
},
301+
]);
302+
303+
const queuesManager = new DefaultQueueManager(prisma, engine, undefined, cache);
304+
const triggerTaskService = new RunEngineTriggerTaskService({
305+
engine,
306+
prisma,
307+
payloadProcessor: new MockPayloadProcessor(),
308+
queueConcern: queuesManager,
309+
idempotencyKeyConcern: new IdempotencyKeyConcern(
310+
prisma,
311+
engine,
312+
new MockTraceEventConcern()
313+
),
314+
validator: new MockTriggerTaskValidator(),
315+
traceEventConcern: new MockTraceEventConcern(),
316+
tracer: trace.getTracer("test", "0.0.0"),
317+
metadataMaximumSize: 1024 * 1024,
318+
});
319+
320+
// Locked → by-worker keyspace → AGENT
321+
const locked = await triggerTaskService.call({
322+
taskId: taskIdentifier,
323+
environment,
324+
body: {
325+
payload: { test: "x" },
326+
options: { lockToVersion: worker.worker.version },
327+
},
328+
});
329+
assertNonNullable(locked);
330+
expect((locked.run.annotations as { taskKind?: string } | null)?.taskKind).toBe("AGENT");
331+
332+
// Not locked → env keyspace → SCHEDULED
333+
const current = await triggerTaskService.call({
334+
taskId: taskIdentifier,
335+
environment,
336+
body: { payload: { test: "y" } },
337+
});
338+
assertNonNullable(current);
339+
expect((current.run.annotations as { taskKind?: string } | null)?.taskKind).toBe("SCHEDULED");
340+
}
341+
);
342+
});

0 commit comments

Comments
 (0)