Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
45 changes: 45 additions & 0 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
122 changes: 104 additions & 18 deletions test/pytest/pgc_vacuity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.
Expand All @@ -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):
Expand All @@ -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}")

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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}")

Expand All @@ -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}")

Expand All @@ -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}")

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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}")

Expand All @@ -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.
#
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading