test(look-at): inject image conversion command runner

src/tools/look-at/image-converter.test.ts

@@ -1,16 +1,21 @@
-import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"
+import { describe, expect, test } from "bun:test"
-import * as childProcess from "node:child_process"
 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<ImageConverterModule["setImageConversionCommandRunnerForTesting"]>[0]
+type CommandCall = {
+  readonly command: Parameters<CommandRunner>[0]
+  readonly args: Parameters<CommandRunner>[1]
+  readonly options: Parameters<CommandRunner>[2]
+}
 
 async function loadImageConverter(): Promise<ImageConverterModule> {
   return import(`./image-converter?test=${Date.now()}-${Math.random()}`)
 }
 
-function writeConvertedOutput(command: string, args: string[]): void {
+function writeConvertedOutput(command: string, args: ReadonlyArray<string>): 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<string>) => 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<TValue>(
   platform: NodeJS.Platform,
   run: () => TValue | Promise<TValue>,
@@ -51,50 +83,25 @@ async function withMockPlatform<TValue>(
 }
 
 describe("image-converter command execution safety", () => {
-  let execFileSyncSpy: ReturnType<typeof spyOn>
-  let execSyncSpy: ReturnType<typeof spyOn>
-
-  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(
+    const outputPath = imageConverter.convertImageToJpeg(inputPath, "image/heic")
-      ((command: string, args: string[]) => {
+    restore()
-        writeConvertedOutput(command, args)
-        return ""
-      }) as typeof childProcess.execFileSync,
-    )
 
-    const outputPath = convertImageToJpeg(inputPath, "image/heic")
+    const firstCall = calls[0]
-
+    expect(firstCall).toBeDefined()
-    expect(execSyncSpy).not.toHaveBeenCalled()
+    expect(typeof firstCall?.command).toBe("string")
-    expect(execFileSyncSpy).toHaveBeenCalled()
+    expect(Array.isArray(firstCall?.args)).toBe(true)
-
+    expect(["sips", "convert", "magick"]).toContain(firstCall?.command)
-    const [firstCommand, firstArgs] = execFileSyncSpy.mock.calls[0] as [string, string[]]
+    expect(firstCall?.args).toContain("--")
-    expect(typeof firstCommand).toBe("string")
+    expect(firstCall?.args).toContain(inputPath)
-    expect(Array.isArray(firstArgs)).toBe(true)
+    expect((firstCall?.args.indexOf("--") ?? Number.MAX_SAFE_INTEGER) < (firstCall?.args.indexOf(inputPath) ?? -1)).toBe(true)
-    expect(["sips", "convert", "magick"]).toContain(firstCommand)
+    expect(firstCall?.args.join(" ")).not.toContain(`"${inputPath}"`)
-    expect(firstArgs).toContain("--")
-    expect(firstArgs).toContain(inputPath)
-    expect(firstArgs.indexOf("--") < firstArgs.indexOf(inputPath)).toBe(true)
-    expect(firstArgs.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(
+    const outputPath = imageConverter.convertImageToJpeg(inputPath, "image/heic")
-      ((command: string, args: string[]) => {
+    restore()
-        writeConvertedOutput(command, args)
-        return ""
-      }) as typeof childProcess.execFileSync,
-    )
-
-    const outputPath = convertImageToJpeg(inputPath, "image/heic")
     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(
+      const outputPath = imageConverter.convertImageToJpeg(inputPath, "image/heic")
-        ((command: string, args: string[]) => {
+      restore()
-          if (command === "magick") {
-            writeFileSync(args[2], "jpeg")
-          }
-          return ""
-        }) as typeof childProcess.execFileSync,
-      )
 
-      const outputPath = convertImageToJpeg(inputPath, "image/heic")
+      const firstCall = calls[0]
-
+      expect(firstCall?.command).toBe("magick")
-      const [command, args] = execFileSyncSpy.mock.calls[0] as [string, string[]]
+      expect(firstCall?.args).toContain("--")
-      expect(command).toBe("magick")
+      expect((firstCall?.args.indexOf("--") ?? Number.MAX_SAFE_INTEGER) < (firstCall?.args.indexOf(inputPath) ?? -1)).toBe(true)
-      expect(args).toContain("--")
-      expect(args.indexOf("--") < args.indexOf(inputPath)).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(
+    const outputPath = imageConverter.convertImageToJpeg(inputPath, "image/heic")
-      ((command: string, args: string[]) => {
+    restore()
-        writeConvertedOutput(command, args)
-        return ""
-      }) as typeof childProcess.execFileSync,
-    )
 
-    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
+    imageConverter.cleanupConvertedImage(outputPath)
-    expect(options).toBeDefined()
-    expect(typeof options?.timeout).toBe("number")
-    expect((options?.timeout ?? 0) > 0).toBe(true)
-
-    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 }
+        const temporaryOutputPath = getTemporaryOutputPath(error)
-        expect(conversionError.temporaryOutputPath).toBeDefined()
+        expect(temporaryOutputPath.endsWith("converted.jpg")).toBe(true)
-        expect(conversionError.temporaryOutputPath?.endsWith("converted.jpg")).toBe(true)
       }
+      restore()
 
       if (existsSync(inputPath)) unlinkSync(inputPath)
       rmSync(testDir, { recursive: true, force: true })

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,