From 7f90c3c76e1d15d1f77a77d973d37478fb6095e1 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 22 Aug 2026 20:30:30 +0000 Subject: [PATCH] fix(TEST-SITE-HUGO-SKIP-GUARD): skip, do not error, when the hugo binary is absent (#1722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1722. `test_rendered_benchmark_index_links_resolve_to_emitted_pages` ran `hugo` through `subprocess.run` with no guard for the binary being absent. `subprocess.run` raises `FileNotFoundError` in that case rather than returning the non-zero code `assertEqual(result.returncode, 0)` was written to report, so the failure arrives before any assertion and `unittest` records it as an ERROR. The `agent-record` job runs on a plain `ubuntu-latest` that installs no Hugo, so the whole suite exited 1 and the job was red on `main` @ `1db7e59cf` (run 32586062675, job 97066276883) and byte-identically on PR #1704 @ `6b37934b8` (run 32594387836, job 97082975595). Every branch cut from `main` inherited a red it did not cause, which is the property that trains readers to skip the job carrying it. The case passed its own review because the developer box that ran it happened to carry Hugo on `PATH`. The case now resolves `shutil.which("hugo")` before it runs anything and skips with `hugo is not on PATH` when the result is `None`, otherwise invoking the resolved path. This is the rule #1661 / #1662 already set for `shellcheck`: a tool-dependent test reports an absent tool as a skip, never as a code verdict. The spelling differs from that fix because the shape differs — the shellcheck case already had a `returncode` probe and only needed the missing `except` arm, while this call site had no probe at all, so it resolves the binary the way `test_cuda_fa2_arch_manifest.py` and `test_check_snapshot_pins.py` already do. The assertion is neither deleted nor widened. With Hugo present the case still holds every rendered benchmark index link to an emitted page, which is the guarantee #1714 added it for. ## Evidence (mudler-ubuntu-box, x86_64, Python 3.12, hugo v0.146.3+extended) Red-before, on the unmodified file at sha256 `4ee0f66dd7c016dbefe21f696512ec5e31ea927f1cbf563e1b2eaa04f3e0484d`, run with a `PATH` that omits `/home/mudler/.local/bin`: `FileNotFoundError: [Errno 2] No such file or directory: 'hugo'`, `Ran 7 tests`, `FAILED (errors=1)`, exit 1. Green-after, both directions, on the guarded file at sha256 `159fe16a72521d72cdd7adaf4a66b4b9de09a52fa73dd0e851fdbfa423127dc4`: - tool absent: `Ran 7 tests`, `OK (skipped=1)`, exit 0, the case reporting `skipped 'hugo is not on PATH'`. - tool present: `Ran 7 tests`, `OK`, exit 0, zero skips, the case reporting `ok`. A guard never exercised in the present-tool direction is not evidence. Three mutations prove the two states are distinguishable and the guard is not a mute switch. Each was applied to a scratch copy, proved applied by a diff against the pristine file, compiled with `python3 -m py_compile` (`compile_errors=0`), then restored byte-for-byte and re-verified by sha256. - `hugo = None` (guard always skips): the tool-present run turns from `OK` into `OK (skipped=1)`. The skip is therefore driven by the resolution and not by the host. - skip arm deleted (`shutil.which("hugo") or "hugo"`): the tool-absent run returns the exact original red, `FileNotFoundError` and `FAILED (errors=1)`. - protected assertion changed from `10` to `11`: with Hugo present the case still fails, `AssertionError: 10 != 11`, `FAILED (failures=1)`. The guard did not silence what it protects. No other instance of this shape survives. `test_check_site.py` has one external tool and no other unguarded invocation — `run_in` uses `sys.executable`. No other file under `tests/` calls `hugo`; the two remaining references are a string assertion in `test_check_gate_commands.py` and a path-class entry in `test_check_pr_size.py`. `scripts/agent-preflight.sh` is `All gates green` on this tree, including `test_cpu_x86_llamacpp_floor`, which this box reds only under load. No spec and no roadmap row: this is a one-line gate repair against an open issue, fixed in the same flow, and it changes no checker semantics. #1661 took exactly this shape and carries an issue-index row under a bare `TEST-SHELLCHECK-SKIP-GUARD` label with no spec and no roadmap row. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/issue-index.md | 1 + tests/scripts/test_check_site.py | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.agents/issue-index.md b/.agents/issue-index.md index f9eb147e3..40b656d5a 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -629,3 +629,4 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1691](https://github.com/mudler/vllm.cpp/issues/1691) | `DOCS-MODELS-HUMAN` | **`docs/QUICKSTART.md` said no container lane had ever been published, and three had.** The page carried `The container lanes have never been published, so no tag below resolves against the registry yet` and, on its `docker run` line, `The package carries only a stage tag today, which is a build artifact and not a lane`. Both were true when written and false on 2026-08-22: `ghcr.io/mudler/vllm.cpp` is public and carries `main-cpu`, `main-cuda` and `main-vulkan`, each a `linux/amd64` + `linux/arm64` manifest, published 07:30 UTC that day. `docs/guides/container-images.md` already documented the `:main-` tags, so the two pages disagreed. VERIFIED by execution rather than by reading the registry listing: `docker run --rm ghcr.io/mudler/vllm.cpp:main-cpu --version` answered `vllm.cpp 0.0.3 c-abi=23` at digest `sha256:7f88301ea282dad778748929e7aa6869d2418c8d295eef0e7900cca8310d06e5`, and the same image with a mounted `Qwen/Qwen3-0.6B` returned tokens through `/v1/completions` on host `mudler-ubuntu-box` (x86_64, Docker 29.1.2). The image also parses the `vllm_cpp` weight-residency document and echoes `mmap=on prefault=off expert_stream=on expert_stream_slots=4000`, which is what lets the Qwen3.8 2.4T page carry a container form of its recipe. FIXED IN FLOW: the false note is corrected, the `docker run` line names a tag that resolves, and the executed-row table gains its first real row. **This does NOT close [#1281](https://github.com/mudler/vllm.cpp/issues/1281)**: `:latest` still does not exist, `--model org/repo` is still blocked by [#1511](https://github.com/mudler/vllm.cpp/issues/1511) so the executed row mounts a local directory, and no GPU-lane row was run. The executed-row obligation stays owned by #1281 | bug | | [#1690](https://github.com/mudler/vllm.cpp/issues/1690) | `DOC-README-ABI-LANDING-SOURCE` | **`check-doc-checkpoint.py`'s `LANDING_SOURCE_FILES` omitted `include/vllm.h`, so a commit that bumps `VLLM_ABI_VERSION` could never repair the README claim it invalidated.** Found by the review of [PR #1655](https://github.com/mudler/vllm.cpp/pull/1655). The README `## Use it as a library (C API)` block quotes the ABI version out of the header; the header was in `USER_USAGE_FILES` (so an ABI change owes `docs/USAGE.md`) but not in `LANDING_SOURCE_FILES`, and the README rule refuses a claim change unaccompanied by a landing source. The claim could therefore be invalidated but not repaired by the same edit, which is how the README reached `VLLM_ABI_VERSION 21` against a header reading `23`, alongside a stale "46 exported functions" for a header declaring 47. That second half is stale by one rather than by six -- an earlier review figure of 51/52 swept in the `#define VLLM_API` visibility block and counted `vllm_*` identifiers that are typedefs and struct fields rather than exported functions -- so the case for deleting the count is that a live count of one file stored in another goes stale on any ABI addition, not that it is badly wrong. The set's own criterion already admitted it -- the checker's comment says every member is "something the README QUOTES" -- and the header was the only such source missing. FIXED IN FLOW: `include/vllm.h` is added to the set, red-before/green-after pinned by `test_the_c_abi_header_is_a_landing_source`, with `test_the_c_abi_header_permits_but_does_not_demand_readme` proving no new README obligation and the pre-existing no-class tests still green | bug | | [#1692](https://github.com/mudler/vllm.cpp/issues/1692) | `KERNEL-ACCEL-PROVIDER-DECLINE-EXACT` | **[#1584](https://github.com/mudler/vllm.cpp/issues/1584) is FIXED by this row, and its gate is CPU-only: both production call sites need a GPU and neither arm was executed.** The fix adds `GetOpFallbackUncounted`, sharing one body with `GetOpFallback` so resolution order, the reference-tier install, the drain and every throw stay identical and only the `fetch_add` is conditional -- in its existing position, BEFORE the "nothing below" check, because a decline that throws is still a decline. The two SHAPE-GATED providers that hoist the lookup into a function-local static switch to it: `cuda_attention_cross.cu` `BlockedFallback` and `metal_mlx_provider.mm` `MlxFallback`. **Candidate 1 of #1584 -- drop the count from `GetOpFallback` -- was REJECTED and measured rather than argued:** it edits five per-call sites (`vulkan_ops.cpp:950,1067,1488,1509`, `tenstorrent_ops.cpp:1341`), one already gated on an exact value at `test_vulkan_backend.cpp:2901` (`after.declines == before.declines + 1`), and its failure mode for a future caller is silent UNDER-counting, the Risk 4 the seam exists to expose. Net blast radius is ZERO backends, not the four #1584 estimated; exactly two providers change what they report, by one. #1555's `WarmDeclineOnce` workaround is REMOVED. **What this issue owns is the missing EXECUTION:** `test_ops_attention_cross` on CUDA (20 cases, ALL 20 SKIP on a CPU-only build -- 32 assertions, every one the skip guard, so that suite gives the change no coverage here), `test_metal_backend` on a `VLLM_CPP_MLX` build, and the `.agents/reachability.md` mutation on `BlockedFallback()` / `MlxFallback()`. The CUDA COMPILE is covered by the `-DVLLM_CPP_CUDA=ON` `vllm`-target job in `.github/workflows/ci.yml`; the Metal `.mm` file is compiled by NO job in this repository, because MLX needs `MLX_ROOT`. Red-before/green-after and the mutation table in [op-provider-decline-exact.md](specs/op-provider-decline-exact.md) | bug | +| [#1722](https://github.com/mudler/vllm.cpp/issues/1722) | `TEST-SITE-HUGO-SKIP-GUARD` | **`test_rendered_benchmark_index_links_resolve_to_emitted_pages` calls `hugo` through `subprocess.run` with no absence guard, so on a runner without the binary the case ERRORs instead of skipping and the whole `agent-record` job goes red.** `subprocess.run` raises `FileNotFoundError` rather than returning the non-zero code `assertEqual(result.returncode, 0)` was written to report, so the failure arrives before any assertion. The case landed with [#1714](https://github.com/mudler/vllm.cpp/pull/1714) and passed review on a developer box that happens to carry Hugo on `PATH`; the `agent-record` runner is a plain `ubuntu-latest` that installs none. Red on `main` @ `1db7e59cf` (run 32586062675, job 97066276883) and byte-identically on [PR #1704](https://github.com/mudler/vllm.cpp/pull/1704) @ `6b37934b8` (run 32594387836, job 97082975595), so every branch cut from `main` inherited a red it did not cause — the property that trains readers to skip the job carrying it. Same class and same rule as [#1661](https://github.com/mudler/vllm.cpp/issues/1661): skip, do not error, when the tool is absent. FIXED IN FLOW by the same change: the case resolves `shutil.which("hugo")` first, skips with `hugo is not on PATH` when it is `None`, and runs the resolved path otherwise. The assertion is NOT deleted or widened — three mutations pin the two states apart on `mudler-ubuntu-box`: forcing `hugo = None` turns the tool-present run from `ok` into `OK (skipped=1)`, deleting the skip arm restores the exact `FileNotFoundError` red, and changing the protected `assertEqual(len(detail_hrefs), 10)` to `11` still fails with Hugo present (`AssertionError: 10 != 11`), which is what proves the guard is not a mute switch. `test_check_site.py` has no other external-tool call (`run_in` uses `sys.executable`), and no other file under `tests/` invokes `hugo` | bug | diff --git a/tests/scripts/test_check_site.py b/tests/scripts/test_check_site.py index e3773e0b5..7f63ba587 100644 --- a/tests/scripts/test_check_site.py +++ b/tests/scripts/test_check_site.py @@ -125,11 +125,20 @@ def test_a_missing_nav_file_fails_closed(self) -> None: self.assertIn("does not exist", result.stderr) def test_rendered_benchmark_index_links_resolve_to_emitted_pages(self) -> None: + # #1722: resolve the renderer BEFORE running it. `subprocess.run` raises + # FileNotFoundError on a host without the binary rather than returning a + # returncode the assertion below could report, so an absent Hugo lands as + # an ERROR and reds the whole suite. The `agent-record` runner installs + # no Hugo, so that red rode on `main` and on every branch cut from it. + # Same rule as #1661: skip, do not error, when the tool is absent. + hugo = shutil.which("hugo") + if hugo is None: + self.skipTest("hugo is not on PATH") public = Path(tempfile.mkdtemp()) self.addCleanup(shutil.rmtree, public, ignore_errors=True) result = subprocess.run( [ - "hugo", + hugo, "--minify", "-s", str(ROOT / "website"),