diff --git a/scripts/check_venv_lock_parity.py b/scripts/check_venv_lock_parity.py new file mode 100644 index 00000000..6bb0736a --- /dev/null +++ b/scripts/check_venv_lock_parity.py @@ -0,0 +1,177 @@ +#!/usr/bin/env python3 +"""Guard a stale postgresql-extra install from silently under-collecting +tests (issue #287). + +`tests_py/infrastructure/test_pg_*.py` (and its siblings across +`tests_py/integration/`, `tests_py/invariants/`) gate on +``pytest.importorskip("psycopg", ...)`` so a SQLite-only dev install +intentionally skips them — that is by design, not the bug. The bug is +different: when ``psycopg`` (or a sibling postgresql-extra package it pulls +in transitively, e.g. ``pgvector``) IS installed but at a version the +current ``uv.lock`` no longer pins, a local ``pytest --collect-only`` can +silently collect a DIFFERENT set of tests than CI's hash-pinned +``requirements/ci-postgresql.txt`` install produces — with no error and no +skip reason, just a smaller number a contributor trusts over CI's. That is +the exact drift issue #287 reports: 6572 collected locally vs. 6582 on CI, +which PR #284 had to work around by using CI's own verified count directly. + +The same class of drift was independently caught the day before, in +``tests_py/scripts/test_launcher_pins_match_lock.py`` — ``pgvector`` 0.4.2 +vs. 0.5.0, ``psycopg`` 3.3.3 vs. 3.3.4 — confirming a stale postgresql-extra +venv is a recurring, not hypothetical, failure mode in this repo, not a +one-off. + +This module makes the divergence LOUD rather than impossible to have +(nothing here can force a contributor to re-run ``uv sync``): +``postgresql_extra_drift()`` is called eagerly from ``tests_py/conftest.py`` +— the same "guard function called at module import time, `pytest.exit()` on +failure" convention that file already uses for its data-safety guards — and +any mismatch between an INSTALLED postgresql-extra package and the version +``requirements/ci-postgresql.txt`` pins for the running interpreter aborts +the session before collection starts, naming the exact packages and the +fix. A package that is not installed at all is left alone: that is either +a genuine SQLite-only dev install (nothing here to guard) or a package this +file does not pin, and both are outside what this check covers. +""" + +from __future__ import annotations + +import importlib.metadata +import re +import sys +from pathlib import Path +from typing import Callable + +from packaging.markers import Marker + +REPO_ROOT = Path(__file__).resolve().parent.parent +CI_POSTGRESQL_TXT = REPO_ROOT / "requirements" / "ci-postgresql.txt" + +# A pinned requirement line: "name==version", optionally followed by +# "; marker" and/or a trailing "\" continuation. Comment, hash-continuation, +# and blank lines are filtered by the caller before this ever sees them. +_PIN_RE = re.compile(r"^([A-Za-z0-9][A-Za-z0-9._-]*)==([^\s;\\]+)") + + +def _normalize(name: str) -> str: + """PEP 503 normalization: pip/uv/importlib.metadata all treat + ``psycopg-pool`` and ``psycopg_pool`` as the same distribution name.""" + return re.sub(r"[-_.]+", "-", name).lower() + + +# No separate "is this a comment/hash/blank line" pre-filter below: every +# such line's first non-whitespace character is "#", "-", "\", or nothing, +# none of which `_PIN_RE` (anchored on an alnum first character) ever +# matches, so they are already filtered by the `if not match` fall-through. +# A pre-filter would be redundant control flow for a result the regex +# already guarantees (confirmed empirically: dropping it parses the real +# `requirements/ci-postgresql.txt` to the identical 131-package set) — +# mutation testing surfaced exactly this as 6 equivalent survivors before +# it was removed (issue #287 PR discussion). +# +# `.split("\\", N)[0]` for any N>=1, an unbounded `.split("\\")[0]`, and +# even `.rsplit("\\", 1)[0]` are all EQUIVALENT here too, not just for +# realistic input: `_PIN_RE`'s character classes exclude backslash +# (`[^\s;\\]+`), so `match()` — which only needs a conforming PREFIX, not a +# full-string match — always halts at the first backslash it meets +# regardless of how much (or little) text trails it. Verified by +# construction and by a 200k-case fuzz across split/rsplit/maxsplit +# variants (issue #287 PR discussion), not asserted from reading the regex. +def parse_pinned_versions(requirements_text: str) -> dict[str, str]: + """``name -> version`` for every requirement line whose marker (if any) + applies to the CURRENTLY RUNNING interpreter. + + A package pinned to two versions by a ``python_full_version`` marker + (e.g. ``aiofile`` forks on 3.11) contributes exactly the one version + this interpreter would receive from ``uv sync`` — resolved via + ``packaging.markers``, the same library ``pip`` and ``uv`` themselves + evaluate markers with, never a hand-rolled comparison. + """ + versions: dict[str, str] = {} + for raw in requirements_text.splitlines(): + line = raw.strip() + requirement = line.split("\\", 1)[0].strip() + match = _PIN_RE.match(requirement) + if not match: + continue + _spec, _, marker_text = requirement.partition(";") + if marker_text.strip() and not Marker(marker_text.strip()).evaluate(): + continue + versions[_normalize(match.group(1))] = match.group(2) + return versions + + +def find_mismatches( + pinned: dict[str, str], + installed_version: Callable[[str], str | None], +) -> list[str]: + """Packages that ARE installed but at a version ``pinned`` disagrees with. + + A package absent from ``installed_version`` (returns ``None``) is not a + mismatch: that is either an intentionally narrower extra set (a + SQLite-only dev install) or a package this file does not pin at all, + and both are outside what this guard checks. + """ + mismatches = [] + for name, pinned_version in sorted(pinned.items()): + actual = installed_version(name) + if actual is not None and actual != pinned_version: + mismatches.append(f"{name}: installed {actual}, lock pins {pinned_version}") + return mismatches + + +def installed_version(name: str) -> str | None: + """Real adapter: the version ``importlib.metadata`` resolves for `name`, + or ``None`` when the distribution is not installed.""" + try: + return importlib.metadata.version(name) + except importlib.metadata.PackageNotFoundError: + return None + + +def postgresql_extra_drift() -> str | None: + """``None`` if the venv is fine or the check does not apply; else a + ready-to-print error naming every mismatched package and the fix. + + Scoped to ``psycopg`` as the activation signal: its presence means a + contributor opted into the ``postgresql`` extra, so the FULL pinned set + ``requirements/ci-postgresql.txt`` resolves to is what their collected + test count should be measured against. + """ + if installed_version("psycopg") is None: + return None # no postgresql extra installed — nothing to guard + + pinned = parse_pinned_versions(CI_POSTGRESQL_TXT.read_text(encoding="utf-8")) + mismatches = find_mismatches(pinned, installed_version) + if not mismatches: + return None + + return ( + "Local venv has drifted from requirements/ci-postgresql.txt — the " + "file CI's 'Check advertised test count' step installs from " + "(issue #287). A version-mismatched postgresql-extra package can " + "change which tests import successfully at collection time, so the " + "locally collected test count silently stops matching CI's. " + "Mismatches:\n" + + "\n".join(f" {m}" for m in mismatches) + + "\nFix: re-run `uv sync --no-default-groups --extra dev --extra " + "postgresql --extra sqlite --extra codebase --extra benchmarks` " + "(CONTRIBUTING.md § Dev setup) to resync this venv to uv.lock." + ) + + +def main(argv: list[str]) -> int: + """Standalone entry point: ``python3 scripts/check_venv_lock_parity.py``.""" + del argv + message = postgresql_extra_drift() + if message is None: + print( + "OK: no postgresql-extra version drift from requirements/ci-postgresql.txt" + ) + return 0 + print(message, file=sys.stderr) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv[1:])) diff --git a/tests_py/conftest.py b/tests_py/conftest.py index 017ffc55..2c1ba1bf 100644 --- a/tests_py/conftest.py +++ b/tests_py/conftest.py @@ -17,6 +17,8 @@ import pytest +from scripts.check_venv_lock_parity import postgresql_extra_drift + # ── Redirect every real-data root — MUST run before importing mcp_server ── # # INCIDENT 2026-07-28 (issue #219): isolation used to be conditional on @@ -326,6 +328,22 @@ def _guard_against_real_data_roots() -> None: ) +def _guard_against_venv_lock_drift() -> None: + """Refuse to run if the postgresql-extra install has drifted from + ``requirements/ci-postgresql.txt`` (issue #287). + + Same shape as the two guards above: a pure check + (``postgresql_extra_drift``, unit-tested on its own in + ``tests_py/scripts/test_check_venv_lock_parity.py``) plus an eager call + here that turns a silent, version-drift-driven change in the collected + test count into an immediate, actionable session abort — never a + quietly-smaller number a contributor trusts over CI's. + """ + message = postgresql_extra_drift() + if message is not None: + pytest.exit(f"REFUSING to run: {message}", returncode=2) + + def _pg_available() -> bool: """Check if PostgreSQL is reachable.""" try: @@ -340,6 +358,7 @@ def _pg_available() -> bool: _guard_against_populated_db() _guard_against_real_data_roots() +_guard_against_venv_lock_drift() _USE_PG = _pg_available() # The SQLite PATHS are already isolated unconditionally by diff --git a/tests_py/scripts/test_check_venv_lock_parity.py b/tests_py/scripts/test_check_venv_lock_parity.py new file mode 100644 index 00000000..e23a842f --- /dev/null +++ b/tests_py/scripts/test_check_venv_lock_parity.py @@ -0,0 +1,272 @@ +"""Tests for scripts/check_venv_lock_parity.py — the venv/CI collect-count +parity guard (issue #287). + +Pins two things: the pure parsing/comparison logic (`parse_pinned_versions`, +`find_mismatches`), and the composed `postgresql_extra_drift` seam via a +monkeypatched `installed_version` — the same seam +`tests_py/conftest.py::_guard_against_venv_lock_drift` calls for real. A +final test reads `tests_py/conftest.py`'s own source to pin that the eager +call is still wired in, mirroring `test_typecheck_env_parity.py`'s pattern +of asserting documentation/CI text directly rather than trusting a human to +keep two files in sync. +""" + +from __future__ import annotations + +import importlib.metadata +import importlib.util +import sys +import unittest +from pathlib import Path +from unittest import mock + +REPO = Path(__file__).resolve().parents[2] + +# Dotted to match the path-derived name mutmut keys mutant trampolines on +# ("scripts.check_venv_lock_parity.*") — a bare module name makes every +# mutant look unreached to a scoped mutation run (issue #262). +_spec = importlib.util.spec_from_file_location( + "scripts.check_venv_lock_parity", REPO / "scripts" / "check_venv_lock_parity.py" +) +parity = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = parity +_spec.loader.exec_module(parity) + + +class ParsePinnedVersionsTests(unittest.TestCase): + def test_parses_a_simple_pinned_line(self) -> None: + versions = parity.parse_pinned_versions("pgvector==0.5.0\n") + self.assertEqual(versions, {"pgvector": "0.5.0"}) + + def test_skips_comments_hashes_continuations_and_blanks(self) -> None: + text = ( + "# a header comment\n" + "\n" + "psycopg==3.3.4 \\\n" + " --hash=sha256:deadbeef \\\n" + " # via some-consumer\n" + ) + versions = parity.parse_pinned_versions(text) + self.assertEqual(versions, {"psycopg": "3.3.4"}) + + def test_marker_gated_line_included_when_marker_true(self) -> None: + text = "foo==1.0.0 ; python_version >= '3.0'\n" + self.assertEqual(parity.parse_pinned_versions(text), {"foo": "1.0.0"}) + + def test_marker_gated_line_excluded_when_marker_false(self) -> None: + text = "foo==1.0.0 ; python_version < '3.0'\n" + self.assertEqual(parity.parse_pinned_versions(text), {}) + + def test_python_version_fork_keeps_only_the_matching_branch(self) -> None: + """The real file forks e.g. aiofile on python_full_version; only the + branch matching THIS interpreter should survive, never both.""" + text = ( + "aiofile==3.9.0 ; python_full_version < '3.0'\n" + "aiofile==3.11.1 ; python_full_version >= '3.0'\n" + ) + self.assertEqual(parity.parse_pinned_versions(text), {"aiofile": "3.11.1"}) + + def test_normalizes_underscores_and_case_like_pep_503(self) -> None: + versions = parity.parse_pinned_versions("Psycopg_Pool==3.3.1\n") + self.assertEqual(versions, {"psycopg-pool": "3.3.1"}) + + def test_a_non_matching_line_is_skipped_not_fatal_to_the_rest(self) -> None: + """A line that reaches the regex but fails it must `continue` to the + next line, never `break` the whole loop — otherwise one malformed + line anywhere in the file would silently truncate every package + after it.""" + text = "not-a-pinned-requirement-line\npgvector==0.5.0\n" + self.assertEqual(parity.parse_pinned_versions(text), {"pgvector": "0.5.0"}) + + +class FindMismatchesTests(unittest.TestCase): + def test_no_mismatch_when_versions_agree(self) -> None: + pinned = {"pgvector": "0.5.0"} + self.assertEqual(parity.find_mismatches(pinned, lambda _name: "0.5.0"), []) + + def test_mismatch_reports_installed_and_pinned_versions(self) -> None: + pinned = {"pgvector": "0.5.0"} + mismatches = parity.find_mismatches(pinned, lambda _name: "0.4.2") + self.assertEqual(mismatches, ["pgvector: installed 0.4.2, lock pins 0.5.0"]) + + def test_absent_package_is_not_a_mismatch(self) -> None: + pinned = {"sqlite-vec": "0.1.9"} + self.assertEqual(parity.find_mismatches(pinned, lambda _name: None), []) + + def test_mismatches_are_sorted_by_name(self) -> None: + pinned = {"zzz-pkg": "1.0", "aaa-pkg": "2.0"} + mismatches = parity.find_mismatches(pinned, lambda _name: "0.0") + self.assertEqual( + mismatches, + [ + "aaa-pkg: installed 0.0, lock pins 2.0", + "zzz-pkg: installed 0.0, lock pins 1.0", + ], + ) + + +class InstalledVersionTests(unittest.TestCase): + def test_returns_none_for_an_uninstalled_distribution(self) -> None: + self.assertIsNone(parity.installed_version("definitely-not-a-real-package-xyz")) + + def test_returns_the_real_version_for_an_installed_distribution(self) -> None: + # pytest itself is always installed in a test-running environment. + self.assertEqual( + parity.installed_version("pytest"), importlib.metadata.version("pytest") + ) + + +class PostgresqlExtraDriftTests(unittest.TestCase): + """`postgresql_extra_drift` composed end-to-end, with `installed_version` + monkeypatched so the test is deterministic regardless of THIS machine's + actual venv state (issue #287 is precisely about that state varying).""" + + def _requirements_file(self, text: str) -> Path: + # `TestCase.enterContext` needs Python 3.11+; this repo's floor is + # 3.10 (pyproject.toml `requires-python`), so the cleanup is wired + # by hand via `addCleanup` instead. + tmp_dir = _tmp_dir() + self.addCleanup(tmp_dir.cleanup) + tmp = Path(tmp_dir.name) / "ci-postgresql.txt" + tmp.write_text(text, encoding="utf-8") + return tmp + + def test_none_when_psycopg_is_not_installed(self) -> None: + with mock.patch.object(parity, "installed_version", return_value=None): + self.assertIsNone(parity.postgresql_extra_drift()) + + def test_none_when_installed_matches_the_pinned_file(self) -> None: + requirements = self._requirements_file("psycopg==3.3.4\npgvector==0.5.0\n") + versions = {"psycopg": "3.3.4", "pgvector": "0.5.0"} + with ( + mock.patch.object(parity, "CI_POSTGRESQL_TXT", requirements), + mock.patch.object(parity, "installed_version", side_effect=versions.get), + ): + self.assertIsNone(parity.postgresql_extra_drift()) + + def test_reports_the_exact_drift_that_caused_issue_287(self) -> None: + """psycopg importable, pgvector present but at the pre-drift version + — the concrete pair `test_launcher_pins_match_lock.py` measured the + day before this issue was filed. Exact equality (not `assertIn`): a + wording tweak to any chunk of the message is a real change a + reviewer should see reflected here, not something a substring check + would silently keep passing under.""" + requirements = self._requirements_file("psycopg==3.3.4\npgvector==0.5.0\n") + versions = {"psycopg": "3.3.4", "pgvector": "0.4.2"} + with ( + mock.patch.object(parity, "CI_POSTGRESQL_TXT", requirements), + mock.patch.object(parity, "installed_version", side_effect=versions.get), + ): + message = parity.postgresql_extra_drift() + self.assertEqual( + message, + "Local venv has drifted from requirements/ci-postgresql.txt — the " + "file CI's 'Check advertised test count' step installs from " + "(issue #287). A version-mismatched postgresql-extra package can " + "change which tests import successfully at collection time, so " + "the locally collected test count silently stops matching CI's. " + "Mismatches:\n" + " pgvector: installed 0.4.2, lock pins 0.5.0\n" + "Fix: re-run `uv sync --no-default-groups --extra dev --extra " + "postgresql --extra sqlite --extra codebase --extra benchmarks` " + "(CONTRIBUTING.md § Dev setup) to resync this venv to uv.lock.", + ) + + def test_multiple_mismatches_are_newline_joined(self) -> None: + """A single-mismatch message can't distinguish the join separator + from a corrupted one — both produce the same one-line output. Two+ + mismatches are required to pin that each gets its own line.""" + requirements = self._requirements_file( + "psycopg==3.3.4\npgvector==0.5.0\npsycopg-pool==3.3.1\n" + ) + versions = { + "psycopg": "3.3.4", + "pgvector": "0.4.2", + "psycopg-pool": "3.3.0", + } + with ( + mock.patch.object(parity, "CI_POSTGRESQL_TXT", requirements), + mock.patch.object(parity, "installed_version", side_effect=versions.get), + ): + message = parity.postgresql_extra_drift() + assert message is not None + mismatch_block = message.split("Mismatches:\n", 1)[1].split("\nFix:", 1)[0] + self.assertEqual( + mismatch_block, + " pgvector: installed 0.4.2, lock pins 0.5.0\n" + " psycopg-pool: installed 3.3.0, lock pins 3.3.1", + ) + + def test_reads_the_pinned_file_as_utf8_explicitly(self) -> None: + """Pins the `encoding="utf-8"` keyword itself (not just its decoded + result): `requirements/*.txt` carries no non-ASCII bytes today, so a + decoded-content assertion alone cannot distinguish `encoding="utf-8"` + from the platform-default `encoding=None` on this machine — only a + call-arguments assertion can.""" + fake_path = mock.Mock(spec=Path) + fake_path.read_text.return_value = "psycopg==3.3.4\n" + with ( + mock.patch.object(parity, "CI_POSTGRESQL_TXT", fake_path), + mock.patch.object(parity, "installed_version", return_value="3.3.4"), + ): + self.assertIsNone(parity.postgresql_extra_drift()) + fake_path.read_text.assert_called_once_with(encoding="utf-8") + + def test_real_ci_postgresql_txt_parses_to_a_realistic_package_set(self) -> None: + """Guard the parser against the real file: a silently-empty parse + (wrong path, changed export format) would make every reconciliation + above vacuously pass on the actual CI-installed set.""" + versions = parity.parse_pinned_versions( + parity.CI_POSTGRESQL_TXT.read_text(encoding="utf-8") + ) + # ci-postgresql.txt installs dev+postgresql+codebase; the set is in + # the hundreds. 50 is a floor far below that and far above empty. + self.assertGreater(len(versions), 50) + self.assertIn("psycopg", versions) + self.assertIn("pgvector", versions) + + +class MainCliTests(unittest.TestCase): + def test_main_prints_ok_and_returns_0_when_no_drift(self) -> None: + with ( + mock.patch.object(parity, "postgresql_extra_drift", return_value=None), + mock.patch("builtins.print") as mock_print, + ): + self.assertEqual(parity.main([]), 0) + mock_print.assert_called_once_with( + "OK: no postgresql-extra version drift from requirements/ci-postgresql.txt" + ) + + def test_main_prints_the_message_and_returns_1_on_drift(self) -> None: + with ( + mock.patch.object( + parity, "postgresql_extra_drift", return_value="drifted: x" + ), + mock.patch("builtins.print") as mock_print, + ): + self.assertEqual(parity.main([]), 1) + mock_print.assert_called_once_with("drifted: x", file=sys.stderr) + + +class ConftestWiringTests(unittest.TestCase): + """Pin that tests_py/conftest.py still calls the guard eagerly — a + future edit that imports the module but drops the call site would leave + the check dead code that never runs.""" + + def test_conftest_imports_and_calls_the_guard(self) -> None: + conftest = (REPO / "tests_py" / "conftest.py").read_text(encoding="utf-8") + self.assertIn( + "from scripts.check_venv_lock_parity import postgresql_extra_drift", + conftest, + ) + self.assertIn("_guard_against_venv_lock_drift()", conftest) + + +def _tmp_dir(): + import tempfile + + return tempfile.TemporaryDirectory() + + +if __name__ == "__main__": + unittest.main()