diff --git a/tests/conformance/README.md b/tests/conformance/README.md index 52ee5f0..65ad469 100644 --- a/tests/conformance/README.md +++ b/tests/conformance/README.md @@ -1,11 +1,11 @@ -# tests/conformance — one suite, three runners +# tests/conformance — one suite, four runners **This is a required check on every lane's PR.** It is the mechanism that makes four parallel lanes safe: the C++ daemon and the TypeScript extension are proved compatible without either having run against the other. ```sh -./tests/conformance/run.sh # starts its own mockd +./tests/conformance/run.sh # starts its own mockd + veloxd ./tests/conformance/run.sh --uds /run/user/1000/velox/velox.sock --ws-port 52000 ``` @@ -15,7 +15,47 @@ without either having run against the other. |---|---|---| | `check_contract.py` | python3, jsonschema | schemas parse and resolve; the documented surface matches the schema surface both ways; every method has a success fixture; every fixture validates; `SettingKey` and `Settings` agree; **committed generated code is not stale** | | `cpp/` | a C++23 compiler, nlohmann | every golden payload parses into the generated structs, serialises back stably, and goes through the real `dispatch()`; privileged methods are refused `-32003` over the WebSocket | -| `ts/replay.ts` | node ≥ 20 | a live server answers every fixture over every transport the contract allows, and the reply passes the generated validator | +| `ts/replay.ts` against mockd | node ≥ 20 | the TS client and the fixtures agree with each other — mockd always answers every fixture correctly by construction, so this cannot catch veloxd disagreeing with the contract | +| `ts/replay.ts` against veloxd | the above, plus a C++23 toolchain (`daemon/CMakeLists.txt`'s deps) | the **real** daemon it builds and starts, isolated (its own `XDG_RUNTIME_DIR`/`XDG_DATA_HOME`/`XDG_CONFIG_HOME`), answers every fixture — except ones hitting a still-stubbed handler, excused by `veloxd-xfail.json` (see below) | + +### `veloxd-xfail.json` + +`daemon/docs/deferrals.md` (D1-D4b) lists the handlers still stubbed out (`-32603 not +implemented`); the fixtures that hit them can't pass against veloxd yet and are listed +here with why, keyed by fixture path. This is a maintained allowlist, not a snapshot: a +listed fixture that unexpectedly *passes* is flipped back to a failure by `replay.ts` +(`applyXfail`) rather than silently accepted, so an entry has to be deleted the same PR +that closes the handler — the list can only shrink, never rot into "things nobody checks." + +This is also where a real veloxd bug shows up before it reaches anyone else: an +implemented handler returning something the schema forbids (e.g. `download.get` with +`segments: 0`, which `TaskSummary.segments` requires >= 1) is **not** in the allowlist, so +it fails the run for real. That is the point of running against veloxd at all, not just +mockd. + +### Current status against veloxd: red, for two reasons — not just the one + +As of this wiring, the veloxd runner does not pass, and shouldn't yet: + +1. **`download.get` / `download.list` can return `segments: 0`.** `store::TaskRow::eff_segments` + defaults to `0` and `to_summary` copies it straight into `TaskSummary.segments` + (`daemon/src/store/tasks.{hpp,cpp}`), which the schema forbids (minimum 1, required). + This isn't only a post-completion thing — it's any task the engine hasn't segmented yet, + which includes a task the instant it's added. This is the regression this runner exists + to catch, and it is deliberately **not** in `veloxd-xfail.json`. +2. **New finding: `download.add` with `startMode: "later"` always fails.** `later` is a + valid, documented `StartMode` (`contracts/schema/types/StartMode.schema.json`: + `["now", "later", "queue"]` — "'later' is the File Info dialog's Download Later + button"), and `on_download_add` stores it verbatim as `tasks.start_mode` + (`daemon/src/rpc/dispatcher.cpp`). But the `start_mode` column's `CHECK` constraint + (`daemon/src/store/migrations/0001_initial.sql`) only allows + `'auto','now','queue','manual'` — no `'later'`, and `'manual'`/`'auto'` aren't contract + values at all. Every `download.add` with `startMode: "later"` — including this runner's + own fixture-binding setup, which needs one to exist before it can replay any + `$taskId`-referencing fixture — fails `-32603` on a SQLite `CHECK` violation. This is not + in `veloxd-xfail.json` either: it's not a stub (D-list), it's a real, currently-shipping + bug, and it's why the veloxd run is red across most of the suite right now, not just on + `download.get`. Filed to DAEMON; not fixed here (out of lane). `run.sh` also runs one scenario that cannot be shown against a healthy server: with the daemon answering slower than `capture.offer`'s 750 ms deadline, the client must give up and @@ -31,22 +71,24 @@ returns its own ids and its own clock — see `contracts/fixtures/README.md`. Adding a method without a fixture fails `check_contract.py`. Regenerating and forgetting to commit the output fails it too. -## Request to lane PKG/QA +## CI -`.github/` belongs to PKG/QA, so this suite is not wired into CI by lane PROTO. Please add -it as a **required status check on every branch**, roughly: +Wired in as `ctest -L conformance` (`.github/workflows/ci.yml`'s `conformance` job, PKG/QA; +see `docs/adr/0014-conformance-runs-through-ctest.md`), required on every branch. It starts +and stops its own `mockd` and its own `veloxd`; nothing else needs to be running. -```yaml - conformance: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 - with: { node-version: '22' } - - run: sudo apt-get update && sudo apt-get install -y nlohmann-json3-dev - - run: pip install jsonschema referencing - - run: ./tests/conformance/run.sh -``` +Needs: `python3` with `jsonschema`, a C++23 compiler, the same deps `daemon/CMakeLists.txt` +needs (SQLite3, nlohmann-json, libsecret — `tools/bootstrap.sh` installs all of it), and +Node ≥ 20. -The suite needs: `python3` with `jsonschema`, a C++23 compiler, `nlohmann-json`, and Node -≥ 20. It starts and stops its own `mockd`; nothing else needs to be running. +### One caveat: veloxd's single-instance lock is not isolation-aware + +veloxd refuses to start a second copy for the same user — an abstract-namespace socket +keyed by UID only, not by `$XDG_RUNTIME_DIR` (`daemon/src/main.cpp`, +`acquire_single_instance_lock`). This run's isolated veloxd collides with that lock exactly +like any other copy would: if a real veloxd (or another worktree's integration run) is +already up for this user when `run.sh` starts, the veloxd step fails fast with "another +instance is already running for this user" rather than silently testing the wrong daemon. +A fresh CI runner never hits this — only concurrent local runs can. If that turns out to +bite CI in practice (two conformance jobs sharing a runner user, say), the fix belongs in +`daemon/` (scope the lock name to the runtime dir), not here. diff --git a/tests/conformance/run.sh b/tests/conformance/run.sh index 24756de..1000d32 100755 --- a/tests/conformance/run.sh +++ b/tests/conformance/run.sh @@ -2,13 +2,19 @@ # # The conformance suite. This is the command CI runs on every lane's PR. # -# ./tests/conformance/run.sh static + C++ + TS against a mockd it starts +# ./tests/conformance/run.sh static + C++ + TS against mockd, then veloxd # ./tests/conformance/run.sh --uds PATH --ws-port N against an already-running daemon # -# Three runners, one set of fixtures: +# Four runners, one set of fixtures: # 1. check_contract.py schemas, fixtures and committed generated code agree -# 2. cpp/ the generated C++ parses, serialises and dispatches every fixture -# 3. ts/replay.ts a live server answers every fixture over both transports +# 2. cpp/ the generated C++ parses, serialises and dispatches every fixture +# 3. ts/replay.ts against mockd — mockd always answers every fixture correctly, so +# this is the TS client and the fixtures agreeing with each other +# 3b. ts/replay.ts against a real, isolated veloxd it builds and starts — the one +# runner that can catch veloxd disagreeing with its own contract. +# Fixtures that hit a still-stubbed handler (daemon/docs/deferrals.md +# D1-D4b) are excused via veloxd-xfail.json; everything else must +# pass for real. # # Plus one scenario that cannot be shown against a healthy server: with the daemon # answering slower than capture.offer's 750 ms deadline, the client must give up and let @@ -23,6 +29,7 @@ EXTERNAL_UDS="" EXTERNAL_WS="" MOCKD_PID="" SLOW_PID="" +VELOXD_PID="" while [ $# -gt 0 ]; do case "$1" in @@ -46,6 +53,7 @@ stop() { cleanup() { stop "$MOCKD_PID" stop "$SLOW_PID" + stop "$VELOXD_PID" rm -rf "$WORK" } trap cleanup EXIT @@ -99,6 +107,78 @@ TS_ARGS=() [ -n "$WS_PORT" ] && TS_ARGS+=(--ws-port "$WS_PORT") ( cd "$HERE/ts" && ./node_modules/.bin/tsx replay.ts "${TS_ARGS[@]}" ) +# ------------------------------------------------------------------ 3b. veloxd +# The same fixtures against a real, isolated veloxd. mockd (above) always answers every +# fixture correctly by construction, so it can only prove the TS client and the fixtures +# agree with each other — it cannot catch veloxd disagreeing with its own contract. This +# is the runner that closed that gap: it is what would have caught veloxd's download.get +# returning `segments: 0`, which TaskSummary forbids (minimum 1, required), before it +# shipped rather than after. +step "generated TypeScript against a real, isolated veloxd" +if [ -z "$EXTERNAL_UDS" ] && [ -z "$EXTERNAL_WS" ]; then + if [ ! -f "$REPO/daemon/CMakeLists.txt" ]; then + echo "skipped: daemon/CMakeLists.txt not present (lane DAEMON has not landed yet)" + else + VBUILD="$REPO/build/dev" + # cmake --preset dev is idempotent to re-run against an existing build dir; a CI leg + # that already configured (the `conformance` job does, before ctest) just reuses it. + if [ ! -f "$VBUILD/CMakeCache.txt" ]; then + ( cd "$REPO" && cmake --preset dev ) >"$WORK/veloxd-configure.log" 2>&1 \ + || { echo "veloxd: cmake configure failed:"; cat "$WORK/veloxd-configure.log"; exit 1; } + fi + cmake --build "$VBUILD" --target veloxd >"$WORK/veloxd-build.log" 2>&1 \ + || { echo "veloxd: build failed:"; cat "$WORK/veloxd-build.log"; exit 1; } + VELOXD_BIN="$VBUILD/bin/veloxd" + + # Isolated: its own runtime dir (socket, ws.port, single-instance lock), data dir + # (velox.db) and config dir, none of them the real user's. veloxd's single-instance + # lock is a UID-scoped abstract socket, not namespaced by XDG_RUNTIME_DIR, so this + # still collides with a veloxd already running for this user outside the sandbox — + # that shows up below as "another instance is already running" and fails loudly + # rather than silently testing the wrong daemon. + VXDG="$WORK/veloxd-xdg" + mkdir -p "$VXDG/runtime" "$VXDG/data" "$VXDG/config" "$VXDG/downloads" + + XDG_RUNTIME_DIR="$VXDG/runtime" XDG_DATA_HOME="$VXDG/data" XDG_CONFIG_HOME="$VXDG/config" \ + "$VELOXD_BIN" >"$WORK/veloxd.log" 2>&1 & + VELOXD_PID=$! + VUDS="$VXDG/runtime/velox/velox.sock" + for _ in $(seq 1 50); do [ -S "$VUDS" ] && break; sleep 0.2; done + [ -S "$VUDS" ] || { echo "veloxd did not start:"; cat "$WORK/veloxd.log"; exit 1; } + + # saveTo.allowedRoots defaults to ["~/Downloads"]; download.add.json (fixture) asks + # for a saveDir under $HOME/Downloads, so both that and download.add's own isolated + # downloads dir need to be allowed roots, or every download.add fixture fails -32011 + # before the point of this runner is even reached. settings.set is itself a D3 stub, + # so this is written straight into the isolated velox.db rather than over the wire. + python3 - "$VXDG/data/velox/velox.db" "$VXDG/downloads" "$HOME/Downloads" <<'PY' +import json, sqlite3, sys +db_path, isolated_downloads, home_downloads = sys.argv[1:4] +db = sqlite3.connect(db_path) +db.execute( + "INSERT INTO settings(key, value) VALUES(?, ?) " + "ON CONFLICT(key) DO UPDATE SET value = excluded.value", + ("saveTo.allowedRoots", json.dumps([isolated_downloads, home_downloads])), +) +db.execute( + "INSERT INTO settings(key, value) VALUES(?, ?) " + "ON CONFLICT(key) DO UPDATE SET value = excluded.value", + ("saveTo.defaultDir", json.dumps(isolated_downloads)), +) +db.commit() +PY + + VELOXD_TS_ARGS=(--uds "$VUDS") + if [ -f "$VXDG/runtime/velox/ws.port" ]; then + VELOXD_TS_ARGS+=(--ws-port "$(cat "$VXDG/runtime/velox/ws.port")") + fi + ( cd "$HERE/ts" && ./node_modules/.bin/tsx replay.ts "${VELOXD_TS_ARGS[@]}" \ + --xfail "$HERE/veloxd-xfail.json" ) + fi +else + echo "skipped: --uds/--ws-port already points at a live daemon" +fi + # ------------------------------------------------- 4. capture fails open step "capture.offer fails open when the daemon is too slow" if [ -z "$EXTERNAL_UDS" ]; then diff --git a/tests/conformance/ts/replay.ts b/tests/conformance/ts/replay.ts index 4644188..144db2f 100644 --- a/tests/conformance/ts/replay.ts +++ b/tests/conformance/ts/replay.ts @@ -222,15 +222,29 @@ function bind(value: unknown, bindings: Record): unknown { /** * Create the tasks the task-referencing fixtures bind to. Doing this per connection is * what lets the same suite run against an empty veloxd and against a seeded mockd. + * + * A server that cannot even complete this setup (not a fixture, so nothing above can + * report it) still needs to be visible as a failure rather than an uncaught exception + * that takes the whole runner down before a single fixture is checked — so a failure + * here becomes an Outcome and setup moves on, leaving that binding unresolved (its + * fixtures will then fail on the placeholder, individually, same as any other bad value). */ -async function setupBindings(conn: Conn): Promise> { +async function setupBindings(conn: Conn): Promise<{ bindings: Record; setup: Outcome[] }> { const bindings: Record = {}; + const setup: Outcome[] = []; for (const [key, url] of [['$taskId', 'https://example.org/conformance-a.bin'], ['$taskId2', 'https://example.org/conformance-b.bin']] as const) { - const added = await conn.call('download.add', { url, startMode: 'later' }); - bindings[key] = added.taskId; + try { + const added = await conn.call('download.add', { url, startMode: 'later' }); + bindings[key] = added.taskId; + setup.push({ fixture: `setup/${key}`, transport: conn.transport, ok: true, + detail: 'download.add for the binding succeeded' }); + } catch (err) { + setup.push({ fixture: `setup/${key}`, transport: conn.transport, ok: false, + detail: `download.add for the binding failed: ${String(err)}` }); + } } - return bindings; + return { bindings, setup }; } async function replay(conn: Conn, fixtures: readonly Fixture[], @@ -319,6 +333,56 @@ async function privilegeChecks(conn: Conn): Promise { return out; } +// ----------------------------------------------------------- expected failure + +/** + * A fixture this runner is allowed to fail against the target server, with why. Used + * against veloxd, which still has stub handlers (daemon/docs/deferrals.md D1-D4b) that + * mockd does not: mockd always answers every fixture correctly, so this list is empty + * there and the mechanism does not apply. + * + * `fixture` matches Outcome.fixture exactly (the path printed in a FAIL line, e.g. + * "contracts/fixtures/download.pause.json"); `transport`, if given, narrows to one + * transport. This is a maintained allowlist, not a captured snapshot: an entry that no + * longer fails is a bug in the list, not a pass, so `applyXfail` turns that back into a + * failure rather than silently dropping the entry. That is what keeps the list shrinking + * as DAEMON lands handlers instead of quietly becoming a list nobody rechecks. + */ +interface XfailEntry { + fixture: string; + transport?: TransportName; + reason: string; +} + +function loadXfail(path: string): XfailEntry[] { + const parsed = JSON.parse(readFileSync(path, 'utf8')) as unknown; + if (!Array.isArray(parsed)) throw new Error(`${path}: expected a JSON array`); + return parsed as XfailEntry[]; +} + +/** + * Reconciles outcomes against the allowlist. A listed fixture that failed is downgraded + * to a pass (its detail says why). A listed fixture that *passed* is flipped to a + * failure: the entry is stale and must be deleted from the list, not left to rot. + */ +function applyXfail(results: readonly Outcome[], xfail: readonly XfailEntry[]): Outcome[] { + const matches = (e: XfailEntry, r: Outcome): boolean => + e.fixture === r.fixture && (e.transport === undefined || e.transport === r.transport); + + return results.map((r) => { + const entry = xfail.find((e) => matches(e, r)); + if (!entry) return r; + if (!r.ok) { + return { ...r, ok: true, detail: `xfail (${entry.reason}): ${r.detail}` }; + } + return { + ...r, ok: false, + detail: `xfail entry unexpectedly passed — delete it from the allowlist ` + + `(was: ${entry.reason})`, + }; + }); +} + // ---------------------------------------------------------------------- main async function main(): Promise { @@ -348,7 +412,8 @@ async function main(): Promise { if (udsPath) { const conn = await connectUds(udsPath); await conn.call('session.hello', { clientType: 'test', clientName: 'conformance', protocolVersion: '1.0.0' }); - results.push(...(await replay(conn, fixtures, await setupBindings(conn), includeRequires))); + const { bindings, setup } = await setupBindings(conn); + results.push(...setup, ...(await replay(conn, fixtures, bindings, includeRequires))); conn.close(); } if (wsPort) { @@ -362,7 +427,8 @@ async function main(): Promise { if (token === undefined) throw new Error('pairing failed: no token issued'); await conn.request('session.hello', { clientType: 'test', clientName: 'conformance', protocolVersion: '1.0.0', token }, 5000); - results.push(...(await replay(conn, fixtures, await setupBindings(conn), includeRequires))); + const { bindings, setup } = await setupBindings(conn); + results.push(...setup, ...(await replay(conn, fixtures, bindings, includeRequires))); results.push(...(await privilegeChecks(conn))); conn.close(); } @@ -370,14 +436,18 @@ async function main(): Promise { process.stdout.write('no --uds or --ws-port given: ran static checks only\n'); } - const failed = results.filter((r) => !r.ok); + const xfailPath = arg('--xfail'); + const finalResults = xfailPath ? applyXfail(results, loadXfail(xfailPath)) : results; + + const failed = finalResults.filter((r) => !r.ok); for (const r of failed) { process.stdout.write(`FAIL [${r.transport}] ${r.fixture}\n ${r.detail}\n`); } const byTransport = new Map(); - for (const r of results) byTransport.set(r.transport, (byTransport.get(r.transport) ?? 0) + 1); + for (const r of finalResults) byTransport.set(r.transport, (byTransport.get(r.transport) ?? 0) + 1); const summary = [...byTransport].map(([k, v]) => `${k}:${v}`).join(' '); - process.stdout.write(`\n${results.length - failed.length}/${results.length} checks passed (${summary})\n`); + process.stdout.write( + `\n${finalResults.length - failed.length}/${finalResults.length} checks passed (${summary})\n`); process.exit(failed.length === 0 ? 0 : 1); } diff --git a/tests/conformance/veloxd-xfail.json b/tests/conformance/veloxd-xfail.json new file mode 100644 index 0000000..5c706f6 --- /dev/null +++ b/tests/conformance/veloxd-xfail.json @@ -0,0 +1,46 @@ +[ + { "fixture": "contracts/fixtures/download.probe.json", "reason": "D2: download.probe -> -32603, needs the engine probe path" }, + { "fixture": "contracts/fixtures/errors/download.probe.probe-failed.json", "reason": "D2: download.probe -> -32603, needs the engine probe path" }, + + { "fixture": "contracts/fixtures/download.pause.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/download.resume.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/download.start.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/download.cancel.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/download.remove.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/download.addBatch.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/download.refreshUrl.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/download.update.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/download.provideAuth.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/errors/download.provideAuth.not-found.json", "reason": "D3: stub handler, -32603 instead of -32010" }, + + { "fixture": "contracts/fixtures/rules.list.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/rules.upsert.json", "reason": "D3: stub handler, -32603" }, + + { "fixture": "contracts/fixtures/settings.get.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/settings.set.json", "reason": "D3: stub handler, -32603" }, + + { "fixture": "contracts/fixtures/limiter.get.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/limiter.set.json", "reason": "D3: stub handler, -32603" }, + + { "fixture": "contracts/fixtures/schedule.get.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/schedule.set.json", "reason": "D3: stub handler, -32603" }, + + { "fixture": "contracts/fixtures/queue.upsert.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/queue.reorder.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/queue.start.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/queue.stop.json", "reason": "D3: stub handler, -32603" }, + + { "fixture": "contracts/fixtures/category.upsert.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/category.remove.json", "reason": "D3: stub handler, -32603" }, + + { "fixture": "contracts/fixtures/grabber.harvest.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/grabber.start.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/grabber.status.json", "reason": "D3: stub handler, -32603" }, + + { "fixture": "contracts/fixtures/media.addVariant.json", "reason": "D3: stub handler, -32603" }, + { "fixture": "contracts/fixtures/media.listVariants.json", "reason": "D3: stub handler, -32603" }, + + { "fixture": "contracts/fixtures/capture.getRules.json", "reason": "stub handler, -32603 -- NOT in daemon/docs/deferrals.md; filed to DAEMON to add a D-row" }, + { "fixture": "contracts/fixtures/capture.offer.take.json", "reason": "stub handler, -32603 -- NOT in daemon/docs/deferrals.md; filed to DAEMON to add a D-row" }, + { "fixture": "contracts/fixtures/errors/capture.offer.ignore.json", "reason": "stub handler, -32603 -- NOT in daemon/docs/deferrals.md; filed to DAEMON to add a D-row" } +]