From 4a15dfe971140770e55f9ea0608ca69014f2203c Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 30 May 2026 23:50:28 +0900 Subject: [PATCH] test(look-at): inject image conversion command runner --- src/tools/look-at/image-converter.test.ts | 170 ++++++++++------------ src/tools/look-at/image-converter.ts | 29 +++- 2 files changed, 107 insertions(+), 92 deletions(-) diff --git a/src/tools/look-at/image-converter.test.ts b/src/tools/look-at/image-converter.test.ts index 12efaacc3..c025ffdd9 100644 --- a/src/tools/look-at/image-converter.test.ts +++ b/src/tools/look-at/image-converter.test.ts @@ -1,16 +1,21 @@ -import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" -import * as childProcess from "node:child_process" +import { describe, expect, test } from "bun:test" import { existsSync, mkdtempSync, writeFileSync, unlinkSync, rmSync } from "node:fs" import { tmpdir } from "node:os" import { dirname, join } from "node:path" type ImageConverterModule = typeof import("./image-converter") +type CommandRunner = Parameters[0] +type CommandCall = { + readonly command: Parameters[0] + readonly args: Parameters[1] + readonly options: Parameters[2] +} async function loadImageConverter(): Promise { return import(`./image-converter?test=${Date.now()}-${Math.random()}`) } -function writeConvertedOutput(command: string, args: string[]): void { +function writeConvertedOutput(command: string, args: ReadonlyArray): void { if (command === "sips") { const outIndex = args.indexOf("--out") const outputPath = outIndex >= 0 ? args[outIndex + 1] : undefined @@ -21,15 +26,42 @@ function writeConvertedOutput(command: string, args: string[]): void { } if (command === "convert") { - writeFileSync(args[2], "jpeg") + const outputPath = args[2] + if (outputPath) { + writeFileSync(outputPath, "jpeg") + } return } if (command === "magick") { - writeFileSync(args[2], "jpeg") + const outputPath = args[2] + if (outputPath) { + writeFileSync(outputPath, "jpeg") + } } } +function installCommandRunner( + imageConverter: ImageConverterModule, + runCommand: (command: string, args: ReadonlyArray) => void = writeConvertedOutput, +): { readonly calls: CommandCall[]; readonly restore: () => void } { + const calls: CommandCall[] = [] + const restore = imageConverter.setImageConversionCommandRunnerForTesting((command, args, options) => { + calls.push({ command, args: [...args], options }) + runCommand(command, args) + }) + + return { calls, restore } +} + +function getTemporaryOutputPath(error: unknown): string { + if (error instanceof Error && "temporaryOutputPath" in error && typeof error.temporaryOutputPath === "string") { + return error.temporaryOutputPath + } + + throw new Error("Expected conversion error to include a temporary output path") +} + async function withMockPlatform( platform: NodeJS.Platform, run: () => TValue | Promise, @@ -51,50 +83,25 @@ async function withMockPlatform( } describe("image-converter command execution safety", () => { - let execFileSyncSpy: ReturnType - let execSyncSpy: ReturnType - - beforeEach(() => { - execSyncSpy = spyOn(childProcess, "execSync").mockImplementation(() => { - throw new Error("execSync should not be called") - }) - - execFileSyncSpy = spyOn(childProcess, "execFileSync").mockImplementation( - ((_command: string, _args: string[], _options?: unknown) => "") as typeof childProcess.execFileSync, - ) - }) - - afterEach(() => { - execFileSyncSpy.mockRestore() - execSyncSpy.mockRestore() - }) - test("uses execFileSync with argument arrays for conversion commands", async () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-test-")) const inputPath = join(testDir, "evil$(touch_pwn).heic") writeFileSync(inputPath, "fake-heic-data") - const { convertImageToJpeg } = await loadImageConverter() + const imageConverter = await loadImageConverter() + const { calls, restore } = installCommandRunner(imageConverter) - execFileSyncSpy.mockImplementation( - ((command: string, args: string[]) => { - writeConvertedOutput(command, args) - return "" - }) as typeof childProcess.execFileSync, - ) + const outputPath = imageConverter.convertImageToJpeg(inputPath, "image/heic") + restore() - const outputPath = convertImageToJpeg(inputPath, "image/heic") - - expect(execSyncSpy).not.toHaveBeenCalled() - expect(execFileSyncSpy).toHaveBeenCalled() - - const [firstCommand, firstArgs] = execFileSyncSpy.mock.calls[0] as [string, string[]] - expect(typeof firstCommand).toBe("string") - expect(Array.isArray(firstArgs)).toBe(true) - expect(["sips", "convert", "magick"]).toContain(firstCommand) - expect(firstArgs).toContain("--") - expect(firstArgs).toContain(inputPath) - expect(firstArgs.indexOf("--") < firstArgs.indexOf(inputPath)).toBe(true) - expect(firstArgs.join(" ")).not.toContain(`"${inputPath}"`) + const firstCall = calls[0] + expect(firstCall).toBeDefined() + expect(typeof firstCall?.command).toBe("string") + expect(Array.isArray(firstCall?.args)).toBe(true) + expect(["sips", "convert", "magick"]).toContain(firstCall?.command) + expect(firstCall?.args).toContain("--") + expect(firstCall?.args).toContain(inputPath) + expect((firstCall?.args.indexOf("--") ?? Number.MAX_SAFE_INTEGER) < (firstCall?.args.indexOf(inputPath) ?? -1)).toBe(true) + expect(firstCall?.args.join(" ")).not.toContain(`"${inputPath}"`) expect(existsSync(outputPath)).toBe(true) @@ -107,21 +114,16 @@ describe("image-converter command execution safety", () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-cleanup-test-")) const inputPath = join(testDir, "photo.heic") writeFileSync(inputPath, "fake-heic-data") - const { convertImageToJpeg, cleanupConvertedImage } = await loadImageConverter() + const imageConverter = await loadImageConverter() + const { restore } = installCommandRunner(imageConverter) - execFileSyncSpy.mockImplementation( - ((command: string, args: string[]) => { - writeConvertedOutput(command, args) - return "" - }) as typeof childProcess.execFileSync, - ) - - const outputPath = convertImageToJpeg(inputPath, "image/heic") + const outputPath = imageConverter.convertImageToJpeg(inputPath, "image/heic") + restore() const conversionDirectory = dirname(outputPath) expect(existsSync(conversionDirectory)).toBe(true) - cleanupConvertedImage(outputPath) + imageConverter.cleanupConvertedImage(outputPath) expect(existsSync(conversionDirectory)).toBe(false) @@ -134,26 +136,19 @@ describe("image-converter command execution safety", () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-platform-test-")) const inputPath = join(testDir, "photo.heic") writeFileSync(inputPath, "fake-heic-data") - const { convertImageToJpeg, cleanupConvertedImage } = await loadImageConverter() + const imageConverter = await loadImageConverter() + const { calls, restore } = installCommandRunner(imageConverter) - execFileSyncSpy.mockImplementation( - ((command: string, args: string[]) => { - if (command === "magick") { - writeFileSync(args[2], "jpeg") - } - return "" - }) as typeof childProcess.execFileSync, - ) + const outputPath = imageConverter.convertImageToJpeg(inputPath, "image/heic") + restore() - const outputPath = convertImageToJpeg(inputPath, "image/heic") - - const [command, args] = execFileSyncSpy.mock.calls[0] as [string, string[]] - expect(command).toBe("magick") - expect(args).toContain("--") - expect(args.indexOf("--") < args.indexOf(inputPath)).toBe(true) + const firstCall = calls[0] + expect(firstCall?.command).toBe("magick") + expect(firstCall?.args).toContain("--") + expect((firstCall?.args.indexOf("--") ?? Number.MAX_SAFE_INTEGER) < (firstCall?.args.indexOf(inputPath) ?? -1)).toBe(true) expect(existsSync(outputPath)).toBe(true) - cleanupConvertedImage(outputPath) + imageConverter.cleanupConvertedImage(outputPath) if (existsSync(inputPath)) unlinkSync(inputPath) rmSync(testDir, { recursive: true, force: true }) }) @@ -163,23 +158,18 @@ describe("image-converter command execution safety", () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-timeout-test-")) const inputPath = join(testDir, "photo.heic") writeFileSync(inputPath, "fake-heic-data") - const { convertImageToJpeg, cleanupConvertedImage } = await loadImageConverter() + const imageConverter = await loadImageConverter() + const { calls, restore } = installCommandRunner(imageConverter) - execFileSyncSpy.mockImplementation( - ((command: string, args: string[]) => { - writeConvertedOutput(command, args) - return "" - }) as typeof childProcess.execFileSync, - ) + const outputPath = imageConverter.convertImageToJpeg(inputPath, "image/heic") + restore() - const outputPath = convertImageToJpeg(inputPath, "image/heic") + const firstCall = calls[0] + expect(firstCall?.options).toBeDefined() + expect(typeof firstCall?.options.timeout).toBe("number") + expect((firstCall?.options.timeout ?? 0) > 0).toBe(true) - const options = execFileSyncSpy.mock.calls[0]?.[2] as { timeout?: number } | undefined - expect(options).toBeDefined() - expect(typeof options?.timeout).toBe("number") - expect((options?.timeout ?? 0) > 0).toBe(true) - - cleanupConvertedImage(outputPath) + imageConverter.cleanupConvertedImage(outputPath) if (existsSync(inputPath)) unlinkSync(inputPath) rmSync(testDir, { recursive: true, force: true }) }) @@ -189,22 +179,22 @@ describe("image-converter command execution safety", () => { const testDir = mkdtempSync(join(tmpdir(), "img-converter-failure-test-")) const inputPath = join(testDir, "photo.heic") writeFileSync(inputPath, "fake-heic-data") - const { convertImageToJpeg } = await loadImageConverter() + const imageConverter = await loadImageConverter() - execFileSyncSpy.mockImplementation((() => { + const { restore } = installCommandRunner(imageConverter, () => { throw new Error("conversion process failed") - }) as typeof childProcess.execFileSync) + }) - const runConversion = () => convertImageToJpeg(inputPath, "image/heic") + const runConversion = () => imageConverter.convertImageToJpeg(inputPath, "image/heic") expect(runConversion).toThrow("No image conversion tool available") try { runConversion() } catch (error) { - const conversionError = error as Error & { temporaryOutputPath?: string } - expect(conversionError.temporaryOutputPath).toBeDefined() - expect(conversionError.temporaryOutputPath?.endsWith("converted.jpg")).toBe(true) + const temporaryOutputPath = getTemporaryOutputPath(error) + expect(temporaryOutputPath.endsWith("converted.jpg")).toBe(true) } + restore() if (existsSync(inputPath)) unlinkSync(inputPath) rmSync(testDir, { recursive: true, force: true }) diff --git a/src/tools/look-at/image-converter.ts b/src/tools/look-at/image-converter.ts index 163e35e28..76d7c9c35 100644 --- a/src/tools/look-at/image-converter.ts +++ b/src/tools/look-at/image-converter.ts @@ -34,6 +34,31 @@ const UNSUPPORTED_FORMATS = new Set([ const CONVERSION_TIMEOUT_MS = 30_000 +type ImageConversionCommandOptions = { + readonly stdio: "pipe" + readonly encoding: "utf-8" + readonly timeout: number +} + +export type ImageConversionCommandRunner = ( + command: string, + args: string[], + options: ImageConversionCommandOptions, +) => void + +let imageConversionCommandRunner: ImageConversionCommandRunner = (command, args, options) => { + childProcess.execFileSync(command, args, options) +} + +export function setImageConversionCommandRunnerForTesting(runner: ImageConversionCommandRunner): () => void { + const previousRunner = imageConversionCommandRunner + imageConversionCommandRunner = runner + + return () => { + imageConversionCommandRunner = previousRunner + } +} + export function needsConversion(mimeType: string): boolean { if (SUPPORTED_FORMATS.has(mimeType)) { return false @@ -59,7 +84,7 @@ export function convertImageToJpeg(inputPath: string, mimeType: string): string try { if (process.platform === "darwin") { try { - childProcess.execFileSync("sips", ["-s", "format", "jpeg", "--", inputPath, "--out", outputPath], { + imageConversionCommandRunner("sips", ["-s", "format", "jpeg", "--", inputPath, "--out", outputPath], { stdio: "pipe", encoding: "utf-8", timeout: CONVERSION_TIMEOUT_MS, @@ -76,7 +101,7 @@ export function convertImageToJpeg(inputPath: string, mimeType: string): string try { const imagemagickCommand = process.platform === "darwin" ? "convert" : "magick" - childProcess.execFileSync(imagemagickCommand, ["--", inputPath, outputPath], { + imageConversionCommandRunner(imagemagickCommand, ["--", inputPath, outputPath], { stdio: "pipe", encoding: "utf-8", timeout: CONVERSION_TIMEOUT_MS,