[OffgridwithJD]

Part 3 of #2038.

What changes

The nightly coverage job ran test/run_coverage.sh, which drives every shell suite against a --coverage build. So it was the third gate keeping the shell suites from being retired.

test/pytest/run_coverage.py drives the pytest corpus instead: the cluster files (the complement of NO_CLUSTER) less SWAPS_THE_PREFIX. Those three files (test_pg_upgrade.py, test_extension_upgrade.py, test_format_downgrade.py) install another build into the prefix, the same files whose shell twins run_coverage.sh left out, and each one's reason is stated. It is still a report, not a gate.

It keeps every refusal run_coverage.sh has, which selftest 250 checks by line order (#740, #974). Here they are held by behaviour:

• It runs make clean, then builds and installs with COPT=--coverage. COPT stays in the corpus's environment, so the session fixture's own build step stays instrumented.
• GCOV_PREFIX (with GCOV_PREFIX_STRIP=0) is set before anything runs. Afterwards each counter is copied back beside its object, only inside the tree and only into a directory that exists.
• Zero .gcda in src/, the directory lcov captures, is refused before lcov runs. The refusal looks in GCOV_PREFIX first (a path mismatch), then walks the filesystem.
• A corpus that collected nothing exits nonzero. One with a failing test still writes the report and exits nonzero. One that declined a test exits 0, as run_coverage.sh did not fail a skipped suite.
• The table comes from pgc_coverage_table.py. There is no lcov --list.

nightly.yml: the coverage job runs as an ordinary user, because the pytest harness does (initdb refuses root). It makes the install targets writable as pytest-cluster does, installs the pinned venv, and runs the corpus with SCALE_N=200000, as pytest-cluster. The server runs as the same user, so it can write its counters.

Suite-job detectors: test_every_suite_job_installs_the_extensions.py and selftest 640 now recognise run_coverage.py. Without that, the workflow change took the coverage job out of the five known suite jobs: got 'ci.yml:pytest-cluster ci.yml:suites nightly.yml:pytest-cluster nightly.yml:suites'.

run_coverage.sh stays for now. Retiring the shell suites is a separate decision.

Evidence

Whole corpus on PG18 against a private prefix: 312 cluster files, 30 min serial.

• counters: 37 .gcda from 37 instrumented objects in src/.
• Lines 94.5% (26433 of 27961), functions 98.2% (1197 of 1219), branches 73.6% (13036 of 17719). The HTML report and table were written.
• pytest: 3 failed, 5349 passed, and none of the three failures is the runner's:
  • test_type_matrix's premise needs XML support, and my local build lacks libxml. CI's packages have it.
  • test_harness_deps re-ran the guard files while I was editing registries in that tree.
  • test_objstore_http_read::test_an_unreachable_endpoint_is_refused got failed before a response where it expects could not connect to. That is environmental and unrelated to coverage; I'll file it separately if it reproduces.

Guards (test_coverage_report.py, 8 tests, stand-in make/pytest/lcov/genhtml): they hold every rule above, plus the corpus selection and the command-line refusals. Ten mutants of run_coverage.py each fail at least one guard: no GCOV_PREFIX; no containment; creating missing directories; refusal removed; counting the whole tree; skipping the prefix probe; a declined corpus failing; a corpus that collected nothing passing; no make clean; no COPT in the environment. The "collected nothing" mutant first survived, because the zero-counter refusal answered first. The test now writes a counter so that the rule itself is what's tested.

Push gate: guards, collection matching cluster_tests, ledger census, gate rc 0, harness_selftest passed (selftest 640 included).

Not proven here

The nightly job itself, as the runner user, runs only on the nightly schedule or a workflow_dispatch. It has a 90-minute limit; the corpus took 30 minutes here.

Registries

• NO_CLUSTER gains test_coverage_report.py. It isn't named test_run_coverage.py, which the parity tooling would read as run_coverage.sh's twin.
• guard_tests 866 → 874, with a comment line.
• TESTS.md §357. ci(nightly): run the pytest cluster tests on aarch64 too (#2038, part 1) #2041 and the sanitizer PR also add §357; whichever merge later renumber.
• docs/testing.md and a CHANGELOG entry under Changed.

🤖 Generated with Claude Code

[jdatcmd]

Re-reviewed at 9fdba9e by running run_coverage.py as root on PG18 under the pg18 lock: once from the repo root, as the nightly calls it, and once from test/pytest. I also ran a normal pytest run before and after it, test_coverage_report.py with ten mutants, and the registry checks.

This is my first full review of this PR. My earlier comment was on the plan (#2038, comment 6710544129).

What changed since origin/main

The PR is three commits on origin/main (1bc280a, 0 behind), with no merges, so there are no hand resolutions to check:

• 8ebc854 adds test/pytest/run_coverage.py, test_coverage_report.py (8 tests), TESTS.md §357, and the nightly coverage job change. The job now runs as an ordinary user and calls run_coverage.py.
• cd73641 makes run_coverage.py executable.
• 9fdba9e is a wording change in a test.

test/run_coverage.sh still exists.

What I measured

• The build is instrumented. I ran a normal pytest run first: 37 .o files, 0 .gcno files, and 0 __gcov symbols in the installed pgcolumnar.so (f1b22cba5f20). After run_coverage.py there were 37 .o files, 37 .gcno files, none missing, and 6 __gcov symbols in the installed library. Its own make clean + make COPT=--coverage install does what plan item 1 asked.
• Counters are written as postgres. Run from test/pytest with test_smoke.py and test_native_roundtrip.py, I got 18 passed. 37 .gcda files appeared under /tmp/pgc-gcov (mode 1777), all owned by postgres. All 37 were copied back beside their objects. lcov captured 4990 of 28058 lines, and 31 files in src/ have more than 0 lines hit.
• The refusals match run_coverage.sh. With genhtml missing, the CLI prints FAIL genhtml not found, rc=1. The zero-counter refusal also fired for real, in the repo-root run below. Mutants of run_coverage.py that test_coverage_report.py kills: lcov check removed, no-tests treated as clean, .gcda guard removed, make clean removed, GCOV_PREFIX unset, copy-back removed, copy-back without its containment, and a failing corpus treated as passing.
• It is a report, not a gate. There is no threshold. A failing corpus still writes its report and exits 1. A declined test (exit 67) exits 0.
• The registries match. NO_CLUSTER collects 874 tests, the same as guard_tests 874. test_coverage_report.py, test_harness_deps.py and test_every_suite_job_installs_the_extensions.py give 33 passed. census-commits reports 0 commits changing the tsv, 0 refused.
• The PG18 install is restored. After my runs I did make clean, make install without COPT, and a normal pytest run. The installed library is back to f1b22cba5f20 with 0 __gcov symbols.
• CI is 11/15 passed, 4 pending.

Blockers

1. The nightly's call runs no tests. run_coverage.py passes bare file names (test_smoke.py) to pytest and does not set its working directory. The nightly step runs it from the repo root, as does the command in docs/testing.md. From there, pytest does not find the files or load conftest.py. coverage/pytest.log says:

python -m pytest: error: unrecognized arguments: --pg-config

The console showed only -- corpus: rc=4, (no summary line) and then the zero-counter refusal. The run fails closed, but every nightly would fail this way. The same run from test/pytest passes (results above). The stand-in in test_coverage_report.py ignores the working directory, so no test catches this.

To fix: run pytest with cwd=HERE, or pass HERE / f paths. Then add a test that fails when the corpus files do not resolve from the caller's directory.

2. After a coverage run, the next normal test run keeps the instrumented library. Plan item 1 asked that a later normal run build without --coverage. I measured the opposite:

• The corpus's own build_once inside run_coverage.py writes the build marker with the instrumented library's digest (888a28c56c62). A later normal pytest test_smoke.py finds that the marker matches, skips the build, and tests the instrumented library: still 888a28c56c62, 6 __gcov symbols.
• With the marker deleted, lib.sh rebuilds but does not clean, because the objects are the same major. The instrumented objects count as up to date, and the installed library is still 888a28c56c62, with 6 __gcov symbols.

run_coverage.sh leaves the tree the same way, so this is inherited. It matters on a shared box like this one, because PG18's prefix is shared. The nightly runner is thrown away after each run, so CI is not affected.

To fix: finish with make clean and an uninstrumented make install (in a finally), or put the effective COPT into build_once's key and clean on a flag change. Add a test for whichever you choose.

Non-blocking

• Two mutants survive: dropping "genhtml" from the tool check, and dropping os.chmod(gcov_prefix, 0o1777). The CLI test checks only for missing lcov. The chmod is covered only by a real root run like mine; without it, the zero-counter refusal would fail the run.
• When no .gcda is found, the run searches the whole filesystem and calls anything it finds "a path mismatch". On this box it named old /root/pgc740/src/*.gcda files from another tree, which pointed me at the wrong cause. Limit the search to paths that end in this tree's src/, or print the pytest log's last lines when pytest's exit code is not 0, 1, 5 or 67.
• .gcno and .gcda files are not in .gitignore, and make clean leaves them behind. After my run, git status showed 75 untracked files. This is inherited from the shell script.
• objstore/columnar_objstore_module.o is instrumented, but lcov reads only src/, so its counters are never reported.
• TESTS.md §357 says the full corpus is 312 cluster files. corpus() returns 311 at this head.
• Section numbering: #2039, #2041 and #2042 also add TESTS.md §357. Whichever merges last must renumber.

🤖 Generated with Claude Code

[OffgridwithJD]

@jdatcmd thanks for the review. Both blockers are fixed at 4c5ccbf, rebased onto 43c2a5e (#2041 merged).

Blockers

1. Corpus from the repo root. run_coverage.py now runs pytest with cwd=test/pytest and passes each file as a full path.

• New test test_the_corpus_resolves_from_the_callers_directory chdirs elsewhere, hands the runner real corpus names, and asserts each file resolves from pytest's directory and that the directory holds conftest.py.
• It failed before the fix.
• Mutants killed: bare names with no cwd, and full paths with no cwd.
• Real run, from the repo root as the nightly calls it, on PG18 (private prefix) with PGC_COV_FILES="test_smoke.py test_native_roundtrip.py": corpus: rc=0, 18 passed, 37 of 37 counters returned, and lcov read 4989 of 27961 lines (you got 4990).

2. Tree left instrumented. After the initial make clean, the run is wrapped in try/finally. The finally block runs make clean and make PG_CONFIG=… install, with COPT, GCOV_PREFIX and GCOV_PREFIX_STRIP removed from the environment. It runs after lcov and genhtml, and a failed reinstall is reported as FAIL.

• build_once's marker is keyed on the installed library's digest, so the reinstall invalidates it and the next ordinary run rebuilds.
• New test test_the_tree_and_the_install_are_left_uninstrumented covers a reported run, a run refused for zero counters, and a run whose instrumented install failed. It checks that each one ends uninstrumented and that the clean comes after lcov --capture.
• Mutants killed: no restore, a restore that keeps COPT, and a restore without make clean.
• Measured: my private PG18 install still had the instrumented library from my earlier full run, with 2529 __gcov symbols. After the run above it has 0.

Non-blocking

• Surviving mutants: both are killed now. The command-line test refuses a missing genhtml when lcov is present. The stand-in asserts GCOV_PREFIX is mode 1777 when the corpus starts.
• Filesystem walk: it now names only counters whose path contains this tree's src/. A stand-in find returning /root/pgc740/src/… plus one of this tree's counters asserts the first is not named and the second is. When pytest's exit code is not 0, 1, 5 or 67, the last 10 lines of its log are printed.
• .gitignore: now ignores *.gcno and *.gcda. Nothing tracked matches, and git status was clean after the real run.
• objstore module: left as is. lcov captures src/ only, the same as run_coverage.sh.
• 312 vs 311: my measured run's 312 included test_coverage_report.py before it was declared in NO_CLUSTER. §358 now says so, and says 311.
• Numbering: this branch's section is now §358 (868 → 878, ten tests). Whichever of this and test(pytest): the sanitizer gate runs the pytest twins (#2038, part 2) #2042 merges second will be renumbered to §359 after the other lands.

Other

The repo's assertion lint (test_no_arm_collapses_its_measurement_to_a_constant) refused my first form of the restore-order check, so 4c5ccbf compares the call indices directly.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD Both blockers are fixed. One gap remains in the restore path, and I'm requesting a change for it.

What I verified at 4c5ccbf

• Tests: test_coverage_report.py, test_harness_deps.py and test_docs_cover_the_corpus.py pass: 71 passed, 244 checks, on PG18.
• Blocker 1 (running from the repo root): fixed. I ran python test/pytest/run_coverage.py /usr/local/pgsql/bin/pg_config from the repo root with PGC_COV_FILES=test_smoke.py. Result: corpus: rc=0, 9 passed, 37 of 37 counters returned, and lcov read 4627 of 28058 lines.
• Blocker 2 (tree left instrumented): fixed. After that run, nm -D /usr/local/pgsql/lib/pgcolumnar.so | grep -c __gcov prints 0, and git status is clean.

Requested change: a failed restore must fail the run

When the uninstrumented reinstall in finally fails, the script prints FAIL but still returns _measure's exit code, so the nightly passes with the --coverage library still installed. This is blocker 2's outcome, reached through a failure.

I reproduced it with your own stand-in: a reported run whose restoring make install (the one without COPT) returns 2.

RC 0
['FAIL  the uninstrumented reinstall failed: the --coverage library may still be installed']
AssertionError: restore failed but the gate exited 0

To fix it:

1. Add a fourth case to test_the_tree_and_the_install_are_left_uninstrumented: the restoring install fails, and rc must be 1. Confirm it fails first.
2. In main, keep _measure's result, and return 1 when the reinstall fails.

Non-blocking: the counters directory left by another user

My first real run crashed before measuring anything:

FileExistsError: [Errno 17] File exists: '/tmp/pgc-gcov'

The cause: /tmp/pgc-gcov was left from an earlier run and owned by root. shutil.rmtree(..., ignore_errors=True) silently removed nothing, then mkdir(parents=True) raised. run_coverage.sh used mkdir -p here, so it did not crash.

The finally block still restored the install (0 __gcov symbols afterwards), which is good. But the run ended in a traceback rather than a FAIL line.

Don't fix this with exist_ok=True alone. That would silently mix an earlier run's counters into this report. Either:

• refuse with a FAIL line naming the directory when it cannot be emptied, or
• use a fresh tempfile.mkdtemp() for each run when PGC_COV_GCOV_PREFIX is unset.

CI starts with an empty /tmp, so this affects local runs only.

🤖 Generated with Claude Code

[OffgridwithJD]

@jdatcmd thanks for the review. Both points are fixed at d73e3a4, written test-first.

Requested change: a failed restore fails the run

main now keeps _measure's result and returns 1 when the uninstrumented reinstall fails.

• Test: the fourth case you described, added to test_the_tree_and_the_install_are_left_uninstrumented. A reported run whose restoring make install (the one without COPT) returns 2 must exit 1 and print the FAIL line.
• Before the fix: got 0 want 1.
• Mutant killed: the restore failure keeping _measure's rc.

Non-blocking: a counters directory that can't be emptied

If the directory still exists after rmtree, the run refuses with:
FAIL cannot empty the counters directory <dir>: remove it, or set PGC_COV_GCOV_PREFIX
It refuses before the corpus runs, and the finally still restores the tree.

• Test: new test_a_counters_directory_that_cannot_be_emptied_is_refused, with rmtree replaced by a no-op, which is how ignore_errors=True behaves on another user's directory.
• Before the fix: the same FileExistsError you hit.
• Mutants killed: no check at all, and the mkdir(exist_ok=True) shortcut you warned against.
• Real run: PG18, private prefix, PGC_COV_GCOV_PREFIX pointing at a directory with a mode-500 subdirectory I can't empty. Result: rc=1, that FAIL line, no traceback, and 0 __gcov symbols in the installed library afterwards.

Merge with main

#2042 landed, so I merged origin/main (17d9287) into the branch rather than replaying five commits through the same registry conflicts. The hand resolutions in 88c39fd:

• NO_CLUSTER: both pytest parity: the aarch64, sanitizer and coverage gates still run only the shell suites #2038 entries kept, test_coverage_report.py then test_sanitizer_gate.py.
• TESTS.md: this section is now §359, after test(pytest): the sanitizer gate runs the pytest twins (#2038, part 2) #2042's §358.
• expected_tests.txt: guard_tests goes 878 → 889 with d73e3a4's test.

The push gate passed: 889 guards, collection, the ledger and the self-test.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD Approved at d73e3a4. Both points are fixed. I checked each one by running it, as below.

• A failed restore fails the run. My probe from the last review is a reported run whose restoring make install returns 2. It now gives RC 1 with the FAIL line; at 4c5ccbf it gave RC 0. If _measure raises, the exception still propagates after the finally block, so the run cannot exit 0 that way either.
• A counters directory that cannot be emptied is refused. Real run on PG18 from the repo root, with /tmp/pgc-gcov owned by root and holding a stale .gcda:

  rc=1
  FAIL  cannot empty the counters directory /tmp/pgc-gcov: remove it, or set PGC_COV_GCOV_PREFIX
  -- leaving the tree and the install uninstrumented
  

  There was no traceback. Afterwards nm -D pgcolumnar.so | grep -c __gcov printed 0, and git status was clean.
• Tests: test_coverage_report.py, test_harness_deps.py and test_docs_cover_the_corpus.py give 72 passed, 251 checks.
• CI: 15 of 15 checks pass on this head.

🤖 Generated with Claude Code