Skip to content

Commit 703a6dc

Browse files
authored
chore(ci): optimise runners, distribute test shards (#4240)
- Use bigger/smaller runners as recommended by warpbuild - Distribute test shards more evenly, move internal tests single big shard
1 parent 022e5c1 commit 703a6dc

15 files changed

Lines changed: 1605 additions & 1236 deletions

.github/workflows/e2e-webapp.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ on:
1414
jobs:
1515
e2eTests:
1616
name: "🧪 E2E Tests: Webapp"
17-
runs-on: warp-ubuntu-latest-x64-8x
17+
runs-on: warp-ubuntu-latest-x64-16x
1818
timeout-minutes: 20
1919
env:
2020
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}

.github/workflows/e2e.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ jobs:
2020
strategy:
2121
fail-fast: false
2222
matrix:
23-
os: [warp-ubuntu-latest-x64-4x, warp-windows-latest-x64-4x]
23+
os: [warp-ubuntu-latest-x64-4x, warp-windows-latest-x64-8x]
2424
package-manager: ["npm", "pnpm"]
2525
steps:
2626
- name: ⬇️ Checkout repo

.github/workflows/release.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ jobs:
4848
4949
release:
5050
name: 🚀 Release npm packages
51-
runs-on: ubuntu-latest
51+
runs-on: ubuntu-latest # this cannot run on non-GH runner
5252
environment: npm-publish
5353
permissions:
5454
contents: write
@@ -281,7 +281,7 @@ jobs:
281281
# The prerelease job needs to be on the same workflow file due to a limitation related to how npm verifies OIDC claims.
282282
prerelease:
283283
name: 🧪 Prerelease
284-
runs-on: ubuntu-latest
284+
runs-on: ubuntu-latest # this cannot run on non-GH runner
285285
environment: npm-publish
286286
permissions:
287287
contents: read

.github/workflows/typecheck.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ permissions:
88

99
jobs:
1010
typecheck:
11-
runs-on: warp-ubuntu-latest-x64-8x
11+
runs-on: warp-ubuntu-latest-x64-16x
1212

1313
steps:
1414
- name: ⬇️ Checkout repo

.github/workflows/unit-tests-internal.yml

Lines changed: 35 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,14 @@ on:
1414
jobs:
1515
unitTests:
1616
name: "🧪 Unit Tests: Internal"
17-
runs-on: warp-ubuntu-latest-x64-8x
18-
strategy:
19-
# one flaky shard shouldn't cancel its siblings - lets us re-run only the failed shard
20-
fail-fast: false
21-
matrix:
22-
shardIndex: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
23-
shardTotal: [12]
17+
# Single big machine instead of a 12-job matrix: the internal suites are serial
18+
# (fileParallelism: false) and container-wait-bound, so 12 in-machine shard processes
19+
# fit comfortably in 32 vCPUs while paying the setup cost (install, prisma generate,
20+
# image pulls) once instead of 12 times.
21+
runs-on: warp-ubuntu-latest-x64-32x
2422
env:
2523
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
26-
SHARD_INDEX: ${{ matrix.shardIndex }}
27-
SHARD_TOTAL: ${{ matrix.shardTotal }}
24+
SHARD_TOTAL: 12
2825
steps:
2926
- name: 🔧 Disable IPv6
3027
run: |
@@ -108,8 +105,34 @@ jobs:
108105
- name: 📀 Generate Prisma Client
109106
run: pnpm run generate
110107

