Skip to content

test(const): fix the stale StrEnum docstring and rename EXCLUDED_STILL_BESPOKE - #894

Merged
JarryShaw merged 1 commit into
mainfrom
fix/892-excluded-still-bespoke-rename
Sep 29, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/892-excluded-still-bespoke-rename

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • test — tests only

Description of your pull request and other information

Closes #892.

Two stale-prose fixes in tests/const/test_const_registry_protocol.py, no behaviour change:

  1. test_default_still_resolves_when_registered_str's docstring called its case "synthetic, since no shipped aenum.StrEnum registry mixes in the base yet". Measured independently by walking pcapkit.const for EnumRegistry+str subclasses: ten already do (Command, FEATCode, Method, OptionType, TLSKeyLabel, AppType and its four transport subclasses). Rewrote the docstring to state that instead.

  2. EXCLUDED_STILL_BESPOKE's comment described "overrides the base" plus TransportProtocol for never mixing in at all — but most entries do mix in EnumRegistry. Renamed to EXCLUDED_FROM_INTENUM_CENSUS and re-grouped its eleven entries by which of three reasons actually applies (not an IntEnum; overrides the base; never mixes in the base — apptype.py carries two of the three in one file). return_code.py/status_code.py's actual override is a custom __new__ plus _unregistered_member, not get()/register(). Updated all four other references (one behavioural, three prose).

all_files/generated/exclusion counts (122/105/11, 6+11+105=122) are unchanged — this branch is now rebased onto #890 (parent commit), which added pcapkit/const/pcapng/tls_key_label.py and is what brought the census from nine to ten and the exclusion set from ten to eleven entries.

@JarryShaw JarryShaw added test Pull requests that add or correct tests (test: subject prefix) bug Issues reporting a defect (set by the bug report template; a default, not an assessment) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 29, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Resolve conflicts.

@JarryShaw
JarryShaw force-pushed the fix/892-excluded-still-bespoke-rename branch 2 times, most recently from ac895ba to 0ef615e Compare September 29, 2026 01:59
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO on 0ef615e7d — cross-review (opus; author sonnet). Its one blocking finding is fixed and I verified the corrected claim by method-resolution introspection rather than from the report:

                     get   register  _unregistered_member
ReturnCode           base    base     own
StatusCode           base    base     own
ResponseKind         base    base     base
GroupingInformation  base    base     base
FEATCode             base    base     base

So the old comment was wrong in both directions: get/get_all/register/register_alias are never overridden in those files, and the real override is _unregistered_member. The corrected text now says exactly that, and separates ReturnCode from ResponseKind/GroupingInformation, which are clean and excluded only as co-residents of return_code.py. Source confirms the __new__ half too — ReturnCode:95 and StatusCode:33 define one; ResponseKind:36, GroupingInformation:60 and FEATCode:26 do not.

One instrument error of mine worth recording, because it nearly produced a false contradiction. My first check used '__new__' in vars(cls) and reported True for all five, which would have made "are clean" wrong. Control: pcapkit.const.mh.packet.Packet, a plain generated registry with no source __new__, also reports True — the enum metaclass installs one on every enum class, so vars() cannot answer this question at all. The source grep can, and does.

The second item is fixed as well: the StrEnumValueFallbackTests sentence at :1176-1180 no longer claims those files are excluded for "bespoke __new__/get() shapes" — it now says they are StrEnum rather than IntEnum, "which is reason (a) there regardless of whether any one of them also overrides the base". That resolves the self-contradiction with the group (a) label, and stops being false about FEATCode.

Both non-blocking notes were folded in rather than deferred: :715-725 now states precisely which classes keep something bespoke (Command, Method, OptionType, plus ReturnCode/StatusCode for _unregistered_member only) against which resolve everything to the base (FEATCode, ResponseKind, GroupingInformation), and the apptype.py tag notes that AppType overrides all four more fully than either file actually tagged (b). The stale post-rebase PR description is updated — eleven entries, 122/105/11, 6 + 11 + 105 = 122, and the "pre-#890" framing dropped now that #890 is its parent.

Still one commit, one file, 74/74 and 33/33 green, and the earlier verification stands: the rebase lost nothing of #890's, every count re-derives, and the set's single operative use is the rename only.

Unpublished and unmerged, yours to merge — it closes #892.

@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 Sep 29, 2026
…L_BESPOKE

- test_default_still_resolves_when_registered_str called its str-valued
  case "synthetic, since no shipped aenum.StrEnum registry mixes in the
  base yet". Measured on main (walking pcapkit.const, filtering for
  EnumRegistry+str subclasses): ten already do (Command, FEATCode,
  Method, OptionType, TLSKeyLabel, AppType and its four transport
  subclasses). Rewrote the docstring to say so instead of the stale claim.
- EXCLUDED_STILL_BESPOKE described its members as overriding the base's
  generic implementation, plus TransportProtocol for never mixing in the
  base at all. In fact most entries do mix in EnumRegistry -- the set's
  real, operative meaning is "outside this file's IntEnum-only historical
  census, anchored at commit 05468a0". Renamed to
  EXCLUDED_FROM_INTENUM_CENSUS and re-grouped its eleven entries by which
  of three reasons applies: not an IntEnum, overrides the base, or never
  mixes in the base (apptype.py carries two of the three, in one file).
  return_code.py/status_code.py's actual override is a custom __new__ plus
  _unregistered_member, not get()/register() -- their get/get_all/
  register/register_alias all resolve to the base's own implementation;
  ResponseKind and GroupingInformation, co-resident in return_code.py,
  override nothing at all. Updated the other four EXCLUDED_STILL_BESPOKE
  references (one behavioural, three prose) to the new name.
- No behaviour change: the set's file-membership, and the 6/11/105/122
  counts it feeds, are unchanged (122/11/105 following #890's merge,
  which added pcapkit/const/pcapng/tls_key_label.py to both).

Build/test: tests/const/test_const_registry_protocol.py (run=74,
failures=0, errors=0) and tests/const/test_const_enum_builtin_parity.py
(run=33, failures=0, errors=0), both via the worktree's own pcapkit.
@JarryShaw
JarryShaw force-pushed the fix/892-excluded-still-bespoke-rename branch from 0ef615e to 098cb51 Compare September 29, 2026 02:19
@JarryShaw

Copy link
Copy Markdown
Owner Author

Rebased onto current main (ea5d34dc6) — 0ef615e7d → 098cb5120, still exactly one commit.

Needed because the main ruleset sets strict_required_status_checks_policy=true, so a BEHIND branch cannot merge until it is current. #893 merging as 486c2a269 put this branch behind.

The review: good-to-go verdict carries to the new head, and here is why that is not a rubber stamp. Before the rebase I merged 0ef615e7d into current main in a scratch worktree and ran the file it touches:

merge exit=0, 'ort' strategy, no conflicts
tests/const/test_const_registry_protocol.py | 130 +++++++++++++++++++---------
Ran 74 tests in 63.992s -- OK
TREE /tmp/verify894-b/pcapkit/__init__.py

The rebase produces the same single-commit diffstat (88 insertions, 42 deletions, same one file), so the tree I tested and the tree now on the branch are the same content. CI is re-running on 098cb5120; I will confirm it lands green before calling this ready.

@JarryShaw
JarryShaw merged commit 1b0832b into main Sep 29, 2026
63 checks passed
@JarryShaw
JarryShaw deleted the fix/892-excluded-still-bespoke-rename branch September 29, 2026 02:45
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Sep 29, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) test Pull requests that add or correct tests (test: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

test(const): the registry-protocol census calls a shipped shape synthetic, and EXCLUDED_STILL_BESPOKE no longer means what it says

1 participant