[e2e] Re-enable concurrent execution of the e2e suite

packages/core/e2e/e2e.test.ts

@@ -16,11 +16,12 @@ import {
   afterAll,
   assert,
   beforeAll,
-  beforeEach,
   describe,
   expect,
-  test,
+  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 {
@@ -32,11 +33,14 @@ import {
   resumeHook,
 } from '../src/runtime';
 import {
+  announceTestStart,
   assertUnsupportedTestsExist,
   cliCancel,
   cliHealthJson,
   cliInspectJson,
   cliInspectJsonUntil,
+  createPerTestState,
+  dumpTrackedRunDiagnostics,
   fetchManifest,
   getCollectedRunIds,
   getWorkflowMetadata,
@@ -46,7 +50,8 @@ import {
   isJsApp,
   isLocalDeployment,
   requireFixture,
-  setupRunTracking,
+  requireSupported,
+  runInTestState,
   setupWorld,
   startTracked,
   trackRun,
@@ -60,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(
@@ -143,6 +153,74 @@ 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 handler wrapper, which owns the
+ * per-test harness plumbing the sequential suites do in a `beforeEach`
+ * (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 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 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<string, unknown>,
+  name: string,
+  optionsOrFn?: unknown,
+  optionsOrTest?: unknown
+) {
+  let options: Record<string, unknown> = {};
+  let handler: (ctx: TestContext) => unknown = () => {};
+  if (typeof optionsOrTest === 'object' && optionsOrTest !== null) {
+    options = optionsOrTest as Record<string, unknown>;
+    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<string, unknown>;
+    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;
 
@@ -320,20 +398,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();

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' });
+  });
+});

packages/core/e2e/utils.ts

@@ -1,3 +1,4 @@
+import { AsyncLocalStorage } from 'node:async_hooks';
 import { spawn } from 'node:child_process';
 import fs from 'node:fs';
 import path, { dirname } from 'node:path';
@@ -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<PerTestState>();
+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<T>(
+  state: PerTestState,
+  fn: () => Promise<T>
+): Promise<T> {
+  return testStateStorage.run(state, fn);
+}
 
 // Global list of run IDs collected for metadata (observability links)
 const globalCollectedRunIds: {
@@ -790,8 +839,9 @@ export function trackRun<T>(
     workflowFn?: string;
   }
 ): Run<T> {
-  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.