[world-testing] Fix Windows Unit Tests flake on event-limit test#3055
[world-testing] Fix Windows Unit Tests flake on event-limit test#3055VaguelySerious wants to merge 1 commit into
Conversation
Every startServer() call ran the Local World against the shared, untagged .workflow-data directory. Since recoverActiveRuns defaults to true, a server starting up while another test file's run was still in-flight would list that foreign run during its startup recovery sweep and re-enqueue it into its own queue, dispatching it against the same on-disk event log and racing the run's actual owner. Under enough concurrency (the full world-testing suite, as CI runs it) this landed as an intermittent ReplayDivergenceError, observed on the Windows Unit Tests job for the new event-limit test. Give each spawned server its own WORKFLOW_LOCAL_DATA_DIR so no state is shared across concurrently-running test files.
🦋 Changeset detectedLatest commit: ae73058 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
📊 Workflow Benchmarkscommit Backend:
Best/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · STSO (1-20) 20/30/60 · STSO (101-120) 30/45/90 · STSO (1001-1020) 40/60/120 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
karthikscale3
left a comment
karthikscale3
left a comment
There was a problem hiding this comment.
One cleanup/lifecycle issue found.
| }); | ||
| onTestFinished(() => { | ||
| proc.kill(); | ||
| void rm(dataDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
proc.kill() only requests termination; it does not wait for the child to exit. On Windows the Local World process may still have files in dataDir open when this rm() runs, so it can reject with EPERM/EBUSY. Because the promise is discarded, that also becomes an unhandled rejection (and the temp directory may leak), potentially replacing the original flake with a teardown flake. Could this hook be made async, await the child's exit/close after killing it, and then await rm() (ideally with maxRetries/retryDelay for Windows)?
2 participants
Summary
Windows Unit Tests failed on main's changeset-release PR (#3028) at the new
event-limit.test.tstest (added in #2986):CORRUPTED_EVENT_LOGmeans the run hitReplayDivergenceError3+ times (REPLAY_DIVERGENCE_MAX_RETRIES) before the max-events guard could cleanly fail it.Root cause
Every
startServer()call inworld-testingspawns a fresh Local World server process against the shared, untagged.workflow-datadirectory (notag, no isolateddataDir). The Local World'sstart()defaultsrecoverActiveRunstotrueand, when untagged, its recovery sweep lists every untagged run in that shared directory — not just the ones the current process created (see the comment inpackages/world-local/src/index.tsaround theisUntaggedfilter).So a server spawned by one test file, starting up while another test file's run is still mid-flight, re-enqueues that foreign run into its own in-process queue and dispatches it against the same on-disk event log — racing the run's actual owner. That produces spurious
ReplayDivergenceErrors under enough concurrency (the full suite, as CI runs it), which is why it reproduced on the Windows job's own head-to-head run but not on a solo re-run of the file.Confirmed locally: running the full
world-testingsuite concurrently (6x in parallel, simulating CI-level file-level concurrency) reliably producedidempotencytimeouts andinline-batches-debugfailures before the fix, and passed cleanly 6/6 after.Fix
Give each spawned server its own
WORKFLOW_LOCAL_DATA_DIR(a unique temp dir), removing the shared state entirely. Cleaned up inonTestFinished.Testing
pnpm build && npx vitest runinpackages/world-testing: 3/3 files, 15/15 tests pass.