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

tests/_dependency_gates.py

@@ -159,6 +159,20 @@
     'chardet': (CORE,),
     'dictdumper': (CORE,),
     'tbtrim': (CORE,),
+    # The one entry no extra carries, and deliberately so. ``mypy`` is a
+    # lint-tier tool: it ships as a :file:`Pipfile` ``[dev-packages]`` entry
+    # (``mypy = "*"``), :file:`.github/workflows/lint.yml` installs it directly
+    # (``pip install -U vermin pylint mypy bandit``), and no
+    # :file:`pyproject.toml` extra declares it at all -- so
+    # :func:`extras_providing` resolves it to the empty set and every pytest
+    # job reaching a ``HAS_MYPY`` gate is reported as a :class:`Gap`. That is
+    # the honest answer rather than the :exc:`KeyError` a missing entry used to
+    # raise: the gate really is dark in CI, and
+    # :data:`DEPENDENCY_GATE_EXCLUSIONS`'s ``HAS_MYPY`` entry is where the reason
+    # for declining to close it lives (#779). Spelled as a requirement string
+    # anyway, so the day some extra does carry mypy the gap closes itself
+    # rather than needing this table rewritten.
+    'mypy': ('mypy',),
     # ``beautifulsoup4`` ships ``bs4``; ``html5lib`` is an *extra of* that
     # distribution, so ``beautifulsoup4`` alone does not provide it. That
     # distinction is the whole reason ``HAS_CRAWLER_DEPS`` is satisfied in CI
@@ -453,6 +467,42 @@ class Exclusion(NamedTuple):
             'do.'
         ),
     ),
+    'HAS_MYPY': Exclusion(
+        dark={'test': ('mypy',), 'engine-tests': ('mypy',), 'gate': ('mypy',)},
+        reason=(
+            'The one entry here that is not a missing *runtime* dependency, and the only '
+            'one whose fix is not an install line. mypy is a type checker: the single '
+            'method it gates runs mypy over the generated '
+            'pcapkit/const/reg/apptype/apptype.py and requires a clean result (#770), '
+            'which is a lint question asked from a unit test rather than a code path '
+            'pcapkit executes. It belongs to the lint tier accordingly -- a Pipfile '
+            '[dev-packages] entry that `make mypy` resolves, and a direct '
+            '"pip install -U vermin pylint mypy bandit" in lint.yml -- and no '
+            'pyproject.toml extra declares it at all.\n\n'
+            'So the fix this guard would otherwise propose -- put an extra carrying mypy '
+            "on a pytest job's install line -- is the wrong one, and declining it is what "
+            'this entry records. A pytest leg that installed mypy would pay for a second, '
+            'slower copy of a check lint.yml already runs across the whole package, and it '
+            'would make every unit-test leg depend on the type checker resolving. Note too '
+            'that lint.yml runs no pytest at all, so pytest_jobs() never sees the one job '
+            'that does install mypy: an extra added for its benefit would satisfy this '
+            'guard without changing what runs anywhere. What the gate buys instead is that '
+            'the check runs wherever mypy happens to be importable -- a developer '
+            "checkout, `make mypy`'s own environment -- and skips *visibly* where it is "
+            'not. Visibly is the whole point of #779: the gate was an inline '
+            'try/except ImportError skipTest until then, which works but is invisible to '
+            'this module by construction, since _gates_of() walks a decorator list and '
+            'never a function body. That is the dark-test hazard #745 exists to stop, and '
+            '#766 carries the same shape for the sibling isort check.\n\n'
+            'test, engine-tests and gate are exactly the three jobs whose selection '
+            'reaches tests/vendor/test_vendor_reg_apptype_generator_unit.py -- the two '
+            'ignore-shape legs and the whole-suite one. integration and pypcap-parity '
+            'select by fixture tier and never collect that module, which is why they are '
+            'not listed, the same reason they are absent from HAS_VENDOR_DEPS above. If a '
+            'lint extra is ever declared in pyproject.toml, the right change is to delete '
+            'this entry and let the gap close on its own rather than to widen it.'
+        ),
+    ),
 }
 
 
@@ -658,6 +708,35 @@ def declared_requirements() -> 'dict[str, tuple[Requirement, ...]]':
     return declared
 
 
+def _top_level_providers(module: 'str') -> 'tuple[str, ...]':
+    """:data:`MODULE_PROVIDERS`'s entry for ``module``'s top-level package.
+
+    The one place that table is subscripted, so that a module nobody has mapped
+    fails with a message naming it and naming the fix rather than with a bare
+    ``KeyError: 'mypy'`` out of a dict lookup three call sites deep. Auditing
+    gates is the whole purpose of this module, so crashing opaquely on the first
+    unrecognised one is the worst answer available; #779 is what that cost, and
+    :func:`_disqualified_providers` below is the message shape this follows.
+
+    Unchanged for a mapped module: the same tuple the subscript returned, so the
+    lookup is a diagnostic improvement and not a semantic one.
+
+    """
+    top_level = module.partition('.')[0]
+    try:
+        return MODULE_PROVIDERS[top_level]
+    except KeyError:
+        raise AssertionError(
+            f'{module!r} has no MODULE_PROVIDERS entry (looked up under its top-level '
+            f'{top_level!r}), so nothing here knows which distribution ships it or which '
+            f'extra would install it. Add a MODULE_PROVIDERS[{top_level!r}] entry naming '
+            f'the requirement string(s) that provide it -- and, when no pyproject.toml '
+            f'extra carries any of them, a DEPENDENCY_GATE_EXCLUSIONS entry for the flag '
+            f'saying why the jobs reaching its gates decline it. A flag whose condition no '
+            f'extra could ever satisfy belongs in NON_DISTRIBUTION_FLAGS instead.'
+        ) from None
+
+
 def module_providers(module: 'str') -> 'frozenset[str]':
     """Every distribution (or :data:`CORE`) :data:`MODULE_PROVIDERS` says could ship ``module``.
 
@@ -667,16 +746,18 @@ def module_providers(module: 'str') -> 'frozenset[str]':
     alone rather than falling back to plain ``pcap``'s two-wide entry. Falls
     back to the top-level package otherwise, the same rule
     :func:`extras_providing` always uses, which is what makes an unmapped
-    module a failure there rather than a silent pass here too (the fallback
-    can raise :exc:`KeyError` exactly as that function's own lookup does).
+    module a failure there rather than a silent pass here too (the fallback goes
+    through :func:`_top_level_providers`, so it raises the same
+    :exc:`AssertionError` naming the module that that function's own lookup
+    does).
 
     :func:`ambiguous_satisfactions` is the reason this exists: it needs the
     narrower, exact-path answer to tell a distribution that genuinely,
     uniquely ships a submodule apart from the wider set that merely ships its
     top-level package -- see that function's own docstring.
 
     """
-    return frozenset(MODULE_PROVIDERS.get(module, MODULE_PROVIDERS[module.partition('.')[0]]))
+    return frozenset(MODULE_PROVIDERS.get(module, _top_level_providers(module)))
 
 
 def contested_imports() -> 'frozenset[str]':
@@ -719,11 +800,15 @@ def _is_live(provider: 'str') -> 'bool':
 def extras_providing(module: 'str') -> 'frozenset[str]':
     """Every extra (or :data:`CORE`) whose requirements make ``module`` importable.
 
-    Raises :exc:`KeyError` for a module with no :data:`MODULE_PROVIDERS` entry,
-    which is what makes a newly gated dependency a failure rather than a pass.
+    Raises :exc:`AssertionError` -- through :func:`_top_level_providers`, which
+    names the module and the table to add it to -- for a module with no
+    :data:`MODULE_PROVIDERS` entry, which is what makes a newly gated dependency
+    a failure rather than a pass. An entry no extra carries is a different thing
+    and is *not* an error: it resolves to the empty set, so every job reaching
+    the gate is reported as a :class:`Gap`, which is what ``mypy`` is.
 
     """
-    providers = MODULE_PROVIDERS[module.partition('.')[0]]
+    providers = _top_level_providers(module)
     declared = declared_requirements()
     satisfying = set()  # type: set[str]
     for provider in providers:
@@ -1356,7 +1441,7 @@ def ambiguous_satisfactions(
                 excluded_modules = exclusions.get((gate.module, gate.flag), frozenset())
                 disqualified = _disqualified_providers(excluded_modules)
 
-                providers = MODULE_PROVIDERS[module.partition('.')[0]]
+                providers = _top_level_providers(module)
                 satisfied = frozenset(
                     provider for provider in providers
                     if provider == CORE or any(

tests/test_tier_guard.py

@@ -1513,6 +1513,58 @@ def test_the_vendor_extra_closes_engine_tests_but_not_test_or_gate(self) -> None
         self.assertNotIn('engine-tests', exclusion.dark)
         self.assertEqual(set(exclusion.dark), {'test', 'gate'})
 
+    def test_the_mypy_gate_is_visible_and_dark_on_every_job_that_reaches_it(self) -> None:
+        """#779: a lint-tier tool gated visibly, and declining the install line anyway.
+
+        ``mypy`` is a :file:`Pipfile` ``[dev-packages]`` entry and is in no
+        :file:`pyproject.toml` extra, so before #779 it had no
+        :data:`~tests._dependency_gates.MODULE_PROVIDERS` entry at all and an
+        ``@unittest.skipUnless`` gate on it was not merely undesirable but
+        *impossible*: adding one to
+        :file:`tests/vendor/test_vendor_reg_apptype_generator_unit.py` and
+        nothing else made this class report 2 failures and 5 errors of its 10
+        tests, every one of those errors a bare ``KeyError: 'mypy'`` out of
+        :func:`~tests._dependency_gates.extras_providing`, upstream of any
+        :data:`~tests._dependency_gates.DEPENDENCY_GATE_EXCLUSIONS` filtering.
+        (#779 reports 4 errors of 9 tests for the same mutation; it was written
+        before #774 added
+        :meth:`test_the_vendor_extra_closes_engine_tests_but_not_test_or_gate`,
+        which is the tenth and the fifth error. Re-measured here rather than
+        copied.)
+        The entry is what makes the gate expressible; the exclusion is what
+        records that *closing* the gap would be the wrong fix, since a type
+        checker belongs to :file:`.github/workflows/lint.yml` rather than to a
+        pytest install line.
+
+        So the outcome pinned here is deliberately not "no gap". It is a gap on
+        exactly the three jobs that collect the module, explained rather than
+        closed -- and, which is the part #745 cares about, countable at all
+        rather than hidden in a function body.
+
+        """
+        gates = {(gate.flag, gate.module) for gate in _dependency_gates.gated_scopes()}
+        self.assertIn(
+            ('HAS_MYPY', 'tests/vendor/test_vendor_reg_apptype_generator_unit.py'), gates,
+            'the mypy gate is invisible to the scan again -- an inline skipTest in a '
+            'function body is exactly the shape _gates_of() cannot see')
+
+        # An empty answer, not a raised exception: no extra carries mypy, which
+        # is why the gap below is real rather than a mapping mistake.
+        self.assertEqual(_dependency_gates.extras_providing('mypy'), frozenset())
+
+        gaps = {(gap.flag, gap.job) for gap in _dependency_gates.dependency_gate_gaps()}
+        for job in ('test', 'engine-tests', 'gate'):
+            with self.subTest(reaches=job):
+                self.assertIn(('HAS_MYPY', job), gaps)
+        # Both select by fixture tier and never collect tests/vendor/ at all --
+        # the same reason they are absent from HAS_VENDOR_DEPS above.
+        for job in ('integration', 'pypcap-parity'):
+            with self.subTest(does_not_reach=job):
+                self.assertNotIn(('HAS_MYPY', job), gaps)
+
+        exclusion = _dependency_gates.DEPENDENCY_GATE_EXCLUSIONS['HAS_MYPY']
+        self.assertEqual(set(exclusion.dark), {'test', 'engine-tests', 'gate'})
+
     def test_each_exclusion_still_describes_a_gap_that_is_really_there(self) -> None:
         """The anti-rot half, and the reason this is an allowlist and not a skip list.
 
@@ -1566,15 +1618,23 @@ def test_every_gated_flag_is_classified(self) -> None:
         never be reported as a gap.
 
         A *required* module missing from :data:`~tests._dependency_gates.MODULE_PROVIDERS`
-        fails right here, with this message naming it. An *excluded* one --
-        the modules :func:`~tests._dependency_gates.flag_exclusions` reads off
-        a negated probe -- had neither: nothing looped over them at all, so
-        the same gap surfaced only as a bare :exc:`KeyError` out of
-        :func:`~tests._dependency_gates.module_providers`, wherever
-        :func:`~tests._dependency_gates.ambiguous_satisfactions` happened to
-        call it. Looping over both here is what gives an excluded module the
-        same deliberate contract a required one already has, instead of an
-        accident of whichever caller reaches it first.
+        fails right here, with this message naming it *and the gate's own file
+        and line*. An *excluded* one -- the modules
+        :func:`~tests._dependency_gates.flag_exclusions` reads off a negated
+        probe -- had neither: nothing looped over them at all, so the same gap
+        surfaced only out of :func:`~tests._dependency_gates.module_providers`,
+        wherever :func:`~tests._dependency_gates.ambiguous_satisfactions`
+        happened to call it. Looping over both here is what gives an excluded
+        module the same deliberate contract a required one already has, instead
+        of an accident of whichever caller reaches it first.
+
+        Both messages below still earn their place now that
+        :func:`~tests._dependency_gates._top_level_providers` names the module
+        and the fix on its own (#779): what that cannot say is *which gate*
+        wanted the module, and the location is most of what makes the failure
+        actionable. This test is also the only one that reaches the excluded
+        half without depending on a job's selection happening to collect the
+        gate.
 
         """
         requirements = _dependency_gates.flag_requirements()
@@ -1607,8 +1667,8 @@ def test_every_gated_flag_is_classified(self) -> None:
                                   _dependency_gates.MODULE_PROVIDERS,
                                   f'{module} is excluded by {gate.flag}\'s own negated probe '
                                   f'but has no MODULE_PROVIDERS entry, so '
-                                  f'ambiguous_satisfactions() would raise a bare KeyError '
-                                  f'resolving it rather than fail with this message')
+                                  f'ambiguous_satisfactions() would fail resolving it '
+                                  f'somewhere that cannot name this gate')
 
     def test_no_provider_mapping_or_exclusion_is_vestigial(self) -> None:
         """Both tables are exactly as wide as the suite needs them to be --
@@ -2126,6 +2186,65 @@ def test_a_flag_no_extra_could_satisfy_is_not_reported_as_a_gap(self) -> None:
             self.assertNotIn('HAS_PYSHARK',
                              {gap.flag for gap in _dependency_gates.dependency_gate_gaps()})
 
+    def test_an_unmapped_module_names_itself_and_the_table_to_add_it_to(self) -> None:
+        """#779's second defect: the table lookup used to die on a bare ``KeyError``.
+
+        Auditing gates is this module's whole purpose, so the worst available
+        answer to an unrecognised one was the one it gave: ``KeyError: 'mypy'``
+        raised from a dict subscript three call sites deep, naming neither the
+        fix nor even the table that wanted the entry. All three subscripts now
+        go through :func:`~tests._dependency_gates._top_level_providers`, so
+        every entry point fails the same way and says the same thing --
+        :func:`~tests._dependency_gates.extras_providing` (which
+        :func:`~tests._dependency_gates.dependency_gate_gaps` reaches),
+        :func:`~tests._dependency_gates.module_providers` (which
+        :func:`~tests._dependency_gates.ambiguous_satisfactions` reaches), and
+        the helper itself.
+
+        The message has to name the *dotted* module as given as well as the
+        top-level key actually looked up, because those differ exactly when the
+        truncation is what a reader would otherwise have to work out for
+        themselves.
+
+        """
+        unmapped = 'nosuchlinter.plugins'
+        for resolve in (_dependency_gates.extras_providing,
+                        _dependency_gates.module_providers,
+                        _dependency_gates._top_level_providers):
+            with self.subTest(resolve=resolve.__name__):
+                with self.assertRaises(AssertionError) as caught:
+                    resolve(unmapped)
+                message = str(caught.exception)
+                self.assertIn(repr(unmapped), message)
+                self.assertIn(repr('nosuchlinter'), message)
+                for table in ('MODULE_PROVIDERS', 'DEPENDENCY_GATE_EXCLUSIONS',
+                              'NON_DISTRIBUTION_FLAGS'):
+                    self.assertIn(table, message)
+
+    def test_a_mapped_module_still_resolves_through_the_same_truncation(self) -> None:
+        """The control for the test above: #779 changed the diagnostic, not the answer.
+
+        :func:`~tests._dependency_gates._top_level_providers` truncates to the
+        top-level package unconditionally -- so ``pcap._pcap`` gets plain
+        ``pcap``'s two-wide entry there, exactly as the bare subscript it
+        replaced did -- while
+        :func:`~tests._dependency_gates.module_providers` keeps its own
+        exact-key preference on top of that, which is what makes ``pcap._pcap``
+        resolve to ``pcap-ct`` alone. Those two answers differing for the same
+        module is load-bearing for #762, so a "harmless" unification of the two
+        lookups has to fail here.
+
+        """
+        self.assertEqual(_dependency_gates._top_level_providers('dpkt'), ('dpkt',))
+        self.assertEqual(_dependency_gates._top_level_providers('pcapfile.savefile'),
+                         ('pypcapfile',))
+        self.assertEqual(_dependency_gates._top_level_providers('mypy'), ('mypy',))
+
+        self.assertEqual(_dependency_gates._top_level_providers('pcap._pcap'),
+                         ('pypcap', 'pcap-ct'))
+        self.assertEqual(_dependency_gates.module_providers('pcap._pcap'),
+                         frozenset({'pcap-ct'}))
+
     def test_an_excluded_module_outside_any_contested_scope_is_ignored(self) -> None:
         """:func:`~tests._dependency_gates._disqualified_providers`, the safe default.