ci: run the cluster-half pytest tests, and make both jobs assert their size (#1016) - #1018
Conversation
jdatcmd
left a comment
There was a problem hiding this comment.
Not approving: the job this PR adds is red, and it is a privilege problem rather than a test problem.
RuntimeError: pgcolumnar failed to build or install from /home/runner/work/pgcolumnar/pgcolumnar;
refusing to report checks against whatever was installed before.
make: *** [.../Makefile.shlib:398: install-lib-shared] Error 1
pgc_cluster.py:363
make install is writing to /usr/lib/postgresql/18/lib/, which is root-owned, and the job asserts non-root two steps earlier:
echo "running as: $(id -un) (uid $(id -u))"
test "$(id -u)" -ne 0Why the suites job does not hit this
It runs the whole matrix under sudo -E:
sudo -E env "PATH=$PATH" PGC_SKIP_TIMING=1 PGC_JOBS=4 \
PGC_REQUIRE_ISOLATION=1 \
bash test/run_all_versions.sh /usr/lib/postgresql/${{ matrix.pg }}/bin/pg_configand lib.sh then drops to runuser -u postgres for anything that must not be root (lib.sh:258). So the shell harness installs as root and runs as postgres. The pytest harness has no equivalent drop, so it must be non-root throughout — and then cannot install.
Three ways out, and the choice is yours because it is about which invariant you want to keep:
sudo chown -R "$USER"the PG18pkglibdir,sharedirandbindirbefore the tests. Keeps the job's non-root assertion, which reads deliberate.- Run the job under
sudo -Elike the suites job, and givepgc_cluster.pythe same drop-to-postgres the shell harness has. Larger, and it makes the two harnesses match. - Install to a user-writable prefix.
I would take 1 — the test "$(id -u)" -ne 0 line looks like a decision, not an accident, and 1 is the only option that keeps it.
The guard that caught this deserves saying
refusing to report checks against whatever was installed before
A failed install that fell through to testing the previously installed .so would have produced a green run measuring the wrong binary — the stale-.so false green this repo has been bitten by before. pgc_cluster.py:363 turns it into a loud red instead. That is the behaviour I would want and it is why this failure is legible at all.
The rest of the change
The expected_tests.txt mechanism is the right shape: both pytest jobs assert their collected count against a declared number, so a job that silently collects nothing can no longer pass. That is the same defect class as an empty statusCheckRollup reading as green, which bit me on #1012 today.
I have not reviewed it further, because a red job is the finding and the rest can wait for a run that executed.
…not keep (commandprompt#432) The governing property of test/differential.sh: load the same data into a heap and a columnar table and every query must answer identically. This ports part 1, the type matrix -- twenty columns, 12,000 rows, a different null modulus per column, across many chunk groups and several stripes. 60 tests, 129 counted checks, 3.5 seconds. Names are the bash suite's character for character, so compare_to_bash.py can diff the two by property. THREE OF THE BASH ARMS CANNOT FAIL, and the port says so rather than reproducing them. Each was found by the vacuity layer refusing a comparison, then measured. 1. `c_int eq` probes `c_int = 600`. c_int is g*7-100, so 600 needs g=100, and 100%5=0 puts a NULL there. Measured: 0 rows. Both sides hash to EMPTY, compare equal, and the arm passes having compared nothing to nothing. The port keeps it with a stated reason and adds `c_int eq present` on 607, which g=101 supplies and no null modulus touches. 2. `c_ztext is null` asks for nulls in a column that has none: CASE WHEN g%2=0 THEN '' ELSE 'z'||g END is never NULL. Measured: 0 rows IS NULL, 6000 rows = ''. Same vacuity. The port asserts what can fail instead -- the empty-string count -- because a decoder that confused '' with NULL would move it. 3. `c_f4 sum/avg` and `c_f8 sum/avg` assert exact equality of a float sum, which has no single right answer. MEASURED on heap alone, one table, three row orders, extra_float_digits=3: ORDER BY id -0.27597385772197924 ORDER BY id DESC -0.2759738577219848 ORDER BY c_f8 -0.2759738578545523 Three answers from one access method, so "columnar equals heap exactly" is false by construction. The bash arm passes because pgc_set_hash hashes the TEXT rendering and psql's default precision rounds the difference away at some magnitudes and not others -- a real tolerance, implicit and magnitude-dependent. The port states it: 1e-6 relative for the two float columns, exact for int, bigint, smallint and numeric, where a tolerance would hide the defect the arm exists to find. With a control that the bound is tight enough to have a direction. Row lists rather than hashes: a failure prints the rows that differ instead of two hex strings, and the vacuity layer can see a both-sides-empty comparison, which a hash cannot. The fixture is module-scoped because 12,000 rows over twenty columns is too slow to rebuild per assertion. That costs pgc_conn's write watch, so the load asserts its own row count, chunk-group count and stripe count -- those three premise arms are what watch_writes would otherwise have done. My first version of those premises queried pgcolumnar.chunk_group, which does not exist. A guessed catalog name is a premise arm that errors instead of asserting; the real ones are zone_map and row_group via get_storage_id, which is what lib.sh's own helpers read. Depends on commandprompt#1018 for the CI job that runs it: this file needs a cluster, so until that lands it is in the half nothing runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…not keep (commandprompt#432) The governing property of test/differential.sh: load the same data into a heap and a columnar table and every query must answer identically. This ports part 1, the type matrix -- twenty columns, 12,000 rows, a different null modulus per column, across many chunk groups and several stripes. 60 tests, 129 counted checks, 3.5 seconds. Names are the bash suite's character for character, so compare_to_bash.py can diff the two by property. THREE OF THE BASH ARMS CANNOT FAIL, and the port says so rather than reproducing them. Each was found by the vacuity layer refusing a comparison, then measured. 1. `c_int eq` probes `c_int = 600`. c_int is g*7-100, so 600 needs g=100, and 100%5=0 puts a NULL there. Measured: 0 rows. Both sides hash to EMPTY, compare equal, and the arm passes having compared nothing to nothing. The port keeps it with a stated reason and adds `c_int eq present` on 607, which g=101 supplies and no null modulus touches. 2. `c_ztext is null` asks for nulls in a column that has none: CASE WHEN g%2=0 THEN '' ELSE 'z'||g END is never NULL. Measured: 0 rows IS NULL, 6000 rows = ''. Same vacuity. The port asserts what can fail instead -- the empty-string count -- because a decoder that confused '' with NULL would move it. 3. `c_f4 sum/avg` and `c_f8 sum/avg` assert exact equality of a float sum, which has no single right answer. MEASURED on heap alone, one table, three row orders, extra_float_digits=3: ORDER BY id -0.27597385772197924 ORDER BY id DESC -0.2759738577219848 ORDER BY c_f8 -0.2759738578545523 Three answers from one access method, so "columnar equals heap exactly" is false by construction. The bash arm passes because pgc_set_hash hashes the TEXT rendering and psql's default precision rounds the difference away at some magnitudes and not others -- a real tolerance, implicit and magnitude-dependent. The port states it: 1e-6 relative for the two float columns, exact for int, bigint, smallint and numeric, where a tolerance would hide the defect the arm exists to find. With a control that the bound is tight enough to have a direction. Row lists rather than hashes: a failure prints the rows that differ instead of two hex strings, and the vacuity layer can see a both-sides-empty comparison, which a hash cannot. The fixture is module-scoped because 12,000 rows over twenty columns is too slow to rebuild per assertion. That costs pgc_conn's write watch, so the load asserts its own row count, chunk-group count and stripe count -- those three premise arms are what watch_writes would otherwise have done. My first version of those premises queried pgcolumnar.chunk_group, which does not exist. A guessed catalog name is a premise arm that errors instead of asserting; the real ones are zone_map and row_group via get_storage_id, which is what lib.sh's own helpers read. Depends on commandprompt#1018 for the CI job that runs it: this file needs a cluster, so until that lands it is in the half nothing runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
051f33a to
a9e6bfc
Compare
linuxhikerpm
left a comment
There was a problem hiding this comment.
The split is the right shape: pytest-cluster as the complement of NO_CLUSTER, --pgc-expect-tests from a tracked file on both jobs, test -n so an empty read cannot fail open, the socket directory pinned to the cluster datadir (packaged postgres cannot write /var/run/postgresql as this user), and a start-log report that names FATAL instead of repeating pg_ctl: could not start server.
I am not approving 67ede8a13cd8498c6034627fbb99dc917fef12cf. The cluster job failed on a9e6bfc0 (install/start), and this head has no check rollup on commandprompt/pgcolumnar yet. The later commits look like the measured fixes; a green rollup on this SHA is what would settle it.
Do not merge until that rollup is green.
67ede8a to
eb467e8
Compare
|
Agreed on the standard, and the rollup is now running on The PR was #1012 merged after I based the branch, the CHANGELOG conflicted, and a PR that cannot merge That is the third meaning of an empty rollup in this repo -- queued, passing, or dirty with What I can offer while the rollup runs, since it is the thing your review actually wants: Three distinct defects, each hiding the next: the install privilege (jdatcmd's diagnosis), None of that substitutes for the rollup. Holding until it is green, same as you. |
eb467e8 to
7b1a73c
Compare
…not keep (commandprompt#432) The governing property of test/differential.sh: load the same data into a heap and a columnar table and every query must answer identically. This ports part 1, the type matrix -- twenty columns, 12,000 rows, a different null modulus per column, across many chunk groups and several stripes. 60 tests, 129 counted checks, 3.5 seconds. Names are the bash suite's character for character, so compare_to_bash.py can diff the two by property. THREE OF THE BASH ARMS CANNOT FAIL, and the port says so rather than reproducing them. Each was found by the vacuity layer refusing a comparison, then measured. 1. `c_int eq` probes `c_int = 600`. c_int is g*7-100, so 600 needs g=100, and 100%5=0 puts a NULL there. Measured: 0 rows. Both sides hash to EMPTY, compare equal, and the arm passes having compared nothing to nothing. The port keeps it with a stated reason and adds `c_int eq present` on 607, which g=101 supplies and no null modulus touches. 2. `c_ztext is null` asks for nulls in a column that has none: CASE WHEN g%2=0 THEN '' ELSE 'z'||g END is never NULL. Measured: 0 rows IS NULL, 6000 rows = ''. Same vacuity. The port asserts what can fail instead -- the empty-string count -- because a decoder that confused '' with NULL would move it. 3. `c_f4 sum/avg` and `c_f8 sum/avg` assert exact equality of a float sum, which has no single right answer. MEASURED on heap alone, one table, three row orders, extra_float_digits=3: ORDER BY id -0.27597385772197924 ORDER BY id DESC -0.2759738577219848 ORDER BY c_f8 -0.2759738578545523 Three answers from one access method, so "columnar equals heap exactly" is false by construction. The bash arm passes because pgc_set_hash hashes the TEXT rendering and psql's default precision rounds the difference away at some magnitudes and not others -- a real tolerance, implicit and magnitude-dependent. The port states it: 1e-6 relative for the two float columns, exact for int, bigint, smallint and numeric, where a tolerance would hide the defect the arm exists to find. With a control that the bound is tight enough to have a direction. Row lists rather than hashes: a failure prints the rows that differ instead of two hex strings, and the vacuity layer can see a both-sides-empty comparison, which a hash cannot. The fixture is module-scoped because 12,000 rows over twenty columns is too slow to rebuild per assertion. That costs pgc_conn's write watch, so the load asserts its own row count, chunk-group count and stripe count -- those three premise arms are what watch_writes would otherwise have done. My first version of those premises queried pgcolumnar.chunk_group, which does not exist. A guessed catalog name is a premise arm that errors instead of asserting; the real ones are zone_map and row_group via get_storage_id, which is what lib.sh's own helpers read. Depends on commandprompt#1018 for the CI job that runs it: this file needs a cluster, so until that lands it is in the half nothing runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…size (commandprompt#1016) ci.yml had one pytest job, pytest-guards, and it installs psycopg deliberately NOT -- that absence is what proves those files need no database. nightly.yml mentions pytest zero times. run_all_versions.sh mentions it zero times and must, because the two harnesses stay independent and the shell runner invoking pytest is the cross-harness call the project forbids. lib.sh names pytest 11 times and every one is a comment. So 8 files ran nowhere: green when somebody ran them by hand, silent when they stopped. They were never broken -- measured on pg18a with the driver present, 99 collected, 289 checks, 289 pass, 40 seconds. --pgc-expect-tests is now passed by BOTH jobs, from test/pytest/expected_tests.txt. The flag existed and nothing used it. What it closes is narrower than "pytest passed having run nothing" and worse: a nonexistent path already fails on its own, but a file list that resolves to REAL files and collects FEWER tests does not. Measured, dropping one file from the guard list: unarmed rc=0 "255 passed" 17 tests gone armed rc=4 "collected 255 test(s) but expected 272" and the flag refuses 0 as vacuous rather than accepting it. THE NUMBERS ARE IN A TRACKED FILE for the reason check_ledger_budget.txt gives about its own: a change to one is then a diff a reviewer sees, next to the test that moved it. PGC_SKIP_TIMING is the precedent for the other choice -- set in two workflow files, suppressing whole suites for months, with no diff ever showing it. The SPLIT stays derived from NO_CLUSTER in both jobs. test -n guards every derived value, because an empty read would omit the flag and fail OPEN. Two new arms hold the new job to what the old one is held to, and a third holds both to passing the flag from the tracked file. The existing arm caught my own work: it forbids a corpus count in a job comment, my comment said "8 files", and it failed before the job ever ran. The count is in the CHANGELOG instead, which is dated. README.md's recorded reason had gone stale twice over: it said CI would first have to install from requirements-test.txt, which pytest-guards already does, and it proposed registering the run in SUITES, which is the forbidden cross-harness call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…ompt#1016) The cluster job's real failure, once the privilege problem jdatcmd diagnosed was fixed: the cluster does not start, and the harness does not say why. RuntimeError: ['pg_ctl', '-D', '/tmp/pgc-pytest-0-.../data', ..., 'start'] failed rc=1: pg_ctl: could not start server Examine the log output. Fifty identical errors, every one naming the command, none naming the cause -- because `pg_ctl` told the harness to examine a log and nothing examined it. The other 50 tests in those files passed, which is what made it legible at all: they are the static arms that need no cluster. lib.sh has had pgc_start_log_report since commandprompt#537 for exactly this. The pytest side had no equivalent. The two harnesses are parallel in FUNCTIONALITY and never in call, so this is that function's job written on this side: FATAL lines with their line numbers, then a tail, and it says so when it found neither, because silence reads as "there was nothing to say". The chown from the previous commit DID work -- the build and install succeeded and 50 tests ran. This is a second, different problem that the first one was hiding, and I would rather add the instrument than guess at it a third time: I have already disproved one hypothesis today (a non-assert build, which passes 101/101 locally on pg18n) and read the code wrongly twice.
…efaults elsewhere (commandprompt#1016) The cluster job's actual cause, named by the diagnostic added in the previous commit rather than by another guess: FATAL: could not create lock file "/var/run/postgresql/.s.PGSQL.29768.lock": Permission denied Measured, same box, same major, two builds: /usr/lib/postgresql/18 (PGDG, configured --runstatedir=/run) #unix_socket_directories = '/var/run/postgresql' /usr/local/pg18a (source build, no such flag) #unix_socket_directories = '/tmp' and /var/run/postgresql is drwxrwsr-x postgres postgres. So on any packaged postgres the postmaster cannot create its lock file and FATALs, and the caller saw only `pg_ctl: could not start server`. lib.sh does not pin this and does not need to: the suites job runs under sudo -E and lib.sh drops to `runuser -u postgres`, which CAN write that directory. This harness must be non-root throughout -- initdb refuses root -- and is not postgres either, so the packaged default is wrong for it and always was. It only ever ran against source builds, where the default is /tmp. Pinned to the cluster's own datadir: it already exists, it is already this cluster's, it goes away with it, and two xdist workers cannot collide in it. Connections are TCP (host=127.0.0.1), so nothing that connects is affected. REPRODUCED THE RUNNER LOCALLY to find this, because running as root proves nothing: _asroot drops to postgres when euid is 0, and postgres can write that directory, so the whole problem disappears. A non-root, non-postgres user against the PACKAGED build gave CI's exact shape -- 1 failed, 50 passed, 50 errors.
…andprompt#1016) test_a_cluster_test_still_needs_the_driver runs pytest in a subprocess with psycopg shimmed out and asserts the failure NAMES the shim. The subprocess got no --pg-config, so it fell back to conftest's DEFAULT_PG_CONFIG -- /usr/local/pg18a/bin/pg_config -- which exists on the audit container and on no GitHub runner. The child then failed on the missing prefix BEFORE anything imported psycopg, so the output never named the shim and the arm reported the shim absent when the shim was fine. Its own premise arm passed throughout, because the shim really did work: the premise and the assertion were about different processes. A latent bug rather than a CI quirk. It passes against a source-built prefix and fails against a packaged one, and nothing in the test said which it needed.
…anent denial (commandprompt#1016) Not a collision -- a kernel-enforced lockout. `fs.protected_regular = 2`, the default on this kernel, forbids opening a regular file for write in a world-writable STICKY directory when the file's owner is neither the directory's owner nor the caller. So once one user creates /tmp/pgc-pytest-build.lock, every other user on the box cannot run the corpus again, and neither can root: running as: root uid=0 lock: -rw-r--r-- 1 ciuser ciuser 0 /tmp/pgc-pytest-build.lock PermissionError: [Errno 13] Permission denied: '/tmp/pgc-pytest-build.lock' CAP_DAC_OVERRIDE does not help, which is what makes it surprising. It cost two runs before I read the sysctl -- it bit while reproducing the CI environment as a second user, then again on the next root run. The DIRECTORY carries the uid, not the filename: a per-user directory is owned by that user and is not world-writable, so protected_regular does not apply inside it. A per-user FILENAME would still be a file in a sticky shared directory. WHAT THIS GIVES UP, said out loud: the lock no longer serialises two DIFFERENT users installing into one shared prefix. That is already covered, and better, by the marker key -- it includes installed_library(pg_config) from commandprompt#956, so another user's install invalidates this user's marker and forces a rebuild rather than being silently accepted. The lock's job is the xdist-worker race within one run, and workers share a uid.
e0dfe21 to
0c4a939
Compare
…#432) Part 2 of test/differential.sh. Part 1 asks whether the two access methods agree about DATA; this asks whether they agree at the SIZES where the format's structure changes. 76 tests, 183 checks, 3.7 seconds. 21 bash arms ported by name. Each fixture is built per test rather than shared, because a different geometry each time is the whole point: the set_options values are the subject, so a module fixture would have to pick one and the rest would go untested. THE GEOMETRY IS ASSERTED, not just the rows. ceil(N/100) chunk groups at N = 99, 100, 101, 200, 201, 250, and ceil(N/1000) stripes at N = 1000, 1001, 2000, 2001. The data can agree while the geometry is wrong -- a writer that never closes a group produces one group and the right rows, and only the count says so. `empty scan` compares two empty results, which is the one arm here that cannot fail on its own. It is kept with a stated reason and a POSITIVE CONTROL: the same query returns a row once one exists, which is what proves the comparison can move. `empty count` and `empty agg` are not vacuous, because 0 and a row of NULLs are values. MY OWN PREMISE ARM CAUGHT MY OWN QUERY. The wide-row test asserted 61 columns via information_schema.columns filtered on table_name alone, which counts every table called t_col in every schema -- including the module-scoped matrix fixture's, which has twenty. It reported 81. Now via 't_col'::regclass, which resolves through search_path to this test's schema and nothing else. Depends on commandprompt#1018 for the job that runs this file: test_differential.py is in the cluster half, and on main that half still runs nowhere. 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, and this is the highest-leverage merge on the board for #432.
I went looking for where the porting effort actually bottlenecks and found it before reading this PR's new head:
25 pytest files
15 database-free -- CI runs these
10 cluster-bound -- CI runs NONE of them
That includes test_differential.py, merged an hour ago. Every port of a real suite lands as a test nobody executes, so until this PR merges, time spent on #432 buys coverage that no gate enforces. Your own note says it better than I would:
The ungated half grew by 67 tests in the time this took to review, which is the argument for gating it rather than a detail about it.
The privilege fix is the right one of the three
LIB="$("$PGC" --pkglibdir)"
SHARE="$("$PGC" --sharedir)"
sudo chown -R "$(id -un)" "$LIB" "$SHARE"Derived from pg_config rather than a hardcoded /usr/lib/postgresql/18/lib, so it cannot drift from the pg_config the tests are actually given. It keeps the test "$(id -u)" -ne 0 assertion, which read as deliberate — and asserting the chown rather than assuming it matters, since a chown that changed nothing fails identically to no chown at all.
The count file is the part I would keep
guard_tests 277 verified against main: pytest --collect-only -q returns exactly 277.
277 tests collected
And the reasoning for putting it in a tracked file rather than the workflow is the PGC_SKIP_TIMING precedent — set in two workflow files, suppressing whole suites for months, with no diff ever showing it. Same argument as check_ledger_budget.txt, applied to a different number.
That the number moved four times during review is the evidence, not the annoyance. 99 → 101 → 106 → 166 as #1012 and #1020 landed. Each move was a rebase re-derivation; a missed one fails naming the drift rather than silently running a different suite than the one declared.
pytest exiting 0 on an empty collection is the same shape as the empty statusCheckRollup that bit me on #1012 today — a summary that reads as success because nothing happened. Refusing 0 as vacuous rather than accepting it is right.
Merging.
…#432) Part 2 of test/differential.sh. Part 1 asks whether the two access methods agree about DATA; this asks whether they agree at the SIZES where the format's structure changes. 76 tests, 183 checks, 3.7 seconds. 21 bash arms ported by name. Each fixture is built per test rather than shared, because a different geometry each time is the whole point: the set_options values are the subject, so a module fixture would have to pick one and the rest would go untested. THE GEOMETRY IS ASSERTED, not just the rows. ceil(N/100) chunk groups at N = 99, 100, 101, 200, 201, 250, and ceil(N/1000) stripes at N = 1000, 1001, 2000, 2001. The data can agree while the geometry is wrong -- a writer that never closes a group produces one group and the right rows, and only the count says so. `empty scan` compares two empty results, which is the one arm here that cannot fail on its own. It is kept with a stated reason and a POSITIVE CONTROL: the same query returns a row once one exists, which is what proves the comparison can move. `empty count` and `empty agg` are not vacuous, because 0 and a row of NULLs are values. MY OWN PREMISE ARM CAUGHT MY OWN QUERY. The wide-row test asserted 61 columns via information_schema.columns filtered on table_name alone, which counts every table called t_col in every schema -- including the module-scoped matrix fixture's, which has twenty. It reported 81. Now via 't_col'::regclass, which resolves through search_path to this test's schema and nothing else. Depends on commandprompt#1018 for the job that runs this file: test_differential.py is in the cluster half, and on main that half still runs nowhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
#1018 made the collected count enforced while this branch was open, so the three arms this PR adds put the guard half at 280 against a declared 277 and the job failed with the drift named. That is the mechanism working: the number moved because the tests moved, and nothing silently ran a different suite than the one declared. Re-derived by collection rather than by adding three: pytest --collect-only -q $(NO_CLUSTER) | tail -1 -> 280 @OffgridwithJD's warning is the reason this is a separate commit: six PRs are open, four of them bump one of these two numbers against the value on main when they were written, so whichever lands second is stale and fails on a PR that was green. Rebase and re-derive between merges. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
#1025 landed between this going green and being merged. Three conflicts, and two of them are the hazard @OffgridwithJD warned about half an hour before it happened. TESTS.md: both sides took section 31. #1025's native_ownership landed first and keeps it; stats_privilege becomes 32, heading and index entry and anchor together. 32 sections, 32 index entries, no duplicates. Fifth number collision today. expected_tests.txt: BOTH branches bumped cluster_tests from 166 -- this one to 169, #1025 to 177 -- so the union left TWO cluster_tests lines in a file that holds one value per key. Resolved by keeping one line and DERIVING the value on the merged tree rather than adding: pytest --collect-only -q <cluster files> -> 180 Not 169, not 177, and not 169 + 11. This is the case #1018's enforcement exists to catch, and it caught it here rather than on the next author's green PR. CHANGELOG: two entries, both kept. guard half 277 passed, 680 checks, enforced cluster half 180 passed, 478 checks, enforced Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
Closes #1016.
93 test functions in 8 files, 26% of the pytest corpus, ran in no CI job. They were
never broken -- nothing ran them.
run_all_versions.shmust stay at zero: the harnesses are independent and the shell runnerinvoking pytest is the cross-harness call the project forbids.
lib.shnames pytest 11times and every one is a comment -- I checked them individually.
What this adds
pytest-cluster, the other half. One major, PGDG PostgreSQL 18 with its headers, everypin from
requirements-test.txtincluding psycopg, and the file list derived as thecomplement of
NO_CLUSTER.--pgc-expect-testson BOTH jobs, fromtest/pytest/expected_tests.txt. The flagalready existed and nothing passed it.
What it closes is narrower than "pytest passed having run nothing", and worse. A nonexistent
path already fails on its own -- that was not the hole. A file list that resolves to real
files and collects fewer tests does not. Measured, dropping one file from the guard
list:
It also refuses
0as vacuous rather than accepting it, which is the other way a countguard gets quietly disabled.
The numbers live in a tracked file, not in the workflow and not in an environment
variable, for the reason
check_ledger_budget.txtgives about its own: a change is then adiff a reviewer sees, sitting next to the test that moved it.
PGC_SKIP_TIMINGis theprecedent for the alternative -- set in two workflow files, suppressing whole suites for
months, with no diff ever showing it.
The split stays derived from
NO_CLUSTERin both jobs.test -nguards every derivedvalue, because an empty read would omit the flag and fail open.
Test plan
Both halves on the branch at
051f33a5, each armed from the tracked file:Controls. Each half must REFUSE a wrong number, or the flag asserts nothing:
The guard venv is built with psycopg absent and that absence asserted, so the pair
still proves the split rather than merely running twice.
requirements-test.txtinstalls clean in a fresh venv: psycopg 3.3.5, pytest 9.1.1,xdist 3.8.0.
The derived file list gives exactly 8 cluster files, and collection gives exactly the
tracked 101.
docs_style.shPASSED.The apt path itself is unverified until CI runs it. I cannot run GitHub Actions
here; the PGDG steps are copied from the
suitesjob, which uses them today, and thecluster half is verified against a local assert build instead. This is the one part of
the change I am taking on inspection rather than measurement.
Three arms, and one of them caught me
test_the_cluster_job_runs_the_other_half_and_derives_it-- the job exists, derives fromNO_CLUSTER, installs the pins, asserts the driver is present (the control forpytest-guardsasserting it is absent), and names no corpus file literally.test_both_pytest_jobs_assert_how_many_tests_they_collected-- both jobs pass the flag,both read it from the tracked file, both guard the read.
The existing arm caught my own work.
test_ci_derives_the_file_list_rather_than_repeating_itforbids a corpus count in a job comment, over the comment block as well as the body. My
new comment said "8 files". It failed before the job ever ran:
The rule is right -- a number in a comment is the same hand-maintained derived value as a
number in a list -- so the comment now states none and the measured figures are in the
CHANGELOG, which is dated. That arm applies to the new job for free, because its region
runs to the next job key.
README.md's recorded reason had gone stale twice over: it said CI would first have toinstall from
requirements-test.txt, whichpytest-guardsalready does, and it proposedregistering the run in
SUITES, which is the forbidden cross-harness call. Corrected.Why this came first
#432 is at 0.66% of product properties (36 of 5,453 distinct bash check names, measured
against a five-major matrix). Every product test ported from here lands in the half that was
ungated. The gate is not a tidy-up for afterwards -- it decides whether the next several
thousand assertions are worth anything.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a