test: the harness guards run in the gate, without a database (#432) - #921
Conversation
…prompt#432) 152 tests in test/pytest/ and NOT ONE of them ran in CI. selftest 350 says it plainly about its own subject: a guard that does not run is a comment. This makes the guard-testing half of the corpus run in the gate, and the change that allows it is one import. ONE EAGER IMPORT COUPLED THE WHOLE CORPUS TO A DATABASE DRIVER --------------------------------------------------------------- `conftest.py` imported psycopg at module scope. conftest is imported before every run, so a DATABASE DRIVER was a hard requirement of every test -- including the 61 that never open a connection. With psycopg absent the run did not fail a test, it failed to COLLECT: ImportError while loading conftest '.../conftest.py' conftest.py:15: in <module> import psycopg E ModuleNotFoundError: No module named 'psycopg' Measured, both directions, on the corpus as it stands: import at module scope 0 of 142 tests run with no driver installed deferred into fixtures 61 of 142 run and pass The layer itself never needed it: pgc_vacuity.py imports ast, numbers, pathlib and pytest. pgc_cluster.py imports no driver either. It was conftest alone. README.md records why the corpus is not in `SUITES`: pgc_skip treats a missing dependency as a failure rather than a skip, so registering it would redden every job until the driver is installed everywhere. THAT ARGUMENT IS ABOUT THE CLUSTER TESTS. It never applied to the guard tests -- and until this import moved there was no way to separate them, because importing conftest imported the driver. THE JOB -------- `.github/workflows/ci.yml` gains `pytest-guards`: ubuntu-latest, no database, no build, an interpreter and two pinned packages. Measured: 61 tests, ~3 seconds. THE FILE LIST AND THE PINS ARE BOTH DERIVED. The list comes from `NO_CLUSTER` in test_harness_deps.py and the pins from requirements-test.txt, because a second copy of either is a hand-maintained value that goes stale silently -- which this repository has spent a day proving, on TESTS.md's totals line (commandprompt#908). Two arms hold the job to that: it must derive the list, and it must name no corpus file literally. The job also asserts psycopg is ABSENT before running. Without that, the tests would pass for the ordinary reason and prove nothing about the coupling. A SHIM RATHER THAN AN UNINSTALL -------------------------------- test_harness_deps.py proves the property behaviourally: it writes a `psycopg.py` that raises on import, puts it FIRST on the path, and requires the no-cluster files to collect and pass anyway. Uninstalling the driver would test the machine rather than the harness, could not run beside anything else, and would leave the environment broken if the test died. Both behavioural arms assert the shim actually bites before believing anything it produces. THE CONTROL IS THE HALF THAT MATTERS. Deferring must make the IMPORT lazy, not the database optional, so a cluster test must STILL fail with the driver gone, and fail naming the shim rather than by being quietly skipped. the guard half, driver shimmed out 61 passed, exit 0 a cluster test, driver shimmed out fails, naming the shim the CI job simulated end to end 61 passed, exit 0, psycopg absent BOTH HARNESSES, per jd's rule. selftest 350 carries the static half -- conftest imports no driver at module scope, the driver IS still imported inside the fixtures that connect (or the first arm is satisfied by a harness that talks to no database at all), and the job derives rather than repeats. harness_selftest 450 passed + 0 failed + 0 unrunnable, rc=0 pytest corpus 154 passed, serial and under -n 4 docs_style 9 checks PASSED shellcheck -S error clean Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
linuxhikerpm
left a comment
There was a problem hiding this comment.
Blocking: the new gate omits test_layer.py, even though it is driverless and is the corpus's direct vacuity-layer test.
Reviewed exact head 668756a1e7320a251d8f15d27839dcb713a4bdff in an isolated worktree. The import deferral is correct and the new job itself passes, but NO_CLUSTER contains only:
[
"test_docs_cover_the_corpus.py",
"test_guards_pinned.py",
"test_ordered.py",
"test_runshape.py",
]test_layer.py has 16 tests, requests only pytester and expect, and never requests pgc_conn or pgc_cluster. I ran it with a psycopg.py shim first on PYTHONPATH whose import raises immediately—the same missing-driver condition this PR is about:
$ PYTHONPATH="$shim:." pytest -q -p no:cacheprovider test_layer.py
................ [100%]
16 passed in 0.43s
So the job runs 61 guard tests where at least 77 are available under its stated no-database/no-driver contract. The omitted file is not peripheral: it directly tests that assertionless tests, empty comparisons, self-comparisons, bare skips, count mismatches, broad exceptions, and unrunnable states are refused. Those are the layer's core guards.
The existing list guard cannot catch this: test_the_no_cluster_list_still_names_files_that_exist proves every listed file exists, not that every eligible file is listed. A valid existing file can be omitted indefinitely while CI remains green.
Please add test_layer.py to NO_CLUSTER, update the measured count/documentation, and add a negative arm proving this specific eligible file cannot disappear from the job's derived set. The behavioral shim run above is the control: it establishes that inclusion is safe rather than inferred from imports.
I also tried test_build_refusal.py; most of it is driverless, but two unprivileged-user arms fail in this container for a separate filesystem-permission reason, so I am not asking to add it wholesale without splitting its environment-dependent cases. The finding here is confined to test_layer.py, which passed cleanly as one file.
The gate job's file list came from NO_CLUSTER, a hand-written list, and nothing
decided whether it was RIGHT. The only arm checked that the names it held exist,
and at_least(len(NO_CLUSTER), 4) is satisfied by any list of five. So a new
database-free test file was silently skipped by the job and nothing went red --
a coverage hole in the mechanism that exists to give those tests coverage.
Membership is now DECIDED, from a property of each file, and reconciled against
the declaration in BOTH directions. The property is read with ast, not a line
regex: this corpus builds tests as strings for pytester, so a file that merely
MENTIONS the driver in prose must not count, and one that reaches a cluster only
through a fixture must. The cluster fixtures are read off conftest.py rather than
named here, so adding one does not need a second edit.
NO_CLUSTER missing a database-free file [1: undeclared:test_ordered.py]
NO_CLUSTER claiming a cluster test [1: needs-a-cluster:test_connection.py]
Both reddened; mutations count-asserted and restored byte-exact. The message
names the offender and which way the disagreement goes, because "the lists
differ" is not something a reader can act on.
AND NO COUNT IS WRITTEN ANYWHERE. Six sentences stated "61 of 142"; the corpus
is now 168 tests, and the ci.yml comment said 152 against a corpus of 154, so it
was wrong the day it was written. The gate's step prints how many files it ran
and pytest prints how many tests passed, which is commandprompt#908's rule: a derived value
that a human maintains is a defect, and the corpus gate provably cannot police
prose -- its totals guard matches only the bold fixed-form line commandprompt#908 removed.
Three smaller things, each found by reading this change rather than the tree:
A table-of-contents link must RESOLVE, not merely name a file. TESTS.md gained
an entry whose anchor stripped the underscores out of test_harness_deps.py, so
the link went nowhere while both existing arms passed -- they sweep for NAMES,
and a broken link still contains the name it points at. The rule is GitHub's and
mechanical, it has ONE definition in this file, and the sweep carries a coverage
premise because without nullglob an unmatched glob stays literal and a loop that
runs no checks reports every check it did run as passing.
Two `printf ... | grep -qxF` pipelines are gone, one of them written by this
change. Under this suite's pipefail grep -q exits on the first match, printf
takes EPIPE, and the pipeline reports failure though the pattern WAS present --
so a name that matched is counted absent. Measured at 10 spurious absences in 40
runs under load; 40 runs of the rewritten sweep over the largest document give
one distinct answer. Selftest 080 states this rule and its sweep has never
entered test/selftest/, which commandprompt#486 is fixing separately.
harness_selftest 465 passed + 0 failed + 0 unrunnable = 465 PASSED
pytest corpus 168 passed
shellcheck -S error -s bash clean
One consequence to merge in order: with this landing first, commandprompt#922 adding a
database-free test file will REDDEN this arm until that file is declared. That is
the hole closing, not a regression -- before this change the file would have been
skipped in silence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
|
Pushed The hole that matteredThe job's file list came from Membership is now decided from a property of each file and reconciled against the declaration in both directions. Read with It bites, both ways, with the offender and the direction named: Each mutation count-asserted, restored byte-exact, green again after. No count is written anywhere nowSix sentences stated "61 of 142". The corpus is 168 tests, and the Three smaller things, all found by reading this change rather than the treeA contents link must resolve, not merely name a file. The entry I added stripped the underscores out of Two GitHub's anchor rule had two copies (a stream form and a per-string helper). Now one. EvidenceRun on the committed tree ( One consequence for the merge orderWith this landing first, #922 adding a database-free test file will redden this arm until that file is declared in |
linuxhikerpm
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 2ad25372b26f8c159e6ec4a5abea8fe92960bfd2. The requested omission is fixed, and the replacement is stronger than a one-file patch: NO_CLUSTER now includes test_layer.py and is reconciled in both directions against an AST-derived partition.
Independent controls in cursor-2604:
--partitionclassifies six database-free files, includingtest_layer.pyandtest_build_refusal.py.--disagreereturns[]on the unmodified tree.- With a first-on-
PYTHONPATHpsycopg.pyshim that raises on import, all six declared files pass:113 passed in 5.09s. - The same run from a root-owned worktree under
/home/jdfailed only the two deliberate cross-user permission arms becausepostgrescould not traverse that private parent; relocating the identical commit to an accessible/tmpworktree made both pass. That control distinguishes an environment premise from driver coupling. - The exact-head driverless GitHub job is green.
The mutation arms name both disagreement directions, and the shell selftest invokes the classifier independently of the pytest file it classifies. This resolves my blocking review.
linuxhikerpm
left a comment
There was a problem hiding this comment.
Blocking at exact head 2ad25372b26f8c159e6ec4a5abea8fe92960bfd2: the deeper adversarial pass found two classifier false-greens that supersede my approval.
-
The classifier’s own controls are not fully gated.
test_harness_deps.pyis classified cluster-bound and excluded frompytest-guards; its own test acknowledges that. Disabling transitive conftest-fixture closure leaves shell selftest 350 green and all 113 CI-selected tests passing. Only the excluded targeted closure test fails. Thus a load-bearing classifier branch can break while both real gates remain green. Move the database-free classifier controls into a gated file or reproduce every classifier branch in the shell gate, with a removal mutation proving both paths bite. -
Standard pytest fixture forms are misclassified as database-free.
_defs()only considers module-level functions and positional parameters, whilepartition()only intersects those names with cluster roots. Direct fixtures using each of these forms were classifiedfreeand then failed under the no-psycopg shim:- a test method inside a class;
@pytest.mark.usefixtures(...);- a keyword-only fixture parameter;
request.getfixturevalue(...);@pytest.fixture(name=...)aliasing.
Support these ordinary dependency forms or mechanically reject them from this corpus, and put each adversarial case behind the actual CI gate.
The declared-list omission proof itself is sound, and the exact CI set passes under the driver shim. The blocker is that the derived property is narrower than pytest dependency resolution and its complete test suite does not run where the property gates coverage.
Stack note: merging current #922 after this head also conflicts in TESTS.md/selftest 350, and #922’s new test_suite_accounting.py is database-free but absent from this head’s declaration. The stacked result must add it rather than silently shrinking coverage.
…ompt#922's new file commandprompt#922 merged first, so main now carries test/pytest/test_suite_accounting.py, which needs no database. This branch's membership arm asserts set equality between the declared NO_CLUSTER list and the files an ast property says are database-free, so an undeclared database-free file reddens it. It did, before the entry was added: FAILED test_the_declaration_is_exactly_the_database_free_half FAILED test_the_gate_runs_the_membership_decision_rather_than_only_this_file 2 failed, 18 passed With "test_suite_accounting.py" declared: 20 passed, the partition is 7 database-free against 6 cluster-bound, and membership_report() returns []. That is the coupling working rather than a cost: before this branch the file would have been skipped by the guards job in silence. TWO CONFLICTS, both resolved deliberately. test/selftest/350 was COMMENT-ONLY. commandprompt#922 and this branch independently fixed _dcv_absent's EPIPE bug and made the SAME fix -- grep -cxF on a here-string -- so the code line is identical on both sides and sits outside the conflict. That was asserted rather than eyeballed: zero non-comment lines on either side, which is what rules out a careless resolution restoring `printf | grep -qxF`. commandprompt#922's comment is the base because it carries the commandprompt#923 provenance, and this branch's measurement is folded in as a second data point: 6 false absences in 400 trials at 170 names under synthetic load, and 10 in 40 in isolation. They bracket the rate rather than disagreeing, so the comment now says it is load- AND size-dependent. test/pytest/TESTS.md was the table of contents and the section bodies. commandprompt#922's section 14 keeps 14 and this branch's becomes 15, with Adding a test, What this corpus does NOT yet refuse, and Traps this corpus records shifting to 16, 17 and 18. Checked structurally rather than by reading: 18 headings, 18 contents entries, contiguous 1..18, and every anchor matches its heading under GitHub's own rule -- which this branch's own sweep is what enforces. Gate on the merged tree, /usr/local/pg17a, PGC_SKIP_BUILD unset: harness_selftest 561 passed + 0 failed + 0 unrunnable = 561, PASSED pytest corpus 182 passed the guards subset 7 files, 127 passed, with psycopg shimmed to raise on import The last line is the job this branch adds, run under its own condition. Its control is that the cluster-bound files still FAIL there: test_connection.py 8 errors and test_hilbert_locality.py 18 errors under the same shim. Without that control, "the guards passed" is equally satisfied by a harness that reaches no database at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
linuxhikerpm
left a comment
There was a problem hiding this comment.
Blocking at exact head 7d51838ef3ddd677482637d09d059510a4abd5fc: the two classifier false-greens remain, and the merge adds an audit artifact.
-
test_harness_deps.pyremains outsideNO_CLUSTER, so its classifier controls do not run inpytest-guards. Removing transitive fixture closure leaves shell selftest 350 green and all 127 integrated CI-selected tests passing; only the excluded targeted test fails. A load-bearing classifier branch therefore still escapes both actual gates. -
The classifier still handles only module-level functions with positional parameters. Class methods,
@pytest.mark.usefixtures, keyword-only fixture parameters,request.getfixturevalue, and@pytest.fixture(name=...)aliases are all classified database-free and then fail under the no-psycopg shim. Support these normal pytest forms or mechanically reject them, with controls behind the real gate. -
The merge commit accidentally adds
.shim/psycopg.pycontaining the gate-probe ImportError. Neither parent contains it; remove this audit artifact.
The #922 integration itself is correct: test_suite_accounting.py is now declared database-free, its 14 tests pass, and the integrated set reaches 127 passing tests. That does not close the two classifier blockers above. Exact-head build/guard jobs are green; PG17/PG18 suites were still pending when reviewed.
|
Status against current main, measured rather than read off the rollup. Merge order matters, and the arm is the thing that says so. This branch requires set equality in both directions between Measured, by copying #923's new pytest files into One file, named by the arm, with three arms agreeing. #925 will add its own ( So:
Also worth knowing before merging either: This branch is 13/13 green at |
…own controls now run in the gate (commandprompt#432) @linuxhikerpm blocked this with three items. All three were real and all three are fixed. The first two I reproduced before fixing; the third I verified is mine. 1. THE CLASSIFIER'S OWN CONTROLS DID NOT RUN IN THE GATE THEY GOVERN. test_harness_deps.py defines the classifier that computes NO_CLUSTER, and the `pytest (harness guards, no database)` job runs exactly NO_CLUSTER -- which does not contain that file, and correctly so: it hands real cluster-bound file names to pytest in a subprocess, so it needs whatever they need. The code deciding the job's contents was the one thing the job could not check. The eight synthetic classifier arms are now in test_harness_deps_classifier.py, which drives a corpus it writes in tmp_path, reads nothing from the real tree, is declared in NO_CLUSTER, and imports the classifier as a LIBRARY -- which does not make it cluster-bound, because the propagation rule reads string constants naming corpus files, not imports. PROVED, which is what the review asked for. Neutering the transitive conftest-fixture closure: before this change the gated job stayed GREEN, selftest 350 stayed green, and only the excluded targeted test failed after this change the gated job goes RED, naming four arms in test_harness_deps_classifier.py 2. FIVE ORDINARY PYTEST DEPENDENCY FORMS WERE CLASSIFIED DATABASE-FREE. `_defs` walked `tree.body` and `args.args` only. Measured against the classifier as it was, each a DIRECT dependency on a cluster root: a test method inside a class free -> bound @pytest.mark.usefixtures free -> bound a keyword-only fixture parameter free -> bound request.getfixturevalue free -> bound an ALIASED cluster root in conftest free -> bound module-level positional bound (the shape it did see) a helper's fixture-named parameter free (and must stay free) a plain test, and `self` free (and must stay free) THE FIFTH NEEDED A SHAPE THE REVIEW DID NOT GIVE, and this matters because I nearly recorded it as not reproducing. An alias on the TEST side is caught anyway: the underlying fixture still takes the root positionally. It is an alias on the ROOT, in conftest, that hides it -- the root was recorded under the def's name while a test requests it under the alias. Measured: roots=['_mk'] before, roots=['conn'] after, and the test requesting `conn` goes free -> bound. `request.getfixturevalue` is NOT supported and NOT banned. The name is computed at run time, so no AST can resolve it, and a file using it is classified CLUSTER-BOUND: wrong in the direction that costs CI time, rather than the direction that greens a gate over tests nothing ran. `self` and `cls` are dropped, because they are bound by Python and no fixture can be requested under either name. Every class is walked rather than only `Test*` ones: a classifier that guesses the collection convention is one convention change from being wrong, and counting a non-collected method errs safely. 3. .shim/psycopg.py WAS AN AUDIT ARTIFACT AND IS GONE. It holds `raise ImportError("psycopg shimmed out by the gate probe")`, it is present at 7d51838 and absent in BOTH parents, so the merge commit added it. Mine, from probing the no-driver path by hand. Removed. MEASURED full pytest corpus 188 passed the gated set exactly as CI runs it 8 files, 141 passed, psycopg absent harness_selftest 561 checks, 561 passed + 0 failed, rc 0 partition / NO_CLUSTER agree in both directions, disagreements [] the real corpus's classification unchanged by the widening Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
|
@linuxhikerpm all three items were real and all three are fixed at 1. The classifier's controls now run in the gate they governYou were right about the shape of it, and the reason is worth stating: The eight synthetic classifier arms are now in The removal mutation you asked for. Neutering the transitive conftest-fixture closure: Four arms rather than one, because the closure is load-bearing for the classification as a whole. 2. The five dependency formsReproduced against the classifier as it was, each a direct dependency on a cluster root, so none of it leans on the closure arms:
One correction, in your favour but not in the shape you gave. An alias on the test side is caught anyway — the underlying fixture still takes the root positionally — so my first attempt at that case did not reproduce and I nearly recorded it as not holding. It is an alias on the root, in conftest, that hides it: the root was recorded under the def's name while a test requests it under the alias. Measured
Two choices inside that worth flagging for review: 3.
|
jdatcmd
left a comment
There was a problem hiding this comment.
Reviewed at b10e3f8a. I verified @linuxhikerpm's three blockers myself rather than taking the "closed" on trust, then attacked what was left.
Their three blockers are genuinely closed. .shim/psycopg.py is absent from a 997-entry tree. The classifier's controls were split into test_harness_deps_classifier.py, which is in NO_CLUSTER, so the job whose file list is that list actually runs them. And the derived job is green driver-free — reproducing CI's own command line in a venv pinned to pytest==9.1.1 with import psycopg raising ModuleNotFoundError: 8 files, 141 passed.
The five new classifier capabilities are load-bearing. Six mutations against the real test_harness_deps.py, each diffed to prove it applied, file restored byte-identical:
drop class descent -> 1 failed test_a_test_method_inside_a_class_is_a_fixture_request
drop usefixtures -> 1 failed test_usefixtures_is_a_fixture_request_without_a_parameter
drop kwonlyargs -> 1 failed test_a_keyword_only_parameter_is_a_fixture_request
drop getfixturevalue -> 1 failed test_a_dynamic_request_is_treated_as_cluster_bound
alias -> def's own name -> 1 failed
bound[name] = True -> 4 failed (the cheap wrong fix, caught by your negative control)
That last one is the one I care about. The cheapest way to make every arm pass is to call everything cluster-bound, and test_a_plain_test_and_a_helpers_parameter_stay_database_free blocks it. Two findings below, neither of them fatal.
1. usefixtures is understood on a function, but not on a class or a module (should-fix)
_usefixtures() is consulted per-function inside _defs(), and _collectable() yields FunctionDefs only. Nothing reads a ClassDef's decorator_list, and nothing reads a module-level pytestmark. Both are ordinary pytest, and the class form is the one your other new feature — class-method descent — exists to serve.
Measured, four files through partition(), with the two working forms as controls:
free (says NO database) : ['test_cls_form.py', 'test_mod_form.py']
bound (says needs one) : ['test_fn_form.py', 'test_method_param.py']
inputs 4 == free 2 + bound 2
test_fn_form.py is @pytest.mark.usefixtures("pgc_conn") on a def — correctly bound, and it is the only form test_usefixtures_is_a_fixture_request_without_a_parameter exercises. test_cls_form.py is the identical mark on a class; test_mod_form.py is pytestmark = pytest.mark.usefixtures("pgc_conn"). Both classified database-free, which puts them in NO_CLUSTER and hands them to the driverless job.
It fails loud, not silent, and that is why this is should-fix rather than blocking:
ERROR test_cls_form.py::TestConnected::test_it - FileNotFoundError: [Errno 2] ...
ERROR test_mod_form.py::test_it - FileNotFoundError: [Errno 2] ...
2 errors in 0.11s
No file in the corpus uses either shape today, so nothing is broken right now. But the classifier is the thing that decides membership, --disagree would demand such a file be declared database-free, and the job would then error on it — the two gate arms cannot both be green. Worth closing while the code is fresh.
2. test_the_job_installs_no_database_driver's first assertion cannot catch a driver install (should-fix)
installs_driver = "psycopg" in job and "pip install" in job and "pip show psycopg" not in job
expect.text(repr(installs_driver), "False", ...)
expect.at_least(job.count("pip show psycopg"), 1, ...)The second assertion requires the very string whose absence the first one's third conjunct needs. So whenever assertion 2 passes, installs_driver is pinned to False no matter what the install line installs. The two assertions guarantee each other's vacuity.
Measured — I changed the job's install line to pip install --quiet $PINS psycopg[binary]==3.3.5 and evaluated the arm's own expression against the mutated file:
assertion 1 installs_driver = False -> want 'False' -> PASSES
assertion 2 count('pip show psycopg') = 1 -> at_least 1 -> PASSES
the job it just described: installs psycopg[binary]==3.3.5
Both green while the job installs the driver the arm is named for. The only edit that reddens assertion 1 is deleting the guard line, which reddens assertion 2 as well — so assertion 1 detects nothing assertion 2 does not.
The product is fine. ! /tmp/pgcvenv/bin/pip show psycopg is in the step under set -euo pipefail, so a real driver install fails the job at runtime. This is a dead test arm, not a hole. A form that can fail: assert no pip install line in the region names the driver, e.g. [l for l in job.splitlines() if "pip install" in l and DRIVER in l] == [] (DRIVER is already defined at line 41), with a control that reddens when it is appended.
What I could not fault
The reconciliation in membership_report is genuinely two-directional — absent, needs-a-cluster, undeclared — and selftest 350 drives it with a fixture corpus that fails in each direction. --disagree over the real corpus returns [] at 8 free / 6 bound / 14 files. The job cannot pass with the driver present: the venv has no --system-site-packages, empty FILES is caught by test -n, and zero collected would exit 5. _walk_own not descending into a nested def is right.
Splitting the controls into their own file so the job that is the list actually runs them is the correct shape, and it is what made @linuxhikerpm's first blocker a real one.
…arm that could not fail, and stop the shell part driving the corpus (commandprompt#432) Three changes. The first is jd's rule, the other two are @jdatcmd's findings. 1. THE SHELL PART NO LONGER DRIVES THE PYTEST CORPUS. jd's rule, set while this branch was in review: the shell tests and the pytest corpus are PARALLEL IN FUNCTIONALITY and must not call, import or reference each other outside docs. Each asserts against the product, in its own terms, never against the other harness's implementation. This branch's additions to test/selftest/350 broke that in the strongest form -- they INVOKED the decider: python3 "$_hd_decide" --disagree "$PGC_TESTDIR/pytest" python3 "$_hd_decide" --partition "$PGC_TESTDIR/pytest" plus arms reading test/pytest/conftest.py as text. A shell arm driving the python decider is not a second measurement of the property: it agrees by construction and can never report the decider wrong. Both blocks are gone. NO COVERAGE IS LOST, and that is checked rather than asserted -- every property they tested is already in the corpus, where it is native: conftest imports no driver at module scope test_harness_deps.py::test_conftest_imports_no_database_driver_at_module_scope the declaration is exactly the database-free half, both directions test_harness_deps.py::test_the_declaration_is_exactly_the_database_free_half the partition accounts for every file, and the three report cases test_harness_deps.py and test_harness_deps_classifier.py What stays in the shell part is the CI WORKFLOW, which belongs to neither harness. Net new cross-harness references in 350 against main: the ci.yml job-name grep, and three synthetic strings in the anchor fixtures -- a string that resembles a filename is a fixture, not a reference. 2. `usefixtures` ON A CLASS AND AT MODULE LEVEL were classified database-free, and the class one is pointed: class-method descent exists to serve exactly that shape, so the two belonged in one change and only one was there. Measured against the previous head: @pytest.mark.usefixtures on a def bound (the only form it saw) @pytest.mark.usefixtures on a class free -> bound pytestmark = pytest.mark.usefixtures free -> bound pytestmark = [ ... ] free -> bound a method taking the fixture bound (unchanged) a plain test free (unchanged) pytest applies a class decorator to every method and a module-level `pytestmark` to every test. Four arms, including the cost side: only `usefixtures` is a dependency, or every parametrised class would be cluster-bound. Removing the class descent reddens its arm alone; removing the module read reddens the two module arms. 3. `test_the_job_installs_no_database_driver`'s FIRST ASSERTION COULD NOT FAIL. It needed `"pip show psycopg" not in job` while the second assertion required that exact string, so it was pinned to False whatever the install line installed. @jdatcmd added `psycopg[binary]==3.3.5` to the install line and both assertions passed. The product was never at risk -- `! pip show psycopg` runs under `set -euo pipefail` -- so it was a dead arm rather than a hole. It now reads the install LINES and asks whether any names the driver, with a control in the same arm that appends such a line to a copy and requires the same expression to see it. MEASURED harness_selftest 550 checks, 550 passed + 0 failed + 0 unrunnable, rc 0 pytest corpus 192 passed the gated set as CI runs it 8 files, 145 passed, psycopg asserted absent the classifier file 18 arms, and each new capability reddens its own arm Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…harness-guards branch Two conflicts, both additive. CHANGELOG.md: two regions, both sides appending at the top of the same section -- kept both. TESTS.md: both sides number a section, and the raises section main landed at 18 collides with this branch, so it is renumbered 20 and its TOC entry and anchor follow. Verified structurally: 20 headings, 20 TOC entries, numbers contiguous 1..20, titles identical between the two lists, every TOC anchor equal to the anchor GitHub derives from its heading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…ies, and the job's list is the intersection (commandprompt#432) commandprompt#927 landing made this branch's own arm fire, by name, which is what it exists to do: disagreements: [1: undeclared:test_raises_sqlstate.py] THE OBVIOUS FIX WAS WRONG, and the arm that caught it was right. Declaring the file turned the driver-free job red: four of its arms fail with psycopg shimmed out, because the modules it hands to `pytester` import the driver. It requests no cluster fixture and it still cannot run where there is no driver. MY SECOND ATTEMPT WAS ALSO WRONG, and an existing arm refused it. Folding the driver condition into `partition()` contradicted `test_the_classifier_is_not_fooled_by_prose_that_names_the_driver`, which asserts that a generated inner test requesting a cluster fixture is the INNER run's requirement and not this file's. That arm is correct, and breaking it was the signal that I was overloading one property with two meanings. SO THERE ARE TWO PROPERTIES, derived separately: partition() -> does this file request a cluster? driver_dependent() -> does it need psycopg IMPORTABLE, even with no cluster? job_runnable() -> the intersection, which is what the job can run and `membership_report` compares the declaration against the intersection, with `needs-the-driver:` as a kind of its own -- `needs-a-cluster:` would be a wrong diagnosis and the reader's next action differs. TWO CONDITIONS FOR THE DRIVER PROPERTY, because a driver import in a string is not enough on its own. `test_harness_deps_classifier.py` writes fixture corpora containing `import psycopg` and only ever PARSES them -- nothing imports those files. Reading the string alone would have thrown that file out of the gate it exists to be in. The difference is whether the file drives `pytester`. cluster-free 9 files, including test_raises_sqlstate.py driver-dependent test_raises_sqlstate.py job runnable 8, which is the declaration disagreements [] Three arms pin it, including both controls: a file that only parses a driver import stays job-runnable, and prose naming the driver is not a dependency on it. MEASURED harness_selftest 550 checks, 550 passed + 0 failed + 0 unrunnable, rc 0 pytest corpus 227 passed the gated set as CI runs it 8 files, 150 passed, psycopg asserted absent Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
|
@jdatcmd both findings are fixed at 1.
|
| form | before | after |
|---|---|---|
@pytest.mark.usefixtures on a def |
bound | bound |
@pytest.mark.usefixtures on a class |
free | bound |
pytestmark = pytest.mark.usefixtures(...) |
free | bound |
pytestmark = [ ... ] |
free | bound |
| a method taking the fixture | bound | bound |
| a plain test | free | free |
You were right that the class one is the pointed case — class-method descent exists to serve exactly that shape, so the two belonged in one change and only one was there. I added the list form too, because that is what a file uses once it has two marks and an arm over the bare form alone would have looked complete.
Four arms, including the cost side: only usefixtures is a dependency, or every parametrised class becomes cluster-bound and the gate empties. Removing the class descent reddens its arm alone; removing the module read reddens the two module arms.
2. The arm whose two assertions guaranteed each other's vacuity
Exactly as you measured. It now reads the install lines and asks whether any names the driver, with the control inside the same arm — it appends such a line to a copy and requires the same expression to see it. So the assertion that detects a driver install and the assertion that proves it can detect one are no longer the same sentence twice.
3. jd's rule, and what it turned up
This branch's additions to selftest/350 were the strongest form of the coupling — they invoked the decider:
python3 "$_hd_decide" --disagree "$PGC_TESTDIR/pytest"
python3 "$_hd_decide" --partition "$PGC_TESTDIR/pytest"
Both blocks are gone, and no coverage went with them — every property they tested already exists in the corpus, which I checked by listing them rather than asserting it. What stays is the ci.yml arms, since CI config belongs to neither harness.
Then #927 landed and this branch's own arm fired, by name: [1: undeclared:test_raises_sqlstate.py].
The obvious fix was wrong. Declaring it turned the driver-free job red: four of its arms fail with psycopg shimmed out, because the modules it hands to pytester import the driver. It requests no cluster fixture and still cannot run where there is no driver.
My second attempt was also wrong, and an existing arm refused it. Folding the driver condition into partition() contradicted test_the_classifier_is_not_fooled_by_prose_that_names_the_driver — which asserts that a generated inner test requesting a cluster fixture is the inner run's requirement, not this file's. That arm is correct, and breaking it was the signal I was giving one property two meanings.
So there are two properties now, derived separately and intersected for the job:
partition() does this file request a cluster?
driver_dependent() does it need psycopg importable, even with no cluster?
job_runnable() the intersection -- what the job can actually run
cluster-free 9 files, including test_raises_sqlstate.py
driver-dependent test_raises_sqlstate.py
job runnable 8 == the declaration
disagreements []
membership_report reports needs-the-driver: as a kind of its own, because needs-a-cluster: would be a wrong diagnosis and the reader's next action differs.
The driver property needs two conditions, and the control is the reason. test_harness_deps_classifier.py writes fixture corpora containing import psycopg and only ever parses them — nothing imports those files. Reading the string alone would have thrown that file out of the gate it exists to be in. The difference is whether the file drives pytester. Three arms, two of them controls.
Evidence
harness_selftest 550 checks, 550 passed + 0 failed + 0 unrunnable, rc 0
pytest corpus 227 passed
the gated set as CI runs it 8 files, 150 passed, psycopg asserted absent
The selftest count dropping from 561 to 550 is the eleven arms the rule removed, and the run is green without them.
On your sequencing note
You were right that nothing runs either guard in CI until this lands, and the driver discovery above is the sharpest version of that: test_raises_sqlstate.py is on main now, it is cluster-free, and it needs the driver — so the job this PR adds would have run it and gone red on its first green main. The arm that would tell anyone is the one in this PR.
Additive conflicts only, resolved by composition rather than choice: CHANGELOG entries from both sides kept, and where TESTS.md section numbers collide the later-numbered section is renumbered with its TOC entry and anchor following. Verified structurally rather than by eye: headings and TOC entries equal in count, numbers contiguous from 1, titles identical between the two lists, and every TOC anchor equal to the anchor GitHub derives from its heading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
jdatcmd
left a comment
There was a problem hiding this comment.
Approving at 11c315ce, 13/13 SUCCESS. Both of my findings are closed, and the third property you found is the most valuable thing in the PR.
Finding 1 — the usefixtures gap. All four request forms are now bound, and the negative control holds, which is what stops this being the cheap "call everything cluster-bound" fix I mutated for last time:
free : ['test_helpers_param.py', 'test_plain.py']
bound : ['test_cls_form.py', 'test_fn_form.py', 'test_method_param.py', 'test_mod_form.py']
inputs 6 == free 2 + bound 4
test_cls_form.py (class-level mark) and test_mod_form.py (module pytestmark) were both classified database-free before; a plain file and one taking only tmp_path still are.
Finding 2 — the vacuous assertion pair. The new form reads the install lines, and the control is in the same arm rather than in a comment. I re-ran my exact mutation — pip install --quiet $PINS psycopg[binary]==3.3.5:
FAILED test_harness_deps.py::test_the_job_installs_no_database_driver
That is the mutation under which both old assertions passed. ci.yml restored byte-identical.
The third property is the part I would point a reviewer at. Splitting "does this file request a cluster" from "does it need psycopg importable" is right, and the reconciliation is printed rather than asserted:
cluster-free 9
driver-dependent 1 ['test_raises_sqlstate.py']
job runnable 8
declared 8
cluster-free 9 == job runnable 8 + driver-dependent 1 -> True
declared == job runnable -> True
membership_report -> []
And the exclusion is a measured property, not a convenience. Running the derived list exactly as the job does, and then the one excluded file, in a venv with no psycopg:
the 8 declared files 150 passed
test_raises_sqlstate.py 24 failed, 6 passed
So the job this PR adds really would have gone red on its first green main — test_raises_sqlstate.py landed with #927 an hour ago, requests no cluster fixture, and cannot run without the driver. The arm that says so is in this PR, which is the argument for it.
That membership_report reports needs-the-driver: as its own kind rather than folding it into needs-a-cluster: is the right call for the reason you give: the next action differs, and a wrong diagnosis costs more than a missing one. Your second attempt being refused by test_the_classifier_is_not_fooled_by_prose_that_names_the_driver is the system working — that arm is right, and a generated inner test's cluster request is the inner run's requirement, not the file's.
One detail for the record, not a fault: you describe four arms failing with psycopg shimmed out; with psycopg absent I get 24 failing. Different mechanism, same conclusion, and the stronger number if you want it in the body.
This is now the PR the other two depend on. #927's and #930's arms are not run by anything until this lands. It is DIRTY against main after #927, #931 and #930 went in — CHANGELOG and probably TESTS.md, the same shape we have each hit several times today. Approving now so the merge is the only thing left; ping me at the new head and I will re-check rather than carry this verdict across the resolution.
…pt#930 landed (commandprompt#432) The arm named it rather than leaving a hole: cluster-free, not driver-dependent, so the driver-free job can run it and the declaration has to say so. Third time this arm has caught a merge-order consequence rather than a mistake -- commandprompt#922 brought test_suite_accounting.py, commandprompt#927 brought test_raises_sqlstate.py (which turned out to need the DRIVER and so is correctly excluded), and commandprompt#930 brings this one. declared 9 · cluster-free 10 · driver-dependent 1 · disagreements [] corpus 238 passed · the gated set 9 files, 161 passed with psycopg absent Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
jdatcmd
left a comment
There was a problem hiding this comment.
Re-approving at 1ce1147d. My previous approval named 11c315ce, so it did not describe this head.
I re-ran the checks rather than carrying the verdict across your conflict resolution, because the resolution changed NO_CLUSTER itself:
cluster-free 10 | driver-dependent 1 ['test_raises_sqlstate.py'] | job runnable 9 | declared 9
cluster-free == job runnable + driver-dependent -> True
declared == job runnable -> True
membership_report -> []
the derived job, driver-free -> 9 files, 161 passed
Declaring test_failed_query_sentinel.py now that #930 has landed is exactly the case this PR's machinery exists to handle, and the arm found it rather than you remembering to. Cluster-free went 9 to 10, job-runnable 8 to 9, and the one driver-dependent file is still the right one.
Both of my findings are still closed at this head:
free : ['test_helpers_param.py', 'test_plain.py']
bound : ['test_cls_form.py', 'test_fn_form.py', 'test_method_param.py', 'test_mod_form.py']
inputs 6 == free 2 + bound 4
driver-install mutation to ci.yml -> 1 failed
ci.yml restored byte-identical; worktree clean.
…new file #921 and #932 landed. #921 brings the classifier that decides which files the driver-free job runs, and it has an opinion about this branch's new file that this branch could not have had when it was written. TESTS.md: main's #921 inserted two file sections at 15 and 16, so the tail sections moved again and this branch's section becomes 22. Same resolution as last time and for the same reason -- main's convention is to append a new file section after the tail -- so one section of this branch is renumbered rather than seven of main's. Checked: 22 headings against 22 TOC entries, every TOC text equal to its heading, every anchor equal to what GitHub derives, numbering contiguous 1..22. NO_CLUSTER gains test_check_results_are_machine_readable.py, because the composed tree failed without it: NO_CLUSTER is exactly the database-free half of the corpus: got '[1: undeclared:test_check_results_are_machine_readable.py]' want '[]' It drives test/lib.sh by subprocess, so what it needs is bash and the tree, not psycopg or a cluster. Measured rather than assumed: 9 passed in a venv with no driver, and the classifier agrees it is not driver-dependent. The derived job goes from 9 files to 10, 161 passed to 170. That failure is only visible where a cluster and the driver are both present, and no CI job runs test_harness_deps.py's membership arms, so it would have landed on main as a red nobody ran. Composing locally is what found it. On the composed tree: selftest 350 53 checks 0 failed, selftest 400 64 checks 0 failed, selftest 080 15 checks 0 failed, the driver-free job 10 files 170 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…epipe sweep branch # Conflicts: # CHANGELOG.md
#923 moved five times while this waited, and #921, #927, #930, #931 and #932 landed on main underneath it. Composing found two things that a clean merge would not have. TESTS.md: the base now carries 22 sections, and this branch had inserted the ledger at 16. Auto-merge kept the base's tail sections AND this branch's copies of them, so the file would have had two of each. Resolved by keeping only the ledger section from this side, renumbered to 23, where the base's own 22 already ends. Checked rather than eyeballed: 23 headings against 23 TOC entries, every TOC title equal to its heading, every anchor equal to GitHub's derivation, numbering contiguous 1..23, no duplicate heading. NO_CLUSTER gains test_mutation_ledger.py. #921's classifier arrived on the base and immediately named it: membership_report: [1: undeclared:test_mutation_ledger.py] It drives test/pgc_ledger.py, a python tool rather than the shell harness, so it needs neither a cluster nor psycopg. Measured rather than assumed: 9 passed in a venv with no driver, and driver_dependent() agrees. The derived job goes to 11 files and 179 passed. THE LEDGER IS REGENERATED, AND THAT IS THE POINT OF THIS MERGE RATHER THAN A SIDE EFFECT. #923 added 17 checks to selftest 400 and converted 25 skip sites, none of which the committed ledger had ever seen. The gate refuses a check it has never seen, so the composed tree would have failed CI for a reason with nothing to do with either change. Regenerating is the documented repair, and the budget file says so. From a real run of the composed tree, not a synthesised log: harness_selftest.sh: PASSED, rc=0 checks run: 735 | accounting: 735 passed + 0 failed + 0 unrunnable + 0 skipped ledger: 701 rows -> 734 | never=734, ever red=0 gate: new this run=0 Reconciled: 735 records == 732 distinct (suite, part, name) + 3 names that each appear twice in one run, and 0 log triples are missing from the ledger, which is exactly what the gate refuses. All 734 rows carry five fields and none ends in a tab. The budget's asserted census follows to 734. Two ledger rows do not appear in this log -- selftest 330's "all three runner functions", which #923 changed to five. rename-scan reports appeared=0, vanished=2. THEY ARE LEFT DELIBERATELY: this log is PG17 only, and pruning rows that a single major did not produce would delete checks that legitimately run elsewhere. The gate refuses unseen checks, not unused rows. Gates on the composed tree: 350 53/53, 400 81/81, 410 96 checks 0 failed, 080 15/15, shellcheck rc=0 over the whole harness, driver-free job 11 files 179 passed, membership_report []. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
152 tests in
test/pytest/and not one of them ran in CI.selftest/350saysit plainly about its own subject: a guard that does not run is a comment. This
runs the guard-testing half in the gate, and what allowed it was one import.
One eager import coupled the whole corpus to a database driver
conftest.pyimported psycopg at module scope, and conftest is imported beforeevery run — so a driver was a hard requirement of every test, including the 61
that never open a connection. With psycopg absent the run did not fail a test, it
failed to collect:
Measured both directions:
The layer never needed it —
pgc_vacuity.pyimportsast,numbers,pathlib,pytest.pgc_cluster.pyimports no driver either. It was conftestalone.
README.mdrecords why the corpus is not inSUITES:pgc_skiptreats a missingdependency as a failure rather than a skip. That argument is about the cluster
tests. It never applied to the guard tests — and until this import moved there
was no way to separate them, because importing conftest imported the driver.
The job
pytest-guards: ubuntu-latest, no database, no build, an interpreter and twopinned packages. 61 tests, ~3 seconds.
The file list and the pins are both derived — the list from
NO_CLUSTERintest_harness_deps.py, the pins fromrequirements-test.txt. A second copy ofeither is a hand-maintained value that goes stale silently, which this repo spent
a day proving on
TESTS.md's totals line (#908). Two arms hold the job to it: itmust derive the list, and it must name no corpus file literally.
The job also asserts psycopg is absent before running. Without that the tests
would pass for the ordinary reason and prove nothing.
A shim rather than an uninstall
test_harness_deps.pyproves the property behaviourally: it writes apsycopg.pythat raises on import, puts it first on the path, and requires the no-cluster
files to collect and pass anyway. Uninstalling would test the machine rather than
the harness, could not run beside anything else, and would leave the environment
broken if the test died. Both behavioural arms assert the shim actually bites
before believing anything it produces.
The control is the half that matters. Deferring must make the import lazy,
not the database optional:
Both harnesses
selftest/350carries the static half — conftest imports no driver at modulescope, the driver is still imported inside the fixtures that connect (or the
first arm is satisfied by a harness that talks to no database at all), and the job
derives rather than repeats.
What this does not do
The cluster tests still do not run in CI, and this makes no claim about them.
They need a build, a server and the driver, which is the expensive half of
README's argument and untouched here.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a