diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 2838ab212..a69a2f8cc 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -1892,7 +1892,7 @@ fixtures are read off `conftest.py` rather than named in the classifier. | `test_ci_derives_the_file_list_rather_than_repeating_it` | the CI job asks this module for `NO_CLUSTER`, names no file literally, and states no count | | `test_the_job_installs_no_database_driver` | the job asserts psycopg is absent rather than assuming it | | `test_the_cluster_job_runs_the_other_half_and_derives_it` | the complement of `NO_CLUSTER` is RUN, derived not listed, and asserts the driver IS present | -| `test_both_pytest_jobs_assert_how_many_tests_they_collected` | both jobs pass `--pgc-expect-tests` from the tracked file, and each guards the read | +| `test_both_pytest_jobs_assert_how_many_tests_they_collected` | each job, in its own code lines rather than a count over the file, passes `--pgc-expect-tests` from the tracked file and guards the read | | `test_each_expected_count_is_stated_exactly_once` | `expected_tests.txt` states each key on one line — a keep-both merge duplicates it, `awk` then hands the flag a multi-line value, and the arm above could not see it because it builds a dict | | `test_a_duplicated_count_is_caught_and_the_dict_form_is_not` | **removal proof**: the same fixture read both ways, so the dict form is shown keeping the LAST line while the line form names the duplicate | | `test_the_shell_reference_detector_sees_code_and_not_prose` | the premise: a docstring is prose, a string passed to bash is a reference, an f-string counts once | diff --git a/test/pytest/pgc_vacuity.py b/test/pytest/pgc_vacuity.py index 753656cc9..cab7165a8 100644 --- a/test/pytest/pgc_vacuity.py +++ b/test/pytest/pgc_vacuity.py @@ -973,7 +973,12 @@ def outcomes(self, result, name, **want): f"{name}: outcomes() with no expectation asserts nothing." ) self._record(name) - result.assert_outcomes(**want) + # pytest's own failure is a bare dict comparison; name the check, or a red + # outcomes() is the one assertion in the corpus that does not say what failed. + try: + result.assert_outcomes(**want) + except AssertionError as exc: + raise AssertionError(f"{name}: {exc}") from None @_resolving def run_failed(self, result, name): diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index be60df42a..20525bb8b 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -472,7 +472,7 @@ def test_the_loop_reader_invents_nothing_in_this_corpus(expect): added += len(got) absent += [n for n in got if n not in _joined(src)] expect.at_least(added, 40, - "premise: the reader really does add names in this tree, so the " + "the reader really does add names in this tree, so the " "assertion below is not vacuous") expect.text(", ".join(absent) or "none", "none", "every name the loop reader returns appears verbatim in the file it " @@ -493,7 +493,7 @@ def test_the_loop_reader_invents_nothing_in_this_corpus(expect): split = ('for label, sql in (("one two "\n' + ' "three", "q"),):\n' ' expect.num(g, 1, label)\n') expect.text(", ".join(_loop_names(ast.parse(split))), "one two three", - "premise: the reader joins a wrapped name, which is why the relaxation " + "the reader joins a wrapped name, which is why the relaxation " "is needed at all") expect.contains( split, "one two three", @@ -507,7 +507,7 @@ def test_the_loop_reader_invents_nothing_in_this_corpus(expect): 'for label, sql in ((f"one {P} three", "q"),):\n' ' expect.num(g, 1, label)\n') expect.text(", ".join(_loop_names(ast.parse(built))), "one {} three", - "premise: an f-string name is read as a TEMPLATE, not refused") + "an f-string name is read as a TEMPLATE, not refused") expect.contains( _joined(built), "one {} three", "and collapsing the concatenation does NOT rescue it -- a template is " @@ -568,7 +568,7 @@ def test_dropping_a_member_brings_the_divergence_back_named(expect): bash = ["c_int range", "c_vc range"] full = set(_matchable_names(_EXPAND_FIXTURE)) expect.num(len([b for b in bash if b not in full]), 0, - "premise: with both members present, neither bash name is missing") + "with both members present, neither bash name is missing") thinned = set(_matchable_names(_EXPAND_FIXTURE.replace( '"c_vc": "c_vc > \'a\'"', ''))) @@ -627,6 +627,22 @@ def test_the_expansion_refuses_what_it_cannot_spell(expect): expect.num(len(_expanded_names(ast.parse(constant))), 0, "a name interpolating a module constant is refused, because 1e-6 " "renders as 1e-06 and bash carries neither") + # THE CONSTANT ALONE (#1339 :468). The fixture above also interpolates `col`, so the + # two-distinct-names rule refuses it too, and admitting constants to the gate + # (`e.id in columns or e.id in consts`) left that arm green. Here the constant is + # the only interpolation, so this rule is the only thing that can refuse it. A gate + # that admitted it then looks the constant up as a column and raises, which is read + # as a value so this arm reddens by name rather than erroring. + alone = ('FLOAT_RTOL = 1e-6\n' + '@pytest.mark.parametrize("col", ["c_int"])\n' + 'def test_x(col, expect):\n' + ' expect.num(g, 1, f"within {FLOAT_RTOL} relative")\n') + try: + got = repr(_expanded_names(ast.parse(alone))) + except KeyError as exc: + got = f"raised KeyError {exc}" + expect.text(got, "[]", + "a name whose only interpolation is a module constant is refused") multi = ('@pytest.mark.parametrize("old", ["a"])\n' '@pytest.mark.parametrize("new", ["b"])\n' @@ -1310,12 +1326,13 @@ def test_a_longer_helper_name_is_not_shadowed_by_a_shorter_one(expect): def test_the_suite_local_helpers_are_known_and_excluded(expect): - """Three helpers are defined by ONE suite each, and the extractor does not read - them. That is a scope decision and it is asserted rather than left implicit. + """Three helpers are defined by one suite, `parallel_copy.sh`, and the extractor + does not read them. That is a scope decision and it is asserted rather than left + implicit. `compare_to_bash.py` grades a `test/.sh` against a - `test/pytest/test_.py`. No suite defining its own helper has a pytest - twin, so none is graded and the exclusion costs nothing TODAY. + `test/pytest/test_.py`. `parallel_copy.sh` has no pytest twin, so it is not + graded and the exclusion costs nothing TODAY. The day one of them is ported, this arm is what says the grader cannot see it. It said so for parquet_export_stats.sh (#1341), whose check_float went when the suite was ported: its two NaN-fixture checks and eight f4 bounds call `check`. @@ -1671,18 +1688,14 @@ def test_the_ported_suites_in_this_tree_are_graded_one_for_one(expect): Only the pairs that reach zero today are listed. A pair with a real gap is not pinned to its gap: that would turn the gap into the expected state. - THE LIST IS HAND-WRITTEN FOR THAT REASON AND NOTHING ENFORCES IT, which is a - different thing from the reason being wrong. The comment below already says a new - port belongs here; no arm reddens when one does not arrive. Today the list happens - to equal the pairs that exist, so nothing has ever been silently ungraded -- but an - eighth complete pair omitted would leave this arm passing while it graded seven, - which is absent-from-the-list and no-gap-found producing the same green. - - #1046 TRACKS MAKING THIS ASSERTION TWO-DIRECTIONAL, in the shape `SHELL_REFERENCES` - already uses: derive the pairs that EXIST and require the declared list to equal that - set. It is not a tidy-up -- it changes what happens to an INCOMPLETE port, which is - quietly absent today and would have to redden, so the issue records the design - question rather than settling it. Kept out of the PR that added the eighth pair. + THE LIST IS ENFORCED BOTH WAYS (#1046, landed). `COMPLETE` and `INCOMPLETE` are + module declarations, and `test_every_pair_in_the_tree_is_declared` requires their + union to equal the pairs that exist on disk, in both directions; the cardinality + premise below counts the same thing from this arm's side. A complete pair left + out of both reddens there -- measured: dropping `advisory_lock_class` from + `COMPLETE` fails both -- so absent-from-the-list and no-gap-found can no longer + produce the same green. An INCOMPLETE port is declared with its reason and + graded by its own arms, not by this loop. """ from compare_to_bash import main import contextlib diff --git a/test/pytest/test_failed_query_sentinel.py b/test/pytest/test_failed_query_sentinel.py index eb59c17e1..0efec3c53 100644 --- a/test/pytest/test_failed_query_sentinel.py +++ b/test/pytest/test_failed_query_sentinel.py @@ -184,9 +184,18 @@ def test_every_public_assertion_is_selected_or_excluded(expect): f"inputs {len(public)} == selected {len(selected)} + excluded {len(excluded)}") expect.num(len(residue), 0, f"no public method matches neither rule; residue: {residue}") - for name, reason in excluded: - expect.at_least(len(reason.split()), 3, - f"{name} is excluded with a stated reason, not a missing entry") + # NO DEAD EXEMPTION (#1339 :186). The reason check this replaces could not fail: + # an exclusion IS a lookup in _LEFT_OPERAND_KIND, so every excluded method has the + # table's reason by construction. What can rot is the table itself -- a kind no + # public method takes any more, waiting to exempt the next assertion that happens + # to name its first parameter that way. Every kind must be in use. + used = {_params(fn)[0] + for n, fn in inspect.getmembers(pgc_vacuity.Expect, inspect.isfunction) + if not n.startswith("_") and _params(fn)} + dead = sorted(k for k in _LEFT_OPERAND_KIND if k not in used) + expect.text(", ".join(dead) or "none", "none", + "every exclusion kind is the first parameter of a public method, " + "so none is a dead exemption") def test_wrote_is_excluded_because_its_left_operand_is_a_cursor(expect): @@ -324,7 +333,7 @@ def test_the_refusal_cannot_be_switched_off_from_the_corpus_it_polices(expect): # prefix, which is the point, and an arm ABOUT sentinels therefore cannot use # the assertion it is describing on one. expect.num(1 if a.split(".")[0] == original else 0, 1, - f"premise: minted under the real prefix ({spelling!r} case)") + f"minted under the real prefix ({spelling!r} case)") pgc_vacuity.QUERY_ERROR = spelling # the corpus rewrites it after try: e.text(a, b, "a rewritten prefix") diff --git a/test/pytest/test_guards_pinned.py b/test/pytest/test_guards_pinned.py index 3cb912634..fe4a4ac05 100644 --- a/test/pytest/test_guards_pinned.py +++ b/test/pytest/test_guards_pinned.py @@ -1,4 +1,11 @@ -"""Every refusal in the layer, pinned to its own message. +"""The layer's guards, each pinned to its own message. + +NOT EVERY REFUSAL IN THE LAYER (#1339 :1). This file pins the guards the census +below found deletable, 17 arms. `pgc_vacuity` raises VacuityError at more sites than +that, and several are pinned where their subject lives rather than here -- the +failed-query refusals in `test_failed_query_sentinel.py`, the collection-time +refusals in `test_layer.py`. A refusal added to the layer is not covered by this +file's existing; it needs an arm here or in the file of its subject. WHY THIS FILE EXISTS. @jdatcmd neutered each guard in turn and found 11 of 17 deletable with `test_layer.py` still green. Repeating the census over the whole @@ -370,7 +377,7 @@ def test_the_empty_plan_refusal_precedes_the_arms_it_protects(expect): if "if not absent and not found:" in l), None) expect.text(f"{i_refusal is not None} {i_absent is not None} {i_present is not None}", "True True True", - "premise: all three were found, so the ordering can mean something") + "all three were found, so the ordering can mean something") # TWO ARMS, NOT ONE FLAG (#1030). `int(a < b and a < c)` collapses two # comparisons into 0 or 1, so a failure says `got 0 want 1` and cannot name # WHICH ordering broke -- with all three indices in scope one line above. Two diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index bc4b66c73..120b198c3 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -523,7 +523,7 @@ def partition(directory=None): # # The closure IS load-bearing inside conftest, where a fixture can reach # a cluster through a sibling without importing anything of its own, and - # `test_the_classifier_follows_a_conftest_fixture_that_connects_indirectly` + # `test_harness_deps_classifier.py::test_the_classifier_follows_a_conftest_fixture_that_connects_indirectly` # kills it there. uses = any(kind in ("fixture", "test") and set(params) & roots for kind, params, _body in defs.values()) @@ -816,8 +816,8 @@ def test_the_partition_accounts_for_every_file_in_the_corpus(expect): expect.num(len(free) + len(bound), len(files), f"every file lands in exactly one bucket: {len(free)}+{len(bound)}") expect.at_least(len(files), 10, "premise: the glob found the corpus") - expect.at_least(len(free), 1, "premise: the partition is not all cluster-bound") - expect.at_least(len(bound), 1, "premise: the partition is not all database-free") + expect.at_least(len(free), 1, "the partition is not all cluster-bound") + expect.at_least(len(bound), 1, "the partition is not all database-free") print("\n-- partition: %d database-free, %d cluster-bound, %d files" % (len(free), len(bound), len(files))) @@ -827,8 +827,12 @@ def test_the_cluster_fixtures_are_read_off_conftest_rather_than_named_here(expec spells out are the driver and the module that provisions a server, so a third connecting fixture in conftest is covered without an edit here.""" roots = cluster_fixtures(HERE / "conftest.py") - expect.at_least(len(roots), 2, - f"conftest names fixtures that reach a cluster: {sorted(roots)}") + # THE SET, not a count of it (#1339 :782). `at_least(2)` passed a derivation that + # lost a fixture or invented one, and the name carried the list it was judging. A + # third connecting fixture still needs no edit to the CLASSIFIER; it reddens here + # by name, which is the reviewer seeing the derivation move. + expect.text(", ".join(sorted(roots)), "pgc_cluster, pgc_conn, pgc_own_db", + "conftest's cluster-reaching fixtures are derived exactly") print("\n-- cluster fixtures derived from conftest.py: %s" % sorted(roots)) @@ -1051,7 +1055,12 @@ def test_the_cluster_job_runs_the_other_half_and_derives_it(expect): # The control for pytest-guards' own assertion. That job proves its files need no # database by psycopg's ABSENCE, so this one has to state its presence or the pair # proves nothing about the split. - expect.at_least(job.count("pip show psycopg"), 1, + # A CODE LINE THAT RUNS IT AND CAN FAIL (#1339 :964). Counting the text passed a + # comment and `pip show psycopg || true`, neither of which asserts anything. + present = [l for l in _code_lines(job) + if re.search(r"\bpip show psycopg\b", l) + and not l.lstrip().startswith("!") and "||" not in l] + expect.at_least(len(present), 1, "and asserts the driver IS present, which is what makes it the other half") # It must not list the corpus either way round: neither the files it runs nor the @@ -1092,15 +1101,17 @@ def test_both_pytest_jobs_assert_how_many_tests_they_collected(expect): expect.at_least(nums.get(key, 0), 1, f"{key} is named and positive, or the flag it feeds asserts nothing") - # BOTH jobs, not one. Arming half of them would leave the other able to collect - # nothing and pass, which is the state this closes. - expect.num(text.count("--pgc-expect-tests"), 2, - "both pytest jobs pass the flag") - expect.num(text.count("expected_tests.txt"), 2, - "and both read the number from the tracked file rather than stating it") - # An empty read would OMIT the flag and fail open, so the value is guarded in the job. - expect.num(text.count('test -n "$WANT"'), 2, - "and each guards the read, because an empty value would fail open") + # BOTH jobs, not one, and EACH ONE IN ITS OWN CODE (#1339 :1007). A count over the + # whole file passed with the guard job's flag moved into a comment, because the + # comment kept the total at two. + for job_name in ("pytest-guards", "pytest-cluster"): + code = "\n".join(_code_lines(_ci_job(text, job_name))) + expect.at_least(code.count("--pgc-expect-tests"), 1, + f"the {job_name} job passes the flag") + expect.at_least(code.count("expected_tests.txt"), 1, + f"the {job_name} job reads the number from the tracked file") + expect.at_least(code.count('test -n "$WANT"'), 1, + f"the {job_name} job guards the read, because an empty value would fail open") # A TRACKED KEY-VALUE FILE MUST NOT REPEAT A KEY, and the arm above could not see one. @@ -1213,7 +1224,7 @@ def installs(text): for line in ("pip install --quiet $PINS " + DRIVER + "[binary]==3.3.5", "pip install --quiet -r test/pytest/requirements-test.txt"): expect.num(len(installs(job + "\n " + line + "\n")), 1, - f"premise: and that test sees `{line.split()[3]}` when it is there") + f"control: and that test sees `{line.split()[3]}` when it is there") # NEGATED, in this job. `pip show psycopg` without the `!` asserts PRESENCE, which # is what the cluster job's line does, and a count could not tell them apart. absent = [l for l in _code_lines(job) diff --git a/test/pytest/test_layer.py b/test/pytest/test_layer.py index a6783953f..b486e3c15 100644 --- a/test/pytest/test_layer.py +++ b/test/pytest/test_layer.py @@ -843,7 +843,9 @@ def visit_Call(self, node): and isinstance(target.value, ast.Name) and target.value.id in ("expect", "e")) if is_expect: - for arg in node.args: + # Keyword values too (#1339 :837): `expect.num(got=int(a != b), ...)` + # is the same collapse, and reading only positional arguments missed it. + for arg in list(node.args) + [k.value for k in node.keywords]: if (isinstance(arg, ast.Call) and isinstance(arg.func, ast.Name) and arg.func.id == "int" @@ -869,6 +871,12 @@ def test_the_inequality_scan_finds_a_planted_offence(expect): "def test_y(expect):\n" " expect.at_least(int(a != b), 1, 'moved')\n")), 1, "and finds it through at_least too") + # PASSED BY KEYWORD (#1339 :837). The scan read positional arguments only, so + # `got=int(a != b)` was invisible to it and to the population arm below. + expect.num(len(_hand_rolled_inequalities( + "def test_k(expect):\n" + " expect.num(got=int(a != b), want=1, name='moved')\n")), 1, + "and finds it when passed by keyword") # THE INVERTED SPELLING, which is the one the manual count missed. The third site # wrote `int(stated == disk) == 0` -- the same assertion with the comparison # flipped -- so a scan that looked only for `!=` would have left it in place and @@ -1019,7 +1027,7 @@ def test_order_collapsed(expect): """ ) plain = pytester.runpytest("-p", "pgc_vacuity") - expect.run_failed(plain, "premise: the collapse is refused when nobody rebinds") + expect.run_failed(plain, "the collapse is refused when nobody rebinds") plain.stderr.fnmatch_lines(["*order-killed*"]) pytester.makeconftest(_rebind("_sorted_ordered_sites")) @@ -1043,7 +1051,7 @@ def test_broad(expect): """ ) plain = pytester.runpytest("-p", "pgc_vacuity") - expect.run_failed(plain, "premise: the broad except is refused") + expect.run_failed(plain, "the broad except is refused") plain.stderr.fnmatch_lines(["*catches Exception broadly*"]) pytester.makeconftest(_rebind("_broad_except_sites")) @@ -1073,7 +1081,7 @@ def test_raises_compound(expect): """ ) plain = pytester.runpytest("-p", "pgc_vacuity") - expect.run_failed(plain, "premise: an unpinned compound raises block is refused") + expect.run_failed(plain, "an unpinned compound raises block is refused") plain.stderr.fnmatch_lines(["*not pinned*"]) pytester.makeconftest(_rebind("_raises_sites")) @@ -1104,7 +1112,7 @@ def test_one_equals_two(expect): """ ) plain = pytester.runpytest("-p", "pgc_vacuity") - expect.outcomes(plain, "premise: a false claim fails when nobody stubs", + expect.outcomes(plain, "a false claim fails when nobody stubs", failed=1, passed=0) import pgc_vacuity @@ -1172,7 +1180,7 @@ def test_honest(expect): """ ) plain = pytester.runpytest("-p", "pgc_vacuity") - expect.outcomes(plain, "premise: an honest test passes", passed=1, failed=0) + expect.outcomes(plain, "an honest test passes", passed=1, failed=0) pytester.makeconftest(_rebind("_broad_except_sites")) hatched = pytester.runpytest("-p", "pgc_vacuity")