diff --git a/CHANGELOG.md b/CHANGELOG.md index 38cedf69..06be141a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2256,6 +2256,46 @@ true until the next version shipped. which is the process serial already used. The in-test control (a body that concludes nothing) is unchanged in both modes. +- A collection-time vacuity refusal is no longer also reported as a silent loss (#991). + + The layer refused a run and then contradicted itself: + + ERROR: the pgColumnar vacuity layer refuses this run: a bare skip is refused ... + VACUITY: 1 collected test(s) never reported an outcome, so the run lost them + silently: tmp/test_offender.py::test_one_offending_arm + + **The run did not lose it silently. It refused it loudly, one line above.** So a reader who + typed one bare `@pytest.mark.skip` got the correct diagnosis and then a second finding + telling them a test vanished without saying so -- and the natural response is to go looking + for a lost test that was never lost. + + `collected - reported` is the right set difference and the wrong **meaning**: a silent loss + is when nobody said anything, and here the layer itself is what stopped the run. The guard + whose whole subject is a silent loss was firing on the one event that is its opposite. + + **One assignment covers all five refusal sites**, because #963 gave the layer a single + chokepoint: `_collection_usage_error` records the refusal, and the reconciliation in + `_RunShape.pytest_sessionfinish` skips the missing-outcome problem when it is set. Only that + problem. The setup-skip problem still prints -- a fixture removing every test that depends on + it is not something a refusal accounts for, and the two are independent findings. + + **Serial was the only path left.** #963 clears `items[:]` in the worker, so the controller's + `collected` set is already empty under `-n` and the contradiction cannot arise there: + + main serial refusal + the silent-loss line + main xdist no refusal at all (that was #963) + #963 serial refusal + the silent-loss line <- what this closes + #963 xdist refusal, no line + + The second arm is the control and the first is worth nothing without it: an item collected + and never reported, with **no** refusal anywhere, is still named and still reddens the run. + Dropping a problem from a reconciliation is one edit away from dropping the guard. The + fixture removes an item after the layer's hook recorded it and without deselecting it, which + is the shape of a crashed xdist worker -- `pytest_deselected` is what tells a deliberate + subset from a loss, and nothing calls it there. + + No shell check moved, so no ledger change: `harness_selftest` is 907 checks on both trees. + ## [1.0-alpha3] - 2026-09-02 ### Added diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 5ff7fcea..1df87bc0 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -263,6 +263,25 @@ a fix that moved the wrong hook reddens here. | `test_a_bare_skip_refusal_keeps_its_reason_under_xdist` | collection skip: rc 4, sentence, no INTERNALERROR, serial and `-n 2` | | `test_a_broad_except_refusal_keeps_its_reason_under_xdist` | the same for a second collection-time rule, so the defect is the hook | | `test_an_in_test_vacuity_refusal_is_unchanged_under_xdist` | **control**: a body that concludes nothing stays rc 1 with the sentence | +| `test_a_collection_refusal_is_not_also_reported_as_a_silent_loss` | a refusal is not ALSO reported as `lost them silently`, serial and `-n 2` | +| `test_a_genuine_silent_loss_is_still_reported` | **control**: an item collected and never reported, with no refusal, is still named | + +**A loud refusal is not a silent loss (#991).** A collection-time refusal printed its +sentence and then, directly beneath it, `VACUITY: N collected test(s) never reported an +outcome, so the run lost them silently`. The run did not lose them silently — it refused +them loudly, one line above — so a reader who typed one bare skip got the right diagnosis +plus a second finding sending them after a test that was never lost. `collected - reported` +is the right set difference and the wrong *meaning*: a silent loss is when nobody said +anything, and here the layer itself stopped the run. + +Every refusal goes through `_collection_usage_error`, so recording it there covers all five +call sites with one assignment, and the reconciliation skips **only** that problem. The +setup-skip problem still prints: a fixture removing every test that depends on it is not +something a refusal accounts for. The control above is what keeps dropping a problem from +becoming dropping the guard — it is one edit away. + +Serial was the only path left after #963, because clearing `items[:]` in the worker already +emptied the controller's `collected` set under `-n`. A worker records the sentence on `workeroutput` and clears the items so no ids cross. The controller re-raises `UsageError` from `pytest_testnodedown`, which diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index b5ec8158..bc2209b6 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -1426,9 +1426,21 @@ def pytest_sessionfinish(self, session, exitstatus): # sees a slice, and the controller receives every worker's reports. if hasattr(session.config, "workerinput"): return + # A LOUD REFUSAL IS NOT A SILENT LOSS (#991). On a collection-time refusal + # every collected item is accounted for BY the refusal: nothing ran, the layer + # said so, and the sentence is printed immediately above this one. Reporting + # "the run lost them silently" there gave a reader two findings where there is + # one, and sent them looking for a lost test that was never lost -- while the + # guard whose whole subject is a SILENT loss fired on the single event that is + # the opposite of silent. + # + # Only this problem is dropped. The setup-skip problem below still prints: a + # fixture removing every test that depends on it is not something the refusal + # covers, and the two are independent findings. + refused = getattr(session.config, "_pgc_vacuity_refused", None) problems = [] missing = sorted(self.collected - self.reported) - if missing: + if missing and not refused: problems.append( f"{len(missing)} collected test(s) never reported an outcome, so the " f"run lost them silently: " + ", ".join(missing[:5]) @@ -1489,6 +1501,11 @@ def _collection_usage_error(session, config, items, msg): pytest_testnodedown, which is the process wrap_session already knows how to print. """ + # RECORDED BEFORE EITHER BRANCH, so the reconciliation in _RunShape cannot call + # this a silent loss (#991). Every refusal in the layer comes through here, which + # is the only reason one assignment covers all five call sites. + config._pgc_vacuity_refused = msg + if hasattr(config, "workerinput"): wo = getattr(config, "workeroutput", None) if wo is None: diff --git a/test/pytest/test_layer.py b/test/pytest/test_layer.py index 9a580cf2..ea63fe7d 100644 --- a/test/pytest/test_layer.py +++ b/test/pytest/test_layer.py @@ -309,6 +309,94 @@ def test_quietly_gone(): ) +@pytest.mark.parametrize("mode", ["serial", "xdist"]) +def test_a_collection_refusal_is_not_also_reported_as_a_silent_loss(pytester, expect, mode): + """#991. The refusal and the reconciliation contradicted each other. + + A collection-time refusal printed its sentence and then, directly beneath it: + + VACUITY: 1 collected test(s) never reported an outcome, so the run lost + them silently: ... + + The run did not lose it silently. It refused it LOUDLY, one line above. So a + reader who typed one bare skip got the correct diagnosis plus a second finding + telling them a test vanished without saying so, and the natural response is to + go looking for a lost test that was never lost. + + `collected - reported` is the right set difference and the wrong MEANING: a + silent loss is when nobody said anything, and here the layer itself is what + stopped the run. + + Measured before the fix -- and note the serial row is the one that survived + #963, because clearing `items[:]` in the worker already emptied the controller's + `collected` set under `-n`: + + main serial refusal + the silent-loss line + main xdist no refusal at all (that was #963) + #963 serial refusal + the silent-loss line <- what this closes + #963 xdist refusal, no line + """ + pytester.makepyfile( + """ + import pytest + @pytest.mark.skip + def test_one_offending_arm(): + assert True + """ + ) + extra = ("-n", "2") if mode == "xdist" else () + result = pytester.runpytest("-p", "pgc_vacuity", *extra) + blob = result.stdout.str() + result.stderr.str() + _collection_refusal_row( + result, expect, rc=4, reason_glob="*bare skip*", + name=f"refusal not a loss {mode}", + ) + expect.num(blob.count("lost them silently"), 0, + f"{mode}: the refusal is not also reported as a silent loss") + expect.num(blob.count("VACUITY:"), 0, + f"{mode}: and with nothing else outstanding there is no VACUITY line at all") + + +def test_a_genuine_silent_loss_is_still_reported(pytester, expect): + """THE CONTROL, and the arm above is worth nothing without it. + + Dropping a problem from a reconciliation is one edit away from dropping the + guard. So: an item collected and never reported, with NO refusal anywhere, must + still be named -- which is the case the guard was written for, a test that + vanishes without anybody saying so. + + The conftest removes an item AFTER the layer's own hook has recorded it, and + without deselecting it, which is exactly the shape of a crashed xdist worker. + `pytest_deselected` is the hook that tells a deliberate subset from a loss, and + nothing calls it here. + """ + pytester.makeconftest( + """ + import pytest + + @pytest.hookimpl(trylast=True) + def pytest_collection_modifyitems(config, items): + del items[-1] + """ + ) + pytester.makepyfile( + """ + def test_the_survivor(expect): + expect.num(1, 1, "this one runs") + + def test_vanishes_without_a_word(expect): + expect.num(1, 1, "collected, then removed without deselection") + """ + ) + result = pytester.runpytest("-p", "pgc_vacuity") + blob = result.stdout.str() + result.stderr.str() + expect.num(blob.count("lost them silently"), 1, + "a genuine silent loss is still reported") + expect.num(blob.count("test_vanishes_without_a_word"), 1, + "and it is NAMED, because the next one will not be this one") + expect.num(result.ret, 1, "and the run is red for it") + + @pytest.mark.parametrize("mode", ["serial", "xdist"]) def test_a_broad_except_refusal_keeps_its_reason_under_xdist(pytester, expect, mode): """#963. Same table, second collection-time row. The defect is the hook,