Skip to content

test: map mypy in MODULE_PROVIDERS and name the module an unmapped lookup wanted - #782

Merged
JarryShaw merged 1 commit into
mainfrom
fix-779-mypy-gate-and-provider-diagnostic
Sep 25, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix-779-mypy-gate-and-provider-diagnostic

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • 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

Closes #779. Tests only; no pcapkit/ file is touched.

Re-derived first. Adding HAS_MYPY + skipUnless to tests/vendor/test_vendor_reg_apptype_generator_unit.py alone gives Ran 10 tests ... FAILED (failures=2, errors=5), all 5 errors a bare KeyError: 'mypy' from extras_providing. #779 says 9 tests / 4 errors because it was written against 110381b63, before #774 added test_the_vendor_extra_closes_engine_tests_but_not_test_or_gate — which is both the tenth test and the fifth error. Two numbers, one cause.

Change. MODULE_PROVIDERS['mypy'] = ('mypy',); a new DEPENDENCY_GATE_EXCLUSIONS['HAS_MYPY'] dark on test, engine-tests and gate; all three bare MODULE_PROVIDERS[...] subscripts routed through _top_level_providers(), which raises an AssertionError naming the module, the top-level key looked up, and the table to add it to (the shape _disqualified_providers already used). Mapped modules return the same tuple as before. The gate itself moves from an inline try/except ImportError: self.skipTest(...) — invisible because _gates_of() walks a decorator list, never a body, the #745 hazard #766 also carries — to the visible form, and the module docstring is rewritten, since it argued for the form being replaced.

Exclusion rationale. No pyproject.toml extra carries mypy (verified: the only such entry in the table); it is a Pipfile [dev-packages] entry that lint.yml installs directly. The fix this guard would otherwise propose — put mypy on a pytest install line — is wrong: it duplicates a whole-package check lint.yml already runs, and lint.yml runs no pytest, so pytest_jobs() never sees the job that does install it. So the gap is explained, not closed.

Gap report. 15 → 18 (flag, job) pairs; the three new rows are HAS_MYPY on test, engine-tests and gate. Not identical, and that is the point — the skip already happened on those legs, the guard just could not count it. integration and pypcap-parity select by fixture tier and never collect the module, so they are absent, exactly as for HAS_VENDOR_DEPS.

Counts. tests/test_tier_guard.py 97 → 100 tests, 512 → 524 subtests, measured under plain unittest with a subTest counter rather than trusting pytest-subtests; pytest agrees (100 passed / 524 subtests). The +12 is 5+3+0 from the three new tests, +1 each for one more gate, one more exclusion ×2, and one more gate-bearing module. Each new test was shown failing against its own mutation: entry deleted, helper reverted to the bare subscript, gate hidden from the scan. Coverage of tests/_dependency_gates.py holds at 99% with all 6 added statements executed, and the vendor module rises 88% → 91% as the now-dead except ImportError arm goes.

…okup wanted

`mypy` had no `MODULE_PROVIDERS` entry, so a `@unittest.skipUnless` gate on it
was impossible rather than merely undesirable, and the guard's response to an
unrecognised module was a bare `KeyError` from a dict subscript.

* Add `MODULE_PROVIDERS['mypy'] = ('mypy',)`. No pyproject.toml extra carries
  it -- it is a Pipfile `[dev-packages]` entry that lint.yml installs directly
  -- so it resolves to the empty set and becomes a real, countable gap instead
  of a missing key.
* Add `DEPENDENCY_GATE_EXCLUSIONS['HAS_MYPY']`, dark on `test`, `engine-tests`
  and `gate`: a type checker belongs to the lint tier, so closing this gap by
  putting mypy on a pytest install line would be the wrong fix.
* Route all three bare `MODULE_PROVIDERS[...]` subscripts through a new
  `_top_level_providers()`, which raises an `AssertionError` naming the module,
  the top-level key looked up, and the table to add it to. Mapped modules
  resolve to the same tuple as before.
* Convert `test_vendor_reg_apptype_generator_unit.py`'s mypy gate from an
  inline `try`/`except ImportError: self.skipTest(...)` to the visible
  `HAS_MYPY` + `skipUnless` form, which `_gates_of()` can see; update the
  module docstring to match, since it argued for the form being replaced.
