fix(draw-image): per-process output TMP in beforeAll for parallel vitest (#186)
* fix(draw-image): per-process output TMP in integration tests * fix(draw-image): per-process output TMP in e2e tests * fix(draw-image): per-process output TMP in cleanup test * docs(handoff): add handoff and ADR for beforeAll output TMP fix * docs(handoff): set PR number * docs(handoff): set PR number 186 --------- Co-authored-by: opencode-agent <agent@opencode.local>
This commit is contained in:
parent
8c00346b76
commit
72caab4097
9 changed files with 77 additions and 13 deletions
|
|
@ -7,7 +7,7 @@ import { spawnSync } from "node:child_process"
|
||||||
|
|
||||||
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
||||||
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
||||||
const TMP = "/tmp/draw-image-cleanup"
|
const TMP = path.join(os.tmpdir(), `draw-image-cleanup-${process.pid}`)
|
||||||
|
|
||||||
beforeAll(() => {
|
beforeAll(() => {
|
||||||
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
|
|
|
||||||
|
|
@ -1,19 +1,24 @@
|
||||||
import { describe, test, expect, beforeAll } from "vitest"
|
import { describe, test, expect, beforeAll, afterAll } from "vitest"
|
||||||
import { existsSync, rmSync, mkdirSync, writeFileSync, copyFileSync, readFileSync } from "node:fs"
|
import { existsSync, rmSync, mkdirSync, writeFileSync, copyFileSync, readFileSync } 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 { validateBrand } from "../src/config"
|
import { validateBrand } from "../src/config"
|
||||||
|
|
||||||
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
||||||
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
||||||
const TMP = "/tmp/draw-image-bad-input"
|
const TMP = path.join(os.tmpdir(), `draw-image-bad-input-${process.pid}`)
|
||||||
|
|
||||||
beforeAll(() => {
|
beforeAll(() => {
|
||||||
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
mkdirSync(TMP, { recursive: true })
|
mkdirSync(TMP, { recursive: true })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
|
})
|
||||||
|
|
||||||
describe("e2e — bad input (unit-level, no file mutation)", () => {
|
describe("e2e — bad input (unit-level, no file mutation)", () => {
|
||||||
test("invalid brand.json rejected by validateBrand", () => {
|
test("invalid brand.json rejected by validateBrand", () => {
|
||||||
expect(() => validateBrand({ base: "#0a0a0a", surface: "#121212", fg: "#ededed", muted: "#a1a1aa" }))
|
expect(() => validateBrand({ base: "#0a0a0a", surface: "#121212", fg: "#ededed", muted: "#a1a1aa" }))
|
||||||
|
|
|
||||||
|
|
@ -1,18 +1,23 @@
|
||||||
import { describe, test, expect, beforeAll } from "vitest"
|
import { describe, test, expect, beforeAll, afterAll } from "vitest"
|
||||||
import { existsSync, readFileSync, rmSync, mkdirSync } from "node:fs"
|
import { existsSync, readFileSync, rmSync, mkdirSync } 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"
|
||||||
|
|
||||||
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
||||||
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
||||||
const TMP = "/tmp/draw-image-e2e-no-icon"
|
const TMP = path.join(os.tmpdir(), `draw-image-e2e-no-icon-${process.pid}`)
|
||||||
|
|
||||||
beforeAll(() => {
|
beforeAll(() => {
|
||||||
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
mkdirSync(TMP, { recursive: true })
|
mkdirSync(TMP, { recursive: true })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
|
})
|
||||||
|
|
||||||
function runCli(args: string[]): { status: number; stdout: string; stderr: string } {
|
function runCli(args: string[]): { status: number; stdout: string; stderr: string } {
|
||||||
return spawnSync("node", ["--experimental-strip-types", path.join(DRAW_IMAGE_DIR, "cli.ts"), ...args], {
|
return spawnSync("node", ["--experimental-strip-types", path.join(DRAW_IMAGE_DIR, "cli.ts"), ...args], {
|
||||||
encoding: "utf-8",
|
encoding: "utf-8",
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import { describe, test, expect, beforeAll } from "vitest"
|
import { describe, test, expect, beforeAll, afterAll } from "vitest"
|
||||||
import { existsSync, readFileSync, rmSync, mkdirSync } from "node:fs"
|
import { existsSync, readFileSync, rmSync, mkdirSync } 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"
|
||||||
|
|
@ -8,13 +9,17 @@ import { loadTemplate, buildSvg } from "../src/render"
|
||||||
|
|
||||||
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
||||||
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
||||||
const TMP = "/tmp/draw-image-optional-e2e"
|
const TMP = path.join(os.tmpdir(), `draw-image-optional-e2e-${process.pid}`)
|
||||||
|
|
||||||
beforeAll(() => {
|
beforeAll(() => {
|
||||||
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
mkdirSync(TMP, { recursive: true })
|
mkdirSync(TMP, { recursive: true })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
|
})
|
||||||
|
|
||||||
function runCli(args: string[]): { status: number; stdout: string; stderr: string } {
|
function runCli(args: string[]): { status: number; stdout: string; stderr: string } {
|
||||||
return spawnSync("node", ["--experimental-strip-types", path.join(DRAW_IMAGE_DIR, "cli.ts"), ...args], {
|
return spawnSync("node", ["--experimental-strip-types", path.join(DRAW_IMAGE_DIR, "cli.ts"), ...args], {
|
||||||
encoding: "utf-8",
|
encoding: "utf-8",
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import { describe, test, expect, beforeAll } from "vitest"
|
import { describe, test, expect, beforeAll, afterAll } from "vitest"
|
||||||
import { existsSync, readFileSync, rmSync, mkdirSync, writeFileSync } from "node:fs"
|
import { existsSync, readFileSync, rmSync, mkdirSync, writeFileSync } 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"
|
||||||
|
|
@ -8,13 +9,17 @@ import { loadTemplate, buildSvg, computeHash } from "../src/render"
|
||||||
|
|
||||||
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
||||||
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
||||||
const TMP = "/tmp/draw-image-idempotency"
|
const TMP = path.join(os.tmpdir(), `draw-image-idempotency-${process.pid}`)
|
||||||
|
|
||||||
beforeAll(() => {
|
beforeAll(() => {
|
||||||
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
mkdirSync(TMP, { recursive: true })
|
mkdirSync(TMP, { recursive: true })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
|
})
|
||||||
|
|
||||||
function renderCli(template: string, title: string, out: string, slots?: string): { status: number; stdout: string; stderr: string } {
|
function renderCli(template: string, title: string, out: string, slots?: string): { status: number; stdout: string; stderr: string } {
|
||||||
const args = ["--experimental-strip-types", path.join(DRAW_IMAGE_DIR, "cli.ts"), "render", template, "--title", title, "--out", out]
|
const args = ["--experimental-strip-types", path.join(DRAW_IMAGE_DIR, "cli.ts"), "render", template, "--title", title, "--out", out]
|
||||||
if (slots) args.push("--slots", slots)
|
if (slots) args.push("--slots", slots)
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
import { describe, test, expect, beforeAll } from "vitest"
|
import { describe, test, expect, beforeAll, afterAll } from "vitest"
|
||||||
import { existsSync, readFileSync, rmSync, mkdirSync, writeFileSync, mkdtempSync } 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 os from "node:os"
|
||||||
|
|
@ -9,13 +9,17 @@ import { loadTemplate, buildSvg, computeHash } from "../src/render"
|
||||||
|
|
||||||
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
||||||
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
||||||
const TMP = "/tmp/draw-image-integration"
|
const TMP = path.join(os.tmpdir(), `draw-image-integration-${process.pid}`)
|
||||||
|
|
||||||
beforeAll(() => {
|
beforeAll(() => {
|
||||||
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
mkdirSync(TMP, { recursive: true })
|
mkdirSync(TMP, { recursive: true })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
|
})
|
||||||
|
|
||||||
function renderToPng(svg: string, outPath: string): void {
|
function renderToPng(svg: string, outPath: string): void {
|
||||||
const tmpDir = mkdtempSync(path.join(os.tmpdir(), `draw-image-test-${process.pid}-`))
|
const tmpDir = mkdtempSync(path.join(os.tmpdir(), `draw-image-test-${process.pid}-`))
|
||||||
const tmpSvg = path.join(tmpDir, "input.svg")
|
const tmpSvg = path.join(tmpDir, "input.svg")
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
import { describe, test, expect, beforeAll } from "vitest"
|
import { describe, test, expect, beforeAll, afterAll } from "vitest"
|
||||||
import { existsSync, readFileSync, rmSync, mkdirSync, writeFileSync, mkdtempSync } 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 os from "node:os"
|
||||||
|
|
@ -9,13 +9,17 @@ import { loadTemplate, buildSvg } from "../src/render"
|
||||||
|
|
||||||
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
const __dirname = path.dirname(fileURLToPath(import.meta.url))
|
||||||
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
const DRAW_IMAGE_DIR = path.resolve(__dirname, "..")
|
||||||
const TMP = "/tmp/draw-image-optional-integration"
|
const TMP = path.join(os.tmpdir(), `draw-image-optional-integration-${process.pid}`)
|
||||||
|
|
||||||
beforeAll(() => {
|
beforeAll(() => {
|
||||||
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
mkdirSync(TMP, { recursive: true })
|
mkdirSync(TMP, { recursive: true })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
if (existsSync(TMP)) rmSync(TMP, { recursive: true, force: true })
|
||||||
|
})
|
||||||
|
|
||||||
function renderToPng(svg: string, outPath: string): void {
|
function renderToPng(svg: string, outPath: string): void {
|
||||||
const tmpDir = mkdtempSync(path.join(os.tmpdir(), `draw-image-test-${process.pid}-`))
|
const tmpDir = mkdtempSync(path.join(os.tmpdir(), `draw-image-test-${process.pid}-`))
|
||||||
const tmpSvg = path.join(tmpDir, "input.svg")
|
const tmpSvg = path.join(tmpDir, "input.svg")
|
||||||
|
|
|
||||||
14
docs/decisions/081-pr-186-beforeall-output-tmp-race.md
Normal file
14
docs/decisions/081-pr-186-beforeall-output-tmp-race.md
Normal file
|
|
@ -0,0 +1,14 @@
|
||||||
|
# ADR-081: Per-process output TMP dirs in draw-image test beforeAll
|
||||||
|
|
||||||
|
## Статус
|
||||||
|
Accepted (2026-07-31)
|
||||||
|
|
||||||
|
## Контекст
|
||||||
|
Тестовые файлы draw-image использовали общий output-каталог на тест-файл (`/tmp/draw-image-<name>`), создаваемый/очищаемый в `beforeAll`. При параллельном запуске нескольких процессов vitest concurrently (внешняя параллель, не intra-pool) процессы с одинаковым набором тест-файлов конкурировали за один каталог: `rmSync(TMP, recursive)` процесса A удалял PNG + `meta.json`, только что написанные процессом B → падение assertions (`existsSync`, `skipped vs rendered`, `unable to open for write`). Аналогично race на `input.svg` (#177, test helper), но для output-каталогов `beforeAll`. Issue #183.
|
||||||
|
|
||||||
|
## Решение
|
||||||
|
Уникализировать output-каталог на процесс через `process.pid` suffix: `TMP = path.join(os.tmpdir(), \`draw-image-<name>-\${process.pid}\`)`. Добавить `afterAll` cleanup где отсутствовал. `process.pid` уникален на каждый параллельный vitest-процесс → каталоги не коллизируют. Не использован `mkdtempSync` — детерминированный suffixed path проще для assertions и достаточно уникален для внешней параллельности (intra-pool workers у vitest по умолчанию forks → разные PID).
|
||||||
|
|
||||||
|
## Альтернативы
|
||||||
|
- `fs.mkdtempSync(path.join(os.tmpdir(), "draw-image-<name>-"))` — гарантированная уникальность даже при одинаковом PID (threads pool), но nondeterministic path усложняет дебаг. PID-suffix достаточен для текущего pool-режима vitest.
|
||||||
|
- Vitest `--no-parallel` / `--pool=threads --poolOptions.threads.singleThread: true` — подавляет параллель, но не устраняет root cause и замедляет CI.
|
||||||
22
docs/handoff/pr-186-beforeall-output-tmp-race.md
Normal file
22
docs/handoff/pr-186-beforeall-output-tmp-race.md
Normal file
|
|
@ -0,0 +1,22 @@
|
||||||
|
---
|
||||||
|
pr: 186
|
||||||
|
title: fix(draw-image): per-process output TMP in beforeAll for parallel vitest
|
||||||
|
---
|
||||||
|
|
||||||
|
## Что сделано
|
||||||
|
- 7 affected тест-файлов `.opencode/draw-image/tests/` переведены с общего output-каталога `/tmp/draw-image-<name>` на per-process: `TMP = path.join(os.tmpdir(), \`draw-image-<name>-\${process.pid}\`)`.
|
||||||
|
- Файлы: `render.integration.test.ts`, `render.optional.integration.test.ts`, `e2e.optional.test.ts`, `e2e.bad-input.test.ts`, `e2e.no-icon.test.ts`, `cleanup.test.ts`, `idempotency.test.ts`.
|
||||||
|
- Добавлен `import os from "node:os"` где отсутствовал (5 файлов).
|
||||||
|
- Добавлен `afterAll` с `rmSync(TMP, { recursive, force })` в 6 файлах (в `cleanup.test.ts` уже был — оставлен as-is).
|
||||||
|
- Импорт `afterAll` добавлен в vitest-импорты 6 файлов.
|
||||||
|
|
||||||
|
## Почему
|
||||||
|
При параллельном запуске 10× `npx vitest run` процессы с одинаковым набором тест-файлов конкурировали за один output-каталог: `beforeAll` процесса A удалял `TMP` (вместе с PNG + `meta.json`, только что написанными процессом B) → assertion `existsSync(outPath)` падал с `expected false to be true`, idempotency-тест падал с `expected 'skipped' to be 'rendered'`, cleanup-тест — с `unable to open for write`. `process.pid` уникален на параллельный vitest-процесс → per-process TMP устраняет race. Аналогично #177 (test helper), но для output-каталогов `beforeAll`.
|
||||||
|
|
||||||
|
## Pending
|
||||||
|
- Issue #185: тот же bug class в `tests/e2e.test.ts` (не вошёл в список #183) — отдельный PR.
|
||||||
|
|
||||||
|
## Watch out
|
||||||
|
- `e2e.test.ts` всё ещё имеет общий `/tmp/draw-image-e2e` — параллельный запуск 10× vitest падает в этом файле (4/10 запусков), фикс трекается в #185. 7 зафиксированных файлов идут зелёные во всех 10 параллельных запусках.
|
||||||
|
- Одиночный `npx vitest run`: 58/58 зелёные.
|
||||||
|
- `mkdtempSync` не использовался — `process.pid` suffixed path достаточен для внешней параллельности (отдельные процессы vitest); для intra-process threads у vitest по умолчанию forks pool, PID разный у каждого воркера.
|
||||||
Loading…
Add table
Reference in a new issue