111-
- name: 🧪 Run Internal Unit Tests
112-
run: pnpm run test:internal --reporter=default --reporter=blob --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --passWithNoTests
108+
- name: 🏗️ Build test dependencies
109+
# Build once up-front so the parallel shard runs below (turbo --only) never race
110+
# to build or cache-restore the same outputs concurrently.
111+
run: pnpm exec turbo run build --filter "@internal/*..."
112+
113+
- name: 🧪 Run Internal Unit Tests (${{ env.SHARD_TOTAL }} in-machine shards)
114+
run: |
115+
# Same shard partitioning as the old 12-job matrix (DurationShardingSequencer
116+
# keys off --shard=i/N), but as parallel local processes. --only skips the
117+
# ^build dependency handled by the step above.
118+
status=0
119+
declare -a pids
120+
for i in $(seq 1 "$SHARD_TOTAL"); do
121+
pnpm exec turbo run test --only --concurrency=1 --filter "@internal/*" -- \
122+
--run --reporter=default --reporter=blob --shard="$i/$SHARD_TOTAL" --passWithNoTests \
123+
> "/tmp/internal-shard-$i.log" 2>&1 &
124+
pids[i]=$!
125+
done
126+
for i in $(seq 1 "$SHARD_TOTAL"); do
127+
if ! wait "${pids[i]}"; then
128+
status=1
129+
echo "::error::internal unit test shard $i/$SHARD_TOTAL failed"
130+
fi
131+
echo "::group::🧪 shard $i/$SHARD_TOTAL"
132+
cat "/tmp/internal-shard-$i.log"
133+
echo "::endgroup::"
134+
done
135+
exit "$status"
113136
114137
- name: Gather all reports
115138
if: ${{ !cancelled() }}
@@ -118,44 +141,6 @@ jobs:
118141
find . -type f -path '*/.vitest-reports/blob-*.json' \
119142
-exec bash -c 'src="$1"; basename=$(basename "$src"); pkg=$(dirname "$src" | sed "s|^\./||;s|/\.vitest-reports$||;s|/|_|g"); cp "$src" ".vitest-reports/${pkg}-${basename}"' _ {} \;
120143
121-
- name: Upload blob reports to GitHub Actions Artifacts
144+
- name: 📊 Merge reports
122145
if: ${{ !cancelled() }}
123-
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
124-
with:
125-
name: internal-blob-report-${{ matrix.shardIndex }}
126-
path: .vitest-reports/*
127-
include-hidden-files: true
128-
retention-days: 1
129-
130-
merge-reports:
131-
name: "📊 Merge Reports"
132-
if: ${{ !cancelled() }}
133-
needs: [unitTests]
134-
runs-on: warp-ubuntu-latest-x64-2x
135-
steps:
136-
- name: ⬇️ Checkout repo
137-
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
138-
with:
139-
fetch-depth: 1
140-
persist-credentials: false
141-
142-
- name: ⎔ Setup pnpm
143-
uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5.0.0
144-
with:
145-
version: 10.33.2
146-
147-
- name: ⎔ Setup node
148-
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
149-
with:
150-
node-version: 22.23.1
151-
# no cache enabled, we're not installing deps
152-
153-
- name: Download blob reports from GitHub Actions Artifacts
154-
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
155-
with:
156-
path: .vitest-reports
157-
pattern: internal-blob-report-*
158-
merge-multiple: true
159-
160-
- name: Merge reports
161146
run: pnpm dlx vitest@4.1.7 run --merge-reports --pass-with-no-tests

.github/workflows/unit-tests-webapp.yml

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,18 @@ on:
1414
jobs:
1515
unitTests:
1616
name: "🧪 Unit Tests: Webapp"
17-
runs-on: warp-ubuntu-latest-x64-8x
17+
# 10 shards on 16x machines: webapp test throughput is limited per-machine (one
18+
# docker daemon + disk absorbing all the per-file Postgres/ClickHouse container
19+
# spin-up), so many machines beats few big ones - fewer/bigger (3x32) measured
20+
# SLOWER than 10x8. The 16x (vs 8x) gives the fork pool the CPU headroom the 8x
21+
# runners lacked. Setup overhead per machine is ~1 min on warm runners.
22+
runs-on: warp-ubuntu-latest-x64-16x
1823
strategy:
1924
# one flaky shard shouldn't cancel its siblings - lets us re-run only the failed shard
2025
fail-fast: false
2126
matrix:
22-
shardIndex: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
23-
shardTotal: [10]
27+
shardIndex: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
28+
shardTotal: [12]
2429
env:
2530
DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }}
2631
SHARD_INDEX: ${{ matrix.shardIndex }}
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
// Split from triggerTask.server.test.ts (combined parent + locked-worker read)
2+
// so CI's duration-based sharding can balance the container-heavy tests.
3+
import { describe, expect, vi } from "vitest";
4+
5+
// Mock the db prisma client. The service is constructed against a real
6+
// testcontainer prisma instead — these empty singletons only satisfy the
7+
// module-level imports of the production wiring (infrastructure boundary).
8+
vi.mock("~/db.server", () => ({
9+
prisma: {},
10+
$replica: {},
11+
runOpsNewPrisma: {},
12+
runOpsLegacyPrisma: {},
13+
runOpsNewReplica: {},
14+
runOpsLegacyReplica: {},
15+
}));
16+
// Inherited harness boilerplate. The parent read under test takes the
17+
// findRun(where, client) overload with this.prisma, so it does not consult this
18+
// flag; the mock only satisfies other wiring imported transitively.
19+
vi.mock("~/v3/runOpsMigration/splitMode.server", () => ({ isSplitEnabled: async () => false }));
20+
21+
vi.mock("~/services/platform.v3.server", async (importOriginal) => {
22+
const actual = (await importOriginal()) as Record<string, unknown>;
23+
return {
24+
...actual,
25+
getEntitlement: vi.fn(),
26+
};
27+
});
28+
29+
import { setupAuthenticatedEnvironment, setupBackgroundWorker } from "@internal/run-engine/tests";
30+
import { assertNonNullable, containerTest } from "@internal/testcontainers";
31+
import { trace } from "@opentelemetry/api";
32+
import { IdempotencyKeyConcern } from "~/runEngine/concerns/idempotencyKeys.server";
33+
import { DefaultQueueManager } from "~/runEngine/concerns/queues.server";
34+
import { RunEngineTriggerTaskService } from "./triggerTask.server";
35+
import {
36+
buildEngine,
37+
CapturingParentRunValidator,
38+
MockPayloadProcessor,
39+
MockTraceEventConcern,
40+
} from "./triggerTask.server.test.helpers";
41+
42+
vi.setConfig({ testTimeout: 60_000 }); // 60 seconds timeout
43+
44+
describe("RunEngineTriggerTaskService combined parent + locked-worker reads", () => {
45+
containerTest(
46+
"issues two independent single-table reads when one call supplies both parentRunId and lockToVersion",
47+
async ({ prisma, redisOptions }) => {
48+
const engine = buildEngine(prisma, redisOptions);
49+
50+
try {
51+
const environment = await setupAuthenticatedEnvironment(prisma, "PRODUCTION");
52+
const taskIdentifier = "test-task";
53+
const { worker } = await setupBackgroundWorker(engine, environment, taskIdentifier);
54+
55+
const workerRow = await prisma.backgroundWorker.findUniqueOrThrow({
56+
where: { id: worker.id },
57+
});
58+
59+
// Count BOTH reads issued by the service on the control-plane client:
60+
// the parent read (runStore.findRun → taskRun.findFirst) and the
61+
// locked-worker read (backgroundWorker.findFirst). Capture every
62+
// findFirst arg so we can assert no read carries a cross-seam include.
63+
let taskRunFindFirstCalls = 0;
64+
let backgroundWorkerFindFirstCalls = 0;
65+
const findFirstArgs: any[] = [];
66+
const countingPrisma = new Proxy(prisma, {
67+
get(target, prop, receiver) {
68+
if (prop === "backgroundWorker") {
69+
const delegate = Reflect.get(target, prop, receiver);
70+
return new Proxy(delegate, {
71+
get(bwTarget, bwProp, bwReceiver) {
72+
if (bwProp === "findFirst") {
73+
return async (args: any) => {
74+
backgroundWorkerFindFirstCalls += 1;
75+
findFirstArgs.push(args);
76+
return (delegate as any).findFirst(args);
77+
};
78+
}
79+
const value = Reflect.get(bwTarget, bwProp, bwReceiver);
80+
return typeof value === "function" ? value.bind(bwTarget) : value;
81+
},
82+
});
83+
}
84+
if (prop === "taskRun") {
85+
const delegate = Reflect.get(target, prop, receiver);
86+
return new Proxy(delegate, {
87+
get(trTarget, trProp, trReceiver) {
88+
if (trProp === "findFirst") {
89+
return async (args: any) => {
90+
taskRunFindFirstCalls += 1;
91+
findFirstArgs.push(args);
92+
return (delegate as any).findFirst(args);
93+
};
94+
}
95+
const value = Reflect.get(trTarget, trProp, trReceiver);
96+
return typeof value === "function" ? value.bind(trTarget) : value;
97+
},
98+
});
99+
}
100+
const value = Reflect.get(target, prop, receiver);
101+
return typeof value === "function" ? value.bind(target) : value;
102+
},
103+
}) as typeof prisma;
104+
105+
const triggerTaskService = new RunEngineTriggerTaskService({
106+
engine,
107+
prisma: countingPrisma,
108+
payloadProcessor: new MockPayloadProcessor(),
109+
// queueConcern/idempotency get the real unproxied prisma so the
110+
// counting proxy only observes reads issued by the service itself.
111+
queueConcern: new DefaultQueueManager(prisma, engine),
112+
idempotencyKeyConcern: new IdempotencyKeyConcern(
113+
prisma,
114+
engine,
115+
new MockTraceEventConcern()
116+
),
117+
validator: new CapturingParentRunValidator(),
118+
traceEventConcern: new MockTraceEventConcern(),
119+
tracer: trace.getTracer("test", "0.0.0"),
120+
metadataMaximumSize: 1024 * 1024 * 1,
121+
});
122+
123+
// ROOT parent first (uses the unproxied prisma via a separate service so
124+
// its internal reads don't pollute the child's counts).
125+
const parentService = new RunEngineTriggerTaskService({
126+
engine,
127+
prisma,
128+
payloadProcessor: new MockPayloadProcessor(),
129+
queueConcern: new DefaultQueueManager(prisma, engine),
130+
idempotencyKeyConcern: new IdempotencyKeyConcern(
131+
prisma,
132+
engine,
133+
new MockTraceEventConcern()
134+
),
135+
validator: new CapturingParentRunValidator(),
136+
traceEventConcern: new MockTraceEventConcern(),
137+
tracer: trace.getTracer("test", "0.0.0"),
138+
metadataMaximumSize: 1024 * 1024 * 1,
139+
});
140+
const parentResult = await parentService.call({
141+
taskId: taskIdentifier,
142+
environment,
143+
body: { payload: { kind: "parent" } },
144+
});
145+
assertNonNullable(parentResult);
146+
147+
// CHILD supplying BOTH parentRunId AND lockToVersion in one call.
148+
const childResult = await triggerTaskService.call({
149+
taskId: taskIdentifier,
150+
environment,
151+
body: {
152+
payload: { kind: "child" },
153+
options: {
154+
parentRunId: parentResult.run.friendlyId,
155+
lockToVersion: workerRow.version,
156+
},
157+
},
158+
});
159+
assertNonNullable(childResult);
160+
161+
const parentRow = await prisma.taskRun.findUniqueOrThrow({
162+
where: { id: parentResult.run.id },
163+
});
164+
const childRow = await prisma.taskRun.findUniqueOrThrow({
165+
where: { id: childResult.run.id },
166+
});
167+
168+
// Child resolved the parent (single-table parent read).
169+
expect(childRow.parentTaskRunId).toBe(parentRow.id);
170+
expect(childRow.depth).toBe(parentRow.depth + 1);
171+
172+
// Child locked to the worker (single-table worker read).
173+
expect(childRow.lockedToVersionId).toBe(workerRow.id);
174+
expect(childRow.taskVersion).toBe(workerRow.version);
175+
176+
// Exactly one backgroundWorker.findFirst fired for the locked-worker read,
177+
// and at least one taskRun.findFirst fired for the parent read.
178+
expect(backgroundWorkerFindFirstCalls).toBe(1);
179+
expect(taskRunFindFirstCalls).toBeGreaterThanOrEqual(1);
180+
181+
// NO-JOIN proof: no captured read carried an `include` joining
182+
// taskRun <-> backgroundWorker. Every findFirst arg has include undefined.
183+
for (const args of findFirstArgs) {
184+
expect(args?.include).toBeUndefined();
185+
}
186+
} finally {
187+
await engine.quit();
188+
}
189+
}
190+
);
191+
});

0 commit comments

Comments
 (0)