* Three new tests, each shown failing against its own mutation.

`tests/test_tier_guard.py` 100 passed / 524 subtests (was 97 / 512); gap report
15 -> 18 pairs, the three new ones all HAS_MYPY.
@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 25, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review at 27d91a5a6 (sonnet, a different model from the author): GOOD TO GO, no required changes. Every load-bearing number reproduced exactly, and the tree was left clean after every probe.

The gap report is right, and for the right reason. dependency_gate_gaps() re-derived on both trees: 15 → 18 (flag, job) pairs, the three new rows all HAS_MYPY with missing=('mypy',) gates=1 on test/engine-tests/gate. The design question I asked — whether "gap classified, not closed" is the right shape for a lint-tier tool — is answered by precedent rather than opinion: HAS_SCAPY, HAS_PYSHARK and HAS_RUNTIME all use the identical Gap + Exclusion(dark=...) pattern for a deliberately-unclosed gap. Both halves of the exclusion's rationale check out too: .github/workflows/lint.yml:187 really is pip install -U vermin pylint mypy bandit, and grep -in pytest on that file returns nothing, so pytest_jobs() genuinely never sees the job that installs mypy. Pipfile:48 is mypy = "*", and pyproject.toml's only mypy hit is the substring inside mypy-extensions.

The visible-gate conversion is real, not cosmetic — which was the risk. gated_scopes() now reports exactly one new Gate(flag='HAS_MYPY', module='tests/vendor/…', lineno=222, func_name='test_undefined_member_infers_as_transport_protocol_under_mypy'), so _gates_of's decorator_list walk does see it.

The +12 subtest accounting was attributed line by line, not spot-checked: +5 / +3 / +0 from the three new tests, then test_every_gated_flag_is_classified 249→250, both exclusion tests 7→8, and test_the_suite_scan_only_looks_at_modules_pytest_collects 114→115. Sums to 12. 97→100 tests, 512→524 subtests under plain unittest with a counting TestResult, and pytest agrees at 100 passed / 524 subtests.

It settled the thing you could not. Monkeypatching importlib.util.find_spec to return None for 'mypy' in a throwaway subprocess, before importing the vendor module: HAS_MYPY reads False and the gated test gives OK (skipped=1) with reason 'mypy is not installed'. The venv was never touched.

And it corrected my own brief. I asked whether AssertionError mattered under python -O. It does not: -O strips bare assert statements, not an explicit raise AssertionError(...). I verified that myself — under -O, the explicit raise is caught normally while a bare assert False vanishes. My concern did not apply.

_top_level_providers survived edge-case attack: dotted-unmapped, empty string, and the one genuinely dotted key (pcap._pcap) all behave correctly, truncation matches the old bare-subscript behaviour at those three call sites exactly, and mapped modules return identical tuples either side. Mutations reproduced: deleting MODULE_PROVIDERS['mypy'] breaks 8 tests in the class; reverting extras_providing to a bare subscript gives KeyError: 'nosuchlinter'. Coverage confirmed at 418→424 statements, misses unchanged at 2, vendor module 88% → 91% as the dead except ImportError arm goes.

Four CI legs still outstanding (23 success, 3 expected skips, 0 failures), so this is a review verdict rather than a merge signal.

@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 25, 2026
@JarryShaw
JarryShaw merged commit 0419c1c into main Sep 25, 2026
31 checks passed
@JarryShaw
JarryShaw deleted the fix-779-mypy-gate-and-provider-diagnostic branch September 25, 2026 13:17
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Sep 25, 2026
JarryShaw added a commit that referenced this pull request Sep 26, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit
(relative to its parent, 4233555) now names all 20 bullets it carries, not
just the 8 this session added on top of the 10 already there -- a first
draft of this message named only its own 8 and left the other 10 silent,
which a cross-review caught. Four are breaking:

- #754 -- AppType split into per-transport registries; the 1,004 portless
  and 704 transportless rows stop being members.
- #764 -- an out-of-range port in `AppType.get` is refused, not minted.
- #778 -- `@final` enforced at runtime on `Info`/`Schema`.
- #575 -- four `.get()`-backed enum fields fall through to
  `_unregistered_member` instead of minting; 14 of 23 sample captures
  change output.
