mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
test: settle 202 jobs instead of returning without asserting (#652)
A 202 means the sync window expired while the job was still running. Tests treated it as a terminal pass: `if (isAsyncFallback(res)) return;` checked the envelope and returned, asserting nothing about the outcome and leaving the job running into the next test, which is the leak cancelAcceptedJobAndWait exists to prevent. Because the window only expires under load, coverage tracked runner load. On CI 44 tests took this path and verified nothing; the same tests on a dev machine asserted in full (one measured 6.4s locally against 31s on CI). settleAsyncFallback waits for a terminal state and asserts the job finished, and that a failure carries a message rather than being a crash. A clean failure stays valid, since the exotic-format fixtures are meant to be rejected. All 82 call sites moved over. per-fork-env no longer floors SYNC_WAIT_MS, so forcing it to 0 drives every request through its 202 path. 570 tests were validated that way and matched their normal-window results exactly. The 29-34s band dropped from 44 tests (23.4% of test time) to 6 (3.0%). Total test time rose 5.8% and CI wall went 12.8 to 13.1 min: the forks were doing real work during that wait, so this buys determinism, not speed. Per-shard totals unchanged at 9903 tests, 9435 passed, 468 skipped.
This commit is contained in:
@@ -35,6 +35,7 @@ import { join } from "node:path";
|
||||
import { apiToolPath } from "@snapotter/shared";
|
||||
import { afterAll, beforeAll, expect } from "vitest";
|
||||
import { fixtureDir } from "../../fixtures/index.js";
|
||||
import { settleAsyncFallback } from "../settle-job.js";
|
||||
import {
|
||||
buildTestApp,
|
||||
createMultipartPayload,
|
||||
@@ -198,22 +199,6 @@ export function needsFallback(fmt: FormatDef): boolean {
|
||||
return fmt.needsCliDecoder || fmt.needsHeifDecoder || fmt.mayFailValidation;
|
||||
}
|
||||
|
||||
/**
|
||||
* A CPU-heavy encode can exceed the sync window (SYNC_WAIT_MS, 30s in tests)
|
||||
* under parallel CI load and fall back to async: 202 {jobId, async: true}. Per
|
||||
* the documented 200-or-202 contract that is a legitimate "accepted & processing"
|
||||
* outcome -- the worker runs the same process fn either way -- not a failure.
|
||||
* Returns true (validating the async body shape) when the response is that
|
||||
* fallback, so callers can treat it as a pass.
|
||||
*/
|
||||
export function isAsyncFallback(res: { statusCode: number; body: string }): boolean {
|
||||
if (res.statusCode !== 202) return false;
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.async).toBe(true);
|
||||
expect(body.jobId).toBeDefined();
|
||||
return true;
|
||||
}
|
||||
|
||||
export function getTimeout(fmt: FormatDef, toolId?: string): number | undefined {
|
||||
if ((fmt.needsHeifDecoder || fmt.needsCliDecoder) && toolId === "image-enhancement")
|
||||
return 300_000;
|
||||
@@ -280,8 +265,11 @@ export async function callTool(toolId: string, fmt: FormatDef, settings: Record<
|
||||
* Assert a standard download response shape (used by most tools).
|
||||
* For fallback formats, accepts 200/400/422. For core formats, expects 200.
|
||||
*/
|
||||
export function assertDownloadResponse(res: { statusCode: number; body: string }, fmt: FormatDef) {
|
||||
if (isAsyncFallback(res)) return undefined;
|
||||
export async function assertDownloadResponse(
|
||||
res: { statusCode: number; body: string },
|
||||
fmt: FormatDef,
|
||||
) {
|
||||
if (await settleAsyncFallback(res)) return undefined;
|
||||
if (needsFallback(fmt)) {
|
||||
expect(ACCEPTABLE_FALLBACK_CODES).toContain(res.statusCode);
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user