diff --git a/tests/_dependency_gates.py b/tests/_dependency_gates.py index a6c750abcf..6c2f0e9989 100644 --- a/tests/_dependency_gates.py +++ b/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,8 +746,10 @@ 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, @@ -676,7 +757,7 @@ def module_providers(module: 'str') -> 'frozenset[str]': 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( diff --git a/tests/test_tier_guard.py b/tests/test_tier_guard.py index 25d36ba64a..5896bb8d86 100644 --- a/tests/test_tier_guard.py +++ b/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. diff --git a/tests/vendor/test_vendor_reg_apptype_generator_unit.py b/tests/vendor/test_vendor_reg_apptype_generator_unit.py index 4e1d408e43..e1b2d26378 100644 --- a/tests/vendor/test_vendor_reg_apptype_generator_unit.py +++ b/tests/vendor/test_vendor_reg_apptype_generator_unit.py @@ -61,30 +61,54 @@ member stays wrapped in ``cast`` rather than reverting to a bare literal, plus a direct run through mypy's own API when :mod:`mypy` is importable, since the source-text shape alone cannot tell a correct fix from a differently-worded -one that stops mypy agreeing. That second pin skips outright when mypy is not -importable, the same shape -:class:`~tests.project.test_isort_clean.TestIsortIsCleanOnThePackage` uses for -isort: mypy is a :file:`Pipfile` ``[dev-packages]`` entry (line 48) and is in -no :file:`pyproject.toml` extra, so no ``pytest`` job in -:file:`.github/workflows/unit-tests.yml` -- every one installs ``.[test,...]`` --- ever has it importable, and this test skips there rather than erroring. -That inline skip is, by #766's own description of the shape, invisible to -:mod:`tests._dependency_gates`'s own guard: gating it instead with a -``HAS_MYPY`` flag and ``@unittest.skipUnless`` was measured and rejected, not -merely not attempted -- ``mypy`` has no entry in that module's -``MODULE_PROVIDERS`` table and is in no :file:`pyproject.toml` extra to add -one for, so doing so breaks -:class:`~tests.test_tier_guard.DependencyGateCoverageTests` outright: a -``KeyError`` from :func:`~tests._dependency_gates.extras_providing` plus two -more failures raising ``AssertionError``, measured as 4 errors and 2 failures -of its 9 tests. #779 tracks closing that gap generally. +one that stops mypy agreeing. + +That second pin is gated the way :mod:`tests._dependency_gates` can actually +see: a module-level ``HAS_MYPY`` flag and an ``@unittest.skipUnless`` +decorator. It was written as an inline +``try``/``except ImportError: self.skipTest(...)`` instead when #770 landed, +for an obstacle that was real at the time -- ``mypy`` had no +:data:`~tests._dependency_gates.MODULE_PROVIDERS` entry, so adding the +decorator without one made +:class:`~tests.test_tier_guard.DependencyGateCoverageTests` 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` -- re-measured on this +change's own base; #779 reports 4 errors of 9 tests because it was written +before #774 added the tenth. #779 closed that end: +``mypy`` is mapped now and its gap is recorded as a deliberate exclusion, so +the gate can take the visible shape. The inline form worked, but was invisible +to the guard *by construction* -- +:func:`~tests._dependency_gates._gates_of` walks a definition's decorator list +and never its body -- which is the "dark test" hazard #745 exists to stop, and +which #766 carries for the sibling isort check. + +What the gate is worth has not changed. mypy is a :file:`Pipfile` +``[dev-packages]`` entry (line 48) and is in no :file:`pyproject.toml` extra, +so no ``pytest`` job in :file:`.github/workflows/unit-tests.yml` -- every one +installs ``.[test,...]`` -- ever has it importable, and this test skips on all +of them; it runs where mypy is installed, which is a developer checkout and +``make mypy``'s own environment. The difference is that the skip is now +counted: it is a :class:`~tests._dependency_gates.Gap` on each job that +reaches it, and +:data:`~tests._dependency_gates.DEPENDENCY_GATE_EXCLUSIONS`\\ ``['HAS_MYPY']`` +carries the reason for declining to close it -- a type checker belongs to the +lint tier, not to a pytest install line. """ +import importlib.util import unittest __all__ = ['AppTypeGeneratorShapeTests'] +#: Whether :mod:`mypy` is importable, for the one test that runs it. A visible +#: ``skipUnless`` flag rather than an inline ``skipTest``, so +#: :func:`~tests._dependency_gates.gated_scopes` counts the gate and +#: :mod:`tests.test_tier_guard` can audit which CI jobs it darkens (#779); see +#: this module's own docstring for what that replaced and why. +HAS_MYPY = importlib.util.find_spec('mypy') is not None + class AppTypeGeneratorShapeTests(unittest.TestCase): """Pins to the three mechanical changes #744, #768 and #770 made.""" @@ -194,6 +218,7 @@ def test_undefined_member_is_cast_rather_than_a_bare_literal(self) -> None: self.fail('found a bare literal near: %r' % source[max(0, match.start() - 40):match.end() + 40]) + @unittest.skipUnless(HAS_MYPY, 'mypy is not installed') def test_undefined_member_infers_as_transport_protocol_under_mypy(self) -> None: """GitHub issue #770, run directly through mypy's own API. @@ -211,14 +236,12 @@ def test_undefined_member_infers_as_transport_protocol_under_mypy(self) -> None: This is not in :file:`.github/workflows/unit-tests.yml`'s reach -- mypy is a :file:`Pipfile` ``[dev-packages]`` entry, not a :file:`pyproject.toml` extra, so no ``pytest`` job there ever has it - importable -- and skips outright rather than erroring when it is not - installed. See this module's own docstring for why that inline skip, - rather than a tracked ``HAS_MYPY`` gate, is the deliberate choice. + importable -- and the ``HAS_MYPY`` gate above skips it there rather + than erroring. That gate is a decorator precisely so + :mod:`tests._dependency_gates` can count the skip instead of it being + invisible; see this module's own docstring. """ - try: - from mypy import api as mypy_api - except ImportError: - self.skipTest('mypy is not installed') + from mypy import api as mypy_api import pcapkit.const.reg.apptype.apptype as base_mod