-
-
Notifications
You must be signed in to change notification settings - Fork 14
refactor(cli): migrate replay SDK validator from zod to valibot #1388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,20 +2,27 @@ | |
| * Shared npm bundle build helper for e2e tests. | ||
| * | ||
| * Serializes bundle builds across parallel test files so `bundle.test.ts` and | ||
| * `library.test.ts` never run `pnpm run bundle` concurrently or delete `dist/` | ||
| * while another file's build is in flight. | ||
| * `library.test.ts` never run `pnpm run bundle` concurrently. vitest runs each | ||
| * test file in its own worker process (`pool: "forks"`), so an in-process | ||
| * promise cannot coordinate them — the lock has to live on the filesystem. | ||
| * Whichever worker wins the `mkdir` lock builds once; the rest wait for the | ||
| * bundle to appear. | ||
| */ | ||
|
|
||
| import { spawn } from "node:child_process"; | ||
| import { existsSync, rmSync } from "node:fs"; | ||
| import { existsSync, mkdirSync, rmSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import { setTimeout as sleep } from "node:timers/promises"; | ||
|
|
||
| function noop(): void { | ||
| // Intentionally empty — absorbs async spawn errors | ||
| } | ||
|
|
||
| const ROOT_DIR = join(import.meta.dirname, "../.."); | ||
|
|
||
| /** Cross-process build lock directory (kept outside `dist/`). */ | ||
| const LOCK_DIR = join(ROOT_DIR, ".bundle-build.lock"); | ||
|
|
||
| /** Bundled library entrypoint used by library-mode e2e tests. */ | ||
| export const BUNDLE_INDEX_PATH = join(ROOT_DIR, "dist/index.cjs"); | ||
|
|
||
|
|
@@ -30,26 +37,60 @@ let buildPromise: Promise<void> | null = null; | |
| /** | ||
| * Ensure the npm bundle exists under `dist/`, building it once if needed. | ||
| * | ||
| * @param options.clean - When true, delete `dist/` before building. Only the | ||
| * first concurrent caller's preference applies while a build is in flight. | ||
| * Safe to call concurrently from multiple test files: a filesystem lock | ||
| * ensures exactly one worker runs `pnpm run bundle` while the others wait for | ||
| * the bundle to appear. | ||
| */ | ||
| export function ensureBundleBuilt(options?: { | ||
| clean?: boolean; | ||
| }): Promise<void> { | ||
| if (!options?.clean && existsSync(BUNDLE_INDEX_PATH)) { | ||
| export function ensureBundleBuilt(): Promise<void> { | ||
| if (existsSync(BUNDLE_INDEX_PATH) && !existsSync(LOCK_DIR)) { | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| buildPromise ??= runBundleBuild(Boolean(options?.clean)); | ||
| buildPromise ??= runBundleBuild(); | ||
| return buildPromise; | ||
| } | ||
|
|
||
| async function runBundleBuild(clean: boolean): Promise<void> { | ||
| const distDir = join(ROOT_DIR, "dist"); | ||
| if (clean && existsSync(distDir)) { | ||
| rmSync(distDir, { recursive: true, force: true }); | ||
| async function runBundleBuild(): Promise<void> { | ||
| // Atomic `mkdir` acts as a cross-process lock: only one worker creates the | ||
| // directory and builds; the rest fall through to wait for the bundle. | ||
| let holdsLock = false; | ||
| try { | ||
| mkdirSync(LOCK_DIR); | ||
| holdsLock = true; | ||
| } catch { | ||
| // Another worker is building — wait for the bundle to appear. | ||
| } | ||
|
|
||
| if (!holdsLock) { | ||
| buildPromise = null; | ||
| await waitForBundle(); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await spawnBundle(); | ||
| } finally { | ||
| rmSync(LOCK_DIR, { recursive: true, force: true }); | ||
| } | ||
|
|
||
| if (!existsSync(BUNDLE_INDEX_PATH)) { | ||
| buildPromise = null; | ||
| throw new Error("Bundle not built — cannot run library/bundle tests"); | ||
| } | ||
| } | ||
|
|
||
| async function waitForBundle(): Promise<void> { | ||
| const deadline = Date.now() + 55_000; | ||
| while (Date.now() < deadline) { | ||
| if (existsSync(BUNDLE_INDEX_PATH) && !existsSync(LOCK_DIR)) { | ||
| return; | ||
| } | ||
| await sleep(250); | ||
| } | ||
| throw new Error("Bundle not built — cannot run library/bundle tests"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Waiters ignore build failuresMedium Severity
Reviewed by Cursor Bugbot for commit 0582807. Configure here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Waiter timeout under builder budgetMedium Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit 0582807. Configure here. |
||
| } | ||
|
|
||
| async function spawnBundle(): Promise<void> { | ||
| const exitCode = await new Promise<number>((resolve) => { | ||
| let buildStderr = ""; | ||
| const proc = spawn("pnpm", ["run", "bundle"], { | ||
|
|
@@ -72,7 +113,7 @@ async function runBundleBuild(clean: boolean): Promise<void> { | |
| }); | ||
| }); | ||
|
|
||
| if (exitCode !== 0 || !existsSync(BUNDLE_INDEX_PATH)) { | ||
| if (exitCode !== 0) { | ||
| buildPromise = null; | ||
| throw new Error("Bundle not built — cannot run library/bundle tests"); | ||
| } | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale lock wedges bundle setup
Medium Severity
If a worker dies while holding
.bundle-build.lock(vitestbeforeAllkill, Ctrl+C), the directory is never removed. LaterensureBundleBuiltcalls skip the early return because the lock exists, failmkdirSync, andwaitForBundletimes out even when a validdist/index.cjsis already present.Additional Locations (2)
packages/cli/test/e2e/bundle-setup.ts#L43-L46packages/cli/test/e2e/bundle-setup.ts#L81-L90Reviewed by Cursor Bugbot for commit 0582807. Configure here.