Skip to content

Python-shelling normal-bake tests time out under full-suite load (vitest 10s budget vs a 240s subprocess allowance) #5856

Description

@atomantic

Problem

server/services/imageTo3d/trellis2NormalBake.test.js fails a full-suite npm test with a runner timeout, and passes in isolation:

FAIL  services/imageTo3d/trellis2NormalBake.test.js > bake_normal_map attaches a usable normal map
      > attaches a normalTexture of the requested size, with real tilt
Error: Test timed out in 10000ms.
 ❯ services/imageTo3d/trellis2NormalBake.test.js:312:3

# same file, run alone:
$ npx vitest run services/imageTo3d/trellis2NormalBake.test.js
Test Files  1 passed (1)      Tests  14 passed (14)      Duration  4.22s

There is no failing assertion — this is a runner budget, the same class the hookTimeout comment in server/vitest.config.js:30-38 already documents for the image-gen suites.

Root cause

These tests shell out to a real Python interpreter with execFileSync and wait synchronously: they write a script to a temp dir, then run numpy + trimesh + PIL through it (the failing case bakes a 64x64 normal map). The cost is a real subprocess whose wall time scales with how loaded the machine is, not with anything the test controls. Alone it is ~4s; on a contended worker during a 1,848-file run it crosses the 10s testTimeout.

The two budgets in this file already contradict each other. The e2e run helper at line 269 passes execFileSync(pyBin, [script], { encoding: 'utf8', timeout: 240000 }) — a deliberate 4-minute allowance for the subprocess — while vitest kills the test at 10s. The execFileSync timeout is dead intent today: vitest always wins. The other run helper (line 62) sets no subprocess timeout at all.

Decision (already made — do not re-litigate)

Give the Python-shelling tests an explicit per-test vitest timeout rather than raising the global testTimeout. The global default is deliberately tight (it catches genuinely hung async work across ~37.5k tests); loosening it repo-wide to accommodate a handful of subprocess suites would blunt that everywhere. The narrow fix states the real budget where the real cost is.

Do not "fix" this by mocking Python. These tests exist specifically to run the actual bake and assert numeric properties of the output (unit-length normals, mean z, tilt fraction) — the arithmetic is the thing under test, and a mock would make the suite vacuous.

Scope

  1. server/services/imageTo3d/trellis2NormalBake.test.js — pass an explicit timeout as it()'s third argument on every test that calls a run helper (both describe blocks). Use a single named constant, e.g. const PY_TEST_TIMEOUT_MS = 120_000;, with a one-line comment saying why (real Python subprocess; wall time scales with machine load, not with the assertion). Keep it at or under the existing execFileSync 240s allowance so the subprocess guard still fires first and produces a useful error instead of a bare vitest timeout.
  2. Add the same { timeout: ... } to the first run helper's execFileSync (line 62), which currently has none — an actually-hung interpreter there hangs until vitest kills the worker.
  3. Sweep the sibling Python-shelling suites for the same shape and apply the same constant where a test genuinely runs an interpreter (not merely resolves a path). Candidates to check — confirm each actually spawns before touching it:
    server/services/loraEffectProbe.test.js, server/services/audioMidiTranscription.test.js, server/services/pipeline/musicGen.test.js, server/lib/pythonSetup.test.js, server/lib/setupScriptRunner.test.js.

Acceptance criteria

  • cd server && npm test completes with zero timeout failures across three consecutive runs on a loaded machine.
  • The file still passes in isolation, and still asserts the numeric bake properties it asserts today — no mocking of the Python side.
  • Global testTimeout / hookTimeout in server/vitest.config.js are unchanged.
  • Every Python-spawning execFileSync in the touched files carries its own subprocess timeout.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

effort:lowEffort: lowin-progressClaimed and being workedmodel:lightModel size: lightplanTracked by /do:replantestsTest suite / test infrastructure

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions