diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ef498bb..24999969 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -116,6 +116,40 @@ true until the next version shipped. same reason: `test_the_document_states_no_totals_for_a_merge_to_get_wrong` exists to keep a totals line OUT. A count in prose that no arm reads is a claim waiting to go wrong. The arms themselves are listed in the table above, where a reader can count them. +- `TESTS.md`'s contents list was out of numeric order and no arm could see it (#1026). + + After #1023 merged, the document read: + + TOC ... 29, 31, 30 and 29, 31, 30, 32 once the next section arrived + sections ... 29, 30, 31 contiguous and correct + + #1023's contents entry for section 30 landed after 31. Resolving this PR's conflict in + that same region meant choosing an order, so the fix lands here: TOC and sections are + both 1..32 with no gap and no inversion. + + WHY NOTHING CAUGHT IT. `test_docs_cover_the_corpus.py` already sweeps every + contents-list link and asserts it reaches a heading. Both orders resolve, so that arm is + green either way. Measured by restoring the broken order under the new arm: + + the new arm FAIL got '[(29, 31), (31, 30), (30, 32)]' want 'none' + the link arms 1 passed + + So the link sweep is not a weaker version of this rule; it answers a different question, + and a shuffled contents list was outside both. + + Two arms. The first reads TESTS.md. It requires the contents numbers and the section + numbers each to count 1..N with no gap, and one contents entry per section. + + The second is the removal proof on a fixture. It uses the `29, 31, 30` shape that actually + shipped rather than a single swap. It also names an omitted entry apart from an inversion: + a missing entry gives `[(1, 3)]` and a shuffle gives `[(1, 3), (3, 2)]`. + + THE GAP RULE CATCHES THE COLLISION TOO. Three open PRs each claimed a section number + another had taken, which is the cause rather than a coincidence. A number used twice + leaves a gap in the section sequence, so `1..N with no gap` reddens on a duplicate and on + an omission with one rule. + + `guard_tests` 282 -> 284, re-derived by collection. - `native_ownership.sh` has a pytest twin, and it asserts the SQLSTATE (#432). @@ -185,6 +219,29 @@ true until the next version shipped. hands back the SET's empty result. The first version collapsed that into a 0 and the arm reported "the OWNER cannot read its stats". Every call site now asserts the error is None rather than folding it into a value. +- The docs gate checks that a markdown table is still a table (#1026). + + `docs_style.sh` enforced seven rules over every user-facing page. Sentence length, the + idiom list, em and en dashes, prose double-hyphens, conflict markers, the nav entry, and + every `VERSION` citation. ALL SEVEN ARE ABOUT PROSE. So a table that had stopped being a + table passed the gate whose purpose is keeping those pages readable. + + The measured case. A note and a second table were spliced into the middle of + `configuration.md`'s `set_options` argument table. That left six of the nine arguments as a + headerless block, and `docs_style.sh` passed with 14 checks. + + It was the second splice in one day. The first gave the GUC table no blank lines, which + made an `awk RS=''` guard read two GUC rows as one record and pass on `main`. Both are the + same fact: a markdown table is a contiguous run of `|` lines, and a blank line is + structural. + + Fences are tracked BY LINE rather than stripped with a regex. The regex form already in + that file is fine for counting, but it loses line numbers and it breaks on an unclosed + fence. State-tracking under-reports there instead, which is the safe direction. + + MEASURED BEFORE LANDING, which is what a static guard here owes. 0 across the gate's own + scope, and 5 elsewhere in the tree. All five are REAL rather than false positives: 3 in + `test/pytest/TESTS.md` and 2 in a design document, none of which the gate covers. - The mutation ledger records WHICH MAJORS each check exists on (#1010). diff --git a/test/plain_language_check.py b/test/plain_language_check.py index cfdf11ff..a6f1fac3 100755 --- a/test/plain_language_check.py +++ b/test/plain_language_check.py @@ -35,6 +35,18 @@ requirement: * No em dash or en dash, anywhere in the checked files. + * Every markdown table block carries a separator row. This is a STRUCTURE rule + rather than a language one, and it is here because nothing else in the gate + checks structure: the other four rules all pass over a table that has stopped + being a table. Measured case (#1026): a note and a second table spliced into + the middle of `configuration.md`'s argument table left six rows with no header, + and `docs_style.sh` passed with 14 checks. It was the second such splice in one + day; the first made an `awk RS=''` guard read two GUC rows as one record. + + A blank line ends a markdown table, so a run of `|` lines that contains no + `| --- |` row renders as a headerless table or as literal pipes. Either way the + page is wrong in a way a reader sees and the gate did not. + * No double hyphen used as a dash in prose. A double hyphen inside a fenced code block is a SQL comment and is left alone. @@ -90,8 +102,49 @@ def sentences(text): out.append(s) return out +# A separator row: pipes, dashes, colons and spaces only. `| --- | :-: |` and +# `|---|---|` both qualify; a row of prose does not. +_SEPARATOR = re.compile(r'^\s*\|[\s:|-]+\|\s*$') + + +def headerless_tables(text): + """[line numbers] of table blocks carrying no separator row. + + BY LINE, tracking fences, rather than by stripping them with a regex. The regex + form used elsewhere in this file is fine for counting but loses line numbers, and + a report that cannot say WHERE is a report somebody has to re-derive. It also + survives an unclosed fence, where `re.sub(r'```.*?```')` does not. + + A block is a contiguous run of lines whose first non-space character is a pipe. + A blank line, prose, or a heading ends it -- which is exactly the markdown rule + that makes the defect possible. + """ + bad, in_fence, in_table, has_sep, start = [], False, False, False, 0 + for n, line in enumerate(text.splitlines(), 1): + if line.lstrip().startswith("```"): + in_fence = not in_fence + if in_table and not has_sep: + bad.append(start) + in_table = False + continue + if in_fence: + continue + is_row = line.lstrip().startswith("|") + if is_row and not in_table: + in_table, has_sep, start = True, False, n + elif not is_row and in_table: + if not has_sep: + bad.append(start) + in_table = False + if in_table and _SEPARATOR.match(line): + has_sep = True + if in_table and not has_sep: + bad.append(start) + return bad + + def violations(path): - """Count only the rules this project enforces: length, idiom, dashes.""" + """Count only the rules this project enforces: length, idiom, dashes, tables.""" t = open(path).read() sents = sentences(t) long_ = [s for s in sents if len(s.split()) > 25] @@ -100,21 +153,25 @@ def violations(path): # A double hyphen in prose, but not one inside a fenced code block. prose = re.sub(r'```.*?```', '', t, flags=re.S) dbl = len(re.findall(r'\S -- \S', prose)) - return long_, idiom, dashes, dbl + tables = headerless_tables(t) + return long_, idiom, dashes, dbl, tables def report(path): - long_, idiom, dashes, dbl = violations(path) - n = len(long_) + len(idiom) + dashes + dbl + long_, idiom, dashes, dbl, tables = violations(path) + n = len(long_) + len(idiom) + dashes + dbl + len(tables) if n == 0: print(f" ok {path}") return 0 print(f" FAIL {path}: {len(long_)} long, {len(idiom)} idiom, " - f"{dashes} em/en dash, {dbl} prose double-hyphen") + f"{dashes} em/en dash, {dbl} prose double-hyphen, " + f"{len(tables)} headerless table") for s in long_[:5]: print(f" {len(s.split())} words: {s[:88]}") for s in idiom[:5]: print(f" idiom: {s[:88]}") + for ln in tables[:5]: + print(f" headerless table block starting at line {ln}") return n if __name__ == '__main__': diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index a9267d7d..fe04c582 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -75,9 +75,10 @@ behaviour, the source of that number is named. - [27. test_skip_loop_arms.py: a skipped arm records under its own name](#27-test_skip_loop_armspy-a-skipped-arm-records-under-its-own-name) - [28. test_docs_join_clustering.py: the runtime filter's layout precondition](#28-test_docs_join_clusteringpy-the-runtime-filters-layout-precondition) - [29. test_join_vector_agg.py: ungrouped fold over a unique-key join](#29-test_join_vector_aggpy-ungrouped-fold-over-a-unique-key-join) -- [31. test_native_ownership.py: every maintenance function is owner-only](#31-test_native_ownershippy-every-maintenance-function-is-owner-only) - [30. test_differential.py: the heap oracle, all seven parts](#30-test_differentialpy-the-heap-oracle-all-seven-parts) +- [31. test_native_ownership.py: every maintenance function is owner-only](#31-test_native_ownershippy-every-maintenance-function-is-owner-only) - [32. test_stats_privilege.py: stats is readable only by a caller who may read the table](#32-test_stats_privilegepy-stats-is-readable-only-by-a-caller-who-may-read-the-table) +- [33. test_docs_table_structure.py: a table must stay a table](#33-test_docs_table_structurepy-a-table-must-stay-a-table) ## 1. How to read a test in here @@ -1041,6 +1042,8 @@ many times. | `test_the_anchor_rule_drops_punctuation_and_keeps_underscores` | GitHub's derivation, on the heading the defect was found in | | `test_an_anchor_that_strips_the_underscores_is_caught` | the exact broken link that shipped, with a control | | `test_every_in_document_link_in_this_directory_reaches_a_heading` | every contents-list link resolves, with a coverage premise | +| `test_the_contents_list_is_numbered_in_order` | the contents list and the sections both count 1..N with no gap or inversion — the link arms above ask only whether a link RESOLVES, and a shuffled list resolves perfectly | +| `test_a_shuffled_contents_list_is_caught_on_a_fixture` | **removal proof**: the `29, 31, 30` shape that shipped, with a clean control and an omitted entry named apart from an inversion | | `test_the_next_steps_list_is_anchored_to_the_inventory` | every section 5 entry names a mode id, so the entry can be checked at all | | `test_no_open_next_step_names_work_the_document_calls_done` | an un-struck entry whose id reached section 2 is stale work to do | | `test_a_stale_next_step_is_caught_on_a_fixture` | **removal proof**: the shape, planted, with the control beside it | @@ -3169,3 +3172,34 @@ version collapsed that into a 0 and the arm reported *"the OWNER cannot read its stats"* — a product failure, from a driver behaviour. The helper now issues the SET as its own execute, and every call site asserts the error is `None` rather than folding it into a value. +## 33. test_docs_table_structure.py: a table must stay a table + +`docs_style.sh` enforced seven rules over every user-facing page. Sentence length, the idiom +list, em and en dashes, prose double-hyphens, conflict markers, the nav entry, and every +`VERSION` citation. **All seven are about prose.** So a table that had stopped being a table +passed the gate whose purpose is keeping those pages readable (#1026). + +The measured case. A note and a second table were spliced into the middle of +`configuration.md`'s `set_options` argument table. That left six of the nine arguments as a +headerless block, and `docs_style.sh` passed with 14 checks. + +It was the second splice that day. The first gave the GUC table no blank lines, which made an +`awk RS=''` guard read two GUC rows as one record and pass on `main`. Both are the same fact: +a markdown table is a contiguous run of `|` lines, and a blank line is structural. + +The rule is in `plain_language_check.py` beside the other four, because that file already +walks every page and reports the per-file counts the shell asserts. It tracks fences **by +line** rather than stripping them with a regex. The regex form loses line numbers, and a +report that cannot say where is one somebody has to re-derive. + +| arm | what it holds | +| --- | --- | +| `test_a_well_formed_table_is_not_flagged` | THE CONTROL, first: a rule flagging every table would catch the defect and be switched off the same day | +| `test_rows_orphaned_by_a_splice_are_flagged_with_their_line` | the defect, and the line the orphaned block starts at | +| `test_a_pipe_inside_a_fenced_code_block_is_not_a_table` | a shell pipeline in a fence is not a table -- with the unfenced control, or the arm passes because nothing is ever flagged | +| `test_an_unclosed_fence_does_not_swallow_the_rest_of_the_file` | the case the regex form gets wrong; state-tracking under-reports, which is the safe direction | +| `test_the_documents_the_gate_checks_are_clean` | the false-positive budget as a standing arm rather than a number measured once | + +**Measured before landing**, which is what a static guard in this tree owes. 0 across the +gate's own scope, and 5 elsewhere in the tree. All five are real: 3 in this file and 2 in a +design document, none of which the gate covers. diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 30e7a20e..72d7d613 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -39,7 +39,9 @@ # 277 -> 280: three arms in test_docs_cover_the_corpus.py about duplicated entries in # VACUITY_MODES.md. Re-derived by collection rather than by adding three, per the recipe # above: `280 tests collected`. -guard_tests 280 +guard_tests 287 +# 282 -> 284: two arms about TESTS.md's own numbering, added with the fix for the +# out-of-order contents list #1023's merge left behind. Re-derived by collection. # 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 diff --git a/test/pytest/test_docs_cover_the_corpus.py b/test/pytest/test_docs_cover_the_corpus.py index 5e9fec84..c27dbf12 100644 --- a/test/pytest/test_docs_cover_the_corpus.py +++ b/test/pytest/test_docs_cover_the_corpus.py @@ -765,6 +765,79 @@ def test_every_in_document_link_in_this_directory_reaches_a_heading(expect): "premise: and it parsed links rather than finding none") +# A NUMBERED CONTENTS LIST MUST BE IN ORDER, which the link arms above cannot see. +# They ask whether a link RESOLVES, and a shuffled list resolves perfectly. #1023's +# merge put TESTS.md's TOC at `29, 31, 30` against sections `29, 30, 31`, and every +# arm here stayed green. +_NUMBERED_TOC = re.compile(r"^- \[(\d+)\. ", re.M) +_NUMBERED_SECTION = re.compile(r"^## (\d+)\. ", re.M) + + +def _numbering(text): + """-> (toc numbers, section numbers) as they appear, in document order.""" + return ([int(n) for n in _NUMBERED_TOC.findall(text)], + [int(n) for n in _NUMBERED_SECTION.findall(text)]) + + +def _gaps(nums): + """-> [(a, b)] for every adjacent pair that is not b == a + 1.""" + return [(a, b) for a, b in zip(nums, nums[1:]) if b != a + 1] + + +def test_the_contents_list_is_numbered_in_order(expect): + """TESTS.md's contents list and its sections must both count 1..N with no gap. + + THE SHAPE THIS CLOSES, measured rather than imagined. After #1023 merged, the + document on main read: + + TOC ... 29, 31, 30 three out-of-order transitions once 32 arrived + sections ... 29, 30, 31 contiguous and correct + + so the list disagreed with the order a reader scrolls through, and the two arms + above were green throughout: both orders RESOLVE, which is all they ask. A + contents list whose numbers are shuffled is a list the reader cannot scan, and it + is the first thing anyone adding a section copies. + + BOTH SEQUENCES, not just the TOC. The collision that produced this is a section + NUMBER taken twice, so the sections are where a duplicate shows up first, and + `1..N with no gap` catches a duplicate and an omission in one rule. + """ + text = (HERE / "TESTS.md").read_text(encoding="utf-8") + toc, sections = _numbering(text) + expect.at_least(len(toc), 20, "premise: the rule found a numbered contents list") + expect.at_least(len(sections), 20, "premise: and it found numbered sections") + expect.num(toc[0], 1, "the contents list starts at 1") + expect.text(str(_gaps(toc)) if _gaps(toc) else "none", "none", + "the contents list is numbered 1..N with no gap or inversion") + expect.text(str(_gaps(sections)) if _gaps(sections) else "none", "none", + "and the sections are numbered 1..N with no gap or inversion") + expect.num(len(toc), len(sections), + "with one contents entry per section") + + +def test_a_shuffled_contents_list_is_caught_on_a_fixture(expect): + """The removal proof, on the exact shape that shipped. + + `29, 31, 30` rather than a single swap, because that is what the merge produced + and because a rule keyed only on "is it sorted" would also flag a list that is + merely missing an entry. Both are caught here, and named apart. + """ + clean = "- [1. a](#a)\n- [2. b](#b)\n- [3. c](#c)\n\n## 1. a\n\n## 2. b\n\n## 3. c\n" + toc, sections = _numbering(clean) + expect.num(len(_gaps(toc)), 0, "premise: the clean fixture is not flagged") + expect.num(len(sections), 3, "premise: and it read the sections too") + + shuffled = clean.replace("- [2. b](#b)\n- [3. c](#c)", "- [3. c](#c)\n- [2. b](#b)") + stoc, _ = _numbering(shuffled) + expect.text(str(_gaps(stoc)), "[(1, 3), (3, 2)]", + "the inversion is caught, and named as the two transitions it is") + + missing = clean.replace("- [2. b](#b)\n", "") + mtoc, _ = _numbering(missing) + expect.text(str(_gaps(mtoc)), "[(1, 3)]", + "and an omitted entry is caught as a gap, not confused with an inversion") + + # --------------------------------------------------------------------------- # Section 5's "what to add next" list must not name work that is already done. # diff --git a/test/pytest/test_docs_table_structure.py b/test/pytest/test_docs_table_structure.py new file mode 100644 index 00000000..da43263b --- /dev/null +++ b/test/pytest/test_docs_table_structure.py @@ -0,0 +1,122 @@ +"""A markdown table that stops being a table must fail the docs gate (#1026). + +`docs_style.sh` enforced seven rules over every user-facing page -- sentence length, the +idiom list, em and en dashes, prose double-hyphens, conflict markers, the nav entry, and +every `VERSION` citation. **All seven are about prose.** So a table that had stopped being a +table passed the gate whose whole purpose is keeping those pages readable. + +**The measured case.** A note and a second table spliced into the middle of +`configuration.md`'s `set_options` argument table left six of the nine arguments as a +headerless block, and `docs_style.sh` passed with 14 checks. Found in review by +@linuxhikerpm, after I had reviewed the same change twice checking sentence length, a +guard's scoping claim and a three-row mutation table -- and never asking whether the +markdown still rendered. + +It was the second splice of the day. The first gave `configuration.md`'s GUC table no blank +lines, which made an `awk RS=''` guard read two GUC rows as one record and pass on `main`. +Both are the same fact: a markdown table is a contiguous run of `|` lines and a blank line +is structural. + +**These tests drive the real checker**, for the reason the other guard tests do: a python +twin of a python rule would agree with itself. +""" + +import pathlib +import sys + +HERE = pathlib.Path(__file__).resolve().parent +sys.path.insert(0, str(HERE.parent)) + +from plain_language_check import headerless_tables # noqa: E402 + +WELL_FORMED = """Some prose. + +| Setting | Default | +| --- | --- | +| `a` | `1` | +| `b` | `2` | + +More prose. +""" + +# The defect, reduced: a blank line ends the first table, and the rows after the note have +# no header of their own. +SPLICED = """Some prose. + +| Setting | Default | +| --- | --- | +| `a` | `1` | + +A note about `a`, and a table of its own: + +| case | result | +| --- | --- | +| one | two | + +More prose about it. +| `b` | `2` | +| `c` | `3` | +""" + + +def test_a_well_formed_table_is_not_flagged(expect): + """THE CONTROL, and it comes first. A rule that flags every table would catch the + defect and be turned off the same day, so the arm that matters is this one.""" + expect.num(len(headerless_tables(WELL_FORMED)), 0, + "a table with its separator row is not flagged") + + +def test_rows_orphaned_by_a_splice_are_flagged_with_their_line(expect): + """The defect itself, and the line number, because a report that cannot say WHERE is + one somebody has to re-derive.""" + bad = headerless_tables(SPLICED) + expect.num(len(bad), 1, "the orphaned rows are flagged exactly once") + # The `| b |` row, which is line 14 of SPLICED. + expect.num(bad[0], 14, "and the report names the line the orphaned block starts at") + + +def test_a_pipe_inside_a_fenced_code_block_is_not_a_table(expect): + """Fences are tracked rather than stripped by regex. + + A shell pipeline in a code fence starts with a pipe often enough to matter, and the + regex form used elsewhere in this file loses line numbers and breaks on an unclosed + fence. Both halves are asserted: inside a fence nothing is flagged, and the same text + outside one IS -- or the arm passes because the rule flags nothing anywhere. + """ + fenced = "Prose.\n\n```\n| grep -c foo\n| wc -l\n```\n\nMore prose.\n" + expect.num(len(headerless_tables(fenced)), 0, + "pipes inside a fence are not a table") + unfenced = "Prose.\n\n| grep -c foo\n| wc -l\n\nMore prose.\n" + expect.num(len(headerless_tables(unfenced)), 1, + "control: the same lines outside a fence ARE flagged, so the fence is " + "what excluded them") + + +def test_an_unclosed_fence_does_not_swallow_the_rest_of_the_file(expect): + """The case the regex form gets wrong. + + `re.sub(r'```.*?```')` needs a closing fence; with one missing it matches nothing and + the fence's contents are scanned as prose. Tracking state by line means an unclosed + fence swallows what follows, which is the safe direction -- it under-reports rather + than inventing a table. + """ + unclosed = "Prose.\n\n```\n| not a table\n\n| `b` | `2` |\n" + expect.num(len(headerless_tables(unclosed)), 0, + "an unclosed fence under-reports rather than inventing a table") + + +def test_the_documents_the_gate_checks_are_clean(expect): + """The false-positive budget, asserted rather than measured once and trusted. + + A static guard in this tree has to be 0 over the tree before it lands, or it arrives + red on existing content and somebody disables it. This keeps that true: the rule is + green over the gate's own scope, and if a page acquires a headerless table the arm + names the page rather than the whole gate going red for an unrelated reason. + """ + root = HERE.parent.parent + pages = sorted(root.glob("docs/*.md")) + [root / "README.md"] + expect.at_least(len(pages), 8, "premise: the gate's scope is the pages it claims") + offenders = {p.name: headerless_tables(p.read_text(errors="replace")) for p in pages} + offenders = {k: v for k, v in offenders.items() if v} + expect.text(", ".join(f"{k}:{v}" for k, v in offenders.items()) or "none", "none", + "every page the gate checks carries well-formed tables only") diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index f3e8b806..63e8b27f 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -105,6 +105,7 @@ # #752 docs. Reads docs/how-to.md and docs/best-practices.md. No cluster, # no driver: the public seam is the published page. "test_docs_join_clustering.py", + "test_docs_table_structure.py", ]