From 9c90c4d45076d179e3487cdd6f5ff3eddfdddbf1 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 22 Oct 2026 01:06:21 +0000 Subject: [PATCH] test: the docs tests read what a reader sees, and each arm can fail (#1339) Fixes the eleven #1339 entries in the four docs tests. Each was shown green under its defect first, then red by name after the fix. test_docs_join_clustering.py - :38 [mislabelled] The file and heading checks lose "premise:". Renaming the heading reddened "premise: the star-schema join heading is present", so a docs change reddens them (#1236). - :41 [weak] "and it names the join key as the clustering column" now needs a sentence that says to cluster ON the join key, read with HTML comments removed. "Cluster" in one sentence and "join key" in another passed, and so did the phrase inside an HTML comment. - :52 [weak] "best-practices names clustering on the join key": the same rule. With the page clustering on the timestamp, "join key" elsewhere passed. test_docs_stripe_floor.py - :90 [vacuous] "and names what lowering past it costs" reads FSST in the section that gives the advice and states the floor. Page-wide, it passed with FSST removed from that section, because the compression section names it throughout. test_docs_cross_database_query.py - :19 [weak] It reads every SQL fence tag and shell blocks, matching case-insensitively. The forbidden query passed in a ```sh psql -c block, under ```postgresql, and spelled PG_DATABASE/PG_EXTENSION. test_docs_cover_the_corpus.py - :55 [weak] A name counts only as a whole word. 22 test names are prefixes of another's, so with `test_whole_row`'s own entry removed, its `test_whole_row_var_on_off` neighbour kept the arm green. - :208 [weak] One duplicate rule, _duplicated_names, is used by the real arm and by the fixture. The fixture carried a copy, and stayed green with the real arm's rule broken (count > 2). - :278 [vacuous] "each file counted once" compares the sweep with an independent listing of the directory. It compared a dict with its own keys. With the glob narrowed to test_d*.py it now names every file it missed. - :320 [vacuous] The fixture drives the no-totals arm's predicate, totals_line_refused, and checks that it refuses a stated total. It compared two tuples both already pinned, and stayed green with the real arm reading nothing. - :407 [weak] Every "N refused" README.md states must be the counted number. "128 refused" and a second, contradicting "30 refused" both passed. - :1172 [weak] One empty-section reader, _empty_sections, serves the real arm and its control. The control carried a copy, and stayed green with the real reader counting blank lines as a body. No test function is renamed, so TESTS.md and guard_tests do not move. Check names change only in the two relabelled join-clustering premises and the :320 arm, and none has a ledger or lossy_arms row. test/docs_style.sh's arm of the same name keeps reading only ```sql blocks. #1653 rewrites that file, so its parity is a follow-up there. Co-Authored-By: Claude Opus 5.5 --- test/pytest/test_docs_cover_the_corpus.py | 95 ++++++++++++++----- test/pytest/test_docs_cross_database_query.py | 14 ++- test/pytest/test_docs_join_clustering.py | 33 +++++-- test/pytest/test_docs_stripe_floor.py | 17 +++- 4 files changed, 122 insertions(+), 37 deletions(-) diff --git a/test/pytest/test_docs_cover_the_corpus.py b/test/pytest/test_docs_cover_the_corpus.py index 63796351e..14377327b 100644 --- a/test/pytest/test_docs_cover_the_corpus.py +++ b/test/pytest/test_docs_cover_the_corpus.py @@ -45,14 +45,25 @@ def corpus_tests(directory): return found +def _words(text): + """-> every whole word in TEXT, file names (`x.py`) included. + + A name counts as named only as a whole word, not as a substring: 22 tests in this + corpus have a name that is the prefix of another's (`test_whole_row` / + `test_whole_row_var_on_off`), and `name in text` counted the shorter one as + documented by the longer one's entry (#1339). + """ + return set(re.findall(r"\w+", text)) | set(re.findall(r"\w+\.py", text)) + + def undocumented(directory, doc_path): """-> sorted list of file and test names the document does not name.""" - text = pathlib.Path(doc_path).read_text() + words = _words(pathlib.Path(doc_path).read_text()) missing = [] for name, tests in corpus_tests(directory).items(): - if name not in text: + if name not in words: missing.append(name) - missing.extend(t for t in tests if t not in text) + missing.extend(t for t in tests if t not in words) return sorted(missing) @@ -67,6 +78,16 @@ def stated_totals(doc_path): return stated_totals_in(pathlib.Path(doc_path).read_text()) +def totals_line_refused(path): + """-> "none" when PATH states no totals line, else what it states, refused. + + The no-totals arm's own predicate, so its fixture below reaches the arm rather + than comparing two constants (#1339). + """ + m = stated_totals(path) + return "none" if m is None else "refused: %d tests in %d files" % m + + def _fixture(tmp_path, doc_body): """A two-test corpus and a document, so the arms can drive a KNOWN answer.""" d = tmp_path / "corpus" @@ -179,6 +200,16 @@ def test_a_documented_file_that_does_not_exist_is_caught(tmp_path, expect): "a documented file that does not exist is named") +def _duplicated_names(names): + """-> the sorted names that occur more than once in NAMES. + + ONE rule, used by the real arm and by the fixture below. The fixture used to carry + its own copy of the expression, so it proved the copy: with the real arm's rule + broken (`count(n) > 2`) the fixture stayed green (#1339). + """ + return sorted({n for n in names if names.count(n) > 1}) + + def test_no_test_name_is_defined_twice_in_the_corpus(expect): """The premise the set-equality argument needs, and it was missing (@jdatcmd). @@ -205,7 +236,7 @@ def test_no_test_name_is_defined_twice_in_the_corpus(expect): found = corpus_tests(HERE) names = [n for tests in found.values() for n in tests] expect.at_least(len(names), 20, "premise: the corpus was found") - dupes = sorted({n for n in names if names.count(n) > 1}) + dupes = _duplicated_names(names) expect.text(", ".join(dupes) or "none", "none", "no test name is defined twice in the corpus") expect.num(len(names), len(set(names)), @@ -220,7 +251,7 @@ def test_a_name_defined_in_two_files_is_caught(tmp_path, expect): names = [n for tests in found.values() for n in tests] expect.num(len(names), 2, "premise: both definitions were seen") expect.num(len(set(names)), 1, "and they share one name") - expect.text(", ".join(sorted({n for n in names if names.count(n) > 1})), + expect.text(", ".join(_duplicated_names(names)), "test_shared_shape", "a name defined in two files is named, not passed over") @@ -253,7 +284,7 @@ def test_the_document_states_no_totals_for_a_merge_to_get_wrong(expect): stays for it: the reader still has to work, or "no totals line" would be indistinguishable from "cannot find one". """ - expect.text(repr(stated_totals(DOC)), "None", + expect.text(totals_line_refused(DOC), "none", "TESTS.md states no totals line for a merge to get wrong") # And the reader that reports it must still be able to FIND one, or the arm @@ -275,7 +306,11 @@ def test_the_corpus_counts_are_reported_rather_than_written(expect): found = corpus_tests(HERE) total = sum(len(v) for v in found.values()) expect.at_least(total, 20, "premise: the corpus was found, so a count means something") - expect.num(len(found), len({f for f in found}), "each file counted once") + # Against the directory itself, listed without corpus_tests(): `found` is a dict, + # so comparing it with the set of its own keys could not fail (#1339). + on_disk = {p.name for p in HERE.iterdir() if re.fullmatch(r"test_\w+\.py", p.name)} + expect.text(", ".join(sorted(on_disk ^ set(found))) or "none", "none", + "each file counted once") print(f"\nCORPUS: {total} test functions in {len(found)} files") @@ -317,9 +352,10 @@ def test_a_stated_total_that_disagrees_with_disk_is_visible(tmp_path, expect): expect.text(repr(stated_totals(doc)), "(9, 4)", "the document states 9 in 4") expect.text(repr((sum(len(v) for v in found.values()), len(found))), "(2, 1)", "while the fixture on disk holds 2 in 1") - expect.differ(stated_totals(doc), - (sum(len(v) for v in found.values()), len(found)), - "a stated total that disagrees with disk does not compare equal") + # The arm that refuses a totals line must refuse THIS one. Comparing the two + # tuples above with each other could not fail: both are pinned already (#1339). + expect.text(totals_line_refused(doc), "refused: 9 tests in 4 files", + "and the no-totals arm refuses a document that states one") # --------------------------------------------------------------------------- @@ -404,8 +440,12 @@ def test_the_readme_and_the_inventory_agree_on_what_is_refused(expect): """ refused, _, _ = _named_modes() readme = (HERE / "README.md").read_text() - expect.at_least(readme.count(f"{len(refused)} refused"), 1, - "README.md quotes the number of modes actually named as refused") + # Every "N refused" README states, read as a whole number: a count of the right + # substring passed with "128 refused", and with a second, contradicting number + # beside the right one (#1339). + said = sorted({int(n) for n in re.findall(r"(? the numbered sections in LINES with nothing between their heading and the next. + + ONE reader for the real arm and its control. The control carried its own copy, + so it proved the copy: with the real reader counting blank lines as a body, the + control stayed green (#1339). + """ + heads = [i for i, l in enumerate(lines) if re.match(r"^## \d+[a-z]?\. ", l)] + empty = [] + for k, i in enumerate(heads): + end = heads[k + 1] if k + 1 < len(heads) else len(lines) + body = [l for l in lines[i + 1:end] if l.strip() and not l.startswith("#")] + if not body: + empty.append(lines[i][3:].split(":")[0].strip()) + return empty + + def test_a_numbered_section_has_a_body_of_its_own(expect): """A numbered heading immediately followed by another heading documents nothing. @@ -1165,23 +1222,13 @@ def test_a_numbered_section_has_a_body_of_its_own(expect): expect.at_least(len(heads), 20, "premise: the numbered headings were found at all") - empty = [] - for k, i in enumerate(heads): - end = heads[k + 1] if k + 1 < len(heads) else len(lines) - body = [l for l in lines[i + 1:end] if l.strip() and not l.startswith("#")] - if not body: - empty.append(lines[i][3:].split(":")[0].strip()) + empty = _empty_sections(lines) expect.text(", ".join(empty) or "none", "none", "every numbered section carries a body of its own") # Control: plant the shape and prove the reader names it, so a green above is the # absence of the defect rather than the absence of a working check. planted = lines[:heads[1]] + ["## 999. planted.py: nothing follows this", ""] + lines[heads[1]:] - heads2 = [i for i, l in enumerate(planted) if re.match(r"^## \d+[a-z]?\. ", l)] - found = [] - for k, i in enumerate(heads2): - end = heads2[k + 1] if k + 1 < len(heads2) else len(planted) - if not [l for l in planted[i + 1:end] if l.strip() and not l.startswith("#")]: - found.append(planted[i][3:].split(":")[0].strip()) + found = _empty_sections(planted) expect.text(", ".join(found), "999. planted.py", "control: a planted empty section is named by the same reader") diff --git a/test/pytest/test_docs_cross_database_query.py b/test/pytest/test_docs_cross_database_query.py index b794b4f7f..41e7d1652 100644 --- a/test/pytest/test_docs_cross_database_query.py +++ b/test/pytest/test_docs_cross_database_query.py @@ -6,8 +6,9 @@ to find the databases that need `ALTER EXTENSION pgcolumnar UPDATE`. The pytest half of the docs_style.sh arm of the same name, written independently: its -own parser for the ```sql blocks, split into statements on ';'. Prose that explains the -rule cannot trip it, because only fenced SQL is read. +own parser for the fenced SQL and shell blocks, split into statements on ';', matched +case-insensitively. Prose that explains the rule cannot trip it, because only fenced code is +read. Needs the repository only, not a cluster or the driver. """ @@ -16,7 +17,11 @@ REPO = pathlib.Path(__file__).resolve().parents[2] DOCS = sorted((REPO / "docs").glob("*.md")) + [REPO / "README.md"] -FENCE = re.compile(r"^[ \t]*```sql[ \t]*\n(.*?)^[ \t]*```", re.S | re.M) +# Every fence a query can be written in: SQL under any of its tags, and shell blocks, where +# the same statement arrives as `psql -c "..."`. Only ```sql was read, so the query passed +# in a ```sh block, under ```postgresql, or spelled PG_EXTENSION (#1339). +FENCE = re.compile(r"^[ \t]*```(?:sql|psql|pgsql|postgresql|postgres|sh|bash|shell|console)[ \t]*\n(.*?)^[ \t]*```", + re.S | re.M | re.I) def _statements(): @@ -31,6 +36,7 @@ def test_no_documented_query_asks_another_database_for_its_extensions(expect): stmts = _statements() expect.at_least(sum(1 for s in stmts if re.search(r"\bSELECT\b", s)), 10, "premise: the scan read SQL statements from the documents") - bad = [s for s in stmts if "pg_database" in s and "pg_extension" in s] + bad = [s for s in stmts + if re.search(r"\bpg_database\b", s, re.I) and re.search(r"\bpg_extension\b", s, re.I)] expect.num(len(bad), 0, "no documented query tests pg_extension while listing databases") diff --git a/test/pytest/test_docs_join_clustering.py b/test/pytest/test_docs_join_clustering.py index cf514044e..4c26d3ae1 100644 --- a/test/pytest/test_docs_join_clustering.py +++ b/test/pytest/test_docs_join_clustering.py @@ -7,6 +7,7 @@ sideways information passing cannot skip. """ +import re from pathlib import Path HERE = Path(__file__).resolve().parent @@ -15,8 +16,19 @@ PRACTICES = ROOT / "docs" / "best-practices.md" +# A sentence that tells the reader to cluster ON the join key: "cluster", then within the +# same sentence "on the join key". The two words anywhere in a section are not that: a +# section can name clustering by time and mention a join key in passing (#1339). +CLUSTER_ON_KEY = re.compile(r"\bcluster\w*\b[^.]*?\bon (?:the |its |a )?join key", re.I) + + +def _read(path): + """The page as a reader sees it: HTML comments are not shown, so they do not count.""" + return re.sub(r"", "", path.read_text(encoding="utf-8"), flags=re.S) + + def _section_after(path, heading): - text = path.read_text(encoding="utf-8") + text = _read(path) start = text.find(heading) if start < 0: return "" @@ -31,14 +43,18 @@ def test_how_to_names_join_key_clustering_for_the_runtime_filter(expect): """The star-schema how-to must say when group skip is a no-op. Public seam: docs/how-to.md. Independently of docs_style.sh, this reads the - section and requires both the layout verb and the join key in that section. + section as a reader sees it (HTML comments removed) and requires a sentence that + says to cluster ON the join key: "cluster" in one sentence and "join key" in another + passed before (#1339). The file and heading checks are not premises: a docs change + reddens them (#1236). """ - expect.num(int(HOWTO.is_file()), 1, "premise: how-to.md is in the tree") + expect.num(int(HOWTO.is_file()), 1, "how-to.md is in the tree") section = _section_after(HOWTO, "## Skip fact-table work under a star-schema join") - expect.at_least(len(section), 1, "premise: the star-schema join heading is present") + expect.at_least(len(section), 1, "the star-schema join heading is present") low = section.lower() expect.contains(low, "cluster", "the runtime-filter how-to names clustering") - expect.contains(low, "join key", "and it names the join key as the clustering column") + expect.at_least(len(CLUSTER_ON_KEY.findall(section)), 1, + "and it names the join key as the clustering column") def test_best_practices_names_join_key_clustering_for_a_fact_table(expect): @@ -47,6 +63,7 @@ def test_best_practices_names_join_key_clustering_for_a_fact_table(expect): Public seam: docs/best-practices.md. The skipping section told readers to cluster on the column they filter by ranges. A join key is not a range. """ - expect.num(int(PRACTICES.is_file()), 1, "premise: best-practices.md is in the tree") - text = PRACTICES.read_text(encoding="utf-8").lower() - expect.contains(text, "join key", "best-practices names clustering on the join key") + expect.num(int(PRACTICES.is_file()), 1, "best-practices.md is in the tree") + text = _read(PRACTICES) + expect.at_least(len(CLUSTER_ON_KEY.findall(text)), 1, + "best-practices names clustering on the join key") diff --git a/test/pytest/test_docs_stripe_floor.py b/test/pytest/test_docs_stripe_floor.py index 03b8e2b86..b63872fa2 100644 --- a/test/pytest/test_docs_stripe_floor.py +++ b/test/pytest/test_docs_stripe_floor.py @@ -16,6 +16,7 @@ this parses the pages itself rather than sharing a helper with the shell arm. """ +import re from pathlib import Path ROOT = Path(__file__).resolve().parents[2] @@ -63,6 +64,18 @@ def _sections_containing(path, needle): return out +def _section_text(path, headings): + """-> the text under the `##` headings named in HEADINGS, HTML comments removed.""" + out, keep = [], False + text = re.sub(r"", "", path.read_text(encoding="utf-8"), flags=re.S) + for line in text.splitlines(): + if line.startswith("## "): + keep = line[3:].strip() in headings + elif keep: + out.append(line) + return "\n".join(out) + + def test_configuration_states_the_floor_where_it_documents_the_setting(expect): expect.num(int(CONFIG.is_file()), 1, "premise: configuration.md is in the tree") expect.at_least( @@ -86,7 +99,9 @@ def test_administration_states_it_in_the_section_that_says_to_lower_it(expect): expect.at_least( len(advice & floor), 1, "and the 1024 floor is stated in that same section") - low = ADMIN.read_text(encoding="utf-8").lower() + # In THAT section too: the page names FSST all through its compression section, + # so a page-wide search passed with the cost gone from the advice (#1339). + low = _section_text(ADMIN, advice & floor).lower() expect.contains(low, "fsst", "and names what lowering past it costs")