From 01f456650941de013dbd9912146acd976961a862 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 24 Sep 2026 17:06:23 -0400 Subject: [PATCH] ci: add a per-engine test job for Scapy, PyShark, PyPCAPFile and pcap-ct Per the ruling on #751: third-party engine coverage gets its own job(s) in unit-tests.yml, installing the engine extras across the full 3.10-3.14 matrix, rather than one more install line on `test`, `integration` or `gate`. - `engine-tests`: installs Scapy, PyShark, PyPCAPFile and PCAP_CT (plus system libpcap and tshark via apt-get) across 3.10-3.14. Closes HAS_SCAPY, HAS_PYSHARK, HAS_PYPCAPFILE (9 of 15), HAS_PCAP_CT and HAS_RUNTIME (test_runtime_engines.py's reused flag, closed as a side effect of installing dpkt+scapy+pyshark together). - `pypcap-parity`: a separate job/venv on 3.10-3.11 only (both extras' own marker ceiling), with a C toolchain and libpcap headers, mirroring `integration`'s fixture-tier selection to reach test_new_engine_parity_runtime.py. Closes HAS_PYPCAP (4 gates, confirmed building on real CI) and the remaining 6 HAS_PYPCAPFILE gates in the same module. Kept apart from `engine-tests`: pypcap and pcap-ct both ship a top-level `pcap` module and cannot share a venv. - Installing `tshark` needed one upstream fix first: test_pyshark_engine.py's test_the_reason_tracks_the_running_interpreter hard-asserted tshark's absence, unlike its sibling test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous, which already self-guards with shutil.which('tshark'). Gave it the same guard so the file no longer disagrees with itself about whether tshark may be present, then installed it (debconf pre-seeded to avoid the postinst prompt). - tests/_dependency_gates.py: update DEPENDENCY_GATE_EXCLUSIONS' reasons for the six gates above, add `engine-tests` to HAS_VENDOR_DEPS' dark jobs, and correct the module docstring's pypcap/pcap-ct ambiguity note now that both extras are on a (different) job's install line -- the ambiguity stays live but happens not to bite here, since neither job's selection reaches the other flag's gate; confirmed with job_reaches() directly rather than assumed. - tests/test_tier_guard.py: extend the three job-selection/removal assertions that enumerated the workflow's jobs by name to include the two new ones. - Fix three comments (on `test`, `integration` and `gate`) left stale by #747/#748: they still described a since-fixed pypcapfile bug as the reason PyPCAPFile is not installed, and one said "or on either job below", no longer true now that `engine-tests` installs it. Cross-review round found four more defects, all fixed here: - tests/_dependency_gates.py: the "ambiguity stays live but happens not to bite" note above was wrong to call safe -- a job installing the *wrong* half of the pypcap/pcap-ct ambiguity read as satisfied too, since extras_providing() only checked whether *some* extra in common shipped the module. Doctoring pypcap-parity to install PCAP_CT (still reaches 4 HAS_PYPCAP gates) and engine-tests to install PyPCAP (still reaches 1 HAS_PCAP_CT gate) both passed the old guard with zero unexplained gaps. Added ambiguous_satisfactions() and AMBIGUOUS_PROVIDER_ALLOWLIST: a satisfied gate resolved through a module more than one *distribution* ships (MODULE_PROVIDERS names more than one for it) now has to resolve to exactly the one distribution the allowlist names for that (job, flag) pair, not merely share some distribution with the job's install line. Both doctored scenarios now report a finding; the real, undoctored workflow reports none. Also corrected the module docstring's own claim that the two flags "both read as needing pcap alone" -- flag_requirements() resolves them apart correctly ({'pcap'} vs {'pcap._pcap'}); the conflation is one level down, in extras_providing()'s top-level-package truncation. - tests/foundation/engines/test_pyshark_engine.py: both tshark-presence checks keyed on shutil.which('tshark'), which disagrees with PyShark.unsupported_reason()'s own oracle (pyshark's get_process_path(), which reads a ./config.ini before PATH) -- pre-existing in test_this_host_really_has_no_tshark_so_the_check_is_not_vacuous and copied into test_the_reason_tracks_the_running_interpreter by this PR. A ./config.ini naming an off-PATH tshark stand-in makes `which` say absent while the real oracle finds it, failing both. Added _tshark_missing(), probing the same way production does, and a regression test constructing exactly that config.ini case. - .github/workflows/unit-tests.yml:456: "unlike the three jobs above" was stale -- this PR's own `engine-tests` and `pypcap-parity` bring the true count to four; corrected and named all four explicitly so a fifth insertion has to confront the list rather than the count alone. - tests/_dependency_gates.py: gated_scopes()'s docstring said HAS_DPKT reaches its 20 methods through 6 class decorators and none through a method decorator; exact-match AST counting (not the substring search that folded HAS_PYPCAPFILE into HAS_PYPCAP in an earlier round) gives 8, not zero. Corrected. Adds no tests for the per-engine job coverage itself; makes 27 previously-skipped unit-tier methods run for real (measured before/after on a throwaway venv, matching `test`'s own install line as the baseline). Confirmed on real PR CI: all 5 `engine-tests` legs and both `pypcap-parity` legs pass, with PyPCAP actually building and its 4 gated methods executing rather than skipping. This round adds 7 tests for the four fixes above (2 falsifiability tests reproducing the doctored ambiguity scenarios, an anti-rot liveness check on the new allowlist, a message-format check, a manual-construction test for the unlisted-pair branch, and a config.ini regression test), each shown to fail against the pre-fix code. CI then went red on real `engine-tests` legs, on the regression test the round above added: - tests/foundation/engines/test_pyshark_engine.py: that test asserted `shutil.which('tshark') is None` as its premise -- true on the machine it was written on, false on every `engine-tests` leg, which apt-get installs a real `/usr/bin/tshark` a few steps earlier in the same job. The claim the test actually needs is narrower: that `which` cannot resolve *the stand-in* it just built, not that the host has no tshark anywhere on PATH. Rewritten to construct both worlds explicitly -- PATH pointing at an empty directory, and PATH pointing at a directory holding an unrelated, real, executable `tshark` standing in for the one `engine-tests` installs -- and to assert against the stand-in's own path rather than against the host's state, so it cannot depend on which world it happens to run in again. - Swept every comment touched by the last two rounds for the same failure mode -- prose asserting what the code used to do rather than what it does now -- and found four more: - unit-tests.yml:316-317 and _dependency_gates.py's own HAS_PYSHARK exclusion both still said the fix was "checks shutil.which('tshark') first" / "self-guard (shutil.which('tshark'))"; both now describe the real oracle (`_tshark_missing()`, itself probing pyshark's get_process_path()). - _dependency_gates.py's HAS_PYSHARK exclusion also said "3 gated methods run nowhere" / "two of them assert what PyShark.unsupported_reason says" -- the regression test above is itself HAS_PYSHARK-gated, so the true counts are 4 and three. - test_pyshark_engine.py's own module docstring claimed tshark "is not installed here" and that "installing Wireshark was not an option" -- both false once `engine-tests` collects this same module with a real tshark on PATH. Rewritten to describe both states without assuming either. - test_pyshark_engine.py's `_tshark_missing()` docstring said "both call sites" and "the two tests below" -- there are three call sites now; reworded to not name a count that the next test added here would have to remember to bump. tests/test_tier_guard.py + test_pyshark_engine.py: 98 passed, 519 subtests (was 84 passed, 508 subtests before this PR's cross-review rounds). A second, independent cross-review found the #762 guard above could still be defeated -- flipping a job's install line and its allowlist entry together stayed silently green, since nothing tied AMBIGUOUS_PROVIDER_ALLOWLIST's values to the distribution that is actually *correct* for a flag, only to whatever the job installs. Removed the allowlist rather than hardening its test: - tests/_dependency_gates.py: added module_flag_exclusions()/ flag_exclusions(), the mirror of module_flag_requirements()/ flag_requirements() that recovers the *negative* half of a flag's probe those two correctly drop (HAS_PYPCAP's own `not importable('pcap._pcap')`). Added module_providers(), which -- unlike extras_providing(), deliberately untouched -- resolves a dotted import name exactly when MODULE_PROVIDERS has a dedicated entry for it, so `pcap._pcap` now maps to `('pcap-ct',)` alone instead of falling back to plain `pcap`'s two-wide entry. Subtracting the exclusion's exact providers from the requirement's leaves exactly one legitimate distribution per flag, derived rather than hand-written. ambiguous_satisfactions() now flags a gate whenever what dependency_gate_gaps() would call "satisfied" disagrees with that derived set -- resolving to the disqualified distribution (the #762 shape) or to more than one legitimate one at once. Both doctored rows are still caught, now with no allowlist to keep in sync; verified by independently stubbing out each half (flag_exclusions returning nothing; MODULE_PROVIDERS's pcap._pcap widened back to both) and confirming each reopens exactly the row it protects. - The same review found the guard's own scope too wide: gating ambiguity on `len(MODULE_PROVIDERS[name]) > 1` false-positives on `html5lib`, which has two entries for the *same* one distribution (`beautifulsoup4[html5lib]`, never bare `html5lib` -- pyproject.toml never declares it) rather than two competing ones. Added MUTUALLY_EXCLUSIVE_IMPORTS, declaring only `pcap` as genuinely contested, and scoped ambiguous_satisfactions() to it. Verified by doctoring `test` to gain the `vendor` extra (closing #738's HAS_VENDOR_DEPS gap): zero findings, where the old length-based scope produced one with no wrong half to report. - A third finding: the NON_DISTRIBUTION_FLAGS-skip assertion added for ambiguous_satisfactions() used HAS_PYSHARK, which never reaches the ambiguity branch at all (`pyshark` has one provider) -- doubly vacuous, since deleting the skip line left it passing too. Dropped that assertion and added a real one on the doctored pypcap-parity workflow, where HAS_PYPCAP does reach the branch and there is a real finding to suppress. - Pre-existing, fixed while in the file: `tests/_dependency_gates.py`'s own module docstring said six of the other seven workflows install `.[all]`; codeql-analysis.yml installs nothing explicitly and python-compatibility.yml installs a bare `.`, so it is five. tests/test_tier_guard.py + test_pyshark_engine.py: 100 passed, 519 subtests. A third, independent cross-review confirmed the mechanism survives (six mutations all turn it red, including doctoring the real workflow and emptying MUTUALLY_EXCLUSIVE_IMPORTS together, which the swap tests' own literal install-line strings still catch) and found four smaller things: - tests/test_tier_guard.py:1615: a deleted blank line before `class DependencyGateFalsifiabilityTests` (PEP 8 E302; `tests/` is not linted by `make pylint`, so nothing else would have caught it). Restored. - Four stale figures in the PR description, all from revision 1 (`HAS_PYSHARK`'s count moved from 3 to 4 once the tshark regression test above became its own third in-file gated method; "both/two touched Python files" is now three; the `tests/test_tier_guard.py` test count needed to be split from `test_pyshark_engine.py`'s rather than presented as one number covering both). Corrected in the description directly. - MUTUALLY_EXCLUSIVE_IMPORTS was the one new hand-written table with no liveness check -- omitting a future contested name would not be caught, only a wrong entry among existing ones. Added contested_imports(): counts, per MODULE_PROVIDERS entry, how many alternatives some declared extra actually resolves (1 for html5lib -- bare html5lib is declared by zero extras -- 2 for pcap), and a name qualifies at 2+. A new test asserts it equals MUTUALLY_EXCLUSIVE_IMPORTS; a falsifiability test doctors in a module with two live alternatives and confirms the comparison disagrees when it is not added. - Two latent gaps in ambiguous_satisfactions(): an excluded module with no MODULE_PROVIDERS entry raised a bare, untested KeyError (test_every_gated_flag_is_classified now loops over excluded modules too, giving them the same deliberate contract required ones already have); and disqualified was computed before the MUTUALLY_EXCLUSIVE_IMPORTS scope check rather than after, so an unrelated flag's negated probe on an unmapped module could crash the whole function instead of being scoped out of it -- moved the computation inside the scope-checked branch. Extracted _disqualified_providers() to also close the related, lower-priority gap: falling back to a contested top-level's full entry for an excluded dotted path with no exact entry of its own would over-disqualify and misdiagnose a real satisfaction as ambiguous; it now fails loudly instead, naming the missing entry. - test_no_provider_mapping_or_exclusion_is_vestigial's docstring claimed the two tables are "exactly as wide as the suite needs them to be" without qualification; its own `needed` computation is self-referential for dotted keys, so deleting `pcap._pcap` moves both sides of the comparison together and goes uncaught by this test specifically (five others still pin it). Narrowed the docstring to say so. tests/test_tier_guard.py + test_pyshark_engine.py: 105 passed, 519 subtests. Fixes #762. --- .github/workflows/unit-tests.yml | 263 +++++++- tests/_dependency_gates.py | 578 ++++++++++++++++-- .../foundation/engines/test_pyshark_engine.py | 144 ++++- tests/test_tier_guard.py | 430 ++++++++++++- 4 files changed, 1309 insertions(+), 106 deletions(-) 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, ())