- #772 (with #790's docstring reword), #766, #759, #787, #794 (with #791's
  citation repoint), #792/#798, #802, #782 (a further #745-hazard instance),
  #704, #723, #739, #743/#746 (cross-dependent, one bullet each), #805
  (closes #802's own filed-as-out-of-scope), #796, #800 -- the other 16,
  non-breaking.

Also restores a measurement an earlier round in this same diff dropped
while updating an adjacent one: the #692 entry's "mypy is unmoved at 112
errors" silently lost its "and pylint ... 364 messages" half when
`EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence.
Restored to the last value earlier rounds signed off on rather than
re-measured, since this branch's own `pcapkit/` tree predates several
since-merged PRs and a fresh run would not be measuring the same thing the
original round measured. Unmoved, and not silently dropped this time:
`93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES`
44 to 43 in the several other places that already carried the fix.

On which PRs get a bullet: there is no clean "user-facing only" rule --
#766 and #791 are pure CI/test/lint-comment entries that are in, while
#773 and #763 are the same kind of thing and are out. The real pattern
across this file's 46 commits is closer to "each round's author judged it
worth a reader's time," which is inconsistent by construction. This round
leaves that inconsistency as found rather than trying to retrofit a rule,
but did add #782 on reconsideration -- its own PR body names it as sharing
#766's hazard, and pre-existing precedent already treats that hazard's
instances as bullet-worthy.

`changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0.
`test_changelog_md.py` 47 passed.
JarryShaw added a commit that referenced this pull request Sep 26, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit
(relative to its parent, 4233555) now names all 20 bullets it carries, not
just the 8 this session added on top of the 10 already there -- a first
draft of this message named only its own 8 and left the other 10 silent,
which a cross-review caught. Six are breaking, matching the crediting PRs'
own `breaking` label in each case:

- #754 -- AppType split into per-transport registries; the 1,004 portless
  and 704 transportless rows stop being members.
- #764 -- an out-of-range port in `AppType.get` is refused, not minted.
- #778 -- `@final` enforced at runtime on `Info`/`Schema`.
- #575 -- four `.get()`-backed enum fields fall through to
  `_unregistered_member` instead of minting; 14 of 23 sample captures
  change output.
- #759 -- `AppType._dispatch` on a multi-transport `proto` now raises
  `ProtocolError` instead of silently resolving to whichever transport
  owns the lowest set bit.
- #805 -- `FieldBase.length` on a negative resolved length now raises
  `ProtocolError` instead of letting a bare `struct.error` escape.
  A second cross-review caught both: their crediting PRs (#783, #811) both
  carry GitHub's own `breaking` label, and neither bullet said so.

- #772 (with #790's docstring reword), #766, #787, #794 (with #791's
  citation repoint), #792/#798, #802, #779 (via #782, a further
  #745-hazard instance), #704, #723, #739, #743/#746 (cross-dependent, one
  bullet each), #796, #800 -- the other 14, non-breaking.

Also restores a measurement an earlier round in this same diff dropped
while updating an adjacent one: the #692 entry's "mypy is unmoved at 112
errors" silently lost its "and pylint ... 364 messages" half when
`EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence.
Restored to the last value earlier rounds signed off on rather than
re-measured, since this branch's own `pcapkit/` tree predates several
since-merged PRs and a fresh run would not be measuring the same thing the
original round measured. Unmoved, and not silently dropped this time:
`93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES`
44 to 43 in the several other places that already carried the fix.

On which PRs get a bullet: there is no clean "user-facing only" rule --
#766 and #791 are pure CI/test/lint-comment entries that are in, while
#773 and #763 are the same kind of thing and are out. The real pattern
across this file's 46 commits is closer to "each round's author judged it
worth a reader's time," which is inconsistent by construction. This round
leaves that inconsistency as found rather than trying to retrofit a rule,
but did add #779 (via #782) on reconsideration -- its own PR body names it
as sharing #766's hazard, and pre-existing precedent already treats that
hazard's instances as bullet-worthy.

`changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0.
`test_changelog_md.py` 47 passed.
JarryShaw added a commit that referenced this pull request Sep 26, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit
(relative to its parent, 4233555) now names all 20 bullets it carries, not
just the 8 this session added on top of the 10 already there -- a first
draft of this message named only its own 8 and left the other 10 silent,
which a cross-review caught. Six are breaking, matching the crediting PRs'
own `breaking` label in each case:

- #754 -- AppType split into per-transport registries; the 1,004 portless
  and 704 transportless rows stop being members.
- #764 -- an out-of-range port in `AppType.get` is refused, not minted.
- #778 -- `@final` enforced at runtime on `Info`/`Schema`.
- #575 -- four `.get()`-backed enum fields fall through to
  `_unregistered_member` instead of minting; 14 of 23 sample captures
  change output.
- #759 -- `AppType._dispatch` on a multi-transport `proto` now raises
  `ProtocolError` instead of silently resolving to whichever transport
  owns the lowest set bit.
- #805 -- `FieldBase.length` on a negative resolved length now raises
  `ProtocolError` instead of letting a bare `struct.error` escape.
  A second cross-review caught both: their crediting PRs (#783, #811) both
  carry GitHub's own `breaking` label, and neither bullet said so.

- #772 (with #790's docstring reword), #766, #787, #794 (with #791's
  citation repoint), #792/#798, #802, #779 (via #782, a further
  #745-hazard instance), #704, #723, #739, #743/#746 (cross-dependent, one
  bullet each), #796, #800 -- the other 14, non-breaking.

Also restores a measurement an earlier round in this same diff dropped
while updating an adjacent one: the #692 entry's "mypy is unmoved at 112
errors" silently lost its "and pylint ... 364 messages" half when
`EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence.
Restored to the last value earlier rounds signed off on rather than
re-measured, since this branch's own `pcapkit/` tree predates several
since-merged PRs and a fresh run would not be measuring the same thing the
original round measured. Unmoved, and not silently dropped this time:
`93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES`
44 to 43 in the several other places that already carried the fix.

On which PRs get a bullet: there is no clean "user-facing only" rule --
#766 and #791 are pure CI/test/lint-comment entries that are in, while
#773 and #763 are the same kind of thing and are out. The real pattern
across this file's 46 commits is closer to "each round's author judged it
worth a reader's time," which is inconsistent by construction. This round
leaves that inconsistency as found rather than trying to retrofit a rule,
but did add #779 (via #782) on reconsideration -- its own PR body names it
as sharing #766's hazard, and pre-existing precedent already treats that
hazard's instances as bullet-worthy.

`changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0.
`test_changelog_md.py` 47 passed.
JarryShaw added a commit that referenced this pull request Sep 26, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit
(relative to its parent, 4233555) now names all 20 bullets it carries, not
just the 8 this session added on top of the 10 already there -- a first
draft of this message named only its own 8 and left the other 10 silent,
which a cross-review caught. Six are breaking, matching the crediting PRs'
own `breaking` label in each case:

- #754 -- AppType split into per-transport registries; the 1,004 portless
  and 704 transportless rows stop being members.
- #764 -- an out-of-range port in `AppType.get` is refused, not minted.
- #778 -- `@final` enforced at runtime on `Info`/`Schema`.
- #575 -- four `.get()`-backed enum fields fall through to
  `_unregistered_member` instead of minting; 14 of 23 sample captures
  change output.
- #759 -- `AppType._dispatch` on a multi-transport `proto` now raises
  `ProtocolError` instead of silently resolving to whichever transport
  owns the lowest set bit.
- #805 -- `FieldBase.length` on a negative resolved length now raises
  `ProtocolError` instead of letting a bare `struct.error` escape.
  A second cross-review caught both: their crediting PRs (#783, #811) both
  carry GitHub's own `breaking` label, and neither bullet said so.

- #772 (with #790's docstring reword), #766, #787, #794 (with #791's
  citation repoint), #792/#798, #802, #779 (via #782, a further
  #745-hazard instance), #704, #723, #739, #743/#746 (cross-dependent, one
  bullet each), #796, #800 -- the other 14, non-breaking.

