From 3c79562820b688bfda90e239b78fc4fcf834856b Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Tue, 6 Oct 2026 11:44:34 -0400 Subject: [PATCH] test: keep the suite off GitHub and away from real gh and zizmor (#548) Tests that ran a full audit called the live GitHub API through gh and ran the installed zizmor, so their results depended on the network, the developer's gh login, rate limits, and the zizmor version. Parity tests that compare several drivers failed whenever those answers differed. An autouse fixture in tests/conftest.py now installs RecordedGhApi({}) for every test (an unrecorded endpoint answers status 0, ERROR unavailable) and puts stand-ins first on PATH: gh exits 4 as if not authenticated, zizmor prints an empty findings list. A test's own responder or stand_in_tool still takes precedence. Tests that must reach the network are marked live and skipped unless the -m expression names the marker; the upstream spec-sync tests, which fetch from raw.githubusercontent.com, now work the same way (and --update-hash still selects them). The tests of gh_api_with_status and gh_api that mock subprocess.run drop the responder so they keep exercising the real gh path. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- docs/getting-started/testing.md | 28 +++++++-- pyproject.toml | 1 + tests/conftest.py | 61 +++++++++++++++++++ .../context/test_dot_project_upstream.py | 5 +- tests/darnit/core/test_gh_api_status.py | 8 +++ tests/darnit/core/test_utils.py | 8 +++ 6 files changed, 105 insertions(+), 6 deletions(-) diff --git a/docs/getting-started/testing.md b/docs/getting-started/testing.md index ce8861ad..594ae460 100644 --- a/docs/getting-started/testing.md +++ b/docs/getting-started/testing.md @@ -36,13 +36,33 @@ uv run pytest tests/darnit/sieve/test_orchestrator.py::test_deterministic_pass - ### Integration tests -Integration tests require GitHub API access (`gh auth login`) and network connectivity: - ```bash uv run pytest tests/integration/ -v ``` -These are excluded from the default test run because they're slower and require external access. +These are excluded from the default test run because they're slower. + +### Network and external tools + +No test reaches GitHub or runs the real `gh` or `zizmor` by default (#548). An autouse +fixture in `tests/conftest.py` gives every test: + +- `RecordedGhApi({})` as the platform responder, so an unrecorded endpoint answers + status 0 (ERROR `unavailable`) on every run. Install your own responder with + `set_gh_api_responder` (restoring the previous one afterwards) to serve the + responses a test needs. +- Stand-ins first on `PATH`: `gh` exits 4 as if not authenticated, and `zizmor` + prints `[]` (no findings). Use `stand_in_tool` from `tests/conftest_helpers.py` + to put a test's own stand-in in front of them. + +A test that must reach the network or run a real tool is marked `live` +(`@pytest.mark.live`); it gets none of the above and is skipped unless the `-m` +expression names `live`. The `upstream` marker works the same way (`--update-hash` +also selects it). + +```bash +uv run pytest tests/ -m live -v +``` ## Test Structure @@ -173,7 +193,7 @@ def test_cel_expression_with_json_output(): - Use descriptive test names: `test___` - One assertion per test when practical - Use `tmp_path` for filesystem operations (auto-cleaned up) -- Mock external services (GitHub API, network calls) +- Mock external services (GitHub API, network calls); see [Network and external tools](#network-and-external-tools) - Keep tests fast — no network calls in unit tests ## Next Steps diff --git a/pyproject.toml b/pyproject.toml index 37353343..b59b6de9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -91,6 +91,7 @@ markers = [ "integration: Integration tests (may require git/gh)", "slow: Slow tests", "upstream: Upstream spec sync tests (run nightly, excluded by default)", + "live: Reaches GitHub or runs a real gh or zizmor (skipped unless -m names it; #548)", ] filterwarnings = [ "ignore::DeprecationWarning:darnit_baseline.rules", diff --git a/tests/conftest.py b/tests/conftest.py index f4489bcd..819810fb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,7 @@ """Pytest configuration and shared fixtures.""" +import os +import re import shutil import subprocess import tempfile @@ -8,6 +10,65 @@ import pytest +from tests.conftest_helpers import write_stand_in_tool + +# Markers whose tests reach the network or run real tools. They run only +# when the -m expression names the marker (#548). +_OPT_IN_MARKERS = ("live", "upstream") + + +def pytest_collection_modifyitems(config: pytest.Config, items: list[pytest.Item]) -> None: + """Skip a ``live`` or ``upstream`` test unless ``-m`` names its marker (#548). + + ``--update-hash`` (tests/darnit/context/conftest.py) also selects the + ``upstream`` tests, since refreshing the tracked hash is what it is for. + """ + markexpr = config.getoption("markexpr", "") or "" + selected = {name for name in _OPT_IN_MARKERS if re.search(rf"\b{name}\b", markexpr)} + if config.getoption("--update-hash", default=False): + selected.add("upstream") + for item in items: + opted_in = [name for name in _OPT_IN_MARKERS if item.get_closest_marker(name)] + if opted_in and not selected.intersection(opted_in): + item.add_marker(pytest.mark.skip(reason=f"needs the network; select with -m {opted_in[0]}")) + + +@pytest.fixture(scope="session") +def _offline_tools(tmp_path_factory: pytest.TempPathFactory) -> Path: + """Stand-ins for ``gh`` (not authenticated) and ``zizmor`` (no findings), written once.""" + bin_dir = tmp_path_factory.mktemp("offline-tools") + write_stand_in_tool( + bin_dir, + "gh", + exit_code=4, + script="echo 'gh: not authenticated in tests (#548); run gh auth login' >&2", + ) + write_stand_in_tool(bin_dir, "zizmor", stdout="[]\n") + return bin_dir + + +@pytest.fixture(autouse=True) +def _offline_platform( + request: pytest.FixtureRequest, _offline_tools: Path, monkeypatch: pytest.MonkeyPatch +) -> Generator[None, None, None]: + """Keep every test off GitHub and away from the real ``gh`` and ``zizmor`` (#548). + + Platform calls get ``RecordedGhApi({})``: an unrecorded endpoint answers + status 0, an ERROR ``unavailable``. A test or fixture that installs its own + responder replaces this one and restores it. Stand-ins for ``gh`` and + ``zizmor`` go first on ``PATH``; ``stand_in_tool`` puts a test's own in + front of them. Tests marked ``live`` get none of this. + """ + if request.node.get_closest_marker("live"): + yield + return + from darnit.core.utils import RecordedGhApi, set_gh_api_responder + + monkeypatch.setenv("PATH", f"{_offline_tools}{os.pathsep}{os.environ.get('PATH', '')}") + previous = set_gh_api_responder(RecordedGhApi({})) + yield + set_gh_api_responder(previous) + @pytest.fixture(autouse=True) def _isolate_operator_config( diff --git a/tests/darnit/context/test_dot_project_upstream.py b/tests/darnit/context/test_dot_project_upstream.py index df7835eb..99c5b72f 100644 --- a/tests/darnit/context/test_dot_project_upstream.py +++ b/tests/darnit/context/test_dot_project_upstream.py @@ -4,8 +4,9 @@ CNCF .project/ specification at: https://github.com/cncf/automation/tree/main/utilities/dot-project -Run locally to check for upstream changes: - uv run pytest tests/darnit/context/test_dot_project_upstream.py -v +Run locally to check for upstream changes (the upstream tests reach the +network, so they are skipped unless selected; #548): + uv run pytest tests/darnit/context/test_dot_project_upstream.py -v -m upstream Update the tracked hash after syncing with upstream: uv run pytest tests/darnit/context/test_dot_project_upstream.py -v --update-hash diff --git a/tests/darnit/core/test_gh_api_status.py b/tests/darnit/core/test_gh_api_status.py index 2d907d0d..01eb1466 100644 --- a/tests/darnit/core/test_gh_api_status.py +++ b/tests/darnit/core/test_gh_api_status.py @@ -17,6 +17,14 @@ from darnit.core import utils +@pytest.fixture(autouse=True) +def _no_responder(): + """These tests drive the real ``gh`` path with a mocked ``subprocess.run``; drop the suite's responder (#548).""" + previous = utils.set_gh_api_responder(None) + yield + utils.set_gh_api_responder(previous) + + def _cp(returncode: int, stdout: str = "", stderr: str = "") -> subprocess.CompletedProcess: return subprocess.CompletedProcess( args=["gh", "api", "..."], returncode=returncode, stdout=stdout, stderr=stderr diff --git a/tests/darnit/core/test_utils.py b/tests/darnit/core/test_utils.py index 8f7c3632..94987ed5 100644 --- a/tests/darnit/core/test_utils.py +++ b/tests/darnit/core/test_utils.py @@ -15,6 +15,7 @@ gh_api_safe, make_result, read_file, + set_gh_api_responder, validate_local_path, ) @@ -236,6 +237,13 @@ def test_get_git_ref_invalid_path(self, temp_dir: Path): class TestGithubCliErrors: """Tests for GitHub CLI helper failures.""" + @pytest.fixture(autouse=True) + def _no_responder(self): + """These tests drive the real ``gh`` path with a mocked ``subprocess.run``; drop the suite's responder (#548).""" + previous = set_gh_api_responder(None) + yield + set_gh_api_responder(previous) + @pytest.mark.unit def test_gh_api_missing_cli_shows_helpful_message(self): """Missing gh CLI should raise a user-facing RuntimeError."""