diff --git a/packages/cli/src/commands/screenshot.ts b/packages/cli/src/commands/screenshot.ts index 67502140d..c2a6b2009 100644 --- a/packages/cli/src/commands/screenshot.ts +++ b/packages/cli/src/commands/screenshot.ts @@ -94,7 +94,7 @@ const MAX_RESERVE_ATTEMPTS = 1000; * driver should return base64 (explicit --path is handled separately; --base64 * opts out of a file entirely). */ -function getDefaultPathFromFlags(flags: { +export function getDefaultPathFromFlags(flags: { path?: string; base64?: boolean; type?: string; @@ -110,7 +110,7 @@ function getDefaultPathFromFlags(flags: { * the name already exists, so two concurrent runs can never claim the same * file. On EEXIST we advance the counter and try the next name. */ -function reserveDefaultScreenshotPath(type: string | undefined): string { +export function reserveDefaultScreenshotPath(type: string | undefined): string { const now = new Date(); const pad = (value: number) => String(value).padStart(2, "0"); const stamp = @@ -137,7 +137,7 @@ function reserveDefaultScreenshotPath(type: string | undefined): string { // Removes the reserved placeholder when the screenshot failed before the driver // wrote to it. `path` is always a file we created via openSync above, so the // isFile guard is just defensive against an unexpected directory/symlink. -function removeIfEmpty(path: string): void { +export function removeIfEmpty(path: string): void { try { const stats = statSync(path); if (stats.isFile() && stats.size === 0) unlinkSync(path); diff --git a/packages/cli/tests/screenshot-default-path.test.ts b/packages/cli/tests/screenshot-default-path.test.ts new file mode 100644 index 000000000..eeaaa8bea --- /dev/null +++ b/packages/cli/tests/screenshot-default-path.test.ts @@ -0,0 +1,98 @@ +import { + closeSync, + existsSync, + mkdtempSync, + openSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + getDefaultPathFromFlags, + removeIfEmpty, + reserveDefaultScreenshotPath, +} from "../src/commands/screenshot.js"; + +/** + * Coverage for the default screenshot path behavior added in #2246 (browse + * screenshot now writes a file by default with a collision counter, cleaning up + * the reserved placeholder on failure). The command shipped without tests. + */ +describe("browse screenshot default-path handling", () => { + let tmp: string; + let originalCwd: string; + + beforeEach(() => { + originalCwd = process.cwd(); + tmp = mkdtempSync(join(tmpdir(), "browse-screenshot-")); + process.chdir(tmp); + }); + + afterEach(() => { + vi.useRealTimers(); + process.chdir(originalCwd); + rmSync(tmp, { recursive: true, force: true }); + }); + + describe("getDefaultPathFromFlags", () => { + it("returns undefined when --path is set (driver writes the explicit file)", () => { + expect(getDefaultPathFromFlags({ path: "out.png" })).toBeUndefined(); + }); + + it("returns undefined when --base64 is set (nothing written to disk)", () => { + expect(getDefaultPathFromFlags({ base64: true })).toBeUndefined(); + }); + + it("reserves a default .png path for a bare invocation", () => { + const reserved = getDefaultPathFromFlags({}); + expect(reserved).toMatch(/screenshot-\d{8}-\d{6}\.png$/); + // The path is reserved by creating an empty placeholder. + expect(existsSync(reserved as string)).toBe(true); + }); + + it("uses the .jpeg extension when --type jpeg", () => { + const reserved = getDefaultPathFromFlags({ type: "jpeg" }); + expect(reserved).toMatch(/screenshot-\d{8}-\d{6}\.jpeg$/); + }); + }); + + describe("reserveDefaultScreenshotPath", () => { + it("advances the collision counter instead of overwriting an existing file", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(2026, 6, 1, 15, 30, 45)); // 2026-07-01 15:30:45 local + + const first = reserveDefaultScreenshotPath(undefined); + const second = reserveDefaultScreenshotPath(undefined); + + expect(first).toMatch(/screenshot-20260701-153045\.png$/); + expect(second).toMatch(/screenshot-20260701-153045-2\.png$/); + expect(first).not.toBe(second); + expect(existsSync(first)).toBe(true); + expect(existsSync(second)).toBe(true); + }); + }); + + describe("removeIfEmpty", () => { + it("removes a zero-byte placeholder left by a failed capture", () => { + const path = join(tmp, "placeholder.png"); + closeSync(openSync(path, "w")); // 0 bytes + removeIfEmpty(path); + expect(existsSync(path)).toBe(false); + }); + + it("keeps a file that already has bytes (a real screenshot)", () => { + const path = join(tmp, "real.png"); + writeFileSync(path, "not empty"); + removeIfEmpty(path); + expect(existsSync(path)).toBe(true); + }); + + it("does not throw when the path is already gone", () => { + expect(() => removeIfEmpty(join(tmp, "missing.png"))).not.toThrow(); + }); + }); +});