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
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,35 @@ true until the next version shipped.

### Added

- `allow_empty` documented a rule the code did not enforce (#1031).

`Expect.rows` documents the argument as taking *"a REASON, not a flag"*. The sentence even
gives the rationale: the hatch should cost more to type than the honest assertion. One line
below it sits a truthiness test:

if _empty(got) and _empty(want) and not allow_empty:

So `allow_empty=True` satisfied it and carried nothing, and the hatch cost LESS than the
assertion rather than more. Measured before the refusal: `allow_empty=True` and
`allow_empty=1` both passed, 3 passed. `row_set` forwards the argument, so it inherited the
hole and now has its own arm asserting it does not route around the refusal.

THE CHECK FIRES WHENEVER THE ARGUMENT IS GIVEN, not only when both sides turn out to be
empty. Otherwise a flag form in a test whose sides happen to be non-empty passes today and
refuses on the day the data changes. That is the worst moment to learn it.

TWO LIVE SITES USED THE FLAG FORM AND BOTH WERE SUBSTANTIVELY CORRECT. Each had its
population premise on the line above. `test_check_records.py` even stated the argument in a
comment: *"an empty offender list is the answer to both, and only one of them is good news."*

So this is not a defect hiding behind the hatch. The cost fell on the next reader. The hatch
exists so every empty-on-both-sides comparison carries its justification where an audit of
`allow_empty=` can read it, and half of them carried none. Both now do.

Pinned in `test_guards_pinned.py`, which exists so a refusal is asserted by its message rather
than by "something failed". That file was built after a census found 12 of 17 guards deletable
with the suite still green.

- The gate refuses two checks that share one ledger key, instead of printing a note
about them (#982).

Expand Down
19 changes: 19 additions & 0 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,9 @@ as the failure.
| `test_num_accepts_real_numbers` | **control**: a genuine numeric comparison still passes |
| `test_text_refuses_an_empty_expectation` | an empty expected string, which anything empty satisfies |
| `test_at_least_refuses_a_non_number` | a bound taken from text |
| `test_rows_refuses_a_flag_where_it_documents_a_reason` | `allow_empty=True` satisfied a truthiness test and carried nothing, so the escape hatch cost LESS to type than the honest assertion |
| `test_rows_accepts_a_reason` | **control**: the documented form still works, or the refusal above is a wall |
| `test_row_set_inherits_the_reason_requirement` | `row_set` delegates to `rows`, so it inherits the refusal rather than routing around it |
| `test_at_least_refuses_a_floor_of_zero` | a floor every possible value clears |
| `test_at_least_accepts_a_real_bound` | **control**: `at_least(7, 3, …)` passes |
| `test_plan_node_refuses_no_criteria` | called with neither `node_type` nor `provider` |
Expand All @@ -421,6 +424,22 @@ as the failure.
| `test_refusal_itself_refuses_an_empty_pattern_list` | the new helper must not become the defect it removes |
| `test_the_empty_plan_refusal_precedes_the_arms_it_protects` | the refusal's **position**: no arm may answer ahead of it |

**`allow_empty` documented a rule the code did not enforce (#1031).** `rows` documents the
argument as taking *"a REASON, not a flag"*. The sentence gives the rationale too: the hatch
should cost more to type than the honest assertion. One line below sits a truthiness test. So
`allow_empty=True` passed and carried nothing, and the hatch cost less rather than more.

Measured before the refusal: `allow_empty=True` and `allow_empty=1` both passed, 3 passed.
`row_set` forwards the argument, so it inherited the hole, which is why it has its own arm.

The check fires whenever the argument is given, not only when both sides turn out empty.
Otherwise a flag form in a test whose sides happen to be non-empty passes today and refuses on
the day the data changes. That is the worst moment to learn it.

Two live sites used the flag form and **both were substantively correct**. Each had its
population premise on the line above, and one stated the argument in a comment. They now carry
that reason in the argument, where an audit of `allow_empty=` can read it.

### plan_marker, and the three ways it could not fail

@jdatcmd named this one first: *"both of its arms can be deleted independently
Expand Down
10 changes: 9 additions & 1 deletion test/pytest/expected_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,15 @@
# distinction, the false-positive budget, and two for the same class one level down, where a
# duplicated row in the tracked ledger file collapsed silently and let the last line win.
# Re-derived by collection on the merged tree, per the recipe above: `304 tests collected`.
guard_tests 304
# 304 -> 307: three arms in test_guards_pinned.py pinning that `allow_empty` takes a
# REASON and not a flag (#1031), with the accepted form as the control and `row_set`
# asserted to inherit the refusal rather than route around it.
#
# RE-DERIVE THIS AFTER EVERY REBASE, not once. Two branches can each derive a number
# correctly against a tree holding only their own arms and both be wrong for the merge --
# measured by @jdatcmd on #1022, where two independent derivations of 280 had 283 as the
# merged truth. Derived here by collection: `307 tests collected`.
guard_tests 307

# The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran
# in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and
Expand Down
21 changes: 21 additions & 0 deletions test/pytest/pgc_vacuity.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,27 @@ def rows(self, got, want, name, allow_empty=None):
the escape hatch costs more to type than the honest assertion.
"""
self._refuse_failed_query(name, got, want)
# THE REASON IS ENFORCED, not merely documented (#1031). This read
# `not allow_empty`, a truthiness test, so `allow_empty=True` satisfied it and
# carried nothing -- which made the escape hatch cost LESS to type than the honest
# assertion, the opposite of what the docstring above argues for. Measured before
# this check: `allow_empty=True` and `allow_empty=1` both passed.
#
# CHECKED WHENEVER IT IS GIVEN, not only when both sides turn out to be empty. A
# flag form in a test whose sides happen to be non-empty would otherwise pass
# today and refuse on the day the data changes, which is the worst moment to
# learn it.
#
# `row_set` forwards this argument, so it inherits the refusal rather than
# offering a way around it.
if allow_empty is not None and not (isinstance(allow_empty, str)
and allow_empty.strip()):
raise VacuityError(
f"{name}: allow_empty takes a REASON, not a flag, and got "
f"{allow_empty!r}. The hatch exists so an empty-on-both-sides "
f"comparison carries its justification where someone auditing "
f"allow_empty= can read it. Pass allow_empty='why it is empty'."
)
if _empty(got) and _empty(want) and not allow_empty:
raise VacuityError(
f"{name}: both sides are empty, so this comparison could not have "
Expand Down
6 changes: 4 additions & 2 deletions test/pytest/test_check_records.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,8 @@ def _self_calls(fn):
# answer to both, and only one of them is good news.
expect.at_least(len(scanned), 15, "premise: the scan found the recording methods")
expect.rows(offenders, [], "no refusal is raised after its record is taken",
allow_empty=True)
allow_empty="an empty offender list is the pass, and the at_least above "
"is the population premise that makes it mean something")


def test_every_recording_method_resolves_its_verdict(expect):
Expand Down Expand Up @@ -344,7 +345,8 @@ def test_every_recording_method_resolves_its_verdict(expect):
if not getattr(getattr(pgc_vacuity.Expect, nm), "_pgc_resolves_verdict", False)
)
expect.rows(unwrapped, [], "every method that takes a record resolves its verdict",
allow_empty=True)
allow_empty="an empty unwrapped list is the pass, and the at_least above "
"is the population premise that makes it mean something")


def test_wrapping_a_method_twice_changes_nothing(expect):
Expand Down
37 changes: 37 additions & 0 deletions test/pytest/test_guards_pinned.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,43 @@ def test_empty_text(expect):
'''), "text refuses an empty expectation", "the expected text is empty")


def test_rows_refuses_a_flag_where_it_documents_a_reason(pytester, expect):
"""`rows` says `allow_empty` takes a REASON, not a flag, and accepted a flag (#1031).

The escape hatch exists so a both-sides-empty comparison carries its justification
where someone auditing `allow_empty=` can read it. A bare `True` satisfies the
truthiness test and carries nothing, so the hatch cost less to type than the honest
assertion -- the opposite of what the docstring argues for.

Measured before this refusal: `allow_empty=True` and `allow_empty=1` both passed, 3
passed. `row_set` forwards the argument, so it inherited the hole.
"""
expect.refusal(_inner(pytester, """
def test_flag_not_reason(expect):
expect.rows([], [], "both sides empty, declared with a flag", allow_empty=True)
"""), "rows refuses a flag where it documents a reason", "not a flag")


def test_rows_accepts_a_reason(pytester, expect):
"""**control**: the documented form still works, or the refusal above is a wall."""
r = _inner(pytester, """
def test_reason(expect):
expect.rows([], [], "empty after truncate", allow_empty="the table was truncated")
""")
expect.outcomes(r, "a reason is accepted", passed=1, failed=0)


def test_row_set_inherits_the_reason_requirement(pytester, expect):
"""`row_set` delegates to `rows`, so it must inherit the refusal rather than be a way
around it. Delegating an assertion does not delegate its refusals when the delegation
transforms the data, which `row_set` already records as a trap for the sentinel case.
"""
expect.refusal(_inner(pytester, """
def test_set_flag(expect):
expect.row_set([], [], "two empty sets, declared with a flag", allow_empty=True)
"""), "row_set refuses a flag too", "not a flag")


def test_at_least_refuses_a_non_number(pytester, expect):
expect.refusal(_inner(pytester, '''
def test_bound_text(expect):
Expand Down
Loading