From 1c15f81f81fad228df8fc5481ea34bb955d46ace Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Mon, 10 Aug 2026 17:56:06 +0200 Subject: [PATCH] Keep cross-runtime gates honest at runtime boundaries Code review exposed empty Bun selection, conservative lock ownership, symlink-race/cause handling, parser edge cases, poisoned iterators, and Bun's unenforced worker resourceLimits. The changes keep gates fail-closed while retaining Node/Deno hosted-worker coverage. Constraint: Bun 1.3.6 does not enforce node:worker_threads resourceLimits. Constraint: Missing or malformed workspace lock ownership is indeterminate and must be preserved. Rejected: Force-exit Node tests | hides leaked handles. Rejected: Reclaim every markerless Bun lock | races live lock initialization. Confidence: high Scope-risk: moderate Directive: Keep Node and Bun gates dependent on build:npm and fail closed when no tests are selected. Tested: deno task test:node (full pass, natural exit 0; dot reporter did not print an exact count). Tested: deno task test:bun (1297 files, 0 failed). Tested: deno task test:unit (3786 files, 28105 steps, 0 failed, 1 ignored). Tested: deno task test:integration --no-lock (283 files, 2726 steps, 0 failed after a single transient external esm.sh AbortError was confirmed by focused rerun). Tested: Linux Deno 2.7.7 deno task lint:ci. Tested: deno task typecheck. Tested: deno task audit. Tested: deno fmt --check. Tested: Focused Node, Bun, and Deno tests plus mutation checks for every new regression. --- extensions/ext-yaml/src/adapter.ts | 4 +- .../declarative-evaluator-worker-protocol.ts | 4 + ...eclarative-evaluator-worker-runner.test.ts | 30 ++- .../declarative-evaluator-worker-runner.ts | 22 +- src/config/declarative-evaluator.ts | 1 + src/extensions/first-party-import.test.ts | 38 ++++ src/modules/server/classify.test.ts | 3 +- .../adapters/runtime/node/http-server.test.ts | 86 ++++++++ src/proxy/retry.test.ts | 29 +++ src/server/shared/renderer/adapter.test.ts | 116 ++++++----- src/transforms/esm/http-cache-helpers.test.ts | 58 +++--- tests/bun/preload.ts | 10 +- tests/bun/run-tests.mjs | 43 +++- tests/bun/runner-args.test.mjs | 28 ++- tests/bun/workspace-packages.mjs | 139 +++++++------ tests/bun/workspace-packages.test.mjs | 188 +++++++++++++----- tests/ensure-npm-links.mjs | 25 ++- tests/ensure-npm-links.test.mjs | 85 ++++++-- 18 files changed, 675 insertions(+), 234 deletions(-) diff --git a/extensions/ext-yaml/src/adapter.ts b/extensions/ext-yaml/src/adapter.ts index d1f3b181a6..5873784a64 100644 --- a/extensions/ext-yaml/src/adapter.ts +++ b/extensions/ext-yaml/src/adapter.ts @@ -10,8 +10,8 @@ import { /** * The tags a JSON-representable document may carry explicitly. `yaml`'s * `Schema.knownTags` fallback resolves YAML 1.1 tags such as `!!binary`, - * `!!timestamp`, `!!set` and `!!omap` even under the 1.2 core schema, and does - * without raising a warning, so the parser options alone cannot express + * `!!timestamp`, `!!set` and `!!omap` even under the 1.2 core schema without + * raising a warning, so the parser options alone cannot express * `@std/yaml`'s JSON schema. Rejecting every other explicit tag does. */ const JSON_SCHEMA_TAGS: ReadonlySet = new Set([ diff --git a/src/config/declarative-evaluator-worker-protocol.ts b/src/config/declarative-evaluator-worker-protocol.ts index 8f4d22a9d1..276caddfd1 100644 --- a/src/config/declarative-evaluator-worker-protocol.ts +++ b/src/config/declarative-evaluator-worker-protocol.ts @@ -75,6 +75,7 @@ export type DeclarativeConfigWorkerInfrastructureReason = | "worker-aborted" | "worker-overloaded" | "worker-protocol" + | "worker-memory-limit-unavailable" | "worker-timeout" | "worker-unavailable"; @@ -175,6 +176,7 @@ const ERROR_REASON_TABLE = ObjectFreeze( "worker-aborted": true, "worker-overloaded": true, "worker-protocol": true, + "worker-memory-limit-unavailable": true, "worker-timeout": true, "worker-unavailable": true, } as const satisfies Readonly>, @@ -347,6 +349,7 @@ function isWorkerReason( return value === "worker-aborted" || value === "worker-overloaded" || value === "worker-protocol" || + value === "worker-memory-limit-unavailable" || value === "worker-timeout" || value === "worker-unavailable"; } @@ -378,6 +381,7 @@ function isLegalErrorTuple( if (phase !== "worker" || !isWorkerReason(reason)) return false; return retryable === ( reason === "worker-overloaded" || + reason === "worker-memory-limit-unavailable" || reason === "worker-timeout" || reason === "worker-unavailable" ); diff --git a/src/config/declarative-evaluator-worker-runner.test.ts b/src/config/declarative-evaluator-worker-runner.test.ts index 91e2ec409d..1e083c1ce4 100644 --- a/src/config/declarative-evaluator-worker-runner.test.ts +++ b/src/config/declarative-evaluator-worker-runner.test.ts @@ -1,5 +1,6 @@ -import { assertEquals } from "#veryfront/testing/assert.ts"; +import { assertEquals, assertRejects } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; +import { isBun } from "#veryfront/platform/compat/runtime.ts"; import { createPreparedDeclarativeConfigWorkerPayload, prepareDeclarativeConfigContext, @@ -7,7 +8,34 @@ import { import { evaluatePreparedDeclarativeConfigInWorker } from "./declarative-evaluator-worker-runner.ts"; describe("declarative config runtime worker", () => { + it("rejects Bun when bounded worker memory limits are unavailable", async () => { + if (!isBun) return; + const context = await prepareDeclarativeConfigContext({ + environmentName: "preview", + environment: {}, + }); + const payload = createPreparedDeclarativeConfigWorkerPayload( + `export default { title: "unreachable" };`, + context, + "veryfront.config.ts", + ); + + const error = await assertRejects(() => evaluatePreparedDeclarativeConfigInWorker(payload)); + + assertEquals( + error instanceof Error && "reason" in error + ? (error as { reason?: unknown }).reason + : undefined, + "worker-memory-limit-unavailable", + ); + assertEquals( + error instanceof Error ? error.message : undefined, + "Hosted configuration rejected (evaluator-unavailable: worker-memory-limit-unavailable)", + ); + }); + it("evaluates a hosted TypeScript config", async () => { + if (isBun) return; const context = await prepareDeclarativeConfigContext({ environmentName: "preview", environment: { TENANT: "tenant-value" }, diff --git a/src/config/declarative-evaluator-worker-runner.ts b/src/config/declarative-evaluator-worker-runner.ts index a7d31e5198..091cd918d9 100644 --- a/src/config/declarative-evaluator-worker-runner.ts +++ b/src/config/declarative-evaluator-worker-runner.ts @@ -8,7 +8,10 @@ */ import { isBun, isDeno, isNode } from "#veryfront/platform/compat/runtime.ts"; -import type { PreparedDeclarativeConfigWorkerPayload } from "./declarative-evaluator.ts"; +import { + DeclarativeConfigEvaluationError, + type PreparedDeclarativeConfigWorkerPayload, +} from "./declarative-evaluator.ts"; import type { ConfigSnapshotRecord } from "./snapshot.ts"; import { createDeclarativeConfigWorkerInfrastructureError, @@ -529,7 +532,11 @@ async function createRuntimeWorkerEndpoint(): Promise< DeclarativeConfigWorkerEndpoint > { if (isDeno) return createDenoWorkerEndpoint(); - if (isBun) return await createNodeWorkerEndpoint(); + if (isBun) { + throw createDeclarativeConfigWorkerInfrastructureError( + "worker-memory-limit-unavailable", + ); + } if (isNode) return await createNodeWorkerEndpoint(); throw createDeclarativeConfigWorkerInfrastructureError("worker-unavailable"); } @@ -664,9 +671,16 @@ function beginEvaluationWithEndpointFactory( let createdEndpoint: DeclarativeConfigWorkerEndpoint; try { createdEndpoint = await endpointFactory(); - } catch { + } catch (error) { drainStartupLifecycle(); - rejectInfrastructure("worker-unavailable"); + if ( + error instanceof DeclarativeConfigEvaluationError && + error.phase === "worker" + ) { + settle({ kind: "reject", error }); + } else { + rejectInfrastructure("worker-unavailable"); + } drainLifecycleIfComplete(); return; } diff --git a/src/config/declarative-evaluator.ts b/src/config/declarative-evaluator.ts index afc42d7b2f..0fc9d261b3 100644 --- a/src/config/declarative-evaluator.ts +++ b/src/config/declarative-evaluator.ts @@ -251,6 +251,7 @@ export type DeclarativeConfigErrorReason = | "worker-aborted" | "worker-overloaded" | "worker-protocol" + | "worker-memory-limit-unavailable" | "worker-timeout" | "worker-unavailable"; diff --git a/src/extensions/first-party-import.test.ts b/src/extensions/first-party-import.test.ts index 0caf362ed7..75f3eaa55e 100644 --- a/src/extensions/first-party-import.test.ts +++ b/src/extensions/first-party-import.test.ts @@ -364,6 +364,44 @@ describe("first-party extension imports", () => { assertEquals(isMissingFirstPartyExtensionModule(unrelated), false); }); + it("parses Bun missing-module reports from strings and object-shaped errors", () => { + const relative = { + message: + "ResolveMessage: Cannot find module './parser-only' from '/app/extensions/ext-parser-babel/src/index.ts'", + }; + assertEquals( + isMissingFirstPartyExtensionModule(relative, [ + "extensions/ext-parser-babel/src/parser-only", + ]), + true, + ); + assertEquals( + isMissingFirstPartyExtensionModule(relative, [ + "extensions/ext-parser-babel/src/other", + ]), + false, + ); + + const packageSpecifier = { + message: + "Cannot find module '@veryfront/ext-parser-babel/parser-only' from '/app/loader.js'", + }; + assertEquals( + isMissingFirstPartyExtensionModule(packageSpecifier, [ + "@veryfront/ext-parser-babel/parser-only", + ]), + true, + ); + + assertEquals( + isMissingFirstPartyExtensionModule( + "Cannot find module '@veryfront/ext-parser-babel' from '/app/loader.js'", + ["@veryfront/ext-parser-babel"], + ), + true, + ); + }); + it("requires a full recognized message when no stable code is present", () => { assertEquals( isMissingFirstPartyExtensionModule( diff --git a/src/modules/server/classify.test.ts b/src/modules/server/classify.test.ts index 9cb42fa547..f9d9333961 100644 --- a/src/modules/server/classify.test.ts +++ b/src/modules/server/classify.test.ts @@ -99,7 +99,7 @@ describe("classifyModuleRequest", () => { } }); - it("normalizes lowercase and uppercase encoded caret operators", () => { + it("normalizes encoded caret version operators before the source marker", () => { for (const encodedCaret of ["%5e", "%5E"]) { const result = classifyModuleRequest( url(`/_vf_modules/_cross/demo@${encodedCaret}1.0.0/@/lib/utils.js`), @@ -107,6 +107,7 @@ describe("classifyModuleRequest", () => { assertEquals(result.kind, "cross-project-versioned"); if (result.kind === "cross-project-versioned") { assertEquals(result.version, "^1.0.0"); + assertEquals(result.path, "lib/utils.js"); } } }); diff --git a/src/platform/adapters/runtime/node/http-server.test.ts b/src/platform/adapters/runtime/node/http-server.test.ts index a2de7638cf..991b48ae86 100644 --- a/src/platform/adapters/runtime/node/http-server.test.ts +++ b/src/platform/adapters/runtime/node/http-server.test.ts @@ -62,6 +62,92 @@ function createDeferred(): { } describe("NodeServer lifecycle", () => { + it("accepts the lowest and highest valid listener ports", async () => { + if (!isNode) return; + const listenDescriptor = Object.getOwnPropertyDescriptor( + NativeHttpServer.prototype, + "listen", + ); + const addressDescriptor = Object.getOwnPropertyDescriptor( + NativeHttpServer.prototype, + "address", + ); + const closeDescriptor = Object.getOwnPropertyDescriptor( + NativeHttpServer.prototype, + "close", + ); + const originalListen = NativeHttpServer.prototype.listen; + const originalAddress = NativeHttpServer.prototype.address; + const originalClose = NativeHttpServer.prototype.close; + const listenedPorts: number[] = []; + let currentPort = 0; + + NativeHttpServer.prototype.listen = function ( + this: NativeHttpServer, + port?: number, + ): NativeHttpServer { + currentPort = port ?? 0; + listenedPorts.push(currentPort); + queueMicrotask(() => this.emit("listening")); + return this; + } as typeof originalListen; + NativeHttpServer.prototype.address = function () { + return { address: "127.0.0.1", family: "IPv4", port: currentPort }; + } as typeof originalAddress; + NativeHttpServer.prototype.close = function ( + this: NativeHttpServer, + callback?: (error?: Error) => void, + ): NativeHttpServer { + queueMicrotask(() => { + this.emit("close"); + callback?.(); + }); + return this; + } as typeof originalClose; + + try { + for (const port of [0, 65_535]) { + const server = await createNodeServer(() => new Response("ok"), { + hostname: "127.0.0.1", + port, + }); + assertEquals(server.addr.port, port); + await server.stop(); + } + } finally { + if (listenDescriptor) { + Object.defineProperty(NativeHttpServer.prototype, "listen", listenDescriptor); + } else { + Reflect.deleteProperty(NativeHttpServer.prototype, "listen"); + } + if (addressDescriptor) { + Object.defineProperty(NativeHttpServer.prototype, "address", addressDescriptor); + } else { + Reflect.deleteProperty(NativeHttpServer.prototype, "address"); + } + if (closeDescriptor) { + Object.defineProperty(NativeHttpServer.prototype, "close", closeDescriptor); + } else { + Reflect.deleteProperty(NativeHttpServer.prototype, "close"); + } + } + assertEquals(listenedPorts, [0, 65_535]); + }); + + it("rejects invalid listener ports with the exact validation message", async () => { + for (const port of [-1, 65_536, 1.5]) { + await assertRejects( + () => + createNodeServer(() => new Response("unreachable"), { + hostname: "127.0.0.1", + port, + }), + RangeError, + `Node server port must be an integer from 0 to 65535, got ${port}`, + ); + } + }); + it("shares shutdown and retries only the failed HTTP close phase", async () => { let upgradeDisposeCalls = 0; let closeCalls = 0; diff --git a/src/proxy/retry.test.ts b/src/proxy/retry.test.ts index 93e06b8fd3..ccfb177a3c 100644 --- a/src/proxy/retry.test.ts +++ b/src/proxy/retry.test.ts @@ -269,6 +269,35 @@ describe("shouldRetryUpstreamRequest", () => { }); describe("getReplayableRequestBodies", () => { + async function readBodyBytes(body: ReadableStream | null): Promise { + const bytes = await new Response(body).arrayBuffer(); + return [...new Uint8Array(bytes)]; + } + + it("replays a multi-chunk body sequentially across retries", async () => { + const encoder = new TextEncoder(); + const chunks = ["alpha", ":", "beta"].map((chunk) => encoder.encode(chunk)); + const expected = chunks.flatMap((chunk) => [...chunk]); + const body = new ReadableStream({ + start(controller) { + for (const chunk of chunks) controller.enqueue(chunk); + controller.close(); + }, + }); + const request = { + method: "POST", + headers: new Headers({ "content-length": "10" }), + body, + } as Request; + + const bodies = getReplayableRequestBodies(request, 3); + + assertEquals(bodies.length, 4); + for (const replay of bodies) { + assertEquals(await readBodyBytes(replay), expected); + } + }); + it("creates an independent signed payload stream for every attempt", async () => { const payload = JSON.stringify({ run: { runId: "run_1" } }); const request = new Request(RUN_STREAM_URL, { diff --git a/src/server/shared/renderer/adapter.test.ts b/src/server/shared/renderer/adapter.test.ts index 28778f1c98..6d2c5dc44a 100644 --- a/src/server/shared/renderer/adapter.test.ts +++ b/src/server/shared/renderer/adapter.test.ts @@ -10,6 +10,7 @@ import { afterEach, beforeEach, describe, it } from "#veryfront/testing/bdd.ts"; import type { Renderer, RendererOptions } from "#veryfront/rendering/renderer.ts"; import { prepareDeclarativeConfigContext } from "#veryfront/config/declarative-evaluator.ts"; import { runWithRequestContext } from "#veryfront/platform/adapters/fs/veryfront/request-context.ts"; +import { isBun } from "#veryfront/platform/compat/runtime.ts"; import { destroyRendererAdapter, getRendererForProject, @@ -17,6 +18,8 @@ import { setRendererInitializer, } from "./adapter.ts"; +const hostedWorkerIt = isBun ? it.skip : it; + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -485,69 +488,72 @@ describe("RendererAdapter with RendererInitializer", () => { assertEquals(pages, ["/"]); }); - it("evaluates shared multi-project config through the request's hosted context", async () => { - const ctx = stubHandlerContext(); - ctx.enriched = undefined; - ctx.config = undefined; - ctx.isLocalProject = false; - ctx.projectDir = "/tmp/hosted-project"; - ctx.resolvedEnvironment = "preview"; - ctx.requestContext = { branch: "feature/hosted-render", mode: "preview" }; - - const sourceContext = { - productionMode: false, - branch: "feature/hosted-render", - } as const; - ctx.prepareHostedConfigContext = async () => ({ - sourceContext, - preparedContext: await prepareDeclarativeConfigContext({ - environmentName: "preview", - environment: { TENANT: "tenant-value" }, - }), - }); + hostedWorkerIt( + "evaluates shared multi-project config through the request's hosted context", + async () => { + const ctx = stubHandlerContext(); + ctx.enriched = undefined; + ctx.config = undefined; + ctx.isLocalProject = false; + ctx.projectDir = "/tmp/hosted-project"; + ctx.resolvedEnvironment = "preview"; + ctx.requestContext = { branch: "feature/hosted-render", mode: "preview" }; + + const sourceContext = { + productionMode: false, + branch: "feature/hosted-render", + } as const; + ctx.prepareHostedConfigContext = async () => ({ + sourceContext, + preparedContext: await prepareDeclarativeConfigContext({ + environmentName: "preview", + environment: { TENANT: "tenant-value" }, + }), + }); - const fs = { - isVeryfrontAdapter: () => true, - getUnderlyingAdapter: () => ({}), - isMultiProjectMode: () => true, - runWithContext: ( - projectSlug: string, - token: string, - fn: () => Promise, - projectId?: string, - opts?: Record, - ) => - runWithRequestContext( - { projectSlug, token, projectId, ...opts }, - fn as () => Promise, - ), - exists: () => Promise.reject(new Error("hosted config must not probe exists")), - readFile: (path: string) => { - if (path !== "/veryfront.config.ts") { - return Promise.reject( - Object.assign(new Error(`File not found: ${path}`), { code: "ENOENT" }), - ); - } - return Promise.resolve(` + const fs = { + isVeryfrontAdapter: () => true, + getUnderlyingAdapter: () => ({}), + isMultiProjectMode: () => true, + runWithContext: ( + projectSlug: string, + token: string, + fn: () => Promise, + projectId?: string, + opts?: Record, + ) => + runWithRequestContext( + { projectSlug, token, projectId, ...opts }, + fn as () => Promise, + ), + exists: () => Promise.reject(new Error("hosted config must not probe exists")), + readFile: (path: string) => { + if (path !== "/veryfront.config.ts") { + return Promise.reject( + Object.assign(new Error(`File not found: ${path}`), { code: "ENOENT" }), + ); + } + return Promise.resolve(` import { defineConfigWithEnv, getEnv } from "veryfront"; export default defineConfigWithEnv((environmentName) => ({ title: \`\${environmentName}:\${getEnv("TENANT") ?? "missing"}\`, })); `); - }, - readDir: async function* () {}, - stat: () => Promise.resolve({ isFile: false, isDirectory: false }), - }; - ctx.adapter = { - fs, - env: { get: () => undefined, set: () => {}, delete: () => {}, toObject: () => ({}) }, - } as unknown as any; + }, + readDir: async function* () {}, + stat: () => Promise.resolve({ isFile: false, isDirectory: false }), + }; + ctx.adapter = { + fs, + env: { get: () => undefined, set: () => {}, delete: () => {}, toObject: () => ({}) }, + } as unknown as any; - await getRendererForProject(ctx); + await getRendererForProject(ctx); - assertEquals(ctx.enriched !== undefined, true); - assertEquals(ctx.enriched.config.title, "preview:tenant-value"); - }); + assertEquals(ctx.enriched !== undefined, true); + assertEquals(ctx.enriched.config.title, "preview:tenant-value"); + }, + ); it("falls back to defaults when the release published no config", async () => { // A release with no config answers 404. adapter-factory.ts already treats diff --git a/src/transforms/esm/http-cache-helpers.test.ts b/src/transforms/esm/http-cache-helpers.test.ts index e1f7fca3b1..83da02ce8f 100644 --- a/src/transforms/esm/http-cache-helpers.test.ts +++ b/src/transforms/esm/http-cache-helpers.test.ts @@ -285,49 +285,59 @@ describe("transforms/esm/http-cache-helpers", () => { assertEquals(urlPrototypeCalls, 0); }); - it("uses captured query append and array iteration for duplicate parameters", () => { - const rawUrl = "https://esm.sh/lodash@4?custom=first&custom=second"; - const baseline = normalizeHttpUrl(rawUrl); - assertEquals( - baseline, - "https://esm.sh/lodash@4?custom=first&custom=second&external=react&target=es2022", - ); - - const iteratorDescriptor = Object.getOwnPropertyDescriptor( - Array.prototype, - Symbol.iterator, - )!; - const appendDescriptor = Object.getOwnPropertyDescriptor( + it("preserves query identity with captured query intrinsics", async () => { + const importMap = { imports: {}, scopes: {} }; + const originalIterator = Object.getOwnPropertyDescriptor(Array.prototype, Symbol.iterator); + const originalAppend = Object.getOwnPropertyDescriptor( URLSearchParams.prototype, "append", - )!; - let hookCalls = 0; - let poisoned: string; + ); + const baselineUrl = "https://esm.sh/pkg@1?dup=one&dup=two&encoded=a%2Bb&q=a+b"; + const expectedNormalized = + "https://esm.sh/pkg@1?dup=one&dup=two&encoded=a%2Bb&external=react&q=a+b&target=es2022"; + const baselineIdentity = await buildHttpCacheIdentity(baselineUrl, { importMap }); + let iteratorCalls = 0; + let appendCalls = 0; + let poisonedNormalized = ""; + let poisonedIdentity = ""; + try { Object.defineProperty(Array.prototype, Symbol.iterator, { configurable: true, value() { - hookCalls++; - throw new Error("poisoned Array.prototype iterator"); + iteratorCalls++; + return (originalIterator?.value as () => Iterator).call([]); }, writable: true, }); Object.defineProperty(URLSearchParams.prototype, "append", { configurable: true, value() { - hookCalls++; + appendCalls++; throw new Error("poisoned URLSearchParams.prototype.append"); }, writable: true, }); - poisoned = normalizeHttpUrl(rawUrl); + + poisonedNormalized = normalizeHttpUrl(baselineUrl); + poisonedIdentity = await buildHttpCacheIdentity(baselineUrl, { importMap }); } finally { - Object.defineProperty(Array.prototype, Symbol.iterator, iteratorDescriptor); - Object.defineProperty(URLSearchParams.prototype, "append", appendDescriptor); + if (originalIterator) { + Object.defineProperty(Array.prototype, Symbol.iterator, originalIterator); + } else { + Reflect.deleteProperty(Array.prototype, Symbol.iterator); + } + if (originalAppend) { + Object.defineProperty(URLSearchParams.prototype, "append", originalAppend); + } else { + Reflect.deleteProperty(URLSearchParams.prototype, "append"); + } } - assertEquals(poisoned, baseline); - assertEquals(hookCalls, 0); + assertEquals(poisonedNormalized, expectedNormalized); + assertEquals(poisonedIdentity, baselineIdentity); + assertEquals(iteratorCalls, 0); + assertEquals(appendCalls, 0); }); it("does not consult inherited toJSON hooks while fingerprinting import maps", async () => { diff --git a/tests/bun/preload.ts b/tests/bun/preload.ts index 23b287004c..965fe1ed3d 100644 --- a/tests/bun/preload.ts +++ b/tests/bun/preload.ts @@ -143,16 +143,20 @@ plugin({ // real test-module imports before Bun resolves them, while the lexer keeps // import-looking fixture strings and comments untouched. build.onLoad( - { filter: /(\.test\.[cm]?[jt]sx?|\/extensions\/ext-[^/]+\/src\/.*\.[cm]?[jt]sx?)$/ }, + { + filter: + /(\.test\.[cm]?[jt]sx?|[/\\]extensions[/\\]ext-[^/\\]+[/\\]src[/\\].*\.[cm]?[jt]sx?)$/, + }, (args) => { + const posixPath = args.path.split(sep).join("/"); const source = readFileSync(args.path, "utf8"); - let contents = args.path.includes(`${sep}extensions${sep}`) + let contents = posixPath.includes("/extensions/") ? rewriteModuleSpecifiers( source, (specifier) => workspaceModuleSpecifier(args.path, specifier), ) ?? source : source; - if (/\.test\.[cm]?[jt]sx?$/.test(args.path)) { + if (/\.test\.[cm]?[jt]sx?$/.test(posixPath)) { contents = rewriteNpmProtocolImports(contents) ?? contents; } const extension = extname(args.path).toLowerCase(); diff --git a/tests/bun/run-tests.mjs b/tests/bun/run-tests.mjs index c473b91402..fe9780f9a9 100644 --- a/tests/bun/run-tests.mjs +++ b/tests/bun/run-tests.mjs @@ -38,15 +38,19 @@ function resolveShardCount(envKeys) { } const args = process.argv.slice(2); -ensureNpmNodeModulesLinks(); const projectRoot = fileURLToPath(new URL("../..", import.meta.url)); -const bunWorkspacePackages = prepareBunWorkspacePackages(projectRoot); -registerBunWorkspaceCleanup(() => bunWorkspacePackages.cleanup()); -const concurrency = resolveConcurrency(["VF_TEST_CONCURRENCY", "BUN_TEST_CONCURRENCY"]); +const concurrency = resolveConcurrency([ + "VF_TEST_CONCURRENCY", + "BUN_TEST_CONCURRENCY", +]); const shardOverride = resolveShardCount(["VF_TEST_SHARDS", "BUN_TEST_SHARDS"]); const processCount = shardOverride ?? Math.max(1, Math.min(4, concurrency)); const defaultRoots = ["src", "tests", "proxy"]; -const includePatterns = (process.env.BUN_TEST_INCLUDE || process.env.VF_TEST_INCLUDE || "") +const includePatterns = ( + process.env.BUN_TEST_INCLUDE || + process.env.VF_TEST_INCLUDE || + "" +) .split(",") .map((value) => value.trim()) .filter(Boolean); @@ -60,7 +64,11 @@ const runtimeIncompatibleTests = [ "src/server/project-env/fetcher.test.ts", "src/routing/api/module-loader/loader.test.ts", ]; -const envExcludePatterns = (process.env.BUN_TEST_EXCLUDE || process.env.VF_TEST_EXCLUDE || "") +const envExcludePatterns = ( + process.env.BUN_TEST_EXCLUDE || + process.env.VF_TEST_EXCLUDE || + "" +) .split(",") .map((value) => value.trim()) .filter(Boolean); @@ -69,10 +77,12 @@ const hasFilters = includePatterns.length > 0 || excludePatterns.length > 0; function isDenoDependentTest(file) { try { const source = readFileSync(file, "utf-8"); - return /\bDeno\./.test(source) || + return ( + /\bDeno\./.test(source) || /\bDeno\.test\s*\(/.test(source) || /tests\/_helpers\/utils\.ts/.test(source) || - /\bcreateMockServer\s*\(/.test(source); + /\bcreateMockServer\s*\(/.test(source) + ); } catch { return false; } @@ -86,14 +96,20 @@ function selectedTestFiles() { const patterns = args.length > 0 ? args : defaultRoots; let files = listTestFiles(patterns); if (hasFilters) { - files = filterTestFiles(files, { include: includePatterns, exclude: excludePatterns }); + files = filterTestFiles(files, { + include: includePatterns, + exclude: excludePatterns, + }); } return removeDenoDependentTests(files); } function runBunProcess(file, bunArgs) { return new Promise((resolvePromise) => { - const child = spawn("bun", bunArgs, { stdio: ["ignore", "pipe", "pipe"], env }); + const child = spawn("bun", bunArgs, { + stdio: ["ignore", "pipe", "pipe"], + env, + }); const stdout = []; const stderr = []; let settled = false; @@ -169,6 +185,13 @@ for ( } const files = selectedTestFiles(); +if (files.length > 0) ensureNpmNodeModulesLinks(); +const bunWorkspacePackages = files.length === 0 + ? undefined + : prepareBunWorkspacePackages(projectRoot); +if (bunWorkspacePackages) { + registerBunWorkspaceCleanup(() => bunWorkspacePackages.cleanup()); +} runIsolatedTests(files) .then((ok) => { process.exitCode = ok ? 0 : 1; diff --git a/tests/bun/runner-args.test.mjs b/tests/bun/runner-args.test.mjs index e9f6a9b8c0..d89e9c4292 100644 --- a/tests/bun/runner-args.test.mjs +++ b/tests/bun/runner-args.test.mjs @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; import { EventEmitter } from "node:events"; import { readFileSync } from "node:fs"; import test from "node:test"; @@ -33,19 +34,32 @@ test("buildIsolatedBunTestRuns puts each test file in its own Bun process", () = }); test("the Bun runner drains child output and exits naturally", () => { - const source = readFileSync(new URL("./run-tests.mjs", import.meta.url), "utf8"); + const source = readFileSync( + new URL("./run-tests.mjs", import.meta.url), + "utf8", + ); - assert.match(source, /child\.on\("close", \(code\) => finish\(code \?\? 1\)\)/); + assert.match( + source, + /child\.on\("close", \(code\) => finish\(code \?\? 1\)\)/, + ); assert.doesNotMatch(source, /process\.exit\(/); assert.match(source, /process\.exitCode = ok \? 0 : 1/); }); -test("the Bun runner fails when selection produces no test files", () => { - const source = readFileSync(new URL("./run-tests.mjs", import.meta.url), "utf8"); +test("the Bun runner fails loudly when filters select no files", () => { + const result = spawnSync( + process.execPath, + [new URL("./run-tests.mjs", import.meta.url).pathname], + { + env: { ...process.env, BUN_TEST_INCLUDE: "missing-bun-fixture.test.ts" }, + encoding: "utf8", + }, + ); - assert.match(source, /if \(files\.length === 0\)/); - assert.match(source, /Bun test runner selected no test files\./); - assert.match(source, /return false;/); + assert.equal(result.status, 1); + assert.match(result.stderr, /Bun test runner selected no test files\./); + assert.doesNotMatch(result.stdout, /0 passed, 0 failed/); }); test("Bun workspace cleanup runs before termination signals are re-raised", () => { diff --git a/tests/bun/workspace-packages.mjs b/tests/bun/workspace-packages.mjs index 3445a872f7..218ce39c61 100644 --- a/tests/bun/workspace-packages.mjs +++ b/tests/bun/workspace-packages.mjs @@ -1,14 +1,7 @@ -import { - existsSync, - mkdirSync, - readFileSync, - rmdirSync, - rmSync, - symlinkSync, - writeFileSync, -} from "node:fs"; +import { existsSync, mkdirSync, readFileSync, rmdirSync, rmSync, writeFileSync } from "node:fs"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { randomUUID } from "node:crypto"; +import { ensureDirectoryLink } from "../ensure-npm-links.mjs"; const MARKER_NAME = ".veryfront-bun-workspace-package.json"; const LOCK_NAME = ".veryfront-bun-workspace-packages.lock"; @@ -18,34 +11,6 @@ function readJson(path) { return JSON.parse(readFileSync(path, "utf8")); } -export function reclaimStalePreparationLock(lockPath, runtimeProcess = process) { - let marker; - try { - marker = readJson(join(lockPath, MARKER_NAME)); - } catch { - rmSync(lockPath, { recursive: true, force: true }); - return true; - } - - if ( - marker?.owner !== LOCK_OWNER || - !Number.isSafeInteger(marker.pid) || - marker.pid <= 0 - ) { - rmSync(lockPath, { recursive: true, force: true }); - return true; - } - - try { - runtimeProcess.kill(marker.pid, 0); - return false; - } catch (error) { - if (error?.code !== "ESRCH") return false; - rmSync(lockPath, { recursive: true, force: true }); - return true; - } -} - function acquirePreparationLock(nodeModulesPath) { mkdirSync(nodeModulesPath, { recursive: true }); const lockPath = join(nodeModulesPath, LOCK_NAME); @@ -83,6 +48,30 @@ function acquirePreparationLock(nodeModulesPath) { return { lockPath, token }; } +export function reclaimStalePreparationLock(lockPath, runtimeProcess = process) { + let marker; + try { + marker = readJson(join(lockPath, MARKER_NAME)); + } catch { + return false; + } + + if (marker?.owner !== LOCK_OWNER) { + return false; + } + if (!Number.isSafeInteger(marker.pid) || marker.pid < 1) { + return false; + } + try { + runtimeProcess.kill(marker.pid, 0); + return false; + } catch (error) { + if (error?.code !== "ESRCH") return false; + rmSync(lockPath, { recursive: true, force: true }); + return true; + } +} + function releasePreparationLock(lockPath, token) { let marker; try { @@ -104,31 +93,48 @@ function packageSegments(name) { function packageTarget(packageName, subpath, target, targetRoot, sourceRoot) { if (typeof target !== "string" || !target.startsWith(".")) { - throw new TypeError(`${packageName} export ${subpath} must target a local path`); + throw new TypeError( + `${packageName} export ${subpath} must target a local path`, + ); } const targetPath = resolve(targetRoot, target); const sourceRelativePath = relative(sourceRoot, targetPath); if ( - isAbsolute(sourceRelativePath) || sourceRelativePath === ".." || + isAbsolute(sourceRelativePath) || + sourceRelativePath === ".." || sourceRelativePath.startsWith(`..${sep}`) ) { - throw new Error(`${packageName} export ${subpath} escapes its generated package source`); + throw new Error( + `${packageName} export ${subpath} escapes its generated package source`, + ); } return `./source/${sourceRelativePath.split(sep).join("/")}`; } -function packageExports(packageName, exports, targetRoot, sourceRoot = targetRoot) { +function packageExports( + packageName, + exports, + targetRoot, + sourceRoot = targetRoot, +) { if (typeof exports === "string") { return packageTarget(packageName, ".", exports, targetRoot, sourceRoot); } - if (!exports || typeof exports !== "object" || Array.isArray(exports)) return null; + if (!exports || typeof exports !== "object" || Array.isArray(exports)) { + return null; + } return Object.fromEntries( Object.entries(exports).map(([subpath, target]) => { if (subpath !== "." && !subpath.startsWith("./")) { - throw new TypeError(`${packageName} export ${subpath} must be . or start with ./`); + throw new TypeError( + `${packageName} export ${subpath} must be . or start with ./`, + ); } - return [subpath, packageTarget(packageName, subpath, target, targetRoot, sourceRoot)]; + return [ + subpath, + packageTarget(packageName, subpath, target, targetRoot, sourceRoot), + ]; }), ); } @@ -140,8 +146,14 @@ function addWorkspaceImportsToRootExports( workspaceConfig, projectRoot, ) { - for (const [specifier, target] of Object.entries(workspaceConfig.imports ?? {})) { - if (!specifier.startsWith(`${rootName}/`) || typeof target !== "string") continue; + for ( + const [specifier, target] of Object.entries( + workspaceConfig.imports ?? {}, + ) + ) { + if (!specifier.startsWith(`${rootName}/`) || typeof target !== "string") { + continue; + } const subpath = `./${specifier.slice(rootName.length + 1)}`; const generatedTarget = packageTarget( rootName, @@ -173,10 +185,13 @@ export function prepareBunWorkspacePackages(projectRoot) { throw new Error(`${name} already exists in node_modules`); } if ( - marker.owner !== LOCK_OWNER || marker.name !== name || + marker.owner !== LOCK_OWNER || + marker.name !== name || marker.source !== sourceRoot ) { - throw new Error(`${name} in node_modules is not owned by the Bun test runner`); + throw new Error( + `${name} in node_modules is not owned by the Bun test runner`, + ); } rmSync(packageRoot, { recursive: true, force: true }); } @@ -184,10 +199,10 @@ export function prepareBunWorkspacePackages(projectRoot) { mkdirSync(packageRoot, { recursive: true }); createdPackages.push(packageRoot); if (segments.length > 1) scopeDirectories.add(dirname(packageRoot)); - symlinkSync( + ensureDirectoryLink( sourceRoot, join(packageRoot, "source"), - process.platform === "win32" ? "junction" : "dir", + `${name}/source`, ); writeFileSync( join(packageRoot, "package.json"), @@ -215,7 +230,9 @@ export function prepareBunWorkspacePackages(projectRoot) { try { rmdirSync(scopeDirectory); } catch (error) { - if (error?.code !== "ENOENT" && error?.code !== "ENOTEMPTY") throw error; + if (error?.code !== "ENOENT" && error?.code !== "ENOTEMPTY") { + throw error; + } } } } finally { @@ -225,11 +242,13 @@ export function prepareBunWorkspacePackages(projectRoot) { try { const rootConfig = readJson(resolve(projectRoot, "deno.json")); - const workspaces = (rootConfig.workspace ?? []).map((workspaceDirectory) => { - const workspaceRoot = resolve(projectRoot, workspaceDirectory); - const workspaceConfig = readJson(resolve(workspaceRoot, "deno.json")); - return { workspaceDirectory, workspaceRoot, workspaceConfig }; - }); + const workspaces = (rootConfig.workspace ?? []).map( + (workspaceDirectory) => { + const workspaceRoot = resolve(projectRoot, workspaceDirectory); + const workspaceConfig = readJson(resolve(workspaceRoot, "deno.json")); + return { workspaceDirectory, workspaceRoot, workspaceConfig }; + }, + ); if (typeof rootConfig.name !== "string") { throw new TypeError("deno.json must name the root package"); @@ -260,7 +279,13 @@ export function prepareBunWorkspacePackages(projectRoot) { } createPackage(rootConfig.name, projectRoot, rootExports); - for (const { workspaceDirectory, workspaceRoot, workspaceConfig } of workspaces) { + for ( + const { + workspaceDirectory, + workspaceRoot, + workspaceConfig, + } of workspaces + ) { if (workspaceConfig.exports === undefined) continue; if (typeof workspaceConfig.name !== "string") { throw new TypeError( diff --git a/tests/bun/workspace-packages.test.mjs b/tests/bun/workspace-packages.test.mjs index 4c42f0524d..986dbc7a56 100644 --- a/tests/bun/workspace-packages.test.mjs +++ b/tests/bun/workspace-packages.test.mjs @@ -1,18 +1,9 @@ import assert from "node:assert/strict"; -import { - existsSync, - mkdirSync, - mkdtempSync, - readFileSync, - realpathSync, - rmSync, - writeFileSync, -} from "node:fs"; +import { existsSync, mkdirSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; -import { tmpdir } from "node:os"; import { fileURLToPath } from "node:url"; import test from "node:test"; -import { prepareBunWorkspacePackages, reclaimStalePreparationLock } from "./workspace-packages.mjs"; +import { prepareBunWorkspacePackages } from "./workspace-packages.mjs"; const projectRoot = fileURLToPath(new URL("../..", import.meta.url)); @@ -26,7 +17,9 @@ test("prepareBunWorkspacePackages derives native packages from every workspace e try { const rootConfig = readJson(resolve(projectRoot, "deno.json")); - const rootPackage = readJson(join(prepared.nodeModulesPath, "veryfront/package.json")); + const rootPackage = readJson( + join(prepared.nodeModulesPath, "veryfront/package.json"), + ); assert.equal( rootPackage.exports["./platform/path"], "./source/src/platform/compat/path/index.ts", @@ -35,26 +28,42 @@ test("prepareBunWorkspacePackages derives native packages from every workspace e rootPackage.exports["./transforms/frontmatter"], "./source/src/transforms/mdx/compiler/frontmatter-extractor.ts", ); - const publishedWorkspaces = rootConfig.workspace.filter((workspaceDirectory) => { - const config = readJson(resolve(projectRoot, workspaceDirectory, "deno.json")); - return typeof config.name === "string" && config.exports !== undefined; - }); - assert.ok(publishedWorkspaces.length > 25); + const publishedWorkspaces = rootConfig.workspace.filter( + (workspaceDirectory) => { + const config = readJson( + resolve(projectRoot, workspaceDirectory, "deno.json"), + ); + return typeof config.name === "string" && config.exports !== undefined; + }, + ); + assert.ok(publishedWorkspaces.length > 0); for (const workspaceDirectory of publishedWorkspaces) { const workspaceRoot = resolve(projectRoot, workspaceDirectory); const sourceConfig = readJson(resolve(workspaceRoot, "deno.json")); - const packageRoot = join(prepared.nodeModulesPath, ...sourceConfig.name.split("/")); + const packageRoot = join( + prepared.nodeModulesPath, + ...sourceConfig.name.split("/"), + ); generatedPackageRoots.push(packageRoot); const packageConfig = readJson(join(packageRoot, "package.json")); assert.equal(packageConfig.name, sourceConfig.name); - assert.equal(realpathSync(join(packageRoot, "source")), realpathSync(workspaceRoot)); + assert.equal( + realpathSync(join(packageRoot, "source")), + realpathSync(workspaceRoot), + ); if (typeof sourceConfig.exports === "string") { - assert.equal(packageConfig.exports, `./source/${sourceConfig.exports.slice(2)}`); + assert.equal( + packageConfig.exports, + `./source/${sourceConfig.exports.slice(2)}`, + ); } else { for (const [subpath, target] of Object.entries(sourceConfig.exports)) { - assert.equal(packageConfig.exports[subpath], `./source/${target.slice(2)}`); + assert.equal( + packageConfig.exports[subpath], + `./source/${target.slice(2)}`, + ); } } } @@ -62,7 +71,9 @@ test("prepareBunWorkspacePackages derives native packages from every workspace e prepared.cleanup(); } - for (const packageRoot of generatedPackageRoots) assert.equal(existsSync(packageRoot), false); + for (const packageRoot of generatedPackageRoots) { + assert.equal(existsSync(packageRoot), false); + } assert.equal(existsSync(join(prepared.nodeModulesPath, "veryfront")), false); assert.equal(existsSync(join(prepared.nodeModulesPath, "react")), true); }); @@ -71,13 +82,19 @@ test("workspace package cleanup is idempotent", () => { const prepared = prepareBunWorkspacePackages(projectRoot); prepared.cleanup(); prepared.cleanup(); - assert.equal(existsSync(join(prepared.nodeModulesPath, "@veryfront/ext-schema-zod")), false); + assert.equal( + existsSync(join(prepared.nodeModulesPath, "@veryfront/ext-schema-zod")), + false, + ); assert.equal(existsSync(join(prepared.nodeModulesPath, "react")), true); }); test("workspace package preparation rejects an overlapping run without disturbing it", () => { const prepared = prepareBunWorkspacePackages(projectRoot); - const rootPackagePath = join(prepared.nodeModulesPath, "veryfront/package.json"); + const rootPackagePath = join( + prepared.nodeModulesPath, + "veryfront/package.json", + ); try { assert.throws( @@ -92,39 +109,118 @@ test("workspace package preparation rejects an overlapping run without disturbin assert.equal(existsSync(rootPackagePath), false); }); -test("workspace package preparation reclaims a lock owned by a dead process", () => { - const root = mkdtempSync(join(tmpdir(), "veryfront-bun-lock-")); - const lockPath = join(root, ".veryfront-bun-workspace-packages.lock"); - mkdirSync(lockPath); +test("workspace package preparation reclaims a stale lock with a dead owner", () => { + const lockPath = join( + projectRoot, + "node_modules", + ".veryfront-bun-workspace-packages.lock", + ); + rmSync(lockPath, { recursive: true, force: true }); + mkdirSync(lockPath, { recursive: true }); writeFileSync( join(lockPath, ".veryfront-bun-workspace-package.json"), - `${JSON.stringify({ owner: "veryfront-bun-tests", pid: 123, token: "stale" })}\n`, + `${JSON.stringify({ owner: "veryfront-bun-tests", pid: 9_999_999, token: "stale" })}\n`, ); - const runtimeProcess = { - kill(pid, signal) { - assert.equal(pid, 123); - assert.equal(signal, 0); - throw Object.assign(new Error("process does not exist"), { code: "ESRCH" }); - }, - }; + const prepared = prepareBunWorkspacePackages(projectRoot); try { - assert.equal(reclaimStalePreparationLock(lockPath, runtimeProcess), true); - assert.equal(existsSync(lockPath), false); + assert.equal( + existsSync(join(prepared.nodeModulesPath, "veryfront/package.json")), + true, + ); } finally { - rmSync(root, { recursive: true, force: true }); + prepared.cleanup(); } }); -test("workspace package preparation reclaims a lock without a valid marker", () => { - const root = mkdtempSync(join(tmpdir(), "veryfront-bun-lock-")); - const lockPath = join(root, ".veryfront-bun-workspace-packages.lock"); - mkdirSync(lockPath); +test("workspace package preparation preserves a live lock owner", () => { + const lockPath = join( + projectRoot, + "node_modules", + ".veryfront-bun-workspace-packages.lock", + ); + rmSync(lockPath, { recursive: true, force: true }); + mkdirSync(lockPath, { recursive: true }); + writeFileSync( + join(lockPath, ".veryfront-bun-workspace-package.json"), + `${JSON.stringify({ owner: "veryfront-bun-tests", pid: process.pid, token: "active" })}\n`, + ); try { - assert.equal(reclaimStalePreparationLock(lockPath), true); - assert.equal(existsSync(lockPath), false); + assert.throws( + () => prepareBunWorkspacePackages(projectRoot), + new Error("Bun workspace package preparation is already active"), + ); + assert.equal(existsSync(lockPath), true); + } finally { + rmSync(lockPath, { recursive: true, force: true }); + } +}); + +test("workspace package preparation preserves a lock with a missing marker", () => { + const lockPath = join( + projectRoot, + "node_modules", + ".veryfront-bun-workspace-packages.lock", + ); + rmSync(lockPath, { recursive: true, force: true }); + mkdirSync(lockPath, { recursive: true }); + + try { + assert.throws( + () => prepareBunWorkspacePackages(projectRoot), + new Error("Bun workspace package preparation is already active"), + ); + assert.equal(existsSync(lockPath), true); + } finally { + rmSync(lockPath, { recursive: true, force: true }); + } +}); + +test("workspace package preparation preserves an invalid owned lock marker", () => { + const lockPath = join( + projectRoot, + "node_modules", + ".veryfront-bun-workspace-packages.lock", + ); + rmSync(lockPath, { recursive: true, force: true }); + mkdirSync(lockPath, { recursive: true }); + writeFileSync( + join(lockPath, ".veryfront-bun-workspace-package.json"), + `${JSON.stringify({ owner: "veryfront-bun-tests", pid: 0, token: "invalid" })}\n`, + ); + + try { + assert.throws( + () => prepareBunWorkspacePackages(projectRoot), + new Error("Bun workspace package preparation is already active"), + ); + assert.equal(existsSync(lockPath), true); + } finally { + rmSync(lockPath, { recursive: true, force: true }); + } +}); + +test("workspace package preparation preserves a foreign lock owner", () => { + const lockPath = join( + projectRoot, + "node_modules", + ".veryfront-bun-workspace-packages.lock", + ); + rmSync(lockPath, { recursive: true, force: true }); + mkdirSync(lockPath, { recursive: true }); + writeFileSync( + join(lockPath, ".veryfront-bun-workspace-package.json"), + `${JSON.stringify({ owner: "external-owner", pid: 9_999_999, token: "external" })}\n`, + ); + + try { + assert.throws( + () => prepareBunWorkspacePackages(projectRoot), + new Error("Bun workspace package preparation is already active"), + ); + assert.equal(existsSync(lockPath), true); } finally { - rmSync(root, { recursive: true, force: true }); + rmSync(lockPath, { recursive: true, force: true }); } }); diff --git a/tests/ensure-npm-links.mjs b/tests/ensure-npm-links.mjs index 5296d989b6..4078f10aeb 100644 --- a/tests/ensure-npm-links.mjs +++ b/tests/ensure-npm-links.mjs @@ -6,7 +6,7 @@ export function resolveDirectoryLinkType(platform = process.platform) { return platform === "win32" ? "junction" : "dir"; } -export function ensureDirectorySymlink( +export function ensureDirectoryLink( sourcePath, targetPath, packageName, @@ -27,9 +27,11 @@ function linkTopLevelPackage(npmModulesRoot, rootModulesRoot, packageName) { const sourcePath = resolve(npmModulesRoot, packageName); const targetPath = resolve(rootModulesRoot, packageName); if (!existsSync(sourcePath)) { - throw new Error(`npm dependency "${packageName}" disappeared while links were prepared.`); + throw new Error( + `npm dependency "${packageName}" disappeared while links were prepared.`, + ); } - ensureDirectorySymlink(sourcePath, targetPath, packageName); + ensureDirectoryLink(sourcePath, targetPath, packageName); } function linkScopedPackages(npmModulesRoot, rootModulesRoot, scopeName) { @@ -38,7 +40,7 @@ function linkScopedPackages(npmModulesRoot, rootModulesRoot, scopeName) { if (!existsSync(sourceScopeDir)) return; if (!existsSync(targetScopeDir)) { - ensureDirectorySymlink(sourceScopeDir, targetScopeDir, scopeName); + ensureDirectoryLink(sourceScopeDir, targetScopeDir, scopeName); if (existsSync(targetScopeDir)) return; } @@ -46,14 +48,16 @@ function linkScopedPackages(npmModulesRoot, rootModulesRoot, scopeName) { try { entries = readdirSync(sourceScopeDir, { withFileTypes: true }); } catch (error) { - throw new Error(`Cannot read npm dependency scope "${scopeName}".`, { cause: error }); + throw new Error(`Cannot read npm dependency scope "${scopeName}".`, { + cause: error, + }); } for (const entry of entries) { if (!entry.isDirectory()) continue; const sourcePath = resolve(sourceScopeDir, entry.name); const targetPath = resolve(targetScopeDir, entry.name); - ensureDirectorySymlink(sourcePath, targetPath, `${scopeName}/${entry.name}`); + ensureDirectoryLink(sourcePath, targetPath, `${scopeName}/${entry.name}`); } } @@ -77,9 +81,12 @@ export function ensureNpmNodeModulesLinks( try { entries = readdirSync(npmModulesRoot, { withFileTypes: true }); } catch (error) { - throw new Error("Cannot read npm/node_modules while preparing runtime tests.", { - cause: error, - }); + throw new Error( + "Cannot read npm/node_modules while preparing runtime tests.", + { + cause: error, + }, + ); } for (const entry of entries) { diff --git a/tests/ensure-npm-links.test.mjs b/tests/ensure-npm-links.test.mjs index e7fed1bdcb..993944247b 100644 --- a/tests/ensure-npm-links.test.mjs +++ b/tests/ensure-npm-links.test.mjs @@ -1,11 +1,11 @@ import { strict as assert } from "node:assert"; import { spawnSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, it } from "node:test"; import { - ensureDirectorySymlink, + ensureDirectoryLink, ensureNpmNodeModulesLinks, resolveDirectoryLinkType, } from "./ensure-npm-links.mjs"; @@ -22,7 +22,7 @@ describe("ensureNpmNodeModulesLinks", () => { it("tolerates a concurrent directory link and preserves other failures", () => { const existsError = Object.assign(new Error("already linked"), { code: "EEXIST" }); assert.doesNotThrow(() => - ensureDirectorySymlink("source", "target", "package", { + ensureDirectoryLink("source", "target", "package", { pathExists: () => false, createSymlink: () => { throw existsError; @@ -33,7 +33,7 @@ describe("ensureNpmNodeModulesLinks", () => { const permissionError = Object.assign(new Error("permission denied"), { code: "EACCES" }); assert.throws( () => - ensureDirectorySymlink("source", "target", "package", { + ensureDirectoryLink("source", "target", "package", { pathExists: () => false, createSymlink: () => { throw permissionError; @@ -52,12 +52,9 @@ describe("ensureNpmNodeModulesLinks", () => { mkdirSync(join(rootDir, "node_modules")); try { - assert.throws( - () => ensureNpmNodeModulesLinks(rootDir), - { - message: MISSING_BUILD_MESSAGE, - }, - ); + assert.throws(() => ensureNpmNodeModulesLinks(rootDir), { + message: MISSING_BUILD_MESSAGE, + }); } finally { rmSync(rootDir, { recursive: true, force: true }); } @@ -76,11 +73,9 @@ describe("ensureNpmNodeModulesLinks", () => { : ["--input-type=module", "--eval", program]; try { - const result = spawnSync( - process.execPath, - evalArgs, - { encoding: "utf8" }, - ); + const result = spawnSync(process.execPath, evalArgs, { + encoding: "utf8", + }); assert.equal(result.status, 1); assert.ok( @@ -91,4 +86,64 @@ describe("ensureNpmNodeModulesLinks", () => { rmSync(rootDir, { recursive: true, force: true }); } }); + + it("tolerates a concurrent EEXIST link race after a stale exists check", () => { + const rootDir = mkdtempSync(join(tmpdir(), "veryfront-npm-links-")); + const sourcePath = join(rootDir, "source"); + const targetPath = join(rootDir, "target"); + mkdirSync(sourcePath); + symlinkSync(join(rootDir, "missing"), targetPath, resolveDirectoryLinkType()); + + try { + assert.equal(false, existsSync(targetPath)); + assert.doesNotThrow(() => ensureDirectoryLink(sourcePath, targetPath, "react")); + } finally { + rmSync(rootDir, { recursive: true, force: true }); + } + }); + + it("preserves the original npm/node_modules read failure as the error cause", () => { + const rootDir = mkdtempSync(join(tmpdir(), "veryfront-npm-links-")); + mkdirSync(join(rootDir, "npm")); + writeFileSync(join(rootDir, "npm/node_modules"), ""); + mkdirSync(join(rootDir, "node_modules")); + + try { + assert.throws( + () => ensureNpmNodeModulesLinks(rootDir), + (error) => { + assert.equal( + error.message, + "Cannot read npm/node_modules while preparing runtime tests.", + ); + assert.equal(error.cause?.code, "ENOTDIR"); + return true; + }, + ); + } finally { + rmSync(rootDir, { recursive: true, force: true }); + } + }); + + it("preserves the original link failure as the error cause", () => { + const rootDir = mkdtempSync(join(tmpdir(), "veryfront-npm-links-")); + const sourcePath = join(rootDir, "missing"); + const targetPath = join(rootDir, "missing-parent", "target"); + + try { + assert.throws( + () => ensureDirectoryLink(sourcePath, targetPath, "react"), + (error) => { + assert.equal( + error.message, + 'Cannot link npm dependency "react" into node_modules.', + ); + assert.equal(error.cause?.code, "ENOENT"); + return true; + }, + ); + } finally { + rmSync(rootDir, { recursive: true, force: true }); + } + }); });