From 67132110f8c4089ada871e5d7c3020b7728d8353 Mon Sep 17 00:00:00 2001 From: Alex Langenfeld Date: Tue, 18 Aug 2026 16:20:39 -0500 Subject: [PATCH 1/2] [e2e] Re-enable concurrent execution of the e2e suite Serial execution has been the dominant wall-clock cost per matrix entry since concurrency was disabled before conf (78048e0f4b): ~128 tests at ~22 of 24 minutes on the Vercel lanes, and lately the slowest lane cannot finish under its 30-minute job timeout on a slow runner day at all. #2083 measured the concurrent suite at ~3x job wall-clock (4-5x on the vitest phase) and identified what broke; its blockers are now fixed: world-local writeExclusive is atomic (write-then-link), abort-fetch tests are hermetic (#3618), the fibonacci tree fits the scheduler (#3619), and source-map assertions are positive-only (#3620). What this change adds is concurrency-safe per-test attribution. The harness tracked runs and test names in module globals reset by a beforeEach - under concurrency every test clobbered every other's state, so a failing test dumped an unrelated sibling's diagnostics. vitest's getCurrentTest() cannot substitute: it is a plain module variable, wrong after any await. Instead an auto fixture - the one place that receives the test's own context unambiguously - binds a per-test state (name, tracked runs, the test's own skip) via AsyncLocalStorage around each test body, and trackRun / recordInfraEvent / requireFixture read it ambiently with no call-site changes. The conformance gates skip through the bound state's skip, so a mid-body requireFixture skips the right test. Sequential suites (dev, agent, region) keep setupRunTracking's module-global fallback. Full suite passes 137/137 concurrently against a local dev server in under 2 minutes. A test that genuinely cannot share a deployment can opt out with test.sequential. Builds on VaguelySerious's investigation in #2083. Signed-off-by: Alex Langenfeld --- packages/core/e2e/e2e.test.ts | 61 ++++++++++++--- packages/core/e2e/utils.test.ts | 37 ++++++++- packages/core/e2e/utils.ts | 128 ++++++++++++++++++++++++-------- 3 files changed, 181 insertions(+), 45 deletions(-) diff --git a/packages/core/e2e/e2e.test.ts b/packages/core/e2e/e2e.test.ts index 5dcfcdf3b6..e492969dd8 100644 --- a/packages/core/e2e/e2e.test.ts +++ b/packages/core/e2e/e2e.test.ts @@ -16,10 +16,9 @@ import { afterAll, assert, beforeAll, - beforeEach, describe, expect, - test, + test as vitestTest, } from 'vitest'; import { getTrustedSourcesHeaders } from '../../../scripts/trusted-sources-headers.mjs'; import type { Run } from '../src/runtime'; @@ -46,7 +45,11 @@ import { isJsApp, isLocalDeployment, requireFixture, - setupRunTracking, + announceTestStart, + createPerTestState, + dumpTrackedRunDiagnostics, + requireSupported, + runInTestState, setupWorld, startTracked, trackRun, @@ -143,6 +146,43 @@ const e2e = (fn: string) => { * Every test not marked here is in scope for cross-language conformance, and is * gated only by `e2e-conformance.json`. No-op for the JS workbench apps. */ +/** + * Every test in this suite runs through this auto fixture, which owns the + * per-test harness plumbing the sequential suites do in a `beforeEach` + * (announce heartbeat, conformance gate, failure diagnostics): + * + * - The suite runs concurrently, and vitest's `getCurrentTest()` is a plain + * module variable that is wrong after any `await`, so nothing per-test can + * live in module globals. The fixture is the one place that receives the + * test's own context unambiguously; it binds a per-test state (name, + * tracked runs, the test's own `skip`) via AsyncLocalStorage around the + * test body, and `trackRun`/`recordInfraEvent`/`requireFixture` read it + * ambiently — no call-site changes. + * - Failure diagnostics dump from the state the fixture bound, so a failing + * test reports its own runs, not a concurrent sibling's. + */ +const test = vitestTest.extend<{ e2eTracking: unknown }>({ + e2eTracking: [ + // biome-ignore lint/correctness/noEmptyPattern: vitest fixture signature + async ({ task, skip, onTestFailed }, use) => { + const state = createPerTestState(task.name, skip); + announceTestStart(task.name); + onTestFailed( + (result) => + dumpTrackedRunDiagnostics(state, result.errors?.[0]?.message), + 30_000 // Allow 30s for diagnostics fetching (default hookTimeout is 10s) + ); + await runInTestState(state, async () => { + // Second conformance gate — inside the bound state so the skip + // targets this test. + requireSupported(task.name); + await use(state); + }); + }, + { auto: true }, + ], +}); + const testJsOnly = isJsApp() ? test : test.skip; const describeJsOnly = isJsApp() ? describe : describe.skip; @@ -320,20 +360,19 @@ async function startWorkflowViaHttp( return run; } -// NOTE: Temporarily disabling concurrent tests to avoid flakiness. -// TODO: Re-enable concurrent tests after conf when we have more time to investigate. -describe('e2e', () => { +// Concurrent: ~128 serial tests were the dominant wall-clock cost per matrix +// entry (~22 of 24 minutes on the Vercel lanes). The known blockers are +// fixed: per-test attribution is concurrency-safe (see the e2eTracking +// fixture), abort-fetch tests are hermetic, the fibonacci tree fits the +// scheduler, and source-map assertions are positive-only. A test that +// genuinely cannot share a deployment can opt out with `test.sequential`. +describe.concurrent('e2e', () => { // Configure the World for the test runner process so that start() and // run.returnValue can communicate with the same backend as the workbench app. beforeAll(async () => { setupWorld(deploymentUrl); }); - // Enable automatic run diagnostics on test failure - beforeEach((ctx) => { - setupRunTracking(ctx.task.name); - }); - // Write E2E metadata and diagnostics files afterAll(() => { writeE2EMetadata(); diff --git a/packages/core/e2e/utils.test.ts b/packages/core/e2e/utils.test.ts index 0e56e5d31c..f6848f4a7a 100644 --- a/packages/core/e2e/utils.test.ts +++ b/packages/core/e2e/utils.test.ts @@ -1,5 +1,12 @@ import { afterEach, describe, expect, test } from 'vitest'; -import { hasStepSourceMaps, waitForRunPickup } from './utils'; +import { + createPerTestState, + getCollectedRunIds, + hasStepSourceMaps, + runInTestState, + trackRun, + waitForRunPickup, +} from './utils'; const ORIGINAL_ENV = { ...process.env }; @@ -130,3 +137,31 @@ describe('waitForRunPickup', () => { await expect(waitForRunPickup(run as any, 5_000)).resolves.toBe(true); }); }); + +describe('per-test state isolation', () => { + test('interleaved contexts attribute runs to their own test', async () => { + const before = getCollectedRunIds().length; + const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); + const fakeRun = (id: string) => ({ runId: id }) as never; + + // Two "tests" interleaving on the event loop, as under + // describe.concurrent: each tracks a run after yielding, so a + // module-global current-test-name would attribute both to whichever + // context touched it last. + await Promise.all([ + runInTestState(createPerTestState('test-a'), async () => { + await sleep(20); + trackRun(fakeRun('wrun_a')); + }), + runInTestState(createPerTestState('test-b'), async () => { + await sleep(10); + trackRun(fakeRun('wrun_b')); + }), + ]); + + const entries = getCollectedRunIds().slice(before); + expect( + Object.fromEntries(entries.map((e) => [e.runId, e.testName])) + ).toEqual({ wrun_a: 'test-a', wrun_b: 'test-b' }); + }); +}); diff --git a/packages/core/e2e/utils.ts b/packages/core/e2e/utils.ts index d4bf552b40..c61400aecb 100644 --- a/packages/core/e2e/utils.ts +++ b/packages/core/e2e/utils.ts @@ -3,6 +3,7 @@ import fs from 'node:fs'; import path, { dirname } from 'node:path'; import { setTimeout as sleep } from 'node:timers/promises'; import { fileURLToPath } from 'node:url'; +import { AsyncLocalStorage } from 'node:async_hooks'; import { getCurrentTest } from '@vitest/runner'; import { createWorkflowUrl } from '@workflow/utils'; import { createWorld as createVercelTestWorld } from '@workflow/world-vercel'; @@ -268,7 +269,7 @@ export function hasFixture(fixtureName: string): boolean { */ export function requireFixture(fixtureName: string): void { if (hasFixture(fixtureName)) return; - getCurrentTest()?.context.skip( + currentSkip()?.( `"${fixtureName}" is not listed in ${CONFORMANCE_CONFIG_FILENAME}` ); } @@ -290,7 +291,7 @@ export function requireSupported(testName: string): void { seenTestNames.add(testName); const reason = getConformanceConfig()?.unsupported?.[testName]; if (!reason) return; - getCurrentTest()?.context.skip( + currentSkip()?.( `${CONFORMANCE_CONFIG_FILENAME} declares this unsupported: ${reason}` ); } @@ -758,8 +759,56 @@ interface TrackedRun { workflowFn?: string; } -// Per-test tracked runs — reset between tests via setupRunTracking() -let trackedRuns: TrackedRun[] = []; +/** + * Per-test harness state: the name used to attribute runs and infra events, + * and the runs whose diagnostics dump if the test fails. + * + * Concurrent suites bind one of these per test via {@link runInTestState} + * (AsyncLocalStorage), so tests interleaving on the event loop cannot + * clobber each other's attribution — `getCurrentTest()` is a plain module + * variable in vitest and is wrong after any `await` under concurrency. + * Sequential suites (dev.test.ts, e2e-agent.test.ts, e2e-region.test.ts) + * keep the classic path: {@link setupRunTracking} resets a module-level + * fallback that is safe when only one test runs at a time. + */ +interface PerTestState { + testName: string; + trackedRuns: TrackedRun[]; + /** + * The test's own `ctx.skip`, captured where the context is unambiguous + * (the fixture), so conformance gates called mid-test-body can skip the + * right test — `getCurrentTest()?.context.skip` would target whichever + * test most recently started. + */ + skip?: (note?: string) => void; +} + +const testStateStorage = new AsyncLocalStorage(); +let fallbackTestState: PerTestState = { + testName: 'unknown', + trackedRuns: [], +}; +const currentTestState = (): PerTestState => + testStateStorage.getStore() ?? fallbackTestState; + +export function createPerTestState( + testName: string, + skip?: (note?: string) => void +): PerTestState { + return { testName, trackedRuns: [], skip }; +} + +/** ALS-bound skip when available, vitest's global otherwise. */ +const currentSkip = (): ((note?: string) => void) | undefined => + testStateStorage.getStore()?.skip ?? getCurrentTest()?.context.skip; + +/** Run `fn` with `state` bound as the ambient per-test state. */ +export function runInTestState( + state: PerTestState, + fn: () => Promise +): Promise { + return testStateStorage.run(state, fn); +} // Global list of run IDs collected for metadata (observability links) const globalCollectedRunIds: { @@ -790,8 +839,9 @@ export function trackRun( workflowFn?: string; } ): Run { - const testName = options?.testName ?? currentTestName; - trackedRuns.push({ + const state = currentTestState(); + const testName = options?.testName ?? state.testName; + state.trackedRuns.push({ run, workflowFile: options?.workflowFile, workflowFn: options?.workflowFn, @@ -832,7 +882,7 @@ export function recordInfraEvent( ) { infraEvents.push({ ...event, - testName: event.testName ?? currentTestName, + testName: event.testName ?? currentTestState().testName, timestamp: new Date().toISOString(), }); } @@ -1103,43 +1153,55 @@ function emitGitHubAnnotation( * beforeEach((ctx) => { setupRunTracking(ctx.task.name); }); */ export function setupRunTracking(testName: string) { - currentTestName = testName; - trackedRuns = []; + fallbackTestState = createPerTestState(testName); // Second conformance gate. Sited here because every test in the suite calls // setupRunTracking from `beforeEach`, which makes this the one place that // sees a test's name without the test having to declare anything. requireSupported(testName); - // Heartbeat: announce the test the moment it starts, written straight to - // stdout to bypass vitest's per-file console buffering. Without this, a - // test that stalls (e.g. polling a run that never progresses) produces no - // output until its timeout, making CI look like a silent hang — the - // reporter only prints a test's result line once it completes. Emitting the - // name on start makes the stalling test immediately identifiable. - process.stdout.write(`\n[e2e] ▶ start: ${testName}\n`); + announceTestStart(testName); + const state = fallbackTestState; onTestFailed( - async (result) => { - const errorMessage = result.errors?.[0]?.message || 'Test failed'; - - for (const tracked of trackedRuns) { - try { - const diagnostics = await getRunDiagnostics(tracked); - console.error(diagnostics); - emitGitHubAnnotation(testName, tracked, errorMessage); - } catch { - console.error( - `[diagnostics] Failed to fetch diagnostics for run ${tracked.run.runId}` - ); - } - } - }, + (result) => dumpTrackedRunDiagnostics(state, result.errors?.[0]?.message), 30_000 // Allow 30s for diagnostics fetching (default hookTimeout is 10s) ); } -// Current test name for auto-tracking -let currentTestName = 'unknown'; +/** + * Heartbeat: announce the test the moment it starts, written straight to + * stdout to bypass vitest's per-file console buffering. Without this, a + * test that stalls (e.g. polling a run that never progresses) produces no + * output until its timeout, making CI look like a silent hang — the + * reporter only prints a test's result line once it completes. Emitting the + * name on start makes the stalling test immediately identifiable. + */ +export function announceTestStart(testName: string) { + process.stdout.write(`\n[e2e] ▶ start: ${testName}\n`); +} + +/** + * Dump diagnostics for every run tracked by `state`. Shared by the + * sequential path (setupRunTracking's onTestFailed) and the concurrent + * fixture, which passes the state it bound for its own test — the one + * thing vitest's globals cannot provide under concurrency. + */ +export async function dumpTrackedRunDiagnostics( + state: PerTestState, + errorMessage = 'Test failed' +) { + for (const tracked of state.trackedRuns) { + try { + const diagnostics = await getRunDiagnostics(tracked); + console.error(diagnostics); + emitGitHubAnnotation(state.testName, tracked, errorMessage); + } catch { + console.error( + `[diagnostics] Failed to fetch diagnostics for run ${tracked.run.runId}` + ); + } + } +} /** * Write diagnostics sidecar file with per-test run info for the aggregation script. From 055695365eb9b5edae5572c8445cde3adef45a3e Mon Sep 17 00:00:00 2001 From: Alex Langenfeld Date: Tue, 18 Aug 2026 17:12:37 -0500 Subject: [PATCH 2/2] [e2e] Bind per-test state via a task collector, not a fixture CI on the first concurrent run caught the fixture design failing in the one place skip-correctness is load-bearing: vitest resolves fixtures in a separate async context, so an AsyncLocalStorage store bound around use() never reaches the test body. Every ambient read silently fell to the module fallback - conformance-gated tests on the Python lane ran instead of skipping and hard-failed on fixtures missing from the manifest ('declared in e2e-conformance.json but not in the deployed manifest'). The suite's test is now built with createTaskCollector, mirroring vitest's own argument normalization, and the wrapper binds the state around the handler call itself - a direct call stack, so propagation is guaranteed rather than assumed. Validated by a collector-pattern ALS experiment (per-test store after awaits, mid-body skip targets self, each/skip/todo chains), a conformance-file simulation reproducing the Python failure shape (fixture-missing tests now skip with the right note), and a 137/137 concurrent local run. Also raises RACE_WINNER_MAX_DURATION_MS 5s -> 8s: the winner takes 1s, the loser 10s, and the bound only has to sit clearly below the loser - concurrent-suite queue latency pushed observed winner durations to ~6.5s on loaded local-dev lanes (vite, first concurrent CI run), so 5s flaked without catching anything 8s misses. Signed-off-by: Alex Langenfeld --- packages/core/e2e/e2e.test.ts | 112 +++++++++++++++++++++++----------- packages/core/e2e/utils.ts | 2 +- 2 files changed, 76 insertions(+), 38 deletions(-) diff --git a/packages/core/e2e/e2e.test.ts b/packages/core/e2e/e2e.test.ts index e492969dd8..590a26f4a1 100644 --- a/packages/core/e2e/e2e.test.ts +++ b/packages/core/e2e/e2e.test.ts @@ -18,8 +18,10 @@ import { beforeAll, describe, expect, - test as vitestTest, + type TestContext, + type test as vitestTest, } from 'vitest'; +import { createTaskCollector, getCurrentSuite } from 'vitest/suite'; import { getTrustedSourcesHeaders } from '../../../scripts/trusted-sources-headers.mjs'; import type { Run } from '../src/runtime'; import { @@ -31,11 +33,14 @@ import { resumeHook, } from '../src/runtime'; import { + announceTestStart, assertUnsupportedTestsExist, cliCancel, cliHealthJson, cliInspectJson, cliInspectJsonUntil, + createPerTestState, + dumpTrackedRunDiagnostics, fetchManifest, getCollectedRunIds, getWorkflowMetadata, @@ -45,9 +50,6 @@ import { isJsApp, isLocalDeployment, requireFixture, - announceTestStart, - createPerTestState, - dumpTrackedRunDiagnostics, requireSupported, runInTestState, setupWorld, @@ -63,7 +65,12 @@ if (!deploymentUrl) { } const DISTRIBUTED_CLOCK_TOLERANCE_MS = 1_000; -const RACE_WINNER_MAX_DURATION_MS = 5_000; +// The race winner takes 1s; the loser would take 10s. The bound only has to +// sit clearly below the loser to catch badly delayed or sequential +// completion — under the concurrent suite, queue latency pushed the winner's +// observed duration to ~6.5s on loaded local-dev lanes, so 5s was tight +// enough to flake without being any better at catching the regression. +const RACE_WINNER_MAX_DURATION_MS = 8_000; const EVENT_POLL_PAGE_SIZE = 100; function expectElapsedAtLeast( @@ -147,41 +154,72 @@ const e2e = (fn: string) => { * gated only by `e2e-conformance.json`. No-op for the JS workbench apps. */ /** - * Every test in this suite runs through this auto fixture, which owns the + * Every test in this suite runs through this handler wrapper, which owns the * per-test harness plumbing the sequential suites do in a `beforeEach` - * (announce heartbeat, conformance gate, failure diagnostics): + * (announce heartbeat, conformance gates, failure diagnostics). * - * - The suite runs concurrently, and vitest's `getCurrentTest()` is a plain - * module variable that is wrong after any `await`, so nothing per-test can - * live in module globals. The fixture is the one place that receives the - * test's own context unambiguously; it binds a per-test state (name, - * tracked runs, the test's own `skip`) via AsyncLocalStorage around the - * test body, and `trackRun`/`recordInfraEvent`/`requireFixture` read it - * ambiently — no call-site changes. - * - Failure diagnostics dump from the state the fixture bound, so a failing - * test reports its own runs, not a concurrent sibling's. + * The suite runs concurrently, and vitest's `getCurrentTest()` is a plain + * module variable that is wrong after any `await`, so nothing per-test can + * live in module globals. The wrapper binds a per-test state (name, tracked + * runs, the test's own `skip`) via AsyncLocalStorage *around the handler + * call itself* — a direct call stack, so the store provably reaches the test + * body — and `trackRun`/`recordInfraEvent`/`requireFixture` read it + * ambiently with no call-site changes. (A `test.extend` auto fixture cannot + * do this: vitest resolves fixtures in a separate async context, so a store + * bound around `use()` never reaches the test body.) Failure diagnostics + * dump from the bound state, so a failing test reports its own runs, not a + * concurrent sibling's. */ -const test = vitestTest.extend<{ e2eTracking: unknown }>({ - e2eTracking: [ - // biome-ignore lint/correctness/noEmptyPattern: vitest fixture signature - async ({ task, skip, onTestFailed }, use) => { - const state = createPerTestState(task.name, skip); - announceTestStart(task.name); - onTestFailed( - (result) => - dumpTrackedRunDiagnostics(state, result.errors?.[0]?.message), - 30_000 // Allow 30s for diagnostics fetching (default hookTimeout is 10s) - ); - await runInTestState(state, async () => { - // Second conformance gate — inside the bound state so the skip - // targets this test. - requireSupported(task.name); - await use(state); - }); - }, - { auto: true }, - ], -}); +const wrapE2EHandler = + (handler: (ctx: TestContext) => unknown) => (ctx: TestContext) => { + const state = createPerTestState(ctx.task.name, ctx.skip); + announceTestStart(ctx.task.name); + ctx.onTestFailed( + (result) => dumpTrackedRunDiagnostics(state, result.errors?.[0]?.message), + 30_000 // Allow 30s for diagnostics fetching (default hookTimeout is 10s) + ); + return runInTestState(state, async () => { + // Second conformance gate — inside the bound state so the skip + // targets this test. + requireSupported(ctx.task.name); + return handler(ctx); + }); + }; + +/** + * Drop-in `test` whose collector mirrors vitest's own argument handling + * (options-second, legacy options/timeout-third, `.each`'s generated + * callbacks) and wraps every handler with {@link wrapE2EHandler}. Built on + * `createTaskCollector`, so the whole chainable surface (`.skip`, `.only`, + * `.each`, `.runIf`, `.sequential`, …) keeps working. + */ +const test = createTaskCollector(function ( + this: Record, + name: string, + optionsOrFn?: unknown, + optionsOrTest?: unknown +) { + let options: Record = {}; + let handler: (ctx: TestContext) => unknown = () => {}; + if (typeof optionsOrTest === 'object' && optionsOrTest !== null) { + options = optionsOrTest as Record; + handler = optionsOrFn as typeof handler; + } else if (typeof optionsOrTest === 'number') { + options = { timeout: optionsOrTest }; + handler = optionsOrFn as typeof handler; + } else if (typeof optionsOrFn === 'object' && optionsOrFn !== null) { + options = optionsOrFn as Record; + handler = optionsOrTest as typeof handler; + } else if (typeof optionsOrFn === 'function') { + handler = optionsOrFn as typeof handler; + } + + getCurrentSuite().task(name, { + ...this, + ...options, + handler: wrapE2EHandler(handler), + }); +}) as typeof vitestTest; const testJsOnly = isJsApp() ? test : test.skip; const describeJsOnly = isJsApp() ? describe : describe.skip; diff --git a/packages/core/e2e/utils.ts b/packages/core/e2e/utils.ts index c61400aecb..587f92918e 100644 --- a/packages/core/e2e/utils.ts +++ b/packages/core/e2e/utils.ts @@ -1,9 +1,9 @@ +import { AsyncLocalStorage } from 'node:async_hooks'; import { spawn } from 'node:child_process'; import fs from 'node:fs'; import path, { dirname } from 'node:path'; import { setTimeout as sleep } from 'node:timers/promises'; import { fileURLToPath } from 'node:url'; -import { AsyncLocalStorage } from 'node:async_hooks'; import { getCurrentTest } from '@vitest/runner'; import { createWorkflowUrl } from '@workflow/utils'; import { createWorld as createVercelTestWorld } from '@workflow/world-vercel';