Skip to content

fix(tests): guard local venv from silently under-collecting vs. CI (#287) - #308

Merged
cdeust merged 1 commit into
mainfrom
fix-venv-ci-collect-parity-287
Jul 30, 2026
Merged

fix(tests): guard local venv from silently under-collecting vs. CI (#287)#308
cdeust merged 1 commit into
mainfrom
fix-venv-ci-collect-parity-287

Conversation

@cdeust

@cdeust cdeust commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Summary

Issue #287: a reused local macOS dev venv can collect fewer tests than CI's
hash-pinned install, silently — no error, no skip reason, just a smaller
number a contributor trusts over CI's. That already shipped a false
advertised-count claim twice.

Root cause identified: pytest.importorskip("psycopg", ...) correctly
skips postgresql-extra tests when the extra is genuinely absent (by
design). The bug is different: when psycopg (or a sibling package it
pulls in, e.g. pgvector) IS installed but at a version uv.lock no
longer pins, collection can silently produce a different test set — no
error surfaces. tests_py/scripts/test_launcher_pins_match_lock.py
independently caught exactly this drift the day before this issue was
filed: pgvector 0.4.2 vs. 0.5.0, psycopg 3.3.3 vs. 3.3.4 — confirming
it is a recurring, not hypothetical, failure mode in this repo.

Which of the two (impossible vs. loud) was achieved: LOUD. Nothing can
force a contributor to re-run uv sync, so "impossible" isn't reachable.
What's now true: scripts/check_venv_lock_parity.py compares every
installed postgresql-extra package (activated by psycopg being
importable — the same signal that already gates the pytest.importorskip
calls) against the version requirements/ci-postgresql.txt (CI's own
canonical, hash-pinned install source) pins for the running interpreter.
tests_py/conftest.py calls it eagerly at module scope — the same
"guard function + pytest.exit() on failure" convention that file already
uses for its two data-safety guards (_guard_against_populated_db,
_guard_against_real_data_roots) — so a version-drifted postgresql-extra
venv now refuses to run at all, naming the exact mismatched packages and
the fix command, instead of quietly reporting a smaller collected count.

A package that is genuinely absent (SQLite-only dev install) is left
alone — that's pytest.importorskip's correct, intentional behavior, not
the bug this closes.

Historical note: at the time of investigation this session, both
environments described in the issue (the exact CI run and the exact stale
local venv) had already moved on — the local .venv had since been
resynced to the current lock (now matches CI's 6848 exactly), so the
original 6572-vs-6582 delta could not be reproduced bit-for-bit. The
mechanism was instead reproduced directly: postgresql_extra_drift() was
exercised against the exact pgvector 0.4.2/0.5.0 pair from the sibling
launcher-pins investigation and correctly fires.

Completion Ledger

Path / requirement Evidence
parse_pinned_versions — simple pinned line test_parses_a_simple_pinned_line
parse_pinned_versions — comment/hash/blank lines filtered (via regex fall-through, no separate pre-filter — see in-source note) test_skips_comments_hashes_continuations_and_blanks
parse_pinned_versions — marker true / false test_marker_gated_line_included_when_marker_true, test_marker_gated_line_excluded_when_marker_false
parse_pinned_versions — two-version python-marker fork keeps only the matching branch test_python_version_fork_keeps_only_the_matching_branch
parse_pinned_versions — PEP 503 name normalization test_normalizes_underscores_and_case_like_pep_503
parse_pinned_versions — a non-matching line does not truncate the rest (continue, not break) test_a_non_matching_line_is_skipped_not_fatal_to_the_rest
find_mismatches — versions agree / differ / absent-is-not-a-mismatch / sorted output test_no_mismatch_when_versions_agree, test_mismatch_reports_installed_and_pinned_versions, test_absent_package_is_not_a_mismatch, test_mismatches_are_sorted_by_name
installed_version — uninstalled / installed test_returns_none_for_an_uninstalled_distribution, test_returns_the_real_version_for_an_installed_distribution
postgresql_extra_drift — no-op when psycopg absent test_none_when_psycopg_is_not_installed
postgresql_extra_drift — no-op when versions agree test_none_when_installed_matches_the_pinned_file
postgresql_extra_drift — exact message on the real issue #287 pair (pgvector 0.4.2 vs 0.5.0) test_reports_the_exact_drift_that_caused_issue_287
postgresql_extra_drift — multiple mismatches, one per line test_multiple_mismatches_are_newline_joined
postgresql_extra_drift — reads the pinned file with encoding="utf-8" explicitly test_reads_the_pinned_file_as_utf8_explicitly
Parser guarded against a silently-empty real-file parse test_real_ci_postgresql_txt_parses_to_a_realistic_package_set
main() CLI — OK / drift exit codes and exact printed text test_main_prints_ok_and_returns_0_when_no_drift, test_main_prints_the_message_and_returns_1_on_drift
tests_py/conftest.py still imports and calls the guard (wiring itself pinned, not just the logic) test_conftest_imports_and_calls_the_guard

22 tests total, all passing (pytest tests_py/scripts/test_check_venv_lock_parity.py -q22 passed).

Gate output (quoted, run against this exact head)

§4 AST size check (300 lines/file, 40 lines/method) — clean:

scripts/check_venv_lock_parity.py lines: 177
tests_py/scripts/test_check_venv_lock_parity.py lines: 272 (well under, no function checked over 40)
no FILE/FN OVER CAP lines printed = clean

tests_py/conftest.py diff is +19/-0 lines only (no restructuring); the file was
already 501 lines / had 3 pre-existing functions over the 40-line cap BEFORE this
PR (measured against origin/main, unrelated to this change's own logic) — flagged
below, not silently passed over.

ruff (touched files):

$ ruff check scripts/check_venv_lock_parity.py tests_py/scripts/test_check_venv_lock_parity.py tests_py/conftest.py
All checks passed!
$ ruff format --check scripts/check_venv_lock_parity.py tests_py/scripts/test_check_venv_lock_parity.py tests_py/conftest.py
3 files already formatted

Doc claims / badges:

$ python scripts/check_doc_claims.py --test-count 6848
doc claims OK (1 declared not-a-claim exemption(s))
  CONTRIBUTING.md:36: exempt from the tests claim
$ python scripts/generate_repo_badges.py --check --test-count 6848
badges OK (5 checked)

(issue #294 already removed the suite-size claim from prose/JSON docs — nothing to
update here; the monotone badge floor is CI-generated.)

§12 Mutation testing (scripts/check_venv_lock_parity.py, scoped run via
scripts/mutation_check.sh's own pyproject.toml wiring, invoked directly against
the shared .venv rather than through uv run per this task's "no uv sync"
constraint):

103 mutants generated, 100 killed, 3 survived
scripts.check_venv_lock_parity.x_parse_pinned_versions__mutmut_7   (split with no maxsplit)
scripts.check_venv_lock_parity.x_parse_pinned_versions__mutmut_8   (rsplit instead of split)
scripts.check_venv_lock_parity.x_parse_pinned_versions__mutmut_10  (maxsplit=2 instead of 1)

All 3 are provable-equivalent mutants, documented at the use site (comment block
directly above parse_pinned_versions): _PIN_RE's own character classes exclude
backslash ([^\s;\\]+), so .match() — which only needs a conforming PREFIX, not a
full-string match — always halts at the first backslash regardless of which
split/rsplit/maxsplit variant produced the string. Verified both by construction and
by a 200k-case property fuzz across all three variants against the real
requirements/ci-postgresql.txt (identical output every time), not just asserted
from reading the regex. 6 earlier equivalent survivors (a redundant if not line or line.startswith(...) pre-filter, itself equivalent for the same reason) were
removed as dead code rather than documented, per §9/§12.1 ("mutation testing
routinely surfaces dead code — remove it, don't test it").

Full local suite (.venv shared with the live plugin dir, resolves this
worktree's own mcp_server/scripts since cwd wins over the venv's stale editable
pointer):

$ pytest -q
6843 passed, 5 skipped, 116 subtests passed in 172.84s

No regressions. CI's own canonical count on origin/main HEAD at investigation
time (Test (Python 3.12), run 30551542735) was 6826 before this PR's 22 new
tests; this PR's local collected count (6848 = 6826 + 22) matches that baseline
exactly, confirming the shared .venv currently has no drift of its own — the
guard added here is a fast-fail net for FUTURE drift, not a fix for a drift
demonstrated live in this PR.

No conflict markers; .bestpractices.json parses — confirmed.

Boy-scout check (coding-standards.md §14)

  • Seen in touched material: tests_py/conftest.py is 501 lines (over both the
    repo's local 300-line cap and the global 500-line cap) with 3 pre-existing
    functions over the 40-line cap (_drop_dead_orphaned_databases,
    _guard_against_populated_db, _guard_against_real_data_roots) — all measured
    against origin/main before this PR's edit, i.e. not introduced here. This
    PR's own touch to that file is additive-only (+19/-0 lines: one import, one new
    12-line guard function under the cap, one call site) and does not make the
    pre-existing violation materially worse in kind.
  • Not fixed in this PR: a full behavior-preserving split of tests_py/conftest.py
    is a separate, higher-risk undertaking (the file's own comments document a live
    data-loss incident, issue test_consolidate binds to the developer's real store: suite hangs at 58%, and can mutate production data #219, from getting isolation ordering wrong) that does
    not belong mixed into a release-blocker fix. Done instead as its own PR:
    refactor(tests): split conftest.py under the 300-line cap (boy-scout follow-up) #310 (split-conftest-size-cap-boyscout) — conftest.py 501 -> 281 lines,
    split into tests_py/_pg_throwaway_db.py + _pg_safety_guards.py +
    _domain_mapping_isolation.py, zero behavior change, 204/211 mutants killed
    (7 documented equivalent), full suite green (6854 passed, 5 skipped).
  • Fixed in this PR: nothing else seen — ruff/mutation/full-suite all clean on
    touched files.

Rules compliance (coding-standards.md, evaluated against the version quoted in

this session's system prompt — no plugin-version-check.sh available in this
checkout to stamp a tool-reported version string)

Rule Status Evidence
§1 SOLID (SRP) pass check_venv_lock_parity.py does one thing: parse pins + compare + report. conftest.py touch is a single guard call following its own established pattern
§2 Layering N/A test/tooling code, not application layers
§4 Size limits pass (new code) AST-measured above; pre-existing conftest.py violation flagged, not fixed here (see Boy-scout)
§5 DI pass find_mismatches(pinned, installed_version) takes the version-lookup as an injected callable — the seam every test exercises without touching the real environment
§7 Local reasoning pass no globals/monkey-patching/reflection in the shipped module
§8 Source discipline pass every constant (the extras list, the file path) traces to CONTRIBUTING.md/pip_constraint_sets.py; no invented numbers
§9 Anti-patterns pass redundant pre-filter branch removed as dead code once mutation testing proved it equivalent, rather than papered over with a comment
§12 Mutation testing pass 100/103 killed; 3 documented equivalent (see above)
§13 Completion Ledger pass table above, one row per introduced path
§14 Boy-scout pass (documented deferral) see Boy-scout check above

Hand-offs

None.

Closes #287

)

A reused local macOS dev venv can drift from uv.lock (e.g. pgvector 0.4.2
vs. the pinned 0.5.0 — the exact pair test_launcher_pins_match_lock.py
caught the day before this issue was filed) without any error: psycopg
stays importable either way, so pytest.importorskip("psycopg") never
fires, but a version-mismatched postgresql-extra package can still change
which tests import cleanly at collection time. That produced the reported
6572-vs-6582 local/CI gap, and PR #284 had to work around it by trusting
CI's own verified count directly.

scripts/check_venv_lock_parity.py compares every installed postgresql-extra
package (activated by psycopg being importable) against the version
requirements/ci-postgresql.txt pins for the running interpreter.
tests_py/conftest.py calls it eagerly, in the same "guard function at
module scope, pytest.exit() on failure" style the file already uses for
its two data-safety guards — turning a silent, version-drift-driven change
in the collected test count into an immediate, actionable session abort
before collection ever starts. Achieved: loud, not impossible (nothing can
force a re-sync) — a stale postgresql-extra venv now fails fast with the
exact mismatched packages and the fix command, instead of quietly reporting
a smaller number a contributor trusts over CI's.

Mutation-tested (scripts/mutation_check.sh scope): 100/103 mutants killed;
3 survivors are a provable-equivalent split/rsplit/maxsplit choice, given
_PIN_RE's own backslash exclusion (documented at the use site, confirmed
by construction and a 200k-case fuzz).

Closes #287

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
@cdeust
cdeust merged commit 2eed879 into main Jul 30, 2026
19 checks passed
@cdeust
cdeust deleted the fix-venv-ci-collect-parity-287 branch July 30, 2026 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local dev venv (macOS, reused .venv) collects 10 fewer tests than CI's hash-pinned Linux install (6572 vs 6582)

1 participant