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."""