diff --git a/CHANGELOG.md b/CHANGELOG.md index 46049bc9..8b48381b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,36 @@ true until the next version shipped. ### Added +- Every counted assertion in the pytest harness produces a record, and the + count is derived from them (#937, first phase). + + The shell harness makes counting and recording the same call, so no path can + do either alone, and reconciles the totals afterwards. The pytest half reaches + the same property through Python rather than through the shell's format. + + It reaches it more strongly, because Python can remove the possibility instead + of policing it. The count is not a second variable kept in step with the + records; it is `len(self._records)`, a property with no setter. A count that + cannot be written cannot drift from the stream it counts. + + Measured before this: `_counted()` at 15 call sites, the counter incremented + by one line and read by one, and zero per-assertion records. + + The record is an object on the recorder, not a formatted line. The shell's + record is tab separated, so `pgc_record` has to strip tabs and newlines out of + a check name. There is no separator here to smuggle, and a name carrying both + is asserted to round-trip byte-identical, so the class of defect cannot return + silently if these ever become a line. + + `cannot_run()` records `UNRUN` rather than a pass. An assertion that declined + to run is an outcome like any other. + + A refused assertion leaves no record: a `VacuityError` means the assertion + never ran, so the stream is outcomes rather than attempts. + + Still to come in #937: the verdict resolved from the outcome, and a session + reconciliation that can fail. + - A serial inner Hash Join can push the build-side keys into a direct columnar scan (#752). diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 4f8e4b30..37db360d 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -71,6 +71,7 @@ behaviour, the source of that number is named. - [23. test_mutation_ledger.py: which checks have ever been red](#23-test_mutation_ledgerpy-which-checks-have-ever-been-red) - [24. test_loop_coverage_premise.py: a loop that never ran asserted nothing](#24-test_loop_coverage_premisepy-a-loop-that-never-ran-asserted-nothing) - [25. test_join_runtime_filter.py: serial join runtime filter](#25-test_join_runtime_filterpy-serial-join-runtime-filter) +- [26. test_check_records.py: every counted assertion is a record](#26-test_check_recordspy-every-counted-assertion-is-a-record) ## 1. How to read a test in here @@ -2363,3 +2364,47 @@ crash on this shape when it drained the tap through `ExecProcNode`. A non-key fact-table qual with late materialization off. The attach used to force the two-pass path with only the join key decoded, so the qual dropped every row. Heap is the oracle. Independent of the shell conjunction arm. + +## 26. test_check_records.py: every counted assertion is a record + +#937, first phase. The shell harness makes counting and recording the same call, +so no path can do either alone, and then reconciles the totals. **The pytest half +reaches the same property through Python instead of through the shell's format**, +which is what "parallel in functionality only" requires: nothing here reads, +sources or derives from `test/*.sh`. + +It is reached more strongly, because Python can remove the possibility rather than +police it. The count is not a second variable kept in step with the records: + +```python +@property +def count(self): + return len(self._records) +``` + +Measured before this file existed: `_counted()` at 15 call sites, `self.count` +incremented by one line and read by one, and **zero** per-assertion records. + +| test | what it pins | +|---|---| +| `test_each_counted_assertion_appends_exactly_one_record` | three assertions leave three records, from a premise of zero | +| `test_the_count_is_the_record_stream` | the count tracks the records at every step, not only at the end | +| `test_the_count_cannot_be_moved_without_a_record` | **the construction proof**: the count has no setter | +| `test_a_record_names_the_assertion_that_made_it` | the names, in order | +| `test_a_refused_assertion_leaves_no_record` | a `VacuityError` is not an outcome; the stream is not a log of attempts | +| `test_a_name_carrying_a_separator_survives_the_record` | a tab and a newline in a name round-trip byte-identical | + +**The construction arm is the one that matters.** A test checking only that the +count agrees with the records would pass on an implementation keeping two numbers +that happen to be updated together — which is exactly the drift the shell side +needs a reconciliation to catch. Asserting the count cannot be written at all is +what makes the agreement structural. + +**The separator arm looks redundant and is not.** There is no separator in an +object, so it cannot fail today. It exists because the moment somebody formats +these records into a line, the shell's tab-and-newline defect returns — measured +there as a record of four fields for a tabbed name and two lines for a newline — +and without this arm nothing would say so. + +Still to come in #937: the verdict resolved from the outcome, and a session +reconciliation that can fail. diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index 99598ad6..d929f8c5 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -125,6 +125,36 @@ def _failed_query(v, _prefix=QUERY_ERROR): _WRITE_TAGS = ("INSERT", "UPDATE", "DELETE", "MERGE", "COPY") +class _Record: + """One counted assertion's outcome. #937. + + THE COUNT IS THIS LIST'S LENGTH, which is the whole design. The shell harness + keeps `PGC_CHECKS` and a record stream honest by reconciling them, because in + bash they must be two things. Here they need not be, so the count is a derived + property and `_records` is the only state -- nothing can increment a count + without a record existing, by construction rather than by discipline. + + AN OBJECT, NOT A FORMATTED LINE, and that is load-bearing rather than + idiomatic. The shell's record is tab separated, so `pgc_record` had to strip + tabs AND newlines out of a check name: measured there, a tab gave a record of + four fields and a newline gave two lines. A name is carried here as an + attribute, so there is no separator to smuggle -- and test_check_records.py + asserts that with a name holding both, because the moment these become a line + the whole class of defect returns and nothing would say so. + + The verdict is filled in by the phase that resolves it from the outcome; a + record created here is an assertion that ran, which is the fact the call site + knows. + """ + + __slots__ = ("name", "verdict", "reason") + + def __init__(self, name, verdict="PASS", reason=""): + self.name = name + self.verdict = verdict + self.reason = reason + + class _Write: """One write statement's outcome. @@ -234,9 +264,31 @@ class Expect: def __init__(self, nodeid): self.nodeid = nodeid - self.count = 0 + self._records = [] self.unrunnable = None + @property + def records(self): + """The assertions this test has concluded, in the order it concluded them. + + A TUPLE, so a caller cannot append to the stream without going through the + recorder. `pytest_runtest_call` reaches the list itself to resolve the + verdict of the assertion that raised; everything else reads this. + """ + return tuple(self._records) + + @property + def count(self): + """How many assertions were counted. NOT a second variable. + + There is no setter, deliberately. An arm in test_check_records.py asserts + that assigning to it raises, because a test checking only that the count + AGREES with the records would pass on an implementation that keeps two + numbers and happens to update both -- which is exactly the drift the shell + side needs a reconciliation to catch. + """ + return len(self._records) + # -- the recorder ------------------------------------------------------- def _refuse_failed_query(self, name, got, want): """Refuse a comparison where either side is a failed query. @@ -257,8 +309,37 @@ def _refuse_failed_query(self, name, got, want): f"failure you expect rather than comparing two of them." ) - def _counted(self): - self.count += 1 + def _record(self, name, verdict="PASS", reason=""): + """Count an assertion by recording it. One operation, no other path. + + Called BEFORE the comparison, at every site, because that is where the + call site knows the assertion ran. The verdict is not passed in: passing it + would need the outcome, which would split this back into two steps. + + SO EVERY RECORD IS `PASS` UNTIL A LATER PHASE SETS IT, AND THAT PHASE + CANNOT RESOLVE IT FROM THE EXCEPTION. The first version of this comment + argued it could: assertions in a body are sequential and a raise ends the + test, so the failing assertion would be the last record. **That is false + here, and the corpus is what makes it false** -- proving a guard refuses + means catching the AssertionError, which five tests do + (test_ordered.py:243, test_failed_query_sentinel.py:236, :326, :357, :382). + Driven on this branch: + + count before/mid/after: 0 / 1 / 2 + record 0 'this comparison must fail' verdict PASS <- this RAISED + record 1 'and the test continues' verdict PASS + 1 passed + + A genuinely failed assertion stays PASS, in a passing test, and nothing + reaches `pytest_runtest_call` to correct it. Found by @OffgridwithJD + attacking the argument rather than the code. + + The verdict therefore has to be set on the comparison's own path, where + the outcome is known and no propagation is needed. That stays one + operation; it is phase 2's work and is not claimed here. What IS claimed + here is the count, which the probe above shows is 2 and correct. + """ + self._records.append(_Record(name, verdict, reason)) # -- numbers ----------------------------------------------------------- def num(self, got, want, name): @@ -273,7 +354,7 @@ def num(self, got, want, name): f"{type(got).__name__}={got!r} and {type(want).__name__}={want!r}. " f"A text comparison here is the defect this harness removes." ) - self._counted() + self._record(name) if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") @@ -328,7 +409,7 @@ def ordered_rows(self, got, want, name): f"is the same, so the reverse ordering is identical and the claim " f"asserts nothing beyond what rows() already asserts." ) - self._counted() + self._record(name) if g != w: for i, (a, b) in enumerate(zip(g, w)): if a != b: @@ -358,7 +439,7 @@ def ordering_observable(self, forward, reverse, name): f, r = list(forward), list(reverse) if not f and not r: raise VacuityError(f"{name}: both directions are empty.") - self._counted() + self._record(name) if f == r: raise AssertionError( f"{name}: the forward and reverse readings are identical, so nothing " @@ -400,7 +481,7 @@ def differ(self, got, want, name): f"this assertion would report the mutation as observable. Assert " f"the failure you expect instead of differencing two of them." ) - self._counted() + self._record(name) if got == want: raise AssertionError( f"{name}: arms-do-not-differ: {got!r} on both arms. An A/B whose arms " @@ -495,7 +576,7 @@ def rows(self, got, want, name, allow_empty=None): f"failed. If an empty result is the point, pass " f"allow_empty='why it is empty'." ) - self._counted() + self._record(name) if list(got) != list(want): raise AssertionError(f"{name}: got {got!r} want {want!r}") @@ -512,7 +593,7 @@ def hash(self, got, want, name): self._refuse_failed_query(name, got, want) if _empty(got) and _empty(want): raise VacuityError(f"{name}: both hashes are empty.") - self._counted() + self._record(name) if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") @@ -524,7 +605,7 @@ def text(self, got, want, name): raise VacuityError( f"{name}: the expected text is empty, so anything empty satisfies it." ) - self._counted() + self._record(name) if got != want: raise AssertionError(f"{name}: got {got!r} want {want!r}") @@ -580,7 +661,7 @@ def sqlstate(self, exc, want, name): f"`exc.value` inside a `with pytest.raises(...) as exc` block, not " f"`exc`." ) - self._counted() + self._record(name) got = exc.sqlstate if got is None: raise AssertionError( @@ -627,7 +708,7 @@ def plan_node(self, plan, node_type=None, provider=None, name=None): continue if provider is not None and pv != provider: continue - self._counted() + self._record(name) return node raise AssertionError( @@ -654,7 +735,7 @@ def at_least(self, got, floor, name): f"{name}: a floor of {floor!r} is satisfied by any count, so this " f"asserts nothing." ) - self._counted() + self._record(name) if not got >= floor: raise AssertionError(f"{name}: got {got!r}, wanted at least {floor!r}") @@ -678,7 +759,7 @@ def refusal(self, result, name, *patterns): f"{name}: refusal() with no pattern asserts only that something " f"failed, which is the defect it exists to remove." ) - self._counted() + self._record(name) result.assert_outcomes(failed=1, passed=0) # ANCHORED TO pytest's ERROR-LINE PREFIX, and that is the whole point. # @@ -719,12 +800,12 @@ def outcomes(self, result, name, **want): raise VacuityError( f"{name}: outcomes() with no expectation asserts nothing." ) - self._counted() + self._record(name) result.assert_outcomes(**want) def run_failed(self, result, name): """Assert an inner run exited non-zero, and count it.""" - self._counted() + self._record(name) if result.ret == 0: raise AssertionError( f"{name}: the inner run exited 0, so nothing refused it." @@ -774,7 +855,7 @@ def plan_marker(self, plan, key, name=None, absent=False): if key in node: found = True - self._counted() + self._record(name) if absent and found: raise AssertionError(f"{label}: the key is present and should not be.") if not absent and not found: @@ -802,7 +883,12 @@ def cannot_run(self, reason, detail=""): f"unrunnable reason {reason!r} is not one of {UNRUNNABLE_REASONS}" ) self.unrunnable = (reason, detail) - self._counted() + # UNRUN, NOT PASS. An assertion that declined to run is an outcome like any + # other -- #937 property 4 -- and the shell's verdict vocabulary has the same + # four values for the same reason. The record is named by the reason CODE, + # which is from a closed list, so the stream stays keyable when the detail is + # free text. + self._record(name=reason, verdict="UNRUN", reason=detail) @pytest.fixture diff --git a/test/pytest/test_check_records.py b/test/pytest/test_check_records.py new file mode 100644 index 00000000..1c74beec --- /dev/null +++ b/test/pytest/test_check_records.py @@ -0,0 +1,122 @@ +"""Every counted assertion produces a record, and the count is derived from them. + +#937. The shell harness records every check outcome as a machine-readable line, +and `pgc_record` counts and records in one call so no path can do either alone. +`pgc_reconcile_records` then reconciles the record count against `checks run:`. + +THE PYTEST HALF REACHED THE SAME PROPERTY THROUGH PYTHON RATHER THAN THROUGH THE +SHELL'S FORMAT, which is what "parallel in functionality only" requires. Nothing +here reads, sources or derives from `test/*.sh`; `test_harness_deps.py` proves +that for the whole corpus rather than this file asserting it about itself. + +AND IT IS STRONGER THAN THE SHELL'S, because Python can remove the possibility +instead of policing it. The shell keeps a counter and a record stream honest by +reconciling two variables that could drift. Here the count IS the record stream: + + @property + def count(self): + return len(self._records) + +A derived count cannot be incremented without a record existing. That is property +1 of #937 reached by construction rather than by discipline, and the arms below +attack the construction rather than trusting the sentence. + +Measured before this file existed: `_counted()` at 15 call sites, `self.count` +incremented by one line and read by one (`pytest_runtest_call`), and ZERO +per-assertion records anywhere. +""" + +import pgc_vacuity + + +def test_each_counted_assertion_appends_exactly_one_record(expect): + """Three assertions, three records. The premise is that a fresh recorder has + none, or the arm would pass on a recorder that ignored every call.""" + e = pgc_vacuity.Expect("records::one-each") + expect.num(len(e.records), 0, "premise: a fresh recorder holds no records") + e.num(1, 1, "first") + e.num(2, 2, "second") + e.num(3, 3, "third") + expect.num(len(e.records), 3, "three assertions left three records") + + +def test_the_count_is_the_record_stream(expect): + """Not "the count agrees with the records" -- that is the shell's property, + and it needs a reconciliation because the two can drift. Here they are the + same object, so the arm asserts identity of the NUMBER at every step rather + than equality at the end.""" + e = pgc_vacuity.Expect("records::derived") + seen = [] + for i in range(4): + e.num(i, i, f"assertion {i}") + seen.append((e.count, len(e.records))) + expect.rows([f"{c}/{r}" for c, r in seen], + ["1/1", "2/2", "3/3", "4/4"], + "the count tracks the records at every step") + + +def test_the_count_cannot_be_moved_without_a_record(expect): + """THE CONSTRUCTION PROOF, and the reason this is not just a second counter. + + A test that only checked `count == len(records)` after a run would pass on an + implementation that keeps two variables and happens to update both. This + asserts the count cannot be written at all, which is what makes the agreement + structural rather than maintained. + """ + e = pgc_vacuity.Expect("records::readonly") + e.num(1, 1, "one assertion") + try: + e.count = 99 + except AttributeError: + expect.num(e.count, 1, "the count refused to be written and did not move") + else: + raise AssertionError( + "the count was assignable, so it is a second variable that can drift " + "from the records rather than being derived from them" + ) + + +def test_a_record_names_the_assertion_that_made_it(expect): + """A record that cannot be traced to a call site answers no question worth + asking. The names are asserted IN ORDER, because the order is what lets a + later phase say which assertion failed.""" + e = pgc_vacuity.Expect("records::named") + e.num(1, 1, "the first question") + e.text("a", "a", "the second question") + e.num(2, 2, "the third question") + expect.ordered_rows([r.name for r in e.records], + ["the first question", "the second question", + "the third question"], + "each record carries the name its call site gave") + + +def test_a_refused_assertion_leaves_no_record(expect): + """A VacuityError means the assertion never ran, so it is not an outcome. + + This is the arm that stops the record stream becoming a log of attempts. It + matters for the reconciliation in phase 5: a refused assertion that left a + record would make the totals disagree with what the run reported. + """ + e = pgc_vacuity.Expect("records::refused") + try: + e.num("100", 100, "a text comparison") + except pgc_vacuity.VacuityError: + expect.num(len(e.records), 0, "a refused assertion recorded nothing") + else: + raise AssertionError("num() accepted a string, so this arm tested nothing") + + +def test_a_name_carrying_a_separator_survives_the_record(expect): + """#937 property 3, asserted rather than assumed. + + The shell had to strip tabs and newlines from a check name because its record + is a tab-separated line: a name with a tab gave four fields, and a name with a + newline gave two lines. The record here is an object, so there is no separator + to smuggle -- but that must be a test, because the moment somebody formats + these into a line the class of defect comes back and nothing would say so. + """ + e = pgc_vacuity.Expect("records::separators") + nasty = "a name with\ta tab and\na newline" + e.num(1, 1, nasty) + expect.num(len(e.records), 1, "a name with separators made exactly one record") + expect.text(e.records[0].name, nasty, "and the name came back byte-identical") diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index 64b2627e..c7d4b517 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -90,6 +90,11 @@ # agrees. The fourth time this arm has caught a merge-order consequence # rather than a mistake, which is the argument for it. "test_mutation_ledger.py", + # #937's first phase. It exercises `Expect` directly -- no connection, no + # cluster, no driver -- so the classifier puts it here and the declaration must + # agree. The fifth time this arm has decided a membership rather than been told + # one. + "test_check_records.py", ]