From dc5527b0bc0228b9dfcb911ab9c3aff7cc7f47ee Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:15:41 +0800 Subject: [PATCH] test(filesystem): verify snapshot admission and retained output (#10091) --- src/adb/AdbInputSnapshot.ts | 18 +--- .../filesystem/adbInputSnapshot.test.ts | 87 ++++++++++--------- .../filesystem/analysisSnapshot.test.ts | 5 +- .../artifactInventorySnapshot.test.ts | 7 +- 4 files changed, 58 insertions(+), 59 deletions(-) diff --git a/src/adb/AdbInputSnapshot.ts b/src/adb/AdbInputSnapshot.ts index b060b91ace..8dd04107f1 100644 --- a/src/adb/AdbInputSnapshot.ts +++ b/src/adb/AdbInputSnapshot.ts @@ -17,11 +17,6 @@ import { import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import type { PrivateRuntimeRoot } from "../process/PrivateRuntimeRoot.js"; -export interface AdbInputSnapshotDependencies { - readonly openSource?: typeof openRegularFile; - readonly openDestination?: typeof open; -} - export interface AdbInputSnapshot { readonly path: string; readonly bytes: number; @@ -51,7 +46,6 @@ export const snapshotAdbInput = async ( resources: ArtifactResourceScope, operation: string, signal?: AbortSignal, - dependencies: AdbInputSnapshotDependencies = {}, ): Promise => { let source: OwnedFileHandle | undefined; let destination: OwnedFileHandle | undefined; @@ -61,21 +55,13 @@ export const snapshotAdbInput = async ( let primaryCause: unknown; try { - const { handle, initial } = await ( - dependencies.openSource ?? openRegularFile - )(sourcePath, { + const { handle, initial } = await openRegularFile(sourcePath, { symlinks: "follow", signal, }); source = new OwnedFileHandle(handle); const destinationPath = join(root.path, basename(sourcePath)); - destination = new OwnedFileHandle( - await (dependencies.openDestination ?? open)( - destinationPath, - "wx", - 0o600, - ), - ); + destination = new OwnedFileHandle(await open(destinationPath, "wx", 0o600)); if (!Number.isSafeInteger(initial.size) || initial.size < 0) throw new RangeError( `Regular file size cannot be represented safely: ${sourcePath}`, diff --git a/tests/boundary/filesystem/adbInputSnapshot.test.ts b/tests/boundary/filesystem/adbInputSnapshot.test.ts index 2308c768a6..63228add5f 100644 --- a/tests/boundary/filesystem/adbInputSnapshot.test.ts +++ b/tests/boundary/filesystem/adbInputSnapshot.test.ts @@ -1,70 +1,79 @@ -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { + mkdtemp, + open, + rm, + writeFile, + type FileHandle, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { expect, it } from "vitest"; +import { expect, it, vi } from "vitest"; import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; -import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; -import { - openRegularFile, - RegularFileAdmissionFailure, -} from "../../../src/filesystem/RegularFile.js"; import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js"; import { AdbInputSnapshotFailure, snapshotAdbInput, } from "../../../src/adb/AdbInputSnapshot.js"; +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, open: vi.fn(actual.open) }; +}); + it("retains and retries a source handle that fails close during admission", async () => { + const actual = + await vi.importActual( + "node:fs/promises", + ); const parent = await mkdtemp(join(tmpdir(), "rea-adb-snapshot-test-")); const root = await PrivateRuntimeRoot.create({ parent }); const resources = new ArtifactResourceScope(); const sourcePath = join(parent, "selected.apk"); await writeFile(sourcePath, "selected bytes"); + const admissionFailure = new Error("controlled descriptor stat failure"); + const closeFailure = new Error("controlled source close failure"); + let source: FileHandle | undefined; let closeAttempts = 0; + let blockClose = true; + vi.mocked(open).mockImplementation(async (...args) => { + const handle = await actual.open(...args); + if (String(args[0]) !== sourcePath) return handle; + source = handle; + vi.spyOn(handle, "stat").mockRejectedValue(admissionFailure); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeAttempts += 1; + // Both admission and snapshot finalization must retain the same handle. + if (blockClose) throw closeFailure; + await close(); + }); + return handle; + }); try { await expect( - snapshotAdbInput( - sourcePath, - root, - resources, - "install_adb_package", - undefined, - { - openSource: async (path, options) => { - const { handle } = await openRegularFile(path, options); - const close = handle.close.bind(handle); - const owner = new OwnedFileHandle(handle); - handle.close = async () => { - closeAttempts += 1; - if (closeAttempts === 1) - throw new Error("controlled close failure"); - await close(); - }; - throw new RegularFileAdmissionFailure( - path, - new Error("controlled admission failure"), - new Error("first close was not confirmed"), - owner, - ); - }, - }, - ), + snapshotAdbInput(sourcePath, root, resources, "install_adb_package"), ).rejects.toMatchObject>({ - primaryCause: expect.objectContaining({ - message: "controlled admission failure", - }), - cleanupResources: [expect.stringContaining("source handle")], + primaryCause: admissionFailure, + cleanupResources: [`${sourcePath} (ADB snapshot source handle)`], + cleanupCause: expect.objectContaining({ errors: [closeFailure] }), }); + expect(source?.fd).toBeGreaterThanOrEqual(0); expect(resources.hasPending).toBe(true); - expect(closeAttempts).toBe(1); + expect(closeAttempts).toBe(2); + blockClose = false; await resources.retryCleanup(); expect(resources.hasPending).toBe(false); - expect(closeAttempts).toBe(2); + expect(closeAttempts).toBe(3); + expect(source?.fd).toBe(-1); } finally { + blockClose = false; await resources.close(); + // An ownership regression must not leave the test's actual descriptor open. + if (source !== undefined && source.fd !== -1) await source.close(); + vi.restoreAllMocks(); await root.close(); await rm(parent, { recursive: true, force: true }); } diff --git a/tests/boundary/filesystem/analysisSnapshot.test.ts b/tests/boundary/filesystem/analysisSnapshot.test.ts index 407751b8c2..969528ea07 100644 --- a/tests/boundary/filesystem/analysisSnapshot.test.ts +++ b/tests/boundary/filesystem/analysisSnapshot.test.ts @@ -2,6 +2,7 @@ import { readFile, stat, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect, it, onTestFinished, vi } from "vitest"; +import canonicalize from "canonicalize"; import { withArtifactResourceScope } from "../../fixtures/artifactInventory.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -130,7 +131,9 @@ describe("analysis snapshots: persistence", () => { writeAnalysisSnapshot(snapshot, path, false, resources), ); if (!written.ok) throw written.error; - expect((await readFile(path, "utf8")).endsWith("\n")).toBe(true); + const encoded = await readFile(path, "utf8"); + // RFC 8785 bytes are a storage contract, independent of snapshot decoding. + expect(encoded).toBe(`${canonicalize(snapshot)}\n`); expect((await stat(path)).mode & 0o777).toBe(0o600); const roundTrip = await readAnalysisSnapshot(path); if (!roundTrip.ok) throw roundTrip.error; diff --git a/tests/boundary/filesystem/artifactInventorySnapshot.test.ts b/tests/boundary/filesystem/artifactInventorySnapshot.test.ts index 6802051cf0..093c73f0e2 100644 --- a/tests/boundary/filesystem/artifactInventorySnapshot.test.ts +++ b/tests/boundary/filesystem/artifactInventorySnapshot.test.ts @@ -85,9 +85,10 @@ describe("artifact inventory snapshot", () => { kind: "artifact-inventory", inventory: { manifest: { occurrence_count: 2 }, - occurrences: expect.arrayContaining([ - expect.objectContaining({ logical_path: "observed.txt" }), - ]), + occurrences: [ + { logical_path: "." }, + { logical_path: "observed.txt" }, + ], }, }, },