Also restores a measurement an earlier round in this same diff dropped
while updating an adjacent one: the #692 entry's "mypy is unmoved at 112
errors" silently lost its "and pylint ... 364 messages" half when
`EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence.
Restored to the last value earlier rounds signed off on rather than
re-measured, since this branch's own `pcapkit/` tree predates several
since-merged PRs and a fresh run would not be measuring the same thing the
original round measured. Unmoved, and not silently dropped this time:
`93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES`
44 to 43 in the several other places that already carried the fix.

On which PRs get a bullet: there is no clean "user-facing only" rule --
#766 and #791 are pure CI/test/lint-comment entries that are in, while
#773 and #763 are the same kind of thing and are out. The real pattern
across this file's 46 commits is closer to "each round's author judged it
worth a reader's time," which is inconsistent by construction. This round
leaves that inconsistency as found rather than trying to retrofit a rule,
but did add #779 (via #782) on reconsideration -- its own PR body names it
as sharing #766's hazard, and pre-existing precedent already treats that
hazard's instances as bullet-worthy.

`changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0.
`test_changelog_md.py` 47 passed.
JarryShaw added a commit that referenced this pull request Sep 28, 2026
`main` moved from 73f09ae to 3cbdf89 while this PR sat open. This commit
(relative to its parent, 4233555) now names all 20 bullets it carries, not
just the 8 this session added on top of the 10 already there -- a first
draft of this message named only its own 8 and left the other 10 silent,
which a cross-review caught. Six are breaking, matching the crediting PRs'
own `breaking` label in each case:

- #754 -- AppType split into per-transport registries; the 1,004 portless
  and 704 transportless rows stop being members.
- #764 -- an out-of-range port in `AppType.get` is refused, not minted.
- #778 -- `@final` enforced at runtime on `Info`/`Schema`.
- #575 -- four `.get()`-backed enum fields fall through to
  `_unregistered_member` instead of minting; 14 of 23 sample captures
  change output.
- #759 -- `AppType._dispatch` on a multi-transport `proto` now raises
  `ProtocolError` instead of silently resolving to whichever transport
  owns the lowest set bit.
- #805 -- `FieldBase.length` on a negative resolved length now raises
  `ProtocolError` instead of letting a bare `struct.error` escape.
  A second cross-review caught both: their crediting PRs (#783, #811) both
  carry GitHub's own `breaking` label, and neither bullet said so.

- #772 (with #790's docstring reword), #766, #787, #794 (with #791's
  citation repoint), #792/#798, #802, #779 (via #782, a further
  #745-hazard instance), #704, #723, #739, #743/#746 (cross-dependent, one
  bullet each), #796, #800 -- the other 14, non-breaking.

Also restores a measurement an earlier round in this same diff dropped
while updating an adjacent one: the #692 entry's "mypy is unmoved at 112
errors" silently lost its "and pylint ... 364 messages" half when
`EXPECTED_FAILURES` was corrected 44 to 43 elsewhere in the same sentence.
Restored to the last value earlier rounds signed off on rather than
re-measured, since this branch's own `pcapkit/` tree predates several
since-merged PRs and a fresh run would not be measuring the same thing the
original round measured. Unmoved, and not silently dropped this time:
`93 of the 95 sites` corrected to `94 of the 95` and `EXPECTED_FAILURES`
44 to 43 in the several other places that already carried the fix.

On which PRs get a bullet: there is no clean "user-facing only" rule --
#766 and #791 are pure CI/test/lint-comment entries that are in, while
#773 and #763 are the same kind of thing and are out. The real pattern
across this file's 46 commits is closer to "each round's author judged it
worth a reader's time," which is inconsistent by construction. This round
leaves that inconsistency as found rather than trying to retrofit a rule,
but did add #779 (via #782) on reconsideration -- its own PR body names it
as sharing #766's hazard, and pre-existing precedent already treats that
hazard's instances as bullet-worthy.

`changelog_md.py` regenerated `CHANGELOG.md`; `--check` exit 0.
`test_changelog_md.py` 47 passed.
@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.

tests: mypy has no MODULE_PROVIDERS entry, so a visible gate on it is impossible and the guard dies on a bare KeyError

1 participant