fix(draw-image): per-process temp dir in renderToPng test helpers (#184)
* fix(draw-image): per-process temp dir in renderToPng test helpers * docs(handoff): add handoff and ADR for renderToPng temp dir fix * docs(project-map): note per-process temp dir fix in renderToPng helpers * docs(handoff): set PR number * docs(handoff): replace PR number placeholder in frontmatter --------- Co-authored-by: opencode-agent <agent@opencode.local>
This commit is contained in:
parent
0bf97cf1c5
commit
8c00346b76
5 changed files with 87 additions and 16 deletions
|
|
@ -1,6 +1,7 @@
|
||||||
import { describe, test, expect, beforeAll } from "vitest"
|
import { describe, test, expect, beforeAll } from "vitest"
|
||||||
import { existsSync, readFileSync, rmSync, mkdirSync } from "node:fs"
|
import { existsSync, readFileSync, rmSync, mkdirSync, writeFileSync, mkdtempSync } from "node:fs"
|
||||||
import path from "node:path"
|
import path from "node:path"
|
||||||
|
import os from "node:os"
|
||||||
import { fileURLToPath } from "node:url"
|
import { fileURLToPath } from "node:url"
|
||||||
import { spawnSync } from "node:child_process"
|
import { spawnSync } from "node:child_process"
|
||||||
import { loadBrand } from "../src/config"
|
import { loadBrand } from "../src/config"
|
||||||
|
|
@ -16,12 +17,17 @@ beforeAll(() => {
|
||||||
})
|
})
|
||||||
|
|
||||||
function renderToPng(svg: string, outPath: string): void {
|
function renderToPng(svg: string, outPath: string): void {
|
||||||
const tmpSvg = path.join(TMP, "input.svg")
|
const tmpDir = mkdtempSync(path.join(os.tmpdir(), `draw-image-test-${process.pid}-`))
|
||||||
require("node:fs").writeFileSync(tmpSvg, svg)
|
const tmpSvg = path.join(tmpDir, "input.svg")
|
||||||
|
try {
|
||||||
|
writeFileSync(tmpSvg, svg)
|
||||||
const r = spawnSync("node", [path.join(DRAW_IMAGE_DIR, "render.mjs"), tmpSvg, outPath], {
|
const r = spawnSync("node", [path.join(DRAW_IMAGE_DIR, "render.mjs"), tmpSvg, outPath], {
|
||||||
encoding: "utf-8",
|
encoding: "utf-8",
|
||||||
})
|
})
|
||||||
if (r.status !== 0) throw new Error(`render.mjs failed: ${r.stderr || r.stdout}`)
|
if (r.status !== 0) throw new Error(`render.mjs failed: ${r.stderr || r.stdout}`)
|
||||||
|
} finally {
|
||||||
|
if (existsSync(tmpDir)) rmSync(tmpDir, { recursive: true, force: true })
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
describe("integration — render pipeline", () => {
|
describe("integration — render pipeline", () => {
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import { describe, test, expect, beforeAll } from "vitest"
|
import { describe, test, expect, beforeAll } from "vitest"
|
||||||
import { existsSync, readFileSync, rmSync, mkdirSync, writeFileSync } from "node:fs"
|
import { existsSync, readFileSync, rmSync, mkdirSync, writeFileSync, mkdtempSync } from "node:fs"
|
||||||
import path from "node:path"
|
import path from "node:path"
|
||||||
|
import os from "node:os"
|
||||||
import { fileURLToPath } from "node:url"
|
import { fileURLToPath } from "node:url"
|
||||||
import { spawnSync } from "node:child_process"
|
import { spawnSync } from "node:child_process"
|
||||||
import { loadBrand } from "../src/config"
|
import { loadBrand } from "../src/config"
|
||||||
|
|
@ -16,12 +17,17 @@ beforeAll(() => {
|
||||||
})
|
})
|
||||||
|
|
||||||
function renderToPng(svg: string, outPath: string): void {
|
function renderToPng(svg: string, outPath: string): void {
|
||||||
const tmpSvg = path.join(TMP, "input.svg")
|
const tmpDir = mkdtempSync(path.join(os.tmpdir(), `draw-image-test-${process.pid}-`))
|
||||||
|
const tmpSvg = path.join(tmpDir, "input.svg")
|
||||||
|
try {
|
||||||
writeFileSync(tmpSvg, svg)
|
writeFileSync(tmpSvg, svg)
|
||||||
const r = spawnSync("node", [path.join(DRAW_IMAGE_DIR, "render.mjs"), tmpSvg, outPath], {
|
const r = spawnSync("node", [path.join(DRAW_IMAGE_DIR, "render.mjs"), tmpSvg, outPath], {
|
||||||
encoding: "utf-8",
|
encoding: "utf-8",
|
||||||
})
|
})
|
||||||
if (r.status !== 0) throw new Error(`render.mjs failed: ${r.stderr || r.stdout}`)
|
if (r.status !== 0) throw new Error(`render.mjs failed: ${r.stderr || r.stdout}`)
|
||||||
|
} finally {
|
||||||
|
if (existsSync(tmpDir)) rmSync(tmpDir, { recursive: true, force: true })
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function assertPng(filePath: string) {
|
function assertPng(filePath: string) {
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,33 @@
|
||||||
|
# ADR-080: Per-process temp dir in renderToPng test helpers
|
||||||
|
|
||||||
|
## Статус
|
||||||
|
Accepted (2026-07-31)
|
||||||
|
|
||||||
|
## Контекст
|
||||||
|
|
||||||
|
Тестовые хелперы `renderToPng` в `tests/render.integration.test.ts` и `tests/render.optional.integration.test.ts` рендерили SVG через двухфазный процесс:
|
||||||
|
1. `writeFileSync(path.join(TMP, "input.svg"), svg)` — запись в единственный fixed-path файл в общем `TMP`-каталоге (`/tmp/draw-image-integration`, общий для всех параллельных vitest-процессов).
|
||||||
|
2. `spawnSync("node", ["render.mjs", tmpSvg, outPath])` — дочерний процесс читает файл и рендерит PNG через sharp.
|
||||||
|
|
||||||
|
При параллельном запуске нескольких vitest-процессов (внешняя параллель — `npx vitest run` × N concurrently) процессы разделяли один `input.svg`: один процесс обнулял/перезаписывал файл, пока другой его читал → `render.mjs` падал с `Input Buffer is empty`. Это тестовый аналог бага #160 (продакшн `cli.ts`), но в хелпере, а не в CLI. Воспроизведено: 10 параллельных прогонов → run 9 упал с `Input Buffer is empty`.
|
||||||
|
|
||||||
|
## Решение
|
||||||
|
|
||||||
|
Использовать уникальный temp-каталог на каждый вызов `renderToPng` через `mkdtempSync` под системным `os.tmpdir()` с PID в префиксе:
|
||||||
|
```ts
|
||||||
|
const tmpDir = mkdtempSync(path.join(os.tmpdir(), `draw-image-test-${process.pid}-`))
|
||||||
|
const tmpSvg = path.join(tmpDir, "input.svg")
|
||||||
|
try {
|
||||||
|
writeFileSync(tmpSvg, svg)
|
||||||
|
// ... spawnSync render.mjs ...
|
||||||
|
} finally {
|
||||||
|
if (existsSync(tmpDir)) rmSync(tmpDir, { recursive: true, force: true })
|
||||||
|
}
|
||||||
|
```
|
||||||
|
и удалять каталог в `finally`-блоке после рендера (cleanup гарантирован даже при `throw`). `render.mjs` не изменён — он уже принимает path к SVG как argv, уникальность обеспечивает хелпер.
|
||||||
|
|
||||||
|
## Альтернативы
|
||||||
|
|
||||||
|
- **PID-суффикс на файл вместо каталога**: `path.join(os.tmpdir(), \`draw-image-test-input-\${process.pid}.svg\`)` — как в `cli.ts` (#160). Достаточен для race между процессами, но НЕ решает race между threads/воркерами одного vitest-процесса (один `process.pid`, несколько параллельных вызовов `renderToPng`). `mkdtempSync` даёт уникальность на каждый вызов, закрывая оба случая (граничный случай из спеки #177).
|
||||||
|
- **Уникальный каталог на процесс без `mkdtempSync`**: `path.join(os.tmpdir(), \`draw-image-test-\${process.pid}\`)` + `mkdirSync` — требует ручной гарантии уникальности и cleanup; `mkdtempSync` атомарно создаёт уникальный каталог за один вызов.
|
||||||
|
- **Передача SVG через stdin в `render.mjs`**: избегает temp-файла целиком, но требует переписывания `render.mjs` (чтение из stdin вместо `readFileSync(argv)`), что выходит за рамки спеки #177 и ломает контракт «`render.mjs` принимает path как argv».
|
||||||
26
docs/handoff/pr-184-draw-image-test-helper-input-svg-race.md
Normal file
26
docs/handoff/pr-184-draw-image-test-helper-input-svg-race.md
Normal file
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
pr: 184
|
||||||
|
title: fix(draw-image): per-process temp dir in renderToPng test helpers
|
||||||
|
---
|
||||||
|
|
||||||
|
## Что сделано
|
||||||
|
|
||||||
|
- `tests/render.integration.test.ts`: хелпер `renderToPng` переписан — вместо общего `path.join(TMP, "input.svg")` (где `TMP = "/tmp/draw-image-integration"` — общий для всех параллельных vitest-процессов) используется уникальный temp-каталог на каждый вызов через `mkdtempSync(path.join(os.tmpdir(), \`draw-image-test-${process.pid}-\`))`, внутри которого пишется `input.svg`.
|
||||||
|
- `tests/render.integration.test.ts`: добавлен `finally`-блок с `rmSync(tmpDir, { recursive: true, force: true })` — temp-каталог удаляется после рендера даже при ошибке/exit.
|
||||||
|
- `tests/render.integration.test.ts`: импорты очищены — `require("node:fs")` заменён на статический `writeFileSync`/`mkdtempSync` из `node:fs`, добавлен `import os from "node:os"`.
|
||||||
|
- `tests/render.optional.integration.test.ts`: аналогичный фикс хелпера `renderToPng` (тот же паттерн `TMP/input.svg`) — уникальный temp-каталог через `mkdtempSync` с PID + cleanup в `finally`.
|
||||||
|
|
||||||
|
## Почему
|
||||||
|
|
||||||
|
Тестовый хелпер `renderToPng` в обоих integration-тестах писал временный SVG в жёстко заданный `path.join(TMP, "input.svg")` где `TMP` — общий каталог для всех параллельных vitest-процессов. При параллельном запуске (CI матрица, `npx vitest run` × N в параллель, несколько vitest-воркеров) процессы конкурировали за один `input.svg`: один процесс обнулял/перезаписывал файл через `writeFileSync`, пока другой его читал через `render.mjs` → `render.mjs` падал с `Input Buffer is empty`. Это аналогично багу #160 (продакшн `cli.ts`), но в тестовом хелпере. Уникальный temp-каталог на вызов (`mkdtempSync` с PID в префиксе) + cleanup в `finally` устраняют race и гарантированно удаляют leftover. `mkdtempSync` выбран вместо PID-суффикса на файл, т.к. решает и race между процессами, и между threads одного процесса (каждый вызов `renderToPng` = свой уникальный каталог).
|
||||||
|
|
||||||
|
## Pending
|
||||||
|
|
||||||
|
—
|
||||||
|
|
||||||
|
## Watch out
|
||||||
|
|
||||||
|
- **Баг вне scope (создан issue #183)**: при верификации параллельным запуском 10× `npx vitest run` обнаружен отдельный race на общих output-каталогах `beforeAll` (`/tmp/draw-image-integration`, `/tmp/draw-image-cleanup`, `/tmp/draw-image-idempotency` и т.д.) — `beforeAll` делает `rmSync(TMP, recursive)` и процесс удаляет output PNG другого процесса → `existsSync(outPath)` = false / idempotency `skipped` вместо `rendered` / `unable to open for write`. Этот race НЕ связан с `input.svg`/`Input Buffer is empty` и чинится отдельно в #183. Критерий приёмки #177 выполнен: `Input Buffer is empty` полностью устранён во всех 10 параллельных прогонах.
|
||||||
|
- **Одиночный прогон ×10 стабильно зелёный** (58/58 каждый прогон) — проверено последовательно.
|
||||||
|
- `render.mjs` не изменён (контракт «принимает path к SVG как argv» сохранён).
|
||||||
|
- Продакшн-код `cli.ts` не тронут (исправлен в #160).
|
||||||
|
|
@ -87,14 +87,14 @@ opencode-config/
|
||||||
│ │ ├── fit.test.ts
|
│ │ ├── fit.test.ts
|
||||||
│ │ ├── recolor.test.ts
|
│ │ ├── recolor.test.ts
|
||||||
│ │ ├── cli.test.ts
|
│ │ ├── cli.test.ts
|
||||||
│ │ ├── render.integration.test.ts
|
│ │ ├── render.integration.test.ts # integration: PNG 1024x1024 + recursive out dir; renderToPng → per-process mkdtempSync tmp dir + finally cleanup (PR#177)
|
||||||
│ │ ├── idempotency.test.ts
|
│ │ ├── idempotency.test.ts
|
||||||
│ │ ├── slot.test.ts
|
│ │ ├── slot.test.ts
|
||||||
│ │ ├── e2e.test.ts
|
│ │ ├── e2e.test.ts
|
||||||
│ │ ├── e2e.no-icon.test.ts
|
│ │ ├── e2e.no-icon.test.ts
|
||||||
│ │ ├── e2e.bad-input.test.ts
|
│ │ ├── e2e.bad-input.test.ts
|
||||||
│ │ ├── render.optional.test.ts # unit: empty badge → no rect, empty subtitle → no text (PR#144)
|
│ │ ├── render.optional.test.ts # unit: empty badge → no rect, empty subtitle → no text (PR#144)
|
||||||
│ │ ├── render.optional.integration.test.ts # integration: PNG valid with/without subtitle+badge (PR#144)
|
│ │ ├── render.optional.integration.test.ts # integration: PNG valid with/without subtitle+badge (PR#144); renderToPng → per-process mkdtempSync tmp dir + finally cleanup (PR#177)
|
||||||
│ │ ├── e2e.optional.test.ts # e2e: title-only CLI → exit 0, clean SVG (PR#144); хелпер переписан — buildSvg напрямую вместо чтения leftover .tmp-render.svg (PR#178)
|
│ │ ├── e2e.optional.test.ts # e2e: title-only CLI → exit 0, clean SVG (PR#144); хелпер переписан — buildSvg напрямую вместо чтения leftover .tmp-render.svg (PR#178)
|
||||||
│ │ └── cleanup.test.ts # unit: temp SVG cleanup — no .tmp-render.svg leftover, PID-based path under os.tmpdir, no stray .svg (PR#178)
|
│ │ └── cleanup.test.ts # unit: temp SVG cleanup — no .tmp-render.svg leftover, PID-based path under os.tmpdir, no stray .svg (PR#178)
|
||||||
│ ├── telegram/ # Telegram Bot API CLI-проект (по паттерну draw-image: plugin-tool + CLI, без runtime-deps) — PR#174
|
│ ├── telegram/ # Telegram Bot API CLI-проект (по паттерну draw-image: plugin-tool + CLI, без runtime-deps) — PR#174
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue