test(const): key the manufactured-name exemption on the name shape, not on file paths

[JarryShaw]

Please follow the guide below

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible -- N/A, changelog centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

• test — adds or corrects a test

Description of your pull request and other information

Closes #879. No longer stacked — it was cut on #878's commit f210ddc71, which was squash-merged into main as 6102bf43f (a different sha), leaving the pre-squash commit on the branch and the PR CONFLICTING. Rebased with git rebase --onto origin/main f210ddc71, so the base is now main and this is one commit on top of it. The replay is tree-identical to the reviewed sha: 8d207553c^{tree} and 933e998a8^{tree} are both 093c5ac1e.

#878 exempted two files from test_const_enum_no_mint.py's is_manufactured() sweep by path, so the preserved hex-suffixed names would pass. That made the exemption shape-blind inside those files: rewriting a branch to 'Xyplex_%d' % value — exactly the value-derived shape the predicate exists to flag — left the sweep green, because exempt_hits stayed at 53.

This keys the exemption on the name-expression shape instead, and drops HEX_SUFFIXED_NAME_EXEMPT_PATHS entirely:

_HEX_SUFFIX_SHAPE = ast.parse('hex(value)[2:].upper().zfill(4)', mode='eval').body
# _is_hex_suffixed_unregistered_name: a %-BinOp whose left is a str constant ending
# in '_0x%s' and whose right is structurally identical to _HEX_SUFFIX_SHAPE
return ast.dump(right) == ast.dump(_HEX_SUFFIX_SHAPE)

An AST structural comparison rather than a source-text match, because the sweep already holds the parsed name_arg node at the point of the check — so it needs no per-prefix pattern (there are 53 distinct prefixes, Xyplex_ through Registered by Xerox_) and is immune to quote-style or whitespace drift in generated source. Every one of the 53 shares that suffix verbatim and no other call site in pcapkit/const/ contains 0x%s at all, so the match hits exactly the intended 53.

The mutation that used to pass now fails — verified independently of the author, by mutating const/reg/ethertype.py:540 in a scratch worktree after asserting the line's exact content:

mutated to: return cls._unregistered_member(value, 'Xyplex_%d' % value)
UnregisteredMemberNameIsBareTests -> run=1 failures=1
AssertionError: 52 != 53 : expected exactly 53 hex-suffixed-name exemptions to fire, found 52

Also proved the other two directions: unmutated the file is 69/0/0, and a value-derived name in an unrelated file is still flagged — mutating const/arp/hardware.py:163 to 'Unassigned_%d' % value fails the sweep with that path named. The exact exempt_hits == 53 bound is kept, so adding or removing a branch still forces a human look.

One commit, one file (git diff --name-only f210ddc71 df62008ce → tests/const/test_const_enum_no_mint.py), nothing under pcapkit/ or docs/.

[JarryShaw]

CI cannot run on this PR while it is stacked, and that is a property of the workflows rather than a problem with the branch. Measured:

.github/workflows/unit-tests.yml:6-7            pull_request:  branches: [main]
.github/workflows/python-compatibility.yml:6-7  pull_request:  branches: [main]
gh api repos/.../commits/df62008ce/check-runs   check_runs=0

Both suites are filtered to pull_request against main, so a PR based on fix/775-ethertype-socket-no-mint triggers nothing. Read ok=0 fail=0 inc=0 here as "not run", not as green. The workflows will fire once #878 merges and GitHub retargets this to main; I will not report it ready to merge before that happens and the checks come back clean.

In the meantime the change is covered locally: tests/const/test_const_enum_no_mint.py is 69/0/0 on this head, and the mutation that motivated the issue now fails with 52 != 53.

[JarryShaw]

GOOD TO GO on df62008ce — cross-review (opus; author sonnet). One finding worth stating precisely, non-blocking.

The shape predicate on its own is wider than the path one was, and the count assertion is what makes the exemption safe. I planted a shape-matching name with an unsanctioned prefix in a file that was never meant to be exempt:

pcapkit/const/arp/operation.py:116 -> return cls._unregistered_member(value, 'Totally_Bogus_0x%s' % hex(value)[2:].upper().zfill(4))
run=1 failures=1   AssertionError: 54 != 53 : expected exactly 53 hex-suffixed-name exemptions to fire, found 54

_is_hex_suffixed_unregistered_name exempts it; the test fails through assertEqual(exempt_hits, 53) instead. So the old path version was narrow on "right shape, wrong file" and blind on "right file, wrong shape"; this inverts that. Since #879's hole was the second one, the trade is correct — and in both directions the test still fails. Worth tightening one sentence of prose rather than any code: the exemption docstring's claim that it "cannot be tricked into covering a differently-shaped manufactured name" is true in that direction only, and does not mean a correctly-shaped name in a wrong file is rejected by the predicate.

Nothing narrow about it in practice, measured. All 53 sites share one distinct right-operand ast.dump, across 49 distinct prefixes — which is exactly why a prefix or source-text match would have been the wrong tool. Corroborated by text search: grep -rn "0x%s" pcapkit/const → 53 (1 in ipx/socket.py, 52 in reg/ethertype.py); zfill(N) → 53 × zfill(4), no other width; no .lower() variant anywhere. call_count is 1065, matching the floor the PR raised it to.

ast.dump does what the change depends on. On 3.14.7 positions are excluded by default and include_attributes=True would make the two operands differ, so the claim is load-bearing and correct. Discriminates zfill(2), .lower(), [2:None], hex(key), [3:]; correctly ignores parentheses, a trailing comma, and line continuations.

All four briefed mutations fail, and failing is right in each: zfill(4)→zfill(2), .upper()→.lower(), %-format→f-string of identical output, and a manufactured name in an unrelated file (that last one names the path in the offenders list). A legitimate future generator change would look exactly like (i)-(iii), and a red test forcing a human to re-bless the exemption is the intended behaviour — that is precisely what #879 complained was missing.

Scope and hygiene: one commit, one file, +56/−22, author and committer both Jarry Shaw <jarryshaw@icloud.com>; 69/0/0 on both this head and f210ddc71, so no test was added or lost — right for a pure re-keying. HEX_SUFFIXED_NAME_EXEMPT_PATHS and the orphaned rel_path are gone repo-wide; repo_root is still used and correctly kept.

Diagnosability nit, pre-existing and unchanged here: for shape-drift mutations assertEqual(exempt_hits, 53) runs before the offenders list is asserted, so the message reports the count and never prints the drifted site. Same ordering at f210ddc71:1313.

Reminder for whoever merges: this is stacked on #878 and has no CI until that lands — ok=0 on this head means not run.

[JarryShaw]

Merge conflicts.

[JarryShaw]

Resolved — you caught it about a minute before the fix landed. Head is now 933e998a8, and GitHub reads MERGEABLE.

The cause was not this change. #881's branch was cut on top of #878's commit f210ddc71, and #878 was squash-merged into main as 6102bf43f — a different sha. So the pre-squash commit stayed on the branch and git saw two commits where one was already merged under another identity:

$ git log --oneline origin/main..origin/fix/879-shape-based-exemption   # before
8d207553c test(const): key the manufactured-name exemption on the name shape, not on file paths
f210ddc71 fix(const,vendor): stop EtherType and Socket minting on an unrecognised value   <- already in main as 6102bf43f

Fixed with git rebase --onto origin/main f210ddc71, which replays only my own commit and drops the merged one. It applied with no conflict, and the result is tree-identical to what was reviewed — git diff 8d207553c 933e998a8 is empty, so the replay introduced nothing.

Now: one commit, one file (tests/const/test_const_enum_no_mint.py), +59/−22, authored as Jarry Shaw <jarryshaw@icloud.com>.

I have pulled the review: good-to-go label even though the tree did not change, because the head sha has moved twice since that verdict was given and a verdict pinned to a superseded head is worse than none. The same opus reviewer is re-checking 933e998a8 — including the docstring rewrite it asked for, which is the one real content change since it last looked. CI is re-running on the new head: 16 green, 0 failed, 42 still in flight. I will report when both land; it stays unpublished and unmerged for you either way.

[JarryShaw]

GOOD TO GO on 933e998a8 — cross-review (opus; author sonnet), re-run after the docstring round and the rebase.

It re-derived both numbers itself rather than taking mine: call_count = 1065, exempt_hits = 53, offenders = 0, whole file Ran 69 tests … OK, against a printed pcapkit.__file__ under its own export tree. It also re-ran the mutation battery on this head — zfill2 → 52 != 53, lower → 52 != 53, a manufactured name planted elsewhere → offenders list non-empty, a correctly-shaped name planted in an unintended file → 54 != 53. So the predicate still has teeth after the replay.

On tree-identity it did better than my check: git rev-parse 8d207553c^{tree} 933e998a8^{tree} gives the same tree object 093c5ac1e both times, so the diff is empty necessarily rather than coincidentally. It also found the old and new bases content-identical (f210ddc71^{tree} == 6102bf43f^{tree}), which is what makes the squash-rebase safe here.

And it corrected me on a point of attribution, which I have verified and accept. I had described the call_count floor as "raised from 1012 to 1065" as if #881 did that. It did not. Measured:

edc1b32e0 (main before #878):  assertGreaterEqual(call_count, 1012   <- and NO exempt_hits assertion
6102bf43f (main, #878 merged): assertGreaterEqual(call_count, 1065 / assertEqual(exempt_hits, 53
933e998a8 (#881 head):         both unchanged; the diff touches only the docstring line that quotes one

Both the raise to 1065 and the introduction of assertEqual(exempt_hits, 53) are #878's work and were already on main before #881 existed. They stay load-bearing for #881 — the shape predicate has to hold exempt_hits at exactly 53, and it does — but a wrong value there would have been #878's to answer for, not this PR's.

Two inherited nits it flagged, neither blocking and neither introduced here: the module docstring calls _is_hex_suffixed_unregistered_name a member of UnregisteredMemberNameIsBareTests when it is a module-level function, and the class docstring says "below" of something defined above — #878's wording made the same two errors about the constant, and #881 carried the sentence shape across. Also worth knowing: measured call_count is exactly 1065 against a >= floor, so there is zero slack and removing any single _unregistered_member call site trips the floor as well as the count.

CI on this head: 53 green, 0 failed, 3 skipped, 5 still in flight. Unpublished and unmerged, yours to merge.