diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 2418d26fd..ad97043b7 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -91,23 +91,17 @@ jobs: # extra's comment in pyproject.toml) -- still cheap in wall time, just not # "a second" cheap. # - # PyPCAPFile (pypcapfile) is deliberately NOT here, or on either job - # below, despite #738 listing HAS_PYPCAPFILE among its cheap subset. - # Measured on 3.10 and 3.11 (where its `python_version < '3.12'` marker - # lets it actually install): installing it does not make the 10 - # HAS_PYPCAPFILE-gated methods pass, it makes 7 of them *fail* -- - # pcapkit/toolkit/pypcapfile.py assumes pypcapfile's `IP.src`/`IP.dst` - # are packed 4-byte addresses, but pypcapfile 0.12.0's real `IP` class - # (pcapfile/protocols/network/ip.py) hands back the dotted-decimal string - # as bytes instead (`ctypes.c_char_p`), so `struct.pack` and - # `ipaddress.IPv4Address(...)` both raise. That is a real, previously - # unexercised bug in the toolkit adapter, not a fixture or environment - # problem -- confirmed by reading pypcapfile's own source, reproduced - # identically across independent runs on both versions. Adding the extra - # would turn 7 invisible skips into 7 required-check failures on every - # 3.10/3.11 leg, which is a worse regression than the skips it would - # replace. Fixing pcapkit/toolkit/pypcapfile.py is out of scope for a - # CI-only change -- left for a follow-up once filed. + # PyPCAPFile (pypcapfile) is deliberately NOT here. It used to be kept off + # every job in this file: on 3.10/3.11 (where its + # `python_version < '3.12'` marker lets it install), installing it turned + # 7 of the 10 HAS_PYPCAPFILE-gated methods this comment used to count + # into failures, against a real bug in pcapkit/toolkit/pypcapfile.py's + # handling of pypcapfile 0.12.0's `IP.src`/`IP.dst`. #747 and #748 fixed + # that bug, and #751 gave the now-safe extra a home: the dedicated + # `engine-tests` and `pypcap-parity` jobs below install it (15 + # HAS_PYPCAPFILE-gated methods between them), deliberately apart from + # this job rather than added to it -- see either job's own comment for + # why. - name: Install package and test dependencies run: | python -m pip install -U pip setuptools wheel @@ -174,10 +168,10 @@ jobs: # covered, across all five Python versions, by the `test` job above. # PyPCAPFile is NOT added here either, even though this job's selection # also reaches test_new_engine_parity_runtime.py's 6 HAS_PYPCAPFILE - # methods -- see the `test` job's comment above for why: on 3.10/3.11, - # where the extra actually installs, 3 of those 6 fail for real against - # a genuine bug in pcapkit/toolkit/pypcapfile.py, not just the 4 that - # skip cleanly on 3.12+. + # methods -- see the `test` job's comment above for the bug that used to + # make installing it here a regression, now fixed by #747/#748. #751's + # `pypcap-parity` job below covers those 6 methods instead, on the same + # fixture-tier selection as this job but in a venv of its own. - name: Install package, test and generator dependencies run: | python -m pip install -U pip setuptools wheel @@ -245,6 +239,215 @@ jobs: echo "Fixture-dependent selection: $selection" python -m pytest -q -n auto --dist load $selection + # Per-engine coverage for the third-party capture engines (Scapy, PyShark, + # PyPCAPFile, pcap-ct) -- ruled onto its own job in #751 rather than one more + # install line on `test`, `integration` or `gate`: "per-engine's tests + # covered by a separate test step and on all supported Python versions... + # so they dont intertwine with the other major tests". `test` had already + # declined Scapy on cost grounds (see its own install-step comment above); + # asking that question three more times, once per engine, would only repeat + # it instead of answering it. + # + # Selection mirrors the `test` job's ignore flags exactly, so it reaches the + # same unit tier -- including + # tests/foundation/engines/test_runtime_engines.py, whose own HAS_RUNTIME + # reuses that name for the four core dependencies plus dpkt, scapy and + # pyshark (see tests/_dependency_gates.py's own exclusion for the history). + # Installing Scapy, PyShark and DPKT here closes that gap too, as a side + # effect of the engine extras rather than a separate install line. + # + # PyPCAPFile installs only on 3.10 and 3.11 here -- its own + # "python_version < '3.12'" marker -- so its 9 HAS_PYPCAPFILE-gated unit + # methods (tests/toolkit/test_pypcapfile_unit.py) run on two of these five + # legs and skip cleanly on the other three. tests/_dependency_gates.py's own + # guard cannot see that partial coverage -- it does not evaluate markers, by + # its own module docstring -- so this is recorded here instead: two legs of + # real coverage is the trade #751 asked to make, not a gap to hide. #747 and + # #748 are what make it worth taking at all -- they fixed the two bugs that + # used to turn those skips into 7 failures. + # + # pcap-ct needs a system libpcap present at run time (no compiler, no + # headers -- see the PCAP_CT extra in pyproject.toml), which is what the + # apt-get step below installs. It must never share a venv with PyPCAP: both + # distributions install a top-level `pcap` module, and pcap-ct's package + # shadows upstream's extension whenever both are importable -- measured in + # pcapkit/foundation/engines/_pcap_backend.py's own module docstring. That is + # why PyPCAP is not in this job's install line at all; it gets the + # `pypcap-parity` job below, entirely to itself. + engine-tests: + name: Engines Python ${{ matrix.python-version }} + if: ${{ inputs.gate-only != true }} + runs-on: ubuntu-latest + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + python-version: + # See the `test` job above for why 3.15 is excluded here. + - "3.10" + - "3.11" + - "3.12" + - "3.13" + - "3.14" + + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v7 + with: + python-version: ${{ matrix.python-version }} + cache: pip + + # pcap-ct's ctypes loader calls find_library("pcap") at run time; without + # a system libpcap, PCAP_CT.unsupported_reason() degrades the engine to + # the default parser instead of running it, and the one HAS_PCAP_CT-gated + # test that exercises the real backend + # (test_the_real_backend_reads_a_committed_capture) would see that + # fallback and fail its own assertion that no EngineWarning was raised -- + # not merely skip. + # + # tshark joins it per #751's later ruling, which asked the same "try it, + # rip it if CI is not a good fit" of PyShark's binary that it asked of + # PyPCAP's toolchain -- not merely "install the distribution and leave + # the binary out", which an earlier reading of this thread had settled + # for. One HAS_PYSHARK-gated method + # (test_the_reason_tracks_the_running_interpreter) used to hard-assert + # tshark's *absence*, which would have flipped from a pass to a failure + # the moment tshark was installed; that assertion now probes the same + # way PyShark.unsupported_reason() itself does -- pyshark's own + # get_process_path(), not shutil.which(), which config.ini precedence + # can make disagree with it -- the same oracle its sibling + # test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous already + # used, so the file no longer disagrees with itself about whether + # tshark is allowed to be present. `DEBIAN_FRONTEND=noninteractive` plus + # the debconf pre-seed below is what stops `tshark`'s postinst script + # from blocking on the "allow non-superusers to capture packets" prompt + # apt would otherwise show. + - name: Install system libpcap and tshark + run: | + sudo apt-get update + echo "wireshark-common wireshark-common/install-setuid boolean false" | sudo debconf-set-selections + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends libpcap0.8 tshark + + - name: Install package and per-engine test dependencies + run: | + python -m pip install -U pip setuptools wheel + python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,PCAP_CT]' + + # See the `test` job above for why this step exists. + - name: Report available parallelism + run: | + nproc + python -c "import os; print('cpu_count', os.cpu_count())" + + - name: Run unit tests + run: >- + python -m pytest -q -n auto --dist load + --ignore=tests/integration + --ignore-glob='*_runtime.py' + --ignore-glob='*_regression.py' + + # Whether upstream PyPCAP is worth building in CI at all -- #751's own + # instruction was "try to build and if the CI is not a good suit, then we + # ripe it". This job is that attempt: a C toolchain plus libpcap headers, on + # the two Python versions its own marker allows + # ("pypcap; python_version < '3.12'", pyproject.toml:151). If a clean run of + # this job's install step goes red -- not a flake -- the fix is deleting this + # job and returning HAS_PYPCAP to + # tests/_dependency_gates.DEPENDENCY_GATE_EXCLUSIONS with that run linked as + # the reason, per the ruling. + # + # Kept apart from `engine-tests` above for two reasons: it needs a compiler + # that job has no other reason to carry, and its `pcap` module would collide + # with pcap-ct's if both were installed into the one venv (see + # `engine-tests`'s own comment on that). The HAS_PYPCAP-gated module this job + # targets, tests/foundation/engines/test_new_engine_parity_runtime.py, reads + # generated captures (arp.pcap, tcp.pcap, ipv4.pcap -- its own module + # docstring), so this mirrors the `integration` job's fixture-tier selection + # and Scapy/DPKT/cli baseline rather than `test`'s ignore-shape one, and pays + # that job's full run cost a second time on top -- accepted here rather than + # discovered, since tests/_dependency_gates.py's guard has no cheaper of its + # three known selection shapes for reaching one fixture-dependent module (see + # that module's own docstring). The same module also carries 6 of + # HAS_PYPCAPFILE's 15 gated methods, which come along for free once + # PyPCAPFile is on this job's install line too. + # + # The matrix is deliberately just 3.10 and 3.11, not the full five: PyPCAP's + # marker and PyPCAPFile's marker are both "python_version < '3.12'", so + # 3.12-3.14 would install neither extra here and pay this job's full + # fixture-tier run for zero new coverage. + pypcap-parity: + name: PyPCAP/PyPCAPFile parity Python ${{ matrix.python-version }} + if: ${{ inputs.gate-only != true }} + runs-on: ubuntu-latest + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + python-version: + - "3.10" + - "3.11" + + steps: + - uses: actions/checkout@v7 + + - uses: actions/setup-python@v7 + with: + python-version: ${{ matrix.python-version }} + cache: pip + + # pypcap ships no wheel: it compiles pcap.c against libpcap, so both the + # headers and the shared library have to be present before pip is asked + # to build it. + - name: Install libpcap headers and a C toolchain + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends build-essential libpcap-dev + + - name: Install package, test and generator dependencies + run: | + python -m pip install -U pip setuptools wheel + python -m pip install -e '.[test,Scapy,DPKT,cli,PyShark,PyPCAP,PyPCAPFile]' + + # See the `integration` job above for why this step is shaped the way it + # is. + - name: Regenerate sample captures + shell: bash + run: | + if ! python examples/generators/make_samples.py 2>&1 | tee "$RUNNER_TEMP/make-samples.log"; then + echo "::error title=Sample fixture generation failed::examples/generators/make_samples.py could not rebuild examples/captures/. This is a fixture-generation failure, not a test failure -- the test suite has not run." + exit 1 + fi + + if grep -q '(download unavailable)' "$RUNNER_TEMP/make-samples.log"; then + echo "::warning title=Sample fixtures degraded::The upstream Wireshark captures were unreachable, so synthesised stand-ins were used. The PCAP-NG tier ran, but not against the upstream bytes." + grep '(download unavailable)' "$RUNNER_TEMP/make-samples.log" + else + echo "Upstream Wireshark captures were used, matching their pinned SHA-256 digests." + fi + + # See the `test` job above for why this step exists. + - name: Report available parallelism + run: | + nproc + python -c "import os; print('cpu_count', os.cpu_count())" + + # See the `integration` job above for why this is asked of + # tests/_tiers.py directly rather than spelled out as literal flags. + - name: Run full test suite + shell: bash + run: | + if ! selection=$(python -c "from tests._tiers import fixture_tier_paths; print(' '.join(fixture_tier_paths()))"); then + echo "::error title=Could not compute the fixture-tier selection::tests._tiers.fixture_tier_paths() failed -- see the traceback above. Refusing to fall back to a bare 'pytest -q', which would silently re-run the entire suite instead of failing loudly." + exit 1 + fi + if [ -z "$selection" ]; then + echo "::error title=Fixture-tier selection is empty::tests._tiers.fixture_tier_paths() returned nothing, which cannot be right -- tests/integration alone should always be part of it. Refusing to run pytest with no arguments." + exit 1 + fi + echo "Fixture-dependent selection: $selection" + python -m pytest -q -n auto --dist load $selection + # ``CHANGELOG.md`` is generated from the newest entry under # ``docs/source/changelog/`` by ``util/changelog_md.py``, so it falls out of step # the moment an entry is edited without regenerating it. That is worth its own @@ -252,8 +455,9 @@ jobs: # to the GitHub Release body: a drifted copy is not merely wrong in the tree, it # is published. # - # Deliberately *not* gated on ``gate-only``, unlike the three jobs above, and - # that is the point of putting it here at all. ``create-release.yml`` calls this + # Deliberately *not* gated on ``gate-only``, unlike the four jobs above + # (`test`, `integration`, `engine-tests`, `pypcap-parity`), and that is the + # point of putting it here at all. ``create-release.yml`` calls this # workflow as its release gate, so an ungated job runs on the release path and # the release body cannot be built from a file that has drifted. The matrix is # skipped per caller because it is expensive and already ran for the commit; @@ -312,12 +516,13 @@ jobs: # --ignore, no tier selection) reaches every one of them regardless of # which tier they live in -- unlike the `test` and `integration` jobs # above, which each cover only the subset their own selection reaches. - # PyPCAPFile is deliberately NOT added, on this job or either of the - # others -- see the `test` job's comment above: it does not merely skip - # cleanly here, it fails for real on 3.10/3.11 against a genuine bug in - # pcapkit/toolkit/pypcapfile.py, and this job runs the full suite - # unfiltered on 3.14, where it would still be a correct no-op today but - # would misleadingly suggest the extra is safe to add everywhere. + # PyPCAPFile is deliberately NOT added here -- see the `test` job's + # comment above for the bug that used to make installing it a + # regression, now fixed by #747/#748. This job runs on 3.14 only, where + # PyPCAPFile's marker resolves to nothing regardless, so adding it here + # would be a no-op that misleadingly suggests the extra is exercised by + # `gate`; #751's `engine-tests` and `pypcap-parity` jobs cover it for + # real, on the 3.10-3.14 (and 3.10-3.11) legs where it actually installs. - name: Install package, test and generator dependencies run: | python -m pip install -U pip setuptools wheel diff --git a/tests/_dependency_gates.py b/tests/_dependency_gates.py index c12b5ef58..dd0c57632 100644 --- a/tests/_dependency_gates.py +++ b/tests/_dependency_gates.py @@ -43,23 +43,50 @@ carries a distribution is read back out of :file:`pyproject.toml`, so renaming an extra or emptying it is caught too. -Two things this deliberately does not model, both recorded here rather than -left to be discovered: - -* **Environment markers.** ``PyPCAPFile = [ "pypcapfile; python_version < - '3.12'" ]`` resolves to nothing on three of the five matrix legs, so an - install line carrying that extra would satisfy this module while two legs - ran the tests and three went on skipping them. :func:`requirement_key` - strips the marker and :func:`provided_by` ignores it; the marker text is kept - on :class:`Requirement` so a diagnostic can say so. -* **Which of two distributions owns a shared import name.** ``pcap`` is shipped - by both ``pypcap`` and ``pcap-ct``, and ``HAS_PYPCAP`` tells them apart by - requiring ``pcap`` and *not* ``pcap._pcap``. :func:`flag_requirements` drops - the negated probe rather than modelling it, so both flags read as needing - ``pcap``. Neither extra is on any :program:`pytest` job's install line today, - so the distinction decides nothing; if one is ever added, the liveness check - on :data:`DEPENDENCY_GATE_EXCLUSIONS` fails and forces this to be thought - about again. +One thing this deliberately does not model, recorded here rather than left to +be discovered: **environment markers.** ``PyPCAPFile = [ "pypcapfile; +python_version < '3.12'" ]`` resolves to nothing on three of the five matrix +legs, so an install line carrying that extra would satisfy this module while +two legs ran the tests and three went on skipping them. :func:`requirement_key` +strips the marker and :func:`provided_by` ignores it; the marker text is kept +on :class:`Requirement` so a diagnostic can say so. + +**Which of two distributions owns a shared import name is modelled, not +tripwired**, and #762 is the history of why that distinction matters. ``pcap`` +is shipped by both ``pypcap`` and ``pcap-ct``, and ``HAS_PYPCAP`` tells them +apart by requiring ``pcap`` and *not* ``pcap._pcap``. :func:`flag_requirements` +resolves the positive half correctly, per flag -- ``HAS_PYPCAP`` needs +``{'pcap'}`` and ``HAS_PCAP_CT`` needs ``{'pcap._pcap'}`` -- but +:func:`extras_providing`'s :data:`MODULE_PROVIDERS` lookup truncates to the +*top-level* package (``module.partition('.')[0]``) before looking anything up, +so ``pcap._pcap`` lands on the very same ``'pcap'`` entry as plain ``pcap`` and +:func:`dependency_gate_gaps` cannot tell PyPCAP's install from PCAP_CT's: a job +installing either would read as satisfying *both* flags' gates. #751 put each +extra on a *different* job's install line and neither job's selection reaches +the *other* flag's gate -- confirmed with :func:`job_reaches` directly -- so +that blind spot never actually bit; a first attempt at closing it (an +allowlist naming the "correct" distribution per ``(job, flag)`` pair) turned +out to be exactly the same shape of trust with an extra layer: it agreed with +whatever the job installed rather than checking it, so flipping a job's +install line and the allowlist entry together stayed silently green. + +:func:`ambiguous_satisfactions` closes it for real, using two pieces of +information :func:`dependency_gate_gaps` never needed on its own. +:func:`module_flag_exclusions` recovers the *negative* half +:func:`flag_requirements` correctly drops -- ``HAS_PYPCAP``'s own ``not +importable('pcap._pcap')`` -- and :func:`module_providers` resolves that +excluded module *exactly*, not truncated: only ``pcap-ct`` really ships +``pcap._pcap``, so it alone is what the negation disqualifies from plain +``pcap``'s two-wide entry. Subtracting the two leaves exactly one legitimate +distribution for each flag, derived from the source rather than hand-written, +so a job installing the wrong half changes what actually resolves without +changing what is "legitimate" -- which is precisely what makes the mismatch +visible. Scoped to :data:`MUTUALLY_EXCLUSIVE_IMPORTS` rather than to every name +:data:`MODULE_PROVIDERS` happens to list more than one requirement string for: +``html5lib`` has two entries there too, but both name the *same* one +distribution (``beautifulsoup4``) reached two ways, not a second one that +could win the import instead -- a length-based scope would have flagged the +first job to gain the unrelated ``vendor`` extra with nothing wrong to report. """ from __future__ import annotations @@ -77,18 +104,24 @@ __all__ = [ 'WORKFLOW', 'PYPROJECT', 'CORE', 'MODULE_PROVIDERS', 'NON_DISTRIBUTION_FLAGS', - 'DEPENDENCY_GATE_EXCLUSIONS', 'Requirement', 'Gate', 'Job', 'Gap', 'Exclusion', + 'MUTUALLY_EXCLUSIVE_IMPORTS', 'DEPENDENCY_GATE_EXCLUSIONS', + 'Requirement', 'Gate', 'Job', 'Gap', 'Exclusion', 'AmbiguousProvider', 'requirement_key', 'provided_by', 'declared_requirements', 'extras_providing', + 'module_providers', 'contested_imports', 'gated_scopes', 'module_gates', 'flag_requirements', 'module_flag_requirements', + 'flag_exclusions', 'module_flag_exclusions', 'pytest_jobs', 'job_sections', 'job_reaches', 'dependency_gate_gaps', 'describe_gap', + 'ambiguous_satisfactions', 'describe_ambiguous_satisfaction', ] #: The workflow holding every job that runs :program:`pytest`, and the only one -#: that runs it at all. Six of the other seven install ``.[all]`` somewhere -- a -#: docs build, a conda recipe, a vendor crawl, the lint pass -- and none of them -#: invokes the suite; :file:`python-compatibility.yml` installs a bare ``.`` and -#: only compiles and imports. That is exactly why this module keys on *jobs that +#: that runs it at all. Five of the other seven install ``.[all]`` somewhere -- +#: a docs build, a conda recipe, a vendor crawl, the lint pass, and the release +#: packaging workflow -- and none of them invokes the suite; +#: :file:`python-compatibility.yml` installs a bare ``.`` and only compiles and +#: imports, and :file:`codeql-analysis.yml` installs nothing explicitly at all +#: (CodeQL's own autobuild step). That is exactly why this module keys on *jobs that #: run pytest* rather than on install lines anywhere in the workflow tree: #: ``.[all]`` carries ``pypcapfile``, ``pyshark`` and ``scapy``, so a guard #: reading those lines would satisfy nearly every flag here vacuously. @@ -104,12 +137,22 @@ #: Import name -> the requirement that has to appear in an extra for it to be #: importable. Several alternatives mean any one of them suffices. #: -#: Keyed on the *top-level* package, so ``pcapfile.savefile`` resolves through -#: ``pcapfile``. Declared rather than derived because an import name and a -#: distribution name are different strings often enough to matter, and the only -#: way to learn the mapping mechanically is to install the distribution and -#: look -- which no test may do. :data:`NON_DISTRIBUTION_FLAGS` covers the -#: flags that ask about something pip cannot install at all. +#: Keyed on the *top-level* package by default, so ``pcapfile.savefile`` +#: resolves through ``pcapfile`` -- :func:`module_providers` and +#: :func:`extras_providing` both fall back to that truncated key when a dotted +#: path has no entry of its own. ``pcap._pcap`` is the one exception, and +#: deliberately so: it has its own, more specific entry below, because unlike +#: every other truncation in this table, the *submodule* and its *top-level +#: package* are shipped by different, mutually exclusive distributions (see +#: :data:`MUTUALLY_EXCLUSIVE_IMPORTS`) -- collapsing them would make the two +#: indistinguishable, which is exactly the #762 defect +#: :func:`ambiguous_satisfactions` exists to catch. +#: +#: Declared rather than derived because an import name and a distribution name +#: are different strings often enough to matter, and the only way to learn the +#: mapping mechanically is to install the distribution and look -- which no +#: test may do. :data:`NON_DISTRIBUTION_FLAGS` covers the flags that ask about +#: something pip cannot install at all. MODULE_PROVIDERS = { # ``pip install -e .`` installs these whatever extras follow. 'aenum': (CORE,), @@ -121,7 +164,10 @@ # distinction is the whole reason ``HAS_CRAWLER_DEPS`` is satisfied in CI # and ``HAS_VENDOR_DEPS`` is not: the ``test`` extra carries plain # ``beautifulsoup4``, and only ``vendor`` and ``all`` carry - # ``beautifulsoup4[html5lib]``. + # ``beautifulsoup4[html5lib]``. The second alternative below is not a + # second *distribution* -- pyproject.toml never declares bare + # ``html5lib`` -- so this is one distribution reached two ways, not the + # kind of ambiguity :data:`MUTUALLY_EXCLUSIVE_IMPORTS` names. 'bs4': ('beautifulsoup4',), 'html5lib': ('beautifulsoup4[html5lib]', 'html5lib'), 'requests': ('requests',), @@ -134,9 +180,18 @@ # compiled; ``pycrate_asn1dir`` is the package the NGAP protocol reads. 'pycrate_asn1dir': ('pycrate',), 'pcapfile': ('pypcapfile',), - # Both distributions ship a top-level ``pcap``; see this module's docstring - # for why that ambiguity is left unmodelled. + # Both distributions ship a top-level ``pcap`` -- genuinely either, which + # is why this entry stays two-wide and why ``pcap`` is the one name in + # :data:`MUTUALLY_EXCLUSIVE_IMPORTS`. 'pcap': ('pypcap', 'pcap-ct'), + # Only ``pcap-ct`` ships this exact submodule; ``pypcap``'s own ``pcap`` + # package has no ``_pcap`` member at all. This is what + # ``HAS_PYPCAP = _importable('pcap') and not _importable('pcap._pcap')`` + # actually distinguishes on, and what :func:`module_flag_exclusions` and + # :func:`ambiguous_satisfactions` use to prune ``pcap-ct`` back out of + # ``HAS_PYPCAP``'s candidates -- see :func:`ambiguous_satisfactions`'s own + # docstring. + 'pcap._pcap': ('pcap-ct',), } #: Flags whose condition is not a distribution, with what it is instead. A flag @@ -149,6 +204,28 @@ ), } +#: Top-level import names two (or more) genuinely *mutually exclusive* +#: distributions can ship, where installing the wrong one silently satisfies +#: the wrong flag. Declared explicitly rather than inferred from +#: ``len(MODULE_PROVIDERS[name]) > 1``, because that length counts alternative +#: *requirement strings*, not competing distributions -- ``html5lib`` also has +#: two entries in :data:`MODULE_PROVIDERS` and is not this: both name the same +#: one distribution (``beautifulsoup4``), reached two ways, and a job that +#: happened to gain the ``vendor`` extra would trip a length-based check with +#: no wrong half to report. ``pcap`` is the one name that is genuinely +#: contested -- ``pypcap`` and ``pcap-ct`` are two different sdists that both +#: create a top-level ``pcap`` package -- and :func:`ambiguous_satisfactions` +#: is scoped to exactly this set. +#: +#: Hand-written rather than computed, for the same reason +#: :data:`DEPENDENCY_GATE_EXCLUSIONS` is: a human decides *that* a name is +#: contested, with a reason worth reading. What is checked, not trusted, is +#: whether this set still matches reality: a liveness test asserts it equals +#: :func:`contested_imports`, so a name going contested without a matching +#: entry here -- the omission a hand-written set cannot itself notice -- fails +#: loudly instead of silently reopening #762 under a different name. +MUTUALLY_EXCLUSIVE_IMPORTS = frozenset({'pcap'}) + class Exclusion(NamedTuple): """A gap that is known, deliberate, and not to be reported as a failure.""" @@ -179,7 +256,15 @@ class Exclusion(NamedTuple): 'the 3.10 and 3.11 legs and nothing at all on the other three. #738 records ' 'the same exclusion, if less precisely -- pypcap needs the headers, pcap-ct ' 'below does not. The flag also requires pcap._pcap to be *absent*, which is ' - "how it tells upstream pypcap from pcap-ct; see this module's docstring." + "how it tells upstream pypcap from pcap-ct; see this module's docstring.\n\n" + "#751's ruling was to try building it in CI rather than declining it forever " + '-- "try to build and if the CI is not a good suit, then we ripe it" -- so ' + 'the new pypcap-parity job now installs a toolchain plus libpcap headers and ' + 'attempts it on the 3.10/3.11 legs the marker allows. integration and gate ' + 'stay dark deliberately: the ruling on #751 was a dedicated job, not one ' + "more install line on either of those two. If pypcap-parity's build step " + 'turns out not to be a good fit for CI, the fix is deleting that job and ' + 'widening this exclusion to name it too, with the failing run linked.' ), ), 'HAS_PCAP_CT': Exclusion( @@ -191,7 +276,14 @@ class Exclusion(NamedTuple): 'pyproject.toml keeps them out of the all extra -- and its ctypes loader ' 'still calls find_library("pcap"), so a system libpcap has to be present on ' 'the runner at run time. A green install would therefore not imply the ' - 'engine can start.' + 'engine can start.\n\n' + "#751's dedicated engine-tests job now takes this: it installs PCAP_CT and a " + 'system libpcap (apt-get libpcap0.8) across the full 3.10-3.14 matrix, kept ' + 'in a venv of its own since pypcap and pcap-ct both install a top-level ' + '``pcap`` module and cannot coexist -- see ' + "pcapkit/foundation/engines/_pcap_backend.py's own docstring. test and gate " + "stay dark on purpose, per #751's ruling that per-engine coverage gets its " + 'own job rather than one more install line on either.' ), ), 'HAS_PYPCAPFILE': Exclusion( @@ -206,11 +298,22 @@ class Exclusion(NamedTuple): 'the five matrix legs. Adding it would satisfy this guard -- which does not ' 'model markers -- while 3.12, 3.13 and 3.14 went on skipping: two legs of ' 'real coverage bought with exactly the false confidence #745 exists to ' - 'remove. Whether that trade is worth taking is #751.' + "remove.\n\n" + "#751's ruling was to take that trade: engine-tests now installs PyPCAPFile " + "across the full 3.10-3.14 matrix, covering the 9 HAS_PYPCAPFILE methods in " + 'test_pypcapfile_unit.py on the 3.10/3.11 legs where the marker lets it ' + 'resolve, and pypcap-parity does the same for the other 6 in ' + 'test_new_engine_parity_runtime.py. This guard still cannot see that only ' + 'two of five legs run for real -- it does not evaluate markers, by this ' + "module's own docstring -- so that partial coverage is recorded here in " + 'prose rather than modelled: the alternative, teaching this guard markers, ' + 'is out of scope for a workflow-only change. test, integration and gate stay ' + "dark on purpose, per #751's ruling that per-engine coverage gets its own " + 'job rather than one more install line on any of the three.' ), ), 'HAS_VENDOR_DEPS': Exclusion( - dark={'test': ('html5lib',), 'gate': ('html5lib',)}, + dark={'test': ('html5lib',), 'gate': ('html5lib',), 'engine-tests': ('html5lib',)}, reason=( "Ruled onto a non-blocking leg by #738's option (b), because the crawlers " 'fetch from IANA and Wikipedia and #518 records four Wikipedia 403s and a ' @@ -220,7 +323,15 @@ class Exclusion(NamedTuple): 'requests and bs4 have shipped in the test extra since #507 and are ' 'installed on every job, so these five classes are one *requirement extra* ' 'short -- html5lib, which only beautifulsoup4[html5lib] provides, i.e. the ' - 'vendor and all extras.' + 'vendor and all extras.\n\n' + "engine-tests (#751) joins test and gate here rather than closing the gap: " + "its selection mirrors test's ignore-shape exactly (same --ignore flags), so " + 'it reaches the same unit-tier HAS_VENDOR_DEPS gates and is dark on html5lib ' + 'for the identical reason test is. pypcap-parity does not appear here even ' + 'though it also declines html5lib, because its fixture-tier selection never ' + "reaches tests/vendor/test_ipx_socket_unit.py in the first place -- same " + 'shape as integration above it, which is why integration is not listed ' + 'either.' ), ), 'HAS_SCAPY': Exclusion( @@ -236,18 +347,43 @@ class Exclusion(NamedTuple): 'integration job never reaches them either. The gate job does, but it runs ' "only where a caller passes gate-only: true -- the three Saturday schedules " '(deploy-pages, cron-vendor, cron-conda) and a v* release tag -- never on a ' - 'pull request or a push to main. So no per-PR leg runs them at all. #751.' + 'pull request or a push to main. So no per-PR leg runs them at all.\n\n' + "#751's engine-tests job now installs Scapy across the full 3.10-3.14 " + 'matrix, closing the "no per-PR leg" problem this exclusion used to ' + 'describe -- it reaches 10 of this guard\'s 14 HAS_SCAPY methods, the same ' + 'unit-tier subset #738 counted; the other 4 live in tests/integration/ or ' + "match the *_runtime.py ignore-glob, already covered by Scapy on the " + "integration and gate jobs. test stays dark on purpose: the ruling on #751 " + 'was a dedicated job precisely so test would not have to reconsider the ' + 'cost question it already answered.' ), ), 'HAS_PYSHARK': Exclusion( dark={'test': ('pyshark',), 'integration': ('pyshark',), 'gate': ('pyshark',)}, reason=( 'One of #738\'s seven, left out of #740\'s "cheap, uncontroversial subset" ' - 'and never since installed, so its 3 gated methods run nowhere. Two of them ' + 'and never since installed, so its 4 gated methods run nowhere. Three of them ' 'assert what PyShark.unsupported_reason says on an interpreter where the ' 'engine cannot work, so they need the distribution importable to have ' - 'anything to ask -- and pyshark wants a tshark binary on the runner to be ' - 'more than importable, which is a separate decision from a pip extra. #751.' + 'anything to ask.\n\n' + "#751's later ruling asked to try a tshark binary the same way it asked to " + 'try building pypcap: "try to build and if the CI is not a good suit, then ' + 'we ripe it… same for HAS_PYSHARK on tshark dependency." ' + 'test_the_reason_tracks_the_running_interpreter used to call ' + 'PyShark.unsupported_reason() unpatched and hard-assert the reason names ' + '"tshark" on every version below the asyncio ceiling -- true only while ' + 'tshark stayed absent, and it would have turned that assertion from a pass ' + 'into a failure the moment tshark was installed. Fixed by probing the same ' + 'way PyShark.unsupported_reason() itself does -- pyshark\'s own ' + "get_process_path(), not shutil.which(), which its own config.ini " + 'precedence can make disagree with -- matching what its sibling ' + 'test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous already ' + 'used, so the file no longer disagrees with itself about whether tshark may ' + 'be present. engine-tests now installs both the distribution and tshark ' + '(apt-get, with a debconf pre-seed so the postinst prompt does not block) ' + 'across the full 3.10-3.14 matrix, closing the test-job gap this exclusion ' + "used to describe. integration and gate stay dark on purpose, per #751's " + 'ruling that per-engine coverage gets its own job.' ), ), 'HAS_RUNTIME': Exclusion( @@ -260,9 +396,17 @@ class Exclusion(NamedTuple): 'four plus dpkt, scapy and pyshark, so its two classes (5 methods) skip ' 'wherever any of the three is absent. It is unit-tier despite the name, ' 'because the ignore-glob is *_runtime.py and the file is ' - 'test_runtime_engines.py, so the test job is what reaches it. #751; the ' - 'narrow fix is to give that flag a name of its own, so the skip reason says ' - 'which dependency was missing.' + 'test_runtime_engines.py, so the test job is what reaches it. The narrow ' + 'fix -- giving that flag a name of its own, so the skip reason says which ' + 'dependency was missing -- is left for a follow-up: it touches a test ' + "module #751 did not otherwise need to change, and this guard does not " + 'care what a flag is named, only whether the job that reaches it installs ' + 'what it asks for.\n\n' + "#751's engine-tests job installs DPKT, Scapy and PyShark together, closing " + "this gap as a side effect of the per-engine extras rather than a separate " + 'install line: all 5 methods run wherever engine-tests does. test and gate ' + 'stay dark on purpose, for the same reason HAS_SCAPY and HAS_PYSHARK above ' + 'do.' ), ), } @@ -325,6 +469,39 @@ class Gap(NamedTuple): gates: 'tuple[Gate, ...]' +class AmbiguousProvider(NamedTuple): + """A satisfied gate resolved through an import name more than one distribution ships. + + Not a :class:`Gap` -- the job in question *did* install something that + provides the module, so :func:`dependency_gate_gaps` calls it satisfied. + What this reports is the question that function never asks: whether the + thing it installed is the *right* one of several possible providers, or + merely one of them. + + """ + + #: The flag. + flag: 'str' + #: The job whose install line satisfied it. + job: 'str' + #: The import name more than one *distribution* ships. + module: 'str' + #: Every distribution :func:`extras_providing`/:func:`dependency_gate_gaps` + #: would credit with shipping ``module`` -- the *unnarrowed* set, keyed on + #: ``module``'s top-level package the way that function always is. + providers: 'frozenset[str]' + #: Of those, the ones the job's install line actually resolves. What + #: :func:`dependency_gate_gaps` treats as proof the gate is satisfied. + satisfied: 'frozenset[str]' + #: The distribution(s) that would *genuinely* leave the flag true -- + #: :func:`module_providers`'s exact-path-aware reading of ``module``, minus + #: whatever :func:`module_flag_exclusions` says this flag needs absent. + #: ``satisfied`` is flagged as ambiguous precisely when it disagrees with + #: this set: resolving to something outside it, or to more than one thing + #: even inside it. + valid: 'frozenset[str]' + + def requirement_key(text: 'str') -> 'Requirement': """Split one requirement string into a :class:`Requirement`. @@ -437,6 +614,63 @@ def declared_requirements() -> 'dict[str, tuple[Requirement, ...]]': return declared +def module_providers(module: 'str') -> 'frozenset[str]': + """Every distribution (or :data:`CORE`) :data:`MODULE_PROVIDERS` says could ship ``module``. + + Unlike :func:`extras_providing`, which this deliberately does not touch, + keyed on the dotted path *exactly as given* when :data:`MODULE_PROVIDERS` + has a dedicated entry for it -- ``pcap._pcap`` resolves to ``('pcap-ct',)`` + 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). + + :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]])) + + +def contested_imports() -> 'frozenset[str]': + """What :data:`MUTUALLY_EXCLUSIVE_IMPORTS` should be, derived rather than trusted. + + For each :data:`MODULE_PROVIDERS` entry, counts how many of its + alternative requirement strings some declared extra actually provides -- + not how many alternatives the entry merely lists. ``html5lib``'s two + alternatives (``beautifulsoup4[html5lib]``, plain ``html5lib``) resolve to + one *live* distribution, because pyproject.toml never declares bare + ``html5lib`` under any extra; ``pcap``'s two (``pypcap``, ``pcap-ct``) + resolve to two. A top-level name qualifies once two or more of its + alternatives are live -- genuinely reachable by installing a real, + different thing -- which is exactly the question + :func:`ambiguous_satisfactions` has to ask, and exactly what separates + ``pcap`` from the ``html5lib`` near-miss automatically. + + This is the liveness half :data:`MUTUALLY_EXCLUSIVE_IMPORTS` does not have + on its own: a hand-written set can go stale exactly the way this module's + own docstring warns a skip list does (#745) -- it would not notice a + *third* contested name go undeclared, only a wrong entry among the ones + already there. Comparing this function's answer against the hand-written + set is what closes that gap. + + """ + declared = declared_requirements() + + def _is_live(provider: 'str') -> 'bool': + return provider == CORE or any( + provided_by(requirements, provider) for requirements in declared.values()) + + contested = set() # type: set[str] + for module, providers in MODULE_PROVIDERS.items(): + if sum(1 for provider in providers if _is_live(provider)) >= 2: + contested.add(module.partition('.')[0]) + return frozenset(contested) + + @functools.lru_cache(maxsize=None) def extras_providing(module: 'str') -> 'frozenset[str]': """Every extra (or :data:`CORE`) whose requirements make ``module`` importable. @@ -491,8 +725,9 @@ def _string_sequence(node: 'ast.expr', names: 'dict[str, tuple[str, ...]]') -> ' def _probed_modules(node: 'ast.AST', names: 'dict[str, tuple[str, ...]]', helpers: 'dict[str, ast.FunctionDef]', negated: 'bool' = False, - seen: 'Optional[frozenset[str]]' = None) -> 'Iterator[str]': - """Module names ``node`` requires to be importable. + seen: 'Optional[frozenset[str]]' = None, + *, want_negated: 'bool' = False) -> 'Iterator[str]': + """Module names ``node`` requires to be importable, or requires *absent*. Walked by hand rather than with :func:`ast.walk` for one reason: polarity. ``HAS_PYPCAP = _importable('pcap') and not _importable('pcap._pcap')`` @@ -500,6 +735,14 @@ def _probed_modules(node: 'ast.AST', names: 'dict[str, tuple[str, ...]]', cannot tell them apart -- it would report ``HAS_PYPCAP`` as needing a module whose presence makes it false. + ``want_negated`` picks which side this call collects: :data:`False` (the + default, and every call site before :func:`module_flag_exclusions` existed) + yields the positively-required modules, as before. :data:`True` yields the + mirror image -- the modules a negated probe asks to be absent -- which is + what :func:`module_flag_exclusions` asks for. A module is never yielded by + both calls on the same expression: exactly one of ``negated == want_negated`` + holds at the point a probe is reached. + A call to a module-level helper contributes both its own string arguments (``_importable('pcap._pcap')``) and whatever its body probes (``_has_pypcapfile()``, which names its modules inside), because the suite @@ -510,25 +753,28 @@ def _probed_modules(node: 'ast.AST', names: 'dict[str, tuple[str, ...]]', seen = frozenset() if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not): - yield from _probed_modules(node.operand, names, helpers, not negated, seen) + yield from _probed_modules(node.operand, names, helpers, not negated, seen, + want_negated=want_negated) return if isinstance(node, ast.Call): helper = node.func.id if isinstance(node.func, ast.Name) else None if _module_probes(node): - if not negated: + if negated == want_negated: yield from _string_args(node, names) return if helper is not None and helper in helpers and helper not in seen: - if not negated: + if negated == want_negated: yield from _string_args(node, names) for statement in helpers[helper].body: yield from _probed_modules(statement, names, helpers, - negated, seen | {helper}) + negated, seen | {helper}, + want_negated=want_negated) return for child in ast.iter_child_nodes(node): - yield from _probed_modules(child, names, helpers, negated, seen) + yield from _probed_modules(child, names, helpers, negated, seen, + want_negated=want_negated) def _string_bindings(tree: 'ast.Module', expression: 'ast.expr') -> 'dict[str, tuple[str, ...]]': @@ -586,6 +832,45 @@ def module_flag_requirements(path: 'pathlib.Path') -> 'dict[str, frozenset[str]] return definitions +def module_flag_exclusions(path: 'pathlib.Path') -> 'dict[str, frozenset[str]]': + """``HAS_*`` flags this module defines, and the modules each requires *absent*. + + The mirror of :func:`module_flag_requirements`, collecting exactly the + negated probes that function drops. Dropping them there is safe -- + :func:`dependency_gate_gaps` only ever asks "is something installed that + provides the positive requirement", and a job's install line cannot make a + module *un*-importable, so the negative half never changes what that + question needs. It stops being safe the moment two distributions can ship + the same positive import name, because then *which* distribution is + providing it decides the negative half's answer too -- which is exactly + what :func:`ambiguous_satisfactions` uses this for (#762): ``HAS_PYPCAP``'s + ``pcap`` requirement is satisfied by either ``pypcap`` or ``pcap-ct``, but + only the first also leaves its own ``not importable('pcap._pcap')`` + requirement true, and this is what says so. + + Most flags have nothing here -- an empty result means "no negated probe", + not "unresolved"; :func:`module_flag_requirements` is still what decides + whether the flag was understood at all. + + """ + try: + tree = ast.parse(path.read_text(encoding='utf-8')) + except (OSError, SyntaxError): + return {} + + helpers = {node.name: node for node in tree.body if isinstance(node, ast.FunctionDef)} + definitions = {} # type: dict[str, frozenset[str]] + for node in tree.body: + if not isinstance(node, ast.Assign): + continue + for target in node.targets: + if isinstance(target, ast.Name) and target.id.startswith('HAS_'): + bindings = _string_bindings(tree, node.value) + definitions[target.id] = frozenset( + _probed_modules(node.value, bindings, helpers, want_negated=True)) + return definitions + + def _flag_imports(path: 'pathlib.Path') -> 'dict[str, str]': """``HAS_*`` flags this module imports, and the module path each came from.""" try: @@ -638,6 +923,36 @@ def flag_requirements() -> 'dict[tuple[str, str], frozenset[str]]': return resolved +@functools.lru_cache(maxsize=1) +def flag_exclusions() -> 'dict[tuple[str, str], frozenset[str]]': + """``(module, flag)`` -> the modules that flag needs *not* importable. + + The mirror of :func:`flag_requirements`, built the same way and for the + same per-file reason (see that function's own docstring) -- and empty for + every ``(module, flag)`` pair :func:`flag_requirements` resolves at all, + except the handful with a genuine negated probe. See + :func:`module_flag_exclusions` for why that handful matters. + + """ + definitions = {} # type: dict[str, dict[str, frozenset[str]]] + for path in sorted(_tiers.TESTS_ROOT.rglob('*.py')): + relative = path.relative_to(_tiers.ROOT).as_posix() + definitions[relative] = module_flag_exclusions(path) + + resolved = {} # type: dict[tuple[str, str], frozenset[str]] + for relative, flags in definitions.items(): + for flag, modules in flags.items(): + resolved[(relative, flag)] = modules + + for gate in gated_scopes(): + if (gate.module, gate.flag) in resolved: + continue + origin = _flag_imports(_tiers.ROOT / gate.module).get(gate.flag) + if origin is not None and gate.flag in definitions.get(origin, {}): + resolved[(gate.module, gate.flag)] = definitions[origin][gate.flag] + return resolved + + @functools.lru_cache(maxsize=1) def gated_scopes() -> 'tuple[Gate, ...]': """Every ``skipUnless(HAS_*, ...)`` gate in a module :program:`pytest` collects. @@ -647,8 +962,10 @@ def gated_scopes() -> 'tuple[Gate, ...]': scan: ``python_files`` in :file:`pyproject.toml` is what pytest collects, so a gate in a helper module gates nothing. Class-level and method-level decorators are both collected, and the class-level ones are the majority -- - ``HAS_DPKT`` reaches 20 methods through 6 class decorators and none through - a method decorator. + ``HAS_DPKT`` reaches 20 methods through 6 class decorators and 8 through a + method decorator -- counted by exact-match AST identifiers the way this + function itself matches them, not by a substring search, which would + (elsewhere) fold ``HAS_PYPCAP`` together with ``HAS_PYPCAPFILE``. """ gates = [] # type: list[Gate] @@ -891,3 +1208,160 @@ def describe_gap(gap: 'Gap') -> 'str': f'line, or record the gap in tests._dependency_gates.' f'DEPENDENCY_GATE_EXCLUSIONS with a reason.\n{where}' ) + + +def _disqualified_providers(excluded_modules: 'frozenset[str]') -> 'frozenset[str]': + """Distributions a flag's own negation rules out, exact entries only. + + Deliberately does *not* fall back to a top-level entry the way + :func:`module_providers` does for a *required* module: an excluded module + with no entry of its own would otherwise borrow its top-level package's + whole provider set, and if that top-level name is itself contested (in + :data:`MUTUALLY_EXCLUSIVE_IMPORTS`) the borrowed set disqualifies *every* + candidate rather than the one the negation actually names -- a finding + with a wrong diagnosis rather than the right one, or none at all. Failing + loudly and naming what is missing is the fix; a caller is only ever + exposed to this for a module that is actually reached by a real gate, + since :func:`ambiguous_satisfactions` only calls this once it has already + confirmed the *required* module it is checking is contested -- an + excluded module unrelated to any contested name never reaches here at + all. + + """ + disqualified = set() # type: set[str] + for excluded in excluded_modules: + exact = MODULE_PROVIDERS.get(excluded) + if exact is None: + top_level = excluded.partition('.')[0] + if top_level in MUTUALLY_EXCLUSIVE_IMPORTS: + raise AssertionError( + f'{excluded!r} is excluded by a negated probe but has no exact ' + f'MODULE_PROVIDERS entry, and its top-level {top_level!r} is in ' + f'MUTUALLY_EXCLUSIVE_IMPORTS -- falling back to that entry would ' + f"disqualify more than the negation actually names. Add a " + f'dedicated MODULE_PROVIDERS[{excluded!r}] entry.' + ) + continue # not contested; module_providers()'s ordinary fallback is fine + disqualified.update(exact) + return frozenset(disqualified) - {CORE} + + +def ambiguous_satisfactions( + workflow: 'Optional[pathlib.Path]' = None, +) -> 'tuple[AmbiguousProvider, ...]': + """Every satisfied gate resolved through the wrong half of a contested import name. + + :func:`dependency_gate_gaps` only ever asks "does at least one distribution + this job installs ship this module" -- which is blind to *which* one. + ``pcap`` is shipped by both ``pypcap`` and ``pcap-ct``, and (via that + function's own top-level truncation) so, as far as it can tell, is + ``pcap._pcap``. A job that installed the wrong one of the two would still + read as satisfied there; see this module's own docstring for the full + shape of that gap (#762). + + This is the check that notices, scoped to exactly the names + :data:`MUTUALLY_EXCLUSIVE_IMPORTS` declares genuinely contested -- + deliberately *not* every name :data:`MODULE_PROVIDERS` lists more than one + requirement string for for. ``html5lib`` also has two entries there, and + is not this: both name the same one distribution reached two ways, not two + distributions that could each independently win the import, so scoping on + ``len(MODULE_PROVIDERS[...]) > 1`` instead would false-positive on it the + moment some job gained the ``vendor`` extra. + + For a gated module in that set, the *true* set of distributions that would + actually leave the flag true is :func:`module_providers`'s exact-path + reading of the module (already narrower for ``pcap._pcap``, which only + ``pcap-ct`` really ships) minus whatever :func:`module_flag_exclusions` + says the same flag needs *absent* (which is how ``HAS_PYPCAP`` -- true + only when ``pcap._pcap`` is *not* importable -- prunes ``pcap-ct`` back + out of plain ``pcap``'s two-wide entry, with no table to hand-maintain). + A finding is reported when what :func:`dependency_gate_gaps` would call + "satisfied" disagrees with that true set: resolving to a distribution + outside it (the #762 shape -- the job installed the wrong half), or to + more than one distribution even inside it (genuinely still ambiguous). + + A module no installed extra resolves to any distribution for is what + :func:`dependency_gate_gaps` already reports as a :class:`Gap`, so it is + skipped here too rather than duplicated. + + """ + requirements = flag_requirements() + exclusions = flag_exclusions() + declared = declared_requirements() + findings = [] # type: list[AmbiguousProvider] + + for job in pytest_jobs(workflow): + available_extras = frozenset(job.extras) | {CORE} + for gate in gated_scopes(): + if gate.flag in NON_DISTRIBUTION_FLAGS: + continue + modules = requirements.get((gate.module, gate.flag)) + if not modules or not job_reaches(job, gate): + continue + + for module in modules: + if module.partition('.')[0] not in MUTUALLY_EXCLUSIVE_IMPORTS: + continue + + # Only computed once the module above is confirmed contested, + # not for every gate this job reaches: an excluded module with + # no MODULE_PROVIDERS entry of its own raises here (see + # _disqualified_providers), and a flag with nothing to do with + # a contested name must never pay for that -- see this + # function's own history (#762 round 3). + excluded_modules = exclusions.get((gate.module, gate.flag), frozenset()) + disqualified = _disqualified_providers(excluded_modules) + + providers = MODULE_PROVIDERS[module.partition('.')[0]] + satisfied = frozenset( + provider for provider in providers + if provider == CORE or any( + provided_by(declared[extra], provider) + for extra in available_extras if extra in declared + ) + ) + if not satisfied: + continue # dependency_gate_gaps already reports this as a Gap + + valid = module_providers(module) - disqualified + if satisfied == (satisfied & valid) and len(satisfied) <= 1: + continue # everything that resolved is legitimate, and unambiguous + + findings.append(AmbiguousProvider(gate.flag, job.name, module, + frozenset(providers), satisfied, valid)) + + seen = set() # type: set[tuple[str, str, str]] + unique = [] # type: list[AmbiguousProvider] + for finding in findings: + key = (finding.flag, finding.job, finding.module) + if key in seen: + continue + seen.add(key) + unique.append(finding) + + return tuple(sorted(unique, key=lambda finding: (finding.flag, finding.job, finding.module))) + + +def describe_ambiguous_satisfaction(finding: 'AmbiguousProvider') -> 'str': + """A failure message naming the flag, the job, and why the satisfaction is untrusted.""" + illegitimate = sorted(finding.satisfied - finding.valid) + if illegitimate: + verdict = ( + f'it resolves to {illegitimate}, which the flag\'s own negated probe ' + f'excludes (module_flag_exclusions), not to {sorted(finding.valid)}' + ) + else: + verdict = ( + f'it resolves to more than one legitimate distribution at once, ' + f'{sorted(finding.satisfied)}' + ) + return ( + f"the {finding.job!r} job reaches a {finding.flag} gate satisfied through " + f'{finding.module!r}, which more than one distribution ships ' + f'({sorted(finding.providers)}) -- {verdict}. This is the #762 shape: a job ' + f'installing the wrong half of an ambiguous shared import name would read as ' + f'satisfied purely because it installs *some* distribution that ships the same ' + f'top-level module. Fix the job\'s install line, or -- if the flag\'s own probe ' + f'genuinely cannot distinguish the two -- correct ' + f'tests._dependency_gates.MODULE_PROVIDERS or the flag\'s negated probe so it can.' + ) diff --git a/tests/foundation/engines/test_pyshark_engine.py b/tests/foundation/engines/test_pyshark_engine.py index 4d9e03fca..9182624bb 100644 --- a/tests/foundation/engines/test_pyshark_engine.py +++ b/tests/foundation/engines/test_pyshark_engine.py @@ -19,11 +19,22 @@ * **the** :program:`tshark` **binary** -- ``pyshark`` shells out to it and parses nothing itself. -What this host could and could not provide, stated plainly rather than left to a -silent skip: :program:`tshark` is **not** installed here, so the "missing binary" -path is exercised for real. The "binary present" path is exercised by patching -``pyshark``'s own resolver, since installing Wireshark was not an option; and the -interpreter is 3.14, so every version below the ceiling is reached by patching +What the running host provides is no longer assumed either way, and that is worth +stating plainly rather than leaving to a silent skip: :program:`tshark` is absent +on most of this file's legs, but #751's ``engine-tests`` job -- which collects this +same module -- installs it via ``apt-get``, so neither "present" nor "absent" can +be relied on. The "missing binary" path is exercised for real by +:meth:`PySharkUnsupportedReasonTests.test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous`, +which self-skips wherever tshark turns out to be on this host's :envvar:`PATH` +instead of assuming the answer; the "binary present" path is exercised mostly by +patching ``pyshark``'s own resolver, since a real tshark is not guaranteed on +every leg that collects this module. The one test that needs *both* states on +demand -- +:meth:`PySharkUnsupportedReasonTests.test_a_config_ini_naming_an_off_path_tshark_is_not_read_as_missing` +-- constructs them explicitly rather than depending on whatever the host +happens to provide, which is what makes it pass on ``engine-tests`` (tshark on +``PATH``) and everywhere else (no tshark on ``PATH``) alike. The interpreter is +3.14, so every version below the ceiling is reached by patching :data:`sys.version_info`, which is the same technique :mod:`tests.foundation.engines.test_pypcapfile_engine` uses. @@ -47,6 +58,34 @@ SUPPORTED_VERSION = (3, 11, 0, 'final', 0) +def _tshark_missing() -> bool: + """Whether pyshark's own resolver cannot find :program:`tshark`, right now. + + Deliberately not :func:`shutil.which`. ``PyShark.unsupported_reason``'s own + docstring says why the two disagree: pyshark reads ``tshark_path`` from a + ``config.ini`` on :func:`pathlib.Path.cwd` *before* consulting + :envvar:`PATH`, so a ``./config.ini`` naming an off-``PATH`` tshark makes + ``which`` report "absent" while pyshark finds it anyway. Probing with + ``which`` here would then disagree with what ``unsupported_reason`` itself + reports, and the tests below that rely on this would fail against a host + they were written to pass on. + + """ + try: + from pyshark.tshark.tshark import get_process_path # isort:skip + except ImportError: + # Not installed; every caller of this function is gated on + # HAS_PYSHARK, so this is not expected, but ``unsupported_reason`` + # treats it as "no reason to report here" rather than "tshark is + # missing", and this mirrors that. + return False + try: + get_process_path() + except Exception: # pylint: disable=broad-except + return True + return False + + @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class PySharkUnsupportedReasonTests(unittest.TestCase): def resolver(self, *, found: bool): @@ -107,11 +146,21 @@ def test_the_reason_tracks_the_running_interpreter(self) -> None: self.assertIn('asyncio', reason) # type: ignore[arg-type] self.assertIn(f'{sys.version_info[0]}.{sys.version_info[1]}', reason) # type: ignore[arg-type] - else: - # below the ceiling the verdict is about tshark, which this host does - # not have -- so still a reason, but a different one + elif _tshark_missing(): + # below the ceiling the verdict is about tshark, on a host that does + # not have it -- so still a reason, but a different one. Probed the + # same way test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous + # is, and the same way PyShark.unsupported_reason() itself is -- + # not shutil.which, which can disagree with it (see + # _tshark_missing's docstring): this test runs on whatever host it + # is given, tshark installed or not, rather than assuming the + # answer. self.assertIsNotNone(reason) self.assertIn('tshark', reason) # type: ignore[arg-type] + else: + # tshark is on this host and the interpreter is supported: nothing + # is wrong, so there is no reason at all. + self.assertIsNone(reason) def test_the_ceiling_is_decided_by_version_not_by_an_import(self) -> None: """The verdict must not depend on whether ``pyshark`` is installed. @@ -182,11 +231,9 @@ def test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous(self) -> Non :meth:`test_a_present_binary_is_no_reason_at_all` is the real check. """ - import shutil - from pcapkit.foundation.engines.pyshark import PyShark - if shutil.which('tshark') is not None: + if not _tshark_missing(): self.skipTest('tshark is installed on this host') with mock.patch.object(sys, 'version_info', SUPPORTED_VERSION): @@ -195,6 +242,81 @@ def test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous(self) -> Non self.assertIsNotNone(reason) self.assertIn('tshark', reason) # type: ignore[arg-type] + @unittest.skipUnless(HAS_PYSHARK, 'pyshark not installed') + def test_a_config_ini_naming_an_off_path_tshark_is_not_read_as_missing(self) -> None: + """Regression for the cross-review's measured "2 failed". + + pyshark's own ``get_process_path()`` reads ``tshark_path`` from a + ``config.ini`` on :func:`pathlib.Path.cwd` *before* it ever consults + :envvar:`PATH` -- see :func:`_tshark_missing`'s docstring. This pins the + exact case that makes :func:`shutil.which` the wrong probe: a tshark + stand-in that ``config.ini`` names but that sits in a directory never on + ``PATH``, so ``which`` cannot resolve *that* path while pyshark finds it + anyway. A probe keyed on ``which`` -- what this file used before -- would + call this "missing" and fail both + :meth:`test_the_reason_tracks_the_running_interpreter` and + :meth:`test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous`; + :func:`_tshark_missing` and ``PyShark.unsupported_reason`` must not. + + Deliberately host-independent, and that is itself the regression: an + earlier version of this test asserted ``shutil.which('tshark') is + None``, i.e. that the *host* has no tshark anywhere on ``PATH`` at all + -- true on the machine it was written on, false on every + ``engine-tests`` CI leg once this PR's own ``apt-get install … + tshark`` step runs, which is exactly the asymmetry that escaped + review. The claim this test actually needs is narrower: that + ``which`` cannot resolve *the stand-in specifically*, because its + directory was never put on ``PATH`` -- true regardless of whether + some unrelated ``tshark`` happens to sit on ``PATH`` elsewhere. Both + worlds are constructed and checked below rather than left to + whatever the running host happens to provide. + + """ + import os + import pathlib + import shutil + import tempfile + + import pyshark.config + + from pcapkit.foundation.engines.pyshark import PyShark + + with tempfile.TemporaryDirectory(prefix='pcapkit-pyshark-config-') as tmpdir, \ + tempfile.TemporaryDirectory(prefix='pcapkit-pyshark-empty-path-') as empty_dir, \ + tempfile.TemporaryDirectory(prefix='pcapkit-pyshark-host-path-') as host_dir: + stand_in = pathlib.Path(tmpdir) / 'tshark' + stand_in.touch() + config_path = pathlib.Path(tmpdir) / 'config.ini' + config_path.write_text(f'[tshark]\ntshark_path = {stand_in}\n', encoding='utf-8') + + # A second, unrelated `tshark` that *is* on PATH -- standing in for + # the real one `engine-tests` apt-get installs. Executable, because + # shutil.which() on POSIX requires os.X_OK. + host_tshark = pathlib.Path(host_dir) / 'tshark' + host_tshark.touch() + host_tshark.chmod(0o755) + + worlds = ( + ('no tshark on PATH at all', empty_dir, None), + ('a different, real tshark on PATH', host_dir, str(host_tshark)), + ) + for world, path_dir, expected_which in worlds: + with self.subTest(world=world): + with mock.patch.dict(os.environ, {'PATH': path_dir}): + # The premise this test depends on: `which` resolves to + # whatever this world's PATH says (nothing, or the + # unrelated host tshark) -- never to the stand-in, + # because the stand-in's directory is not in PATH + # either way. + which_result = shutil.which('tshark') + self.assertEqual(which_result, expected_which) + self.assertNotEqual(which_result, str(stand_in)) + + with mock.patch.object(pyshark.config, 'fp_config_path', config_path): + self.assertFalse(_tshark_missing()) + with mock.patch.object(sys, 'version_info', SUPPORTED_VERSION): + self.assertIsNone(PyShark.unsupported_reason()) + def test_an_absent_pyshark_is_left_to_the_import_test(self) -> None: from pcapkit.foundation.engines.pyshark import PyShark diff --git a/tests/test_tier_guard.py b/tests/test_tier_guard.py index edb2f1c1a..c283833ea 100644 --- a/tests/test_tier_guard.py +++ b/tests/test_tier_guard.py @@ -1142,6 +1142,12 @@ def test_a_negated_probe_is_not_a_requirement(self) -> None: flat :func:`ast.walk` would report the flag as *needing* ``pcap._pcap`` -- the one module whose presence makes it false. + :func:`~tests._dependency_gates.module_flag_exclusions` is the mirror + image, pinned on the same module: it is what recovers ``pcap._pcap`` + as the thing this flag needs *absent*, which is exactly what + :func:`~tests._dependency_gates.module_flag_requirements` correctly + drops. + """ module = write_module(self.tmp_path, 'test_negated_unit.py', """ import importlib @@ -1160,6 +1166,8 @@ def _importable(*modules): """) self.assertEqual(_dependency_gates.module_flag_requirements(module), {'HAS_PYPCAP': frozenset({'pcap'})}) + self.assertEqual(_dependency_gates.module_flag_exclusions(module), + {'HAS_PYPCAP': frozenset({'pcap._pcap'})}) def test_a_flag_that_asks_about_something_pip_cannot_install_requires_nothing(self) -> None: """``HAS_PROC_FD``'s shape: no probe, so no requirement, so no gap. @@ -1279,7 +1287,8 @@ def test_each_job_selection_is_one_of_the_three_recognised_shapes(self) -> None: selections = {job.name: job.selection for job in _dependency_gates.pytest_jobs()} self.assertEqual(selections, {'test': 'ignore', 'integration': 'fixture-tier', - 'gate': 'whole-suite'}) + 'gate': 'whole-suite', 'engine-tests': 'ignore', + 'pypcap-parity': 'fixture-tier'}) def test_the_selection_is_read_off_the_step_that_runs_pytest(self) -> None: """Not off the job, whose comments contradict it. @@ -1307,7 +1316,8 @@ def test_a_job_whose_pytest_step_is_renamed_is_still_classified(self) -> None: selections = {job.name: job.selection for job in _dependency_gates.pytest_jobs(doctored)} self.assertEqual(selections, {'test': 'ignore', 'integration': 'fixture-tier', - 'gate': 'whole-suite'}) + 'gate': 'whole-suite', 'engine-tests': 'ignore', + 'pypcap-parity': 'fixture-tier'}) def test_two_install_lines_in_one_pytest_job_is_refused(self) -> None: """Ambiguity fails loudly instead of the first line winning.""" @@ -1525,8 +1535,20 @@ def test_every_gated_flag_is_classified(self) -> None: is a failure, because an unresolved flag requires nothing and so can 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. + """ requirements = _dependency_gates.flag_requirements() + exclusions = _dependency_gates.flag_exclusions() gates = _dependency_gates.gated_scopes() # Without this the whole loop passes on an empty scan, which is the one # way a classification check can be wrong and silent at the same time. @@ -1550,13 +1572,51 @@ def test_every_gated_flag_is_classified(self) -> None: f'{module} has no MODULE_PROVIDERS entry, so nothing ' f'knows which extra installs it') + for module in sorted(exclusions.get((gate.module, gate.flag), ())): + self.assertIn(module.partition('.')[0], + _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') + def test_no_provider_mapping_or_exclusion_is_vestigial(self) -> None: - """Both tables are exactly as wide as the suite needs them to be.""" + """Both tables are exactly as wide as the suite needs them to be -- + for every drift this comparison can actually see. + + A required module resolves through the *exact* key when + :data:`~tests._dependency_gates.MODULE_PROVIDERS` has one (``pcap._pcap``, + which does not want plain ``pcap``'s two-wide entry) and through its + top-level truncation otherwise -- the same rule + :func:`~tests._dependency_gates.module_providers` and + :func:`~tests._dependency_gates.extras_providing` both apply, so + ``needed`` is built the same way rather than by truncating every + required module unconditionally. + + One blind spot, not fixed here because nothing else needs it fixed: + ``needed`` decides *whether* to truncate a required module by asking + the very dictionary being checked, so deleting a *dotted* key + (``pcap._pcap``) removes it from both sides of the comparison in the + same step -- ``needed`` truncates to ``'pcap'`` the moment the exact + key is gone, and the assertion below stays green. That deletion is + not vestigial -- :func:`~tests._dependency_gates.module_providers`'s + callers still need the entry -- it is just invisible to this + particular test; + :meth:`~tests.test_tier_guard.DependencyGateScanTests.test_a_negated_probe_is_not_a_requirement`, + the two ``#762`` swap tests below, and + :meth:`~tests.test_tier_guard.DependencyGateFalsifiabilityTests\ +.test_removing_the_negation_or_the_exact_path_reopens_762`'s own mutation all + pin ``pcap._pcap`` directly and would catch it. + + """ requirements = _dependency_gates.flag_requirements() gated = {gate.flag for gate in _dependency_gates.gated_scopes()} - needed = {module.partition('.')[0] - for (_, flag), modules in requirements.items() if flag in gated - for module in modules} + required = {module + for (_, flag), modules in requirements.items() if flag in gated + for module in modules} + needed = {module if module in _dependency_gates.MODULE_PROVIDERS + else module.partition('.')[0] + for module in required} self.assertEqual(set(_dependency_gates.MODULE_PROVIDERS), needed) self.assertLessEqual(set(_dependency_gates.NON_DISTRIBUTION_FLAGS), @@ -1566,6 +1626,47 @@ def test_no_provider_mapping_or_exclusion_is_vestigial(self) -> None: & set(_dependency_gates.DEPENDENCY_GATE_EXCLUSIONS), set(), 'a flag no extra could satisfy does not also need an exclusion') + def test_mutually_exclusive_imports_matches_its_derivation(self) -> None: + """The liveness half :data:`~tests._dependency_gates.MUTUALLY_EXCLUSIVE_IMPORTS` needs. + + A hand-written set can misname which distribution is right for an + entry it already has, but it cannot notice a *third* name going + contested that nobody added -- the exact shape #745's own docstring + warns a skip list rots into. Comparing it against + :func:`~tests._dependency_gates.contested_imports`, which counts how + many of a :data:`~tests._dependency_gates.MODULE_PROVIDERS` entry's + alternatives some declared extra actually resolves, is what would + catch that: an entry gaining a second *live* alternative without a + matching addition here fails this assertion rather than silently + reopening #762 under a name nobody scoped + :func:`~tests._dependency_gates.ambiguous_satisfactions` to. + + """ + self.assertEqual(_dependency_gates.MUTUALLY_EXCLUSIVE_IMPORTS, + _dependency_gates.contested_imports()) + + def test_no_gate_is_satisfied_through_the_wrong_half_of_an_ambiguous_import(self) -> None: + """#762: a satisfied gate has to resolve to the *right* distribution. + + :meth:`test_every_gate_a_job_reaches_has_its_dependency_installed` only + asks whether at least one distribution the job installs ships the + module a gate needs -- which cannot tell PyPCAP's ``pcap`` from + pcap-ct's, since both are registered under the one + :data:`~tests._dependency_gates.MODULE_PROVIDERS` entry. This is that + check's complement: every ``(job, gate)`` pair the gaps pass calls + satisfied has to resolve to exactly the distribution that is legitimate + for it -- derived from :func:`~tests._dependency_gates.module_providers` + and :func:`~tests._dependency_gates.module_flag_exclusions`, not a + hand-maintained table -- wherever more than one distribution could have + supplied the import. + + """ + findings = _dependency_gates.ambiguous_satisfactions() + self.assertEqual( + findings, (), + '\n\n'.join(_dependency_gates.describe_ambiguous_satisfaction(finding) + for finding in findings)) + class DependencyGateFalsifiabilityTests(unittest.TestCase): """Break the install lines on a copy, and watch the guard fire. @@ -1610,13 +1711,23 @@ def test_removing_crypto_from_the_test_job_is_caught(self) -> None: ) def test_removing_dpkt_is_caught_on_every_job_that_installs_it(self) -> None: - """#729's defect, restaged. Three jobs install ``DPKT``; all three go dark.""" + """#729's defect, restaged. Five jobs install ``DPKT``; all five go dark. + + #751 added ``engine-tests`` and ``pypcap-parity`` to the three this test + used to name, each carrying its own ``DPKT`` for the same reason as the + original three -- ``engine-tests`` for + :file:`tests/foundation/engines/test_runtime_engines.py`'s reused + ``HAS_RUNTIME``, ``pypcap-parity`` for + :file:`examples/generators/make_samples.py`. + + """ text = _dependency_gates.WORKFLOW.read_text(encoding='utf-8') doctored = doctored_workflow(self, text, text.replace('DPKT,', '').replace(',DPKT', '')) gaps = {gap.job: gap for gap in _dependency_gates.dependency_gate_gaps(doctored) if gap.flag == 'HAS_DPKT'} - self.assertEqual(sorted(gaps), ['gate', 'integration', 'test']) + self.assertEqual(sorted(gaps), + ['engine-tests', 'gate', 'integration', 'pypcap-parity', 'test']) for job, gap in sorted(gaps.items()): with self.subTest(job=job): self.assertEqual(gap.missing, ('dpkt',)) @@ -1625,10 +1736,13 @@ def test_removing_cli_only_darkens_the_jobs_that_reach_the_emoji_gates(self) -> """Precision, not just detection. Every ``HAS_EMOJI`` gate is in :file:`tests/integration/test_cli_subprocess.py`, - which the ``test`` job ignores wholesale. So dropping ``cli`` must be - reported against ``integration`` and ``gate`` and *not* against - ``test``: a guard that flagged all three would be noise, and noise is - what gets a guard allowlisted into uselessness. + which the ``test`` and ``engine-tests`` jobs both ignore wholesale. So + dropping ``cli`` must be reported against ``integration``, ``gate`` and + ``pypcap-parity`` -- #751's job that mirrors ``integration``'s + fixture-tier selection and so reaches the same gate -- and *not* + against ``test`` or ``engine-tests``: a guard that flagged those two as + well would be noise, and noise is what gets a guard allowlisted into + uselessness. """ text = _dependency_gates.WORKFLOW.read_text(encoding='utf-8') @@ -1636,13 +1750,235 @@ def test_removing_cli_only_darkens_the_jobs_that_reach_the_emoji_gates(self) -> jobs = sorted(gap.job for gap in _dependency_gates.dependency_gate_gaps(doctored) if gap.flag == 'HAS_EMOJI') - self.assertEqual(jobs, ['gate', 'integration']) + self.assertEqual(jobs, ['gate', 'integration', 'pypcap-parity']) + + def test_swapping_pypcap_parity_to_pcap_ct_is_an_ambiguous_satisfaction(self) -> None: + """#762, direction one: the wrong half of the ``pcap`` ambiguity, installed. + + ``pypcap-parity`` swapped from the ``PyPCAP`` extra to ``PCAP_CT`` still + reaches all 4 ``HAS_PYPCAP`` gates, and + :func:`~tests._dependency_gates.dependency_gate_gaps` still calls them + satisfied -- pcap-ct ships the same top-level ``pcap`` module, so this is + exactly the blind spot :func:`~tests._dependency_gates.dependency_gate_gaps` + cannot see on its own. :func:`~tests._dependency_gates.ambiguous_satisfactions` + is what catches it. + + """ + doctored = doctored_workflow( + self, + "python -m pip install -e '.[test,Scapy,DPKT,cli,PyShark,PyPCAP,PyPCAPFile]'", + "python -m pip install -e '.[test,Scapy,DPKT,cli,PyShark,PCAP_CT,PyPCAPFile]'", + ) + + gaps = {(gap.flag, gap.job) for gap in _dependency_gates.dependency_gate_gaps(doctored)} + self.assertNotIn( + ('HAS_PYPCAP', 'pypcap-parity'), gaps, + 'the premise of this test: dependency_gate_gaps must still see no gap on this ' + 'job, which is exactly what makes the blind spot invisible to it') + + findings = {(finding.job, finding.flag): finding + for finding in _dependency_gates.ambiguous_satisfactions(doctored)} + self.assertIn(('pypcap-parity', 'HAS_PYPCAP'), findings) + finding = findings[('pypcap-parity', 'HAS_PYPCAP')] + self.assertEqual(finding.module, 'pcap') + self.assertEqual(finding.providers, frozenset({'pypcap', 'pcap-ct'})) + self.assertEqual(finding.satisfied, frozenset({'pcap-ct'})) + + self.assertEqual(finding.valid, frozenset({'pypcap'})) + + message = _dependency_gates.describe_ambiguous_satisfaction(finding) + self.assertIn("'pypcap-parity'", message) + self.assertIn('HAS_PYPCAP', message) + self.assertIn("it resolves to ['pcap-ct']", message) + self.assertIn("not to ['pypcap']", message) + + def test_swapping_engine_tests_to_pypcap_is_an_ambiguous_satisfaction(self) -> None: + """#762, direction two: the wrong half of the ``pcap._pcap`` ambiguity. + + ``engine-tests`` swapped from ``PCAP_CT`` to ``PyPCAP`` still reaches the + 1 ``HAS_PCAP_CT`` gate and still reads as satisfied to + :func:`~tests._dependency_gates.dependency_gate_gaps`, for the mirrored + reason: ``extras_providing`` truncates ``pcap._pcap`` to ``pcap`` before + its lookup, per this module's own docstring. + + """ + doctored = doctored_workflow( + self, + "python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,PCAP_CT]'", + "python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,PyPCAP]'", + ) + + gaps = {(gap.flag, gap.job) for gap in _dependency_gates.dependency_gate_gaps(doctored)} + self.assertNotIn(('HAS_PCAP_CT', 'engine-tests'), gaps) + + findings = {(finding.job, finding.flag): finding + for finding in _dependency_gates.ambiguous_satisfactions(doctored)} + self.assertIn(('engine-tests', 'HAS_PCAP_CT'), findings) + finding = findings[('engine-tests', 'HAS_PCAP_CT')] + self.assertEqual(finding.module, 'pcap._pcap') + self.assertEqual(finding.providers, frozenset({'pypcap', 'pcap-ct'})) + self.assertEqual(finding.satisfied, frozenset({'pypcap'})) + self.assertEqual(finding.valid, frozenset({'pcap-ct'})) + + def test_gaining_the_vendor_extra_is_not_an_ambiguous_satisfaction(self) -> None: + """The html5lib near-miss: two requirement strings, one distribution. + + ``html5lib`` has two entries in + :data:`~tests._dependency_gates.MODULE_PROVIDERS` + (``beautifulsoup4[html5lib]`` and plain ``html5lib``), the same shape + as ``pcap``'s two. The difference is that both name the *same* + distribution -- pyproject.toml never declares bare ``html5lib`` -- so + gaining the ``vendor`` extra (closing #738's ``HAS_VENDOR_DEPS`` gap) + must not read as ambiguous the way installing the wrong ``pcap`` + distribution does. Scoping + :func:`~tests._dependency_gates.ambiguous_satisfactions` to + :data:`~tests._dependency_gates.MUTUALLY_EXCLUSIVE_IMPORTS` rather than + to every :data:`~tests._dependency_gates.MODULE_PROVIDERS` entry with + more than one requirement string is what keeps it that way. + + """ + doctored = doctored_workflow( + self, + "python -m pip install -e '.[test,DPKT,crypto,NGAP]'", + "python -m pip install -e '.[test,DPKT,crypto,NGAP,vendor]'", + ) + + gaps = {(gap.flag, gap.job) for gap in _dependency_gates.dependency_gate_gaps(doctored)} + self.assertNotIn( + ('HAS_VENDOR_DEPS', 'test'), gaps, + 'the premise of this test: gaining vendor must actually close the gap on ' + 'this job, or there is no ambiguity question to ask about it') + + findings = _dependency_gates.ambiguous_satisfactions(doctored) + self.assertEqual( + findings, (), + '\n\n'.join(_dependency_gates.describe_ambiguous_satisfaction(finding) + for finding in findings)) + + def test_removing_the_negation_or_the_exact_path_reopens_762(self) -> None: + """Anti-rot: both halves of the fix are load-bearing, checked by removing each. + + Neither doctored scenario above is caught by :func:`~tests._dependency_gates + .dependency_gate_gaps` on its own (that is their whole premise). If + either of :func:`~tests._dependency_gates.ambiguous_satisfactions`'s + two extra sources of information stopped being consulted, the + corresponding scenario would go back to being invisible -- which is + exactly what removing each one in turn demonstrates. + + """ + doctored_pypcap_parity = doctored_workflow( + self, + "python -m pip install -e '.[test,Scapy,DPKT,cli,PyShark,PyPCAP,PyPCAPFile]'", + "python -m pip install -e '.[test,Scapy,DPKT,cli,PyShark,PCAP_CT,PyPCAPFile]'", + ) + doctored_engine_tests = doctored_workflow( + self, + "python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,PCAP_CT]'", + "python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,PyPCAP]'", + ) + + with self.subTest(mutation='flag_exclusions stubbed to report nothing'): + with unittest.mock.patch.object(_dependency_gates, 'flag_exclusions', lambda: {}): + findings = {(finding.job, finding.flag) + for finding in + _dependency_gates.ambiguous_satisfactions(doctored_pypcap_parity)} + self.assertNotIn( + ('pypcap-parity', 'HAS_PYPCAP'), findings, + 'without HAS_PYPCAP\'s own negated probe, nothing disqualifies pcap-ct from ' + 'plain pcap\'s two-wide entry, and the #762 shape goes uncaught again') + + with self.subTest(mutation="MODULE_PROVIDERS['pcap._pcap'] widened back to both"): + with unittest.mock.patch.dict(_dependency_gates.MODULE_PROVIDERS, + {'pcap._pcap': ('pypcap', 'pcap-ct')}): + findings = {(finding.job, finding.flag) + for finding in + _dependency_gates.ambiguous_satisfactions(doctored_engine_tests)} + self.assertNotIn( + ('engine-tests', 'HAS_PCAP_CT'), findings, + 'without the exact-path entry, pcap._pcap falls back to the same two-wide ' + 'set as plain pcap, and the #762 shape goes uncaught again') + + def test_a_third_contested_name_going_undeclared_is_caught(self) -> None: + """Falsifiability for the liveness comparison itself. + + A hand-written set passing a comparison against itself proves + nothing; this shows the comparison actually distinguishes a set that + has drifted from one that has not. Doctoring in a module with two + *live* alternatives -- ``requests`` and ``cryptography`` are both + already installed everywhere, so both resolve for real, unlike + ``html5lib``'s second -- and never adding it to + :data:`~tests._dependency_gates.MUTUALLY_EXCLUSIVE_IMPORTS` is exactly + the omission the real table must not make. + + """ + with unittest.mock.patch.dict(_dependency_gates.MODULE_PROVIDERS, + {'fakemod': ('requests', 'cryptography')}): + derived = _dependency_gates.contested_imports() + self.assertIn( + 'fakemod', derived, + 'the premise of this test: two live alternatives must make it contested') + self.assertNotEqual( + _dependency_gates.MUTUALLY_EXCLUSIVE_IMPORTS, derived, + 'the real table was never told about fakemod, so it must disagree here') + + def test_a_non_distribution_flag_is_not_reported_even_when_doctored_ambiguous(self) -> None: + """:data:`~tests._dependency_gates.NON_DISTRIBUTION_FLAGS`, exercised for real. + + ``HAS_PYSHARK`` (used to stand in for this mechanism elsewhere) never + reaches :func:`~tests._dependency_gates.ambiguous_satisfactions`'s + ``MUTUALLY_EXCLUSIVE_IMPORTS`` branch at all -- ``pyshark`` has one + provider -- so naming it in ``NON_DISTRIBUTION_FLAGS`` would still + report nothing whether or not this function's own skip line ran, + which proves nothing about that line specifically. ``HAS_PYPCAP`` on + the doctored ``pypcap-parity`` workflow does reach it -- there is a + real finding to suppress -- so naming *that* flag here is what + actually exercises the skip. + + """ + doctored = doctored_workflow( + self, + "python -m pip install -e '.[test,Scapy,DPKT,cli,PyShark,PyPCAP,PyPCAPFile]'", + "python -m pip install -e '.[test,Scapy,DPKT,cli,PyShark,PCAP_CT,PyPCAPFile]'", + ) + + findings = {(f.job, f.flag) for f in _dependency_gates.ambiguous_satisfactions(doctored)} + self.assertIn( + ('pypcap-parity', 'HAS_PYPCAP'), findings, + 'the premise of this test: there must be a real finding here to suppress') + + with unittest.mock.patch.dict(_dependency_gates.NON_DISTRIBUTION_FLAGS, + {'HAS_PYPCAP': 'stood in for this test'}): + findings = {(f.job, f.flag) + for f in _dependency_gates.ambiguous_satisfactions(doctored)} + self.assertNotIn(('pypcap-parity', 'HAS_PYPCAP'), findings) def test_the_undoctored_workflow_produces_no_unexplained_gap(self) -> None: - """The control: the three tests above fail for the doctoring, not by default.""" + """The control: the tests above fail for the doctoring, not by default.""" unexplained = [gap for gap in _dependency_gates.dependency_gate_gaps() if gap.flag not in _dependency_gates.DEPENDENCY_GATE_EXCLUSIONS] self.assertEqual(unexplained, []) + self.assertEqual(_dependency_gates.ambiguous_satisfactions(), ()) + + def test_describe_ambiguous_satisfaction_names_residual_ambiguity_too(self) -> None: + """The other branch of :func:`~tests._dependency_gates.describe_ambiguous_satisfaction`. + + Both real findings above are the "resolved to something illegitimate" + shape. The other shape -- resolved to more than one distribution even + after narrowing to the legitimate set -- has no real workflow state + that produces it today (the legitimate set for both watched flags is + always a singleton), so it needs its own construction to reach. + + """ + finding = _dependency_gates.AmbiguousProvider( + flag='HAS_MADE_UP', job='made-up-job', module='pcap', + providers=frozenset({'pypcap', 'pcap-ct'}), + satisfied=frozenset({'pypcap', 'pcap-ct'}), + valid=frozenset({'pypcap', 'pcap-ct'})) + + message = _dependency_gates.describe_ambiguous_satisfaction(finding) + self.assertIn('more than one legitimate distribution at once', message) + self.assertIn('HAS_MADE_UP', message) + self.assertIn('made-up-job', message) class DependencyGateDegradationTests(unittest.TestCase): @@ -1670,11 +2006,13 @@ class Tests(: """) self.assertEqual(_dependency_gates.module_gates(module, 'test_broken_unit.py'), ()) self.assertEqual(_dependency_gates.module_flag_requirements(module), {}) + self.assertEqual(_dependency_gates.module_flag_exclusions(module), {}) def test_a_missing_module_is_skipped_too(self) -> None: absent = self.tmp_path / 'test_absent_unit.py' self.assertEqual(_dependency_gates.module_gates(absent, 'test_absent_unit.py'), ()) self.assertEqual(_dependency_gates.module_flag_requirements(absent), {}) + self.assertEqual(_dependency_gates.module_flag_exclusions(absent), {}) def test_an_unrecognised_flag_shape_resolves_to_nothing(self) -> None: """And :class:`DependencyGateCoverageTests` is what makes that a failure. @@ -1753,3 +2091,67 @@ def test_a_flag_no_extra_could_satisfy_is_not_reported_as_a_gap(self) -> None: {'HAS_PYSHARK': 'stood in for this test'}): self.assertNotIn('HAS_PYSHARK', {gap.flag for gap in _dependency_gates.dependency_gate_gaps()}) + + def test_an_excluded_module_outside_any_contested_scope_is_ignored(self) -> None: + """:func:`~tests._dependency_gates._disqualified_providers`, the safe default. + + A negated probe naming a module this scan has never heard of, whose + top-level package is not in + :data:`~tests._dependency_gates.MUTUALLY_EXCLUSIVE_IMPORTS` either, + contributes nothing to disqualification rather than raising -- + there is no ambiguity to protect here, so there is nothing this + function needs to know about the module at all. + + """ + self.assertEqual( + _dependency_gates._disqualified_providers(frozenset({'totally.unmapped.thing'})), + frozenset()) + + def test_an_excluded_module_under_a_contested_top_level_needs_its_own_entry(self) -> None: + """The over-disqualification case: fails loud, with the right diagnosis. + + ``pcap._pcap``'s real entry is what stops ``pcap``'s two-wide one from + being borrowed wholesale. Removing that entry (simulating an excluded + dotted path nobody has mapped yet, under a top-level that *is* + contested) must not silently fall back to the broader entry -- that + would disqualify ``pypcap`` too, collapsing every legitimate candidate + to none and reporting a real satisfaction as ambiguous for the wrong + reason. It has to fail instead, and name what is missing. + + """ + with unittest.mock.patch.dict(_dependency_gates.MODULE_PROVIDERS): + del _dependency_gates.MODULE_PROVIDERS['pcap._pcap'] + with self.assertRaises(AssertionError) as caught: + _dependency_gates._disqualified_providers(frozenset({'pcap._pcap'})) + self.assertIn("'pcap._pcap'", str(caught.exception)) + self.assertIn('MUTUALLY_EXCLUSIVE_IMPORTS', str(caught.exception)) + + def test_an_unrelated_flags_unmapped_exclusion_never_reaches_disqualification(self) -> None: + """The ordering fix: the contested-scope check has to run *first*. + + :func:`~tests._dependency_gates.ambiguous_satisfactions` used to + compute disqualified providers for every gate a job reaches, before + checking whether the gate's own required module was even in + :data:`~tests._dependency_gates.MUTUALLY_EXCLUSIVE_IMPORTS`. A flag + with a negated probe on some module with no + :data:`~tests._dependency_gates.MODULE_PROVIDERS` entry, and nothing + to do with a contested name, would crash the entire function via + :func:`~tests._dependency_gates._disqualified_providers` rather than + being scoped out of it. This pins a synthetic flag shaped exactly + that way and shows it no longer crashes. + + """ + with unittest.mock.patch.object( + _dependency_gates, 'flag_exclusions', + lambda: {('fake/module.py', 'HAS_FAKE'): frozenset({'totally.unmapped.nonsense'})}): + with unittest.mock.patch.object( + _dependency_gates, 'flag_requirements', + lambda: {('fake/module.py', 'HAS_FAKE'): frozenset({'dpkt'})}): + with unittest.mock.patch.object( + _dependency_gates, 'gated_scopes', + lambda: (_dependency_gates.Gate('HAS_FAKE', 'fake/module.py', 1, None, + 'test_fake'),)): + with unittest.mock.patch.object( + _dependency_gates, 'job_reaches', lambda job, gate: True): + result = _dependency_gates.ambiguous_satisfactions() + self.assertEqual(result, ())