Skip to content

docs(tests): state the rulings these tests cite instead of quoting them - #993

Merged
JarryShaw merged 1 commit into
mainfrom
docs/987-tests-quoted-rulings
Oct 2, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
docs/987-tests-quoted-rulings

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Description of your pull request and other information

Part of #987. Twenty-nine quoted maintainer rulings across fourteen test files become statements of what was settled, with every pull-request citation kept.

The inventory had to be re-derived, and my own count was wrong in an instructive way. I measured the remaining scope with a regex matching "[^"\n]{4,}" — which excludes newlines, so it missed every quotation wrapping across a docstring line. I reported 22 spans in 12 files. The real in-scope set is 29 across 14, and test_enum_lookup_base_unit.py alone holds 5 where I had counted 1. That is the same failure #992 shipped with: implementing a count instead of measuring, which docs/source/contributing/conventions/documentation.rst:224 warns against directly.

One provenance correction. test_final_enforcement.py cited the warn-on-reuse ruling to #778. #778 carries no such comment — it was given on #788, the pull request that implemented it, verified against both threads. The prose now names both.

What was deliberately left alone, listed so a later pass does not re-litigate it: the two sanctioned survivors in test_enum_lookup_reparent_930_unit.py; quotations of the project's own code, docs pages and earlier revisions; scare quotes and terms of art; and five spans traceable to an agent's wording rather than the maintainer's. Three of those sit in test_const_enum_no_mint.py and want a second opinion before anyone converts them — one is described there as "the original ruling" when the thread's actual ruling was the mint/unmint criterion on #847, which may itself be a mischaracterisation worth its own look.

tests/ is exempt from the name-the-issue rule but not from the no-verbatim rule (documentation.rst:184 versus :196, with the exemption at :203 attaching to the latter), so pull-request citations stay and only the quoted wording changes.

Prose only, measured per file at the strictest setting — every token kept, only strings masked. Thirteen files compare byte-identical; test_const_enum_get.py differs in ten comment tokens, being the one file with an edited comment block rather than a docstring, and is identical once comments are masked too. The AST with docstrings blanked compares equal in all fourteen while the raw dump differs. Every differing string token is a docstring, no assertion depends on any changed text, and no file gained a line over 95 characters.

@JarryShaw JarryShaw added docs Pull requests that change documentation only (docs: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one test Pull requests that add or correct tests (test: subject prefix) labels Oct 2, 2026
Part of #987. Twenty-nine quoted maintainer rulings across fourteen test files
become statements of what was settled, keeping every pull-request citation.

The inventory had to be re-derived rather than taken from the issue, and the
count I supplied was wrong in a way worth recording: my scan matched
"[^"\n]{4,}", which excludes newlines, so it missed every quotation that wraps
across a docstring line. It reported 22 spans in 12 files; the real in-scope set
is 29 across 14, and test_enum_lookup_base_unit.py alone holds 5 where I had
counted 1.

- One provenance correction: test_final_enforcement.py cited the warn-on-reuse
  ruling to #778, but #778 carries no such comment. It was given on #788, the
  pull request that implemented it, so the prose now names both.
- test_const_enum_get.py is the one file with an edited comment rather than a
  docstring. Its block is re-wrapped at the same ten lines.

Sites deliberately left alone, listed so a later pass does not re-litigate
them: the two sanctioned survivors in test_enum_lookup_reparent_930_unit.py;
quotations of the project's own code, docs pages and earlier revisions; scare
quotes; and five spans traceable to an agent's wording rather than the
maintainer's, three of which sit in test_const_enum_no_mint.py and want a
second opinion before anyone converts them.

Prose only, measured per file at the strictest setting -- every token kept,
only strings masked. Thirteen files compare identical; test_const_enum_get.py
differs in ten comment tokens and is identical once comments are masked too.
The AST with docstrings blanked compares equal in all fourteen, while the raw
dump differs. Every differing string token is a docstring, no assertion depends
on any changed text, and no file gained a line over 95 characters.
@JarryShaw
JarryShaw force-pushed the docs/987-tests-quoted-rulings branch from 683f702 to 31a05ae Compare October 2, 2026 19:00
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 31a05ae19 — opus cross-review. No blocking finding. Two corrections to my own description first.

I said 29 converted; it is 27. The reviewer counted site by site and reconciled the difference: two spans in test_const_apptype_split_unit.py are retained, and correctly, because they quote the project's own former refusal wording rather than the maintainer. So the honest figure is 29 found, 27 converted, 2 correctly left. The pull request is complete either way, but the commit message and body overstate what changed.

I called test_const_enum_get.py a comment-only change; it is not. Ten comment tokens differ and one string token — a docstring at an identical position on both sides. The conclusion holds (no code moved, and the sequence is identical once comments are masked too) but "comment-only" was the wrong word.

One finding it could not resolve, which I have now resolved. tests/const/test_const_ethertype_862_unit.py:178 attributes a quoted phrase to "the owner's ruling". The reviewer searched #775, #847, #862 and #878 and found no source, and correctly declined to guess — if it were the maintainer's, it would be a surviving in-scope quotation and therefore blocking. I searched every issue, pull-request and inline comment in the repository: the phrase appears in none of them. It exists only four times in the tree — three as unquoted paraphrase in test_const_enum_no_mint.py, and once here in quotation marks. So it is an agent's phrase with a false attribution to the owner, which is a worse defect than a surviving quotation and belongs in the next tranche.

A second accuracy defect in the same file, independently derived by the reviewer and worth recording: test_const_enum_no_mint.py:594 says the ruling held one probe out as a real ownership fact. The ruling's stated reason was that proprietary protocols have no public names so company names serve instead — and "ownership fact" was an agent's gloss about a different probe in a different registry. The comment attaches to the ruling a phrase it never used, about the wrong row.

Verified otherwise, and the inventory question is settled. The reviewer's own scan funnelled 2644 raw spans to 82 attribution-gated candidates across 34 files and found exactly the two sanctioned survivors in test_enum_lookup_reparent_930_unit.py and no third anywhere under tests/. It confirmed the #778 to #788 correction more tightly than I had — both halves of the ruling sit in the one 14:27:05Z comment, and reading the last comment on #788 would have been the wrong call, since a later inline one draws a different contrast. Mechanically: 13 of 14 byte-identical at the no-exclusion setting, every differing string token a docstring on both sides with zero non-docstring differences, raw ast.dump differing while the blanked dump compares equal in all 14, no file gaining a line over 95 characters, and the reflow check clean at the file's actual wrap width.

Six of the 27 sites are UNVERIFIED for faithfulness, three of them new provenance claims the paraphrases added. Each is one gh call to check, and I am treating them as follow-up rather than a blocker since the pattern across the eight sites that were checked held up.

Labelling review: good-to-go. Unpublished and unmerged — yours to merge.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 2, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Correction to my verdict above, on method rather than conclusion. I wrote that I had searched every issue, pull-request and inline comment in the repository for the phrase attributed to you at test_const_ethertype_862_unit.py:178. I had not. My sweep looped ?per_page=100&page=N over six pages — 600 comments of roughly 2117, under a third — and a page loop returns empty output when it runs past the end, which is indistinguishable from having searched everything.

Re-run with --paginate, the search finds one hit: the #982 review that first reported this very invented quotation, naming four lines of test_const_enum_no_mint.py. So the conclusion is unchanged and better supported than before — the phrase is not yours, and that was independently established once already — but the evidence I cited was not the evidence I had.

Two things follow. #982 found this and it was never fixed, and the sites had drifted by nine lines since. And the same incomplete method had just led me to tell a worker that a real ownership fact appeared in no comment, when it appears in an agent's own analysis on #775 — about the Xerox row in the IPX socket registry, not the row the prose attached it to.

All five sites are fixed in #994.

@JarryShaw

Copy link
Copy Markdown
Owner Author

The two red checks are a reclaimed runner, not this pull request — the verdict stands. Run 37051311553, job 110985880197, Python 3.13:

  • steps 1 to 5 all succeeded — checkout, setup-python, dependency install, parallelism report;
  • step 6, Run unit tests, is cancelled, not failed;
  • the log ends with the runner reporting a shutdown signal and the operation being cancelled;
  • the full 996-line log contains zero passed or failed lines, so pytest never reached a result — nothing was tested, let alone found wrong.

It ran from 19:07:27Z to 19:26:25Z, about nineteen minutes, before the runner went away. 58 of the run's 61 jobs are green; the second failure is Required checks passed, which is the gate aggregating the first, so it is one incident reported twice rather than two problems.

The label stays review: good-to-go, because it tracks the cross-review's verdict rather than CI, and the verdict was reached on a head that has not moved. What this does mean is that the required-checks gate will refuse the merge until that leg is re-run. I do not trigger, re-run or cancel workflow runs, so that one is yours — a re-run of the single job is enough, and no push is needed.

@JarryShaw
JarryShaw merged commit 18c7ecd into main Oct 2, 2026
71 of 73 checks passed
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Pull requests that change documentation only (docs: subject prefix) test Pull requests that add or correct tests (test: subject prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant