From d4d45f835e06dd67322940303eb7fe1233a1bfcb Mon Sep 17 00:00:00 2001 From: claude Date: Fri, 24 Jul 2026 01:41:36 +0000 Subject: [PATCH] ci: reuse backend.has_backend / `backend status` for readiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: the hand-rolled `/dev/kvm` + docker-daemon capability probes duplicated logic the CLI already owns. Delegate instead. - tests/_backend.py skip guards now gate on `bot_bottle.backend.has_backend` (each backend's `is_available()` classmethod — the same probe behind `./cli.py backend status`), dropping the bespoke `Capability` probes. - Remove tests/backend_preflight.py; the docker integration job runs `./cli.py backend status --backend=docker` as its preflight (clear per-check summary, non-zero exit when unready), matching the firecracker job. The firecracker preflight reverts to its original binary/KVM checks (backend status doesn't cover those). - Unit test + docs updated to match. Co-Authored-By: Claude Opus 4.8 --- .gitea/workflows/test.yml | 18 ++- docs/ci.md | 14 +-- tests/README.md | 18 +-- tests/_backend.py | 130 ++++------------------ tests/backend_preflight.py | 27 ----- tests/unit/test_backend_skip_guards.py | 146 +++---------------------- 6 files changed, 62 insertions(+), 291 deletions(-) delete mode 100644 tests/backend_preflight.py diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml index f2e1a16..1fbf962 100644 --- a/.gitea/workflows/test.yml +++ b/.gitea/workflows/test.yml @@ -105,13 +105,14 @@ jobs: # Fail loudly if the backend this job promises isn't actually usable, # rather than letting every test silently `unittest.skip` and the job - # go green on zero coverage. Same capability probe the skip guards use. + # go green on zero coverage. `backend status` prints a clear per-check + # summary (docker on PATH, daemon reachable) and exits non-zero when a + # prerequisite is missing — the same readiness check the skip guards + # gate on via `has_backend`. - name: Preflight — Docker backend is ready - env: - BOT_BOTTLE_BACKEND: docker run: | python3 --version - python3 -m tests.backend_preflight docker + python3 cli.py backend status --backend=docker - name: Run integration tests (docker) with coverage env: @@ -158,13 +159,10 @@ jobs: uses: actions/checkout@v4 - name: Preflight — Firecracker host is ready - env: - BOT_BOTTLE_BACKEND: firecracker run: | - # Standardized capability line — same probe (and PASS/FAIL wording) - # the integration skip guards use, so a missing backend is visible - # at the job level rather than hidden among `unittest.skip` lines. - python3 -m tests.backend_preflight firecracker + command -v firecracker >/dev/null || { + echo "firecracker not on PATH — provision the runner (README: Firecracker on Linux)"; exit 1; } + test -e /dev/kvm || { echo "/dev/kvm missing — KVM not available on this runner"; exit 1; } # `backend status` exits non-zero unless the TAP pool is up + no # range overlap; it prints the exact `backend setup` fix. python3 cli.py backend status --backend=firecracker diff --git a/docs/ci.md b/docs/ci.md index c0230c2..3cc5e74 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -8,15 +8,15 @@ It runs the unit suite plus one integration job per backend - every push to `main`. Each integration job selects its backend via `BOT_BOTTLE_BACKEND` and -runs a **preflight** (`python3 -m tests.backend_preflight `) -that prints a clear PASS/FAIL capability line and fails the job when the +runs a **preflight** (`./cli.py backend status --backend=`) that +prints a clear per-check readiness summary and fails the job when the backend is missing — so absent infrastructure is visible at the job level -rather than hidden among per-test `unittest.skip` lines. The same -capability probes back the skip guards in -[`tests/_backend.py`](../tests/_backend.py): backend-agnostic tests use +rather than hidden among per-test `unittest.skip` lines. The skip guards in +[`tests/_backend.py`](../tests/_backend.py) gate on the same readiness +check (`bot_bottle.backend.has_backend`): backend-agnostic tests use `skip_unless_selected_backend_available()` and run through whichever -backend is selected (checking, e.g., `/dev/kvm` for Firecracker rather -than unrelated Docker availability); Docker-implementation tests use +backend is selected (checking, e.g., Linux + `/dev/kvm` for Firecracker +rather than unrelated Docker availability); Docker-implementation tests use `skip_unless_backend("docker")` and no-op under a non-Docker run. A small subset of integration tests skip when running specifically diff --git a/tests/README.md b/tests/README.md index 25391fc..28985cd 100644 --- a/tests/README.md +++ b/tests/README.md @@ -10,8 +10,7 @@ tests run through the backend named by `BOT_BOTTLE_BACKEND` (default ``` tests/ fixtures.py # JSON manifest builders (shared) - _backend.py # backend capability probes + skip guards - backend_preflight.py # `python -m tests.backend_preflight` (CI) + _backend.py # backend selection + skip guards (shared) unit/ test_egress.py test_egress_addon_core.py @@ -90,16 +89,19 @@ BOT_BOTTLE_RUN_CANARIES=1 python -m unittest discover -t . -s tests/canaries -v if __name__ == "__main__": unittest.main() ``` -4. Skip guards live in `tests._backend`: +4. Skip guards live in `tests._backend` and gate on the backend's own + readiness check, `bot_bottle.backend.has_backend` — the same probe + behind `./cli.py backend status`: - Backend-agnostic tests (go through `get_bottle_backend()`) decorate the class with `@skip_unless_selected_backend_available()` — the test runs against whichever backend `BOT_BOTTLE_BACKEND` selects and skips - unless that backend's capability is present (a reachable Docker daemon, - or an accessible `/dev/kvm` + `firecracker` for Firecracker). + unless that backend is available (checking, e.g., Linux + `/dev/kvm` + for Firecracker rather than unrelated Docker availability). - Backend-specific tests (exercise `DockerBroker`, `DockerGateway`, `backend.docker.*`, …) decorate with `@skip_unless_backend("docker")` so they no-op under a run targeting a different backend. - The same capability probes back the CI preflight, - `python -m tests.backend_preflight []`, which prints a clear - PASS/FAIL line and exits non-zero when the selected backend is missing. + Each CI integration job runs `./cli.py backend status --backend=` + as a preflight, which prints a clear per-check summary and exits non-zero + when the backend is missing — so absent infrastructure fails the job + instead of hiding among per-test `unittest.skip` lines. diff --git a/tests/_backend.py b/tests/_backend.py index 39ed57c..00f5f38 100644 --- a/tests/_backend.py +++ b/tests/_backend.py @@ -1,34 +1,22 @@ -"""Backend-aware capability probes and skip guards for the integration suite. +"""Backend selection + capability-aware skip guards for the integration suite. -Every integration test selects its backend from ``BOT_BOTTLE_BACKEND`` -(default ``docker``) and exercises the full test through that backend. Skip -guards check the *capability* the selected backend actually needs — a -reachable Docker daemon for ``docker``, an accessible ``/dev/kvm`` plus a -``firecracker`` binary for ``firecracker`` — rather than gating every backend -on unrelated Docker availability. - -The same probes back the CI preflight (``python3 -m tests.backend_preflight``) -so a job surfaces missing infrastructure at the job level with a clear -PASS/FAIL line instead of turning every test into a silent ``unittest.skip``. +Each integration test targets the backend named by ``BOT_BOTTLE_BACKEND`` +(default ``docker``) and skips on that backend's own readiness check — +``bot_bottle.backend.has_backend`` (the same probe behind +``./cli.py backend status``), not an unrelated "is Docker installed" test. """ from __future__ import annotations import os -import shutil -import subprocess import unittest -from dataclasses import dataclass + +from bot_bottle.backend import has_backend # Default when ``BOT_BOTTLE_BACKEND`` is unset. Docker preserves the historical # Docker-backed CI path (and mirrors the pin in ``test_sandbox_escape``). DEFAULT_BACKEND = "docker" -# `/dev/kvm` must exist and be openable by the invoking user for the -# Firecracker backend to boot a guest (mirrors -# ``bot_bottle.backend.firecracker.util``). -_KVM_DEVICE = "/dev/kvm" - def selected_backend() -> str: """The backend this test run targets, from ``BOT_BOTTLE_BACKEND``. @@ -40,114 +28,34 @@ def selected_backend() -> str: return os.environ.get("BOT_BOTTLE_BACKEND") or DEFAULT_BACKEND -@dataclass(frozen=True) -class Capability: - """Outcome of a backend capability probe. - - ``ok`` gates the tests; ``detail`` is a one-line human string reused for - both preflight output and ``unittest.skip`` reasons so the same wording - shows up at the job level and next to a skipped test. - """ - - backend: str - ok: bool - detail: str - - -def docker_capability() -> Capability: - """Whether a Docker daemon is reachable (and not opted out).""" - name = "docker" - if os.environ.get("SKIP_DOCKER_TESTS"): - return Capability(name, False, "SKIP_DOCKER_TESTS is set") - if shutil.which("docker") is None: - return Capability(name, False, "docker not on PATH") - try: - returncode = subprocess.run( - ["docker", "info"], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - check=False, - timeout=5, - ).returncode - except subprocess.TimeoutExpired: - return Capability(name, False, "docker info timed out") - if returncode != 0: - return Capability(name, False, "docker daemon unreachable") - return Capability(name, True, "docker daemon reachable") - - -def firecracker_capability() -> Capability: - """Whether this host can boot a Firecracker guest: an accessible - ``/dev/kvm`` and a ``firecracker`` binary on ``PATH``. - - Deliberately does not probe unrelated Docker availability — the - Firecracker backend is Docker-independent at runtime. - """ - name = "firecracker" - if not os.path.exists(_KVM_DEVICE): - return Capability(name, False, f"{_KVM_DEVICE} missing — KVM unavailable") - if not os.access(_KVM_DEVICE, os.R_OK | os.W_OK): - return Capability( - name, False, f"{_KVM_DEVICE} not accessible — add your user to the kvm group" - ) - if shutil.which("firecracker") is None: - return Capability(name, False, "firecracker not on PATH") - return Capability(name, True, f"{_KVM_DEVICE} accessible; firecracker on PATH") - - -_PROBES = { - "docker": docker_capability, - "firecracker": firecracker_capability, -} - - -def backend_capability(backend: str | None = None) -> Capability: - """Probe ``backend`` (default: the selected backend) for readiness.""" - backend = backend or selected_backend() - probe = _PROBES.get(backend) - if probe is None: - return Capability(backend, False, f"no capability probe for backend {backend!r}") - return probe() - - -# ---- back-compat boolean probe ------------------------------------------- - - -def docker_available() -> bool: - """Boolean Docker probe (kept for callers that only need the flag).""" - return docker_capability().ok - - -# ---- skip guards --------------------------------------------------------- - - def skip_unless_backend(backend: str): """Skip a backend-specific test unless the selected backend matches AND - that backend's capability is present. + that backend is available on the host. Docker-implementation tests (``DockerBroker``, ``DockerGateway``, ``backend.docker.*``) use ``skip_unless_backend("docker")`` so they no-op - under a Firecracker run instead of testing Docker internals that run - doesn't target — the guard reads ``BOT_BOTTLE_BACKEND`` rather than - "is Docker installed". + under a run targeting a different backend instead of testing Docker + internals that run doesn't exercise — the guard reads + ``BOT_BOTTLE_BACKEND`` rather than "is Docker installed". """ sel = selected_backend() if sel != backend: return unittest.skip( f"backend {backend!r} not selected (BOT_BOTTLE_BACKEND={sel})" ) - cap = backend_capability(backend) - return unittest.skipUnless(cap.ok, f"{backend} backend unavailable: {cap.detail}") + return unittest.skipUnless( + has_backend(backend), f"{backend} backend prerequisites unavailable" + ) def skip_unless_selected_backend_available(): - """Skip a backend-agnostic test unless the *selected* backend can run it. + """Skip a backend-agnostic test unless the *selected* backend is available. - The test then exercises whichever backend ``BOT_BOTTLE_BACKEND`` names, - checking that backend's real capability (e.g. ``/dev/kvm`` for + The test then runs through whichever backend ``BOT_BOTTLE_BACKEND`` names, + gated on that backend's real prerequisites (e.g. Linux + KVM for Firecracker) rather than unrelated Docker availability. """ - cap = backend_capability() + backend = selected_backend() return unittest.skipUnless( - cap.ok, f"selected backend {cap.backend!r} unavailable: {cap.detail}" + has_backend(backend), f"selected backend {backend!r} prerequisites unavailable" ) diff --git a/tests/backend_preflight.py b/tests/backend_preflight.py deleted file mode 100644 index 0659586..0000000 --- a/tests/backend_preflight.py +++ /dev/null @@ -1,27 +0,0 @@ -"""Backend capability preflight for the integration CI jobs. - -Run as ``python3 -m tests.backend_preflight`` (optionally with a backend name -argument; default: ``BOT_BOTTLE_BACKEND`` or ``docker``). Prints a single -clear PASS/FAIL line for the selected backend's capability and exits non-zero -on failure, so a job surfaces missing infrastructure at the job level instead -of hiding it among ``unittest.skip`` lines further down the output. -""" - -from __future__ import annotations - -import sys - -from tests._backend import backend_capability, selected_backend - - -def main(argv: list[str] | None = None) -> int: - argv = sys.argv[1:] if argv is None else argv - backend = argv[0] if argv else selected_backend() - cap = backend_capability(backend) - status = "PASS" if cap.ok else "FAIL" - print(f"[preflight] backend={backend} {status}: {cap.detail}") - return 0 if cap.ok else 1 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/tests/unit/test_backend_skip_guards.py b/tests/unit/test_backend_skip_guards.py index 54265a1..0695b85 100644 --- a/tests/unit/test_backend_skip_guards.py +++ b/tests/unit/test_backend_skip_guards.py @@ -1,9 +1,9 @@ -"""Tests for the backend-aware capability probes and skip guards -(``tests/_backend.py``) and the CI preflight (``tests/backend_preflight.py``). +"""Tests for the backend-aware skip guards in ``tests/_backend.py``. -The probes shell out to Docker / inspect ``/dev/kvm``; here they are driven -entirely through mocks so the unit job asserts the decision logic without -needing either backend present on the runner. +The guards delegate their readiness check to +``bot_bottle.backend.has_backend`` (the probe behind ``./cli.py backend +status``); here that probe is mocked so the unit job asserts the +selection/skip logic without either backend present on the runner. """ from __future__ import annotations @@ -12,12 +12,7 @@ import os import unittest from unittest.mock import patch -from tests import _backend, backend_preflight from tests._backend import ( - Capability, - backend_capability, - docker_capability, - firecracker_capability, selected_backend, skip_unless_backend, skip_unless_selected_backend_available, @@ -42,147 +37,42 @@ class TestSelectedBackend(unittest.TestCase): self.assertEqual("firecracker", selected_backend()) -class TestDockerCapability(unittest.TestCase): - def test_opt_out_via_skip_docker_tests(self): - with patch.dict(os.environ, {"SKIP_DOCKER_TESTS": "1"}, clear=True): - cap = docker_capability() - self.assertFalse(cap.ok) - self.assertIn("SKIP_DOCKER_TESTS", cap.detail) - - def test_not_on_path(self): - with patch.dict(os.environ, {}, clear=True), \ - patch("tests._backend.shutil.which", return_value=None): - cap = docker_capability() - self.assertFalse(cap.ok) - self.assertIn("PATH", cap.detail) - - def test_daemon_reachable(self): - with patch.dict(os.environ, {}, clear=True), \ - patch("tests._backend.shutil.which", return_value="/usr/bin/docker"), \ - patch("tests._backend.subprocess.run") as run: - run.return_value.returncode = 0 - cap = docker_capability() - self.assertTrue(cap.ok) - - def test_daemon_unreachable(self): - with patch.dict(os.environ, {}, clear=True), \ - patch("tests._backend.shutil.which", return_value="/usr/bin/docker"), \ - patch("tests._backend.subprocess.run") as run: - run.return_value.returncode = 1 - cap = docker_capability() - self.assertFalse(cap.ok) - self.assertIn("unreachable", cap.detail) - - -class TestFirecrackerCapability(unittest.TestCase): - def test_kvm_missing(self): - with patch("tests._backend.os.path.exists", return_value=False): - cap = firecracker_capability() - self.assertFalse(cap.ok) - self.assertIn("KVM", cap.detail) - - def test_kvm_inaccessible(self): - with patch("tests._backend.os.path.exists", return_value=True), \ - patch("tests._backend.os.access", return_value=False): - cap = firecracker_capability() - self.assertFalse(cap.ok) - self.assertIn("kvm group", cap.detail) - - def test_binary_missing(self): - with patch("tests._backend.os.path.exists", return_value=True), \ - patch("tests._backend.os.access", return_value=True), \ - patch("tests._backend.shutil.which", return_value=None): - cap = firecracker_capability() - self.assertFalse(cap.ok) - self.assertIn("firecracker not on PATH", cap.detail) - - def test_ready(self): - with patch("tests._backend.os.path.exists", return_value=True), \ - patch("tests._backend.os.access", return_value=True), \ - patch("tests._backend.shutil.which", return_value="/usr/bin/firecracker"): - cap = firecracker_capability() - self.assertTrue(cap.ok) - - -class TestBackendCapability(unittest.TestCase): - def test_unknown_backend(self): - cap = backend_capability("qemu") - self.assertFalse(cap.ok) - self.assertIn("no capability probe", cap.detail) - - def test_defaults_to_selected(self): - def probe() -> Capability: - return Capability("docker", True, "ok") - - with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "docker"}, clear=True), \ - patch.dict("tests._backend._PROBES", {"docker": probe}): - cap = backend_capability() - self.assertEqual("docker", cap.backend) - self.assertTrue(cap.ok) - - class TestSkipUnlessBackend(unittest.TestCase): def test_skips_when_other_backend_selected(self): - with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "firecracker"}, clear=True): + # A different backend is selected — no host probe needed, skip. + with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "firecracker"}, clear=True), \ + patch("tests._backend.has_backend") as has: decorated = skip_unless_backend("docker")(_new_case()) self.assertTrue(_skipped(decorated)) + has.assert_not_called() - def test_runs_when_selected_and_capable(self): + def test_runs_when_selected_and_available(self): with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "docker"}, clear=True), \ - patch("tests._backend.backend_capability", - return_value=Capability("docker", True, "ok")): + patch("tests._backend.has_backend", return_value=True): decorated = skip_unless_backend("docker")(_new_case()) self.assertFalse(_skipped(decorated)) - def test_skips_when_selected_but_incapable(self): + def test_skips_when_selected_but_unavailable(self): with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "docker"}, clear=True), \ - patch("tests._backend.backend_capability", - return_value=Capability("docker", False, "docker daemon unreachable")): + patch("tests._backend.has_backend", return_value=False): decorated = skip_unless_backend("docker")(_new_case()) self.assertTrue(_skipped(decorated)) class TestSkipUnlessSelectedBackendAvailable(unittest.TestCase): - def test_runs_when_selected_backend_capable(self): + def test_runs_when_selected_backend_available(self): with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "firecracker"}, clear=True), \ - patch("tests._backend.backend_capability", - return_value=Capability("firecracker", True, "ok")): + patch("tests._backend.has_backend", return_value=True) as has: decorated = skip_unless_selected_backend_available()(_new_case()) self.assertFalse(_skipped(decorated)) + has.assert_called_once_with("firecracker") - def test_skips_when_selected_backend_incapable(self): + def test_skips_when_selected_backend_unavailable(self): with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "firecracker"}, clear=True), \ - patch("tests._backend.backend_capability", - return_value=Capability("firecracker", False, "/dev/kvm missing")): + patch("tests._backend.has_backend", return_value=False): decorated = skip_unless_selected_backend_available()(_new_case()) self.assertTrue(_skipped(decorated)) -class TestPreflightMain(unittest.TestCase): - def test_pass_exits_zero(self): - with patch.object(backend_preflight, "backend_capability", - return_value=Capability("docker", True, "docker daemon reachable")): - rc = backend_preflight.main(["docker"]) - self.assertEqual(0, rc) - - def test_fail_exits_nonzero(self): - with patch.object(backend_preflight, "backend_capability", - return_value=Capability("firecracker", False, "/dev/kvm missing")): - rc = backend_preflight.main(["firecracker"]) - self.assertEqual(1, rc) - - def test_defaults_to_selected_backend(self): - with patch.dict(os.environ, {"BOT_BOTTLE_BACKEND": "docker"}, clear=True), \ - patch.object(backend_preflight, "backend_capability", - return_value=Capability("docker", True, "ok")) as cap: - rc = backend_preflight.main([]) - self.assertEqual(0, rc) - cap.assert_called_once_with("docker") - - def test_module_exposes_selected_backend(self): - # Import-surface guard: the preflight re-exports the selector it uses. - self.assertIs(backend_preflight.selected_backend, _backend.selected_backend) - - if __name__ == "__main__": unittest.main()