Skip to content

tests: seven more skipUnless gates have never executed on any CI path (106 methods) #738

Description

@JarryShaw

#729 fixed HAS_DPKT. Seven more gates are in the identical never-executes state — the dependency lives in an extra no CI job installs, so the tests report as skips, which read as passes.

CI installs .[test] on the test legs and .[test,Scapy] on integration/gate. The test extra is pytest, pytest-xdist, typing-extensions — nothing else. Counts are my own AST pass over tests/, resolving class-level and method-level skipUnless gates, against origin/main at 55513f69e.

flag methods extra needed ships wheels? files
HAS_VENDOR_DEPS 41 vendor (requests) yes tests/vendor/*
HAS_CRYPTO 14 crypto (cryptography) yes, everywhere protocols/internet/test_esp_unit.py
HAS_EMOJI 14 cli (emoji) yes, pure Python integration/test_cli_subprocess.py
HAS_CRAWLER_DEPS 14 vendor yes tests/vendor/*
HAS_PYCRATE 10 NGAP (pycrate) yes protocols/application/test_ngap_unit.py
HAS_PYPCAPFILE 10 PyPCAPFile yes toolkit/test_pypcapfile_unit.py
HAS_PYSHARK 3 PyShark yes (binary tshark is separate) foundation/engines/test_pyshark_engine.py

106 methods, against #729's 28.

Two nuances that make this a judgement call rather than a pure defect:

  • HAS_VENDOR_DEPS and HAS_CRAWLER_DEPS (55 of the 106) exercise the vendor crawlers, which reach the network. Those may be deliberately dark in CI — see Vendor crawlers: three take a Wikipedia 403 on the default User-Agent, three point at a dead IETF URL #518, where four Wikipedia 403s and a dead IETF URL make them flaky by nature. Installing requests would make them run and possibly fail on upstream availability rather than on our code. This wants your ruling, not a blanket fix.
  • HAS_SCAPY (14) is not in this list — Scapy is installed on the integration and gate legs, so those run. They are dark only on the test leg.

HAS_PYPCAP (4) and HAS_PCAP_CT (1) are excluded as defensibly dark: C extensions needing libpcap headers.

The cheap, uncontroversial subset is HAS_CRYPTO + HAS_EMOJI + HAS_PYCRATE + HAS_PYPCAPFILE = 48 methods, all pure-Python or wheel-shipping, and test_cli_subprocess.py is already selected wholesale by the integration job — so that one is literally the same one-line install-list edit #737 made. test_esp_unit.py is real ESP crypto correctness, which is the highest-value of the set.

Why the whole class survived: Pipfile [dev-packages] carries dpkt, cryptography and emoji, so local make test has always run these. Only CI was ever blind.

Activity

  1. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    testPull requests that add or correct tests (test: subject prefix)
    ciPull requests that change CI or workflow configuration (ci: subject prefix)
    on Sep 24, 2026
  2. JarryShaw commented on Sep 24, 2026

    @JarryShaw
    OwnerAuthor

    Blocked on two things, recorded so nobody dispatches into a conflict:

    1. The fix needs .github/workflows/unit-tests.yml, which open PR ci(unit-tests): install DPKT so the dpkt-gated tests actually run #737 owns. ci(unit-tests): install DPKT so the dpkt-gated tests actually run #737 is ✅ and awaiting merge; editing the same install lines now would conflict. This clears the moment it merges — exactly how tests: dpkt-gated tests have never run on any CI path (28 methods, 6 files) #729 waited on ci: parallelise pytest with xdist and drop 3.15 from the blocking matrix #725.
    2. 55 of the 106 methods (HAS_VENDOR_DEPS 41, HAS_CRAWLER_DEPS 14) need your ruling, not a fix. They exercise the vendor crawlers over the network, and Vendor crawlers: three take a Wikipedia 403 on the default User-Agent, three point at a dead IETF URL #518 records four Wikipedia 403s plus a dead IETF URL — so installing requests would make them run and possibly fail on upstream availability rather than on our code. Deliberately dark may be the right answer there.

    The 48-method subset needs no ruling and is ready to dispatch once #737 lands: HAS_CRYPTO 14, HAS_EMOJI 14, HAS_PYCRATE 10, HAS_PYPCAPFILE 10 — all pure-Python or wheel-shipping, no network. tests/integration/test_cli_subprocess.py is already selected wholesale by the integration job, so HAS_EMOJI is the identical one-line edit #737 made.

  3. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 24, 2026
  4. JarryShaw commented on Sep 24, 2026

    @JarryShaw
    OwnerAuthor

    What's needed exactly from me?

  5. JarryShaw commented on Sep 24, 2026

    @JarryShaw
    OwnerAuthor

    Fair — my earlier comment buried the ask. One question:

    Should the 55 vendor-crawler test methods run in CI at all?

    HAS_VENDOR_DEPS (41) and HAS_CRAWLER_DEPS (14) gate on the vendor extra, i.e. requests. Adding it to the install lines is a one-line change and they would start running. The reason I did not just do it:

    I would take (b) if you want the coverage and (a) if you do not; (c) seems wrong given #518. Your call.

    Nothing else here needs you. The other 48 methods — HAS_CRYPTO 14, HAS_EMOJI 14, HAS_PYCRATE 10, HAS_PYPCAPFILE 10 — are pure-Python or wheel-shipping with no network, and a PR for them is already in flight. HAS_PYPCAP (4) and HAS_PCAP_CT (1) I left out as C extensions needing libpcap headers; say so if you want those too and I will treat the header install as its own decision.

  6. JarryShaw commented on Sep 24, 2026

    @JarryShaw
    OwnerAuthor

    I agree on (b).

  7. JarryShaw commented on Sep 24, 2026

    @JarryShaw
    OwnerAuthor

    Ruling recorded: (b) — the vendor-crawler tests run, but off the PR-blocking matrix.

    So the remaining scope on this issue is a non-blocking leg carrying the vendor extra, covering HAS_VENDOR_DEPS (41) and HAS_CRAWLER_DEPS (14) = 55 methods. The precedent to follow is how 3.15 was handled in b7f51401b: the job exists and reports, but is absent from ruleset 23497679's 15 required checks, so a red there informs without gating.

    Two things whoever implements it must get right, both learned the hard way here:

    • The new job's name must not collide with a required context. The 15 required are Python 3.10–3.14, Integration Python 3.10–3.14, Compat Python 3.10–3.14. Reusing one of those names silently makes the new leg gating — the opposite of this ruling.
    • Skips must be provably resolved, not merely absent. CI runs pytest -q with no -r, so skip reasons never print and "0 occurrences of requests not installed" reads identically whether or not the fix worked. The skip delta is the only usable signal — ci(unit-tests): install DPKT so the dpkt-gated tests actually run #737 established this.

    Expect real upstream flake: #518 records four Wikipedia 403s and a dead IETF URL. That is the cost being accepted deliberately, and it is the reason the leg is non-blocking.

    needs: decision removed — nothing here waits on the maintainer now. The 48-method subset is already in flight as #740 (Part of #738), which leaves this issue tracking the 55, and separately HAS_PYPCAP (4) and HAS_PCAP_CT (1) which remain excluded as C extensions needing libpcap headers — say the word if those should be revisited.

  8. removed
    needs: decisionWaiting on the maintainer to decide — not blocked by other work
    on Sep 24, 2026
  9. JarryShaw commented on Sep 24, 2026

    @JarryShaw
    OwnerAuthor

    Correction to this issue's remaining scope, found while writing #745's guard: it is 41 methods, not 55.

    HAS_CRAWLER_DEPS (14) asks only for requests and bs4, and #507 put both in the test extra — so those 14 have been running on all three pytest jobs ever since, and need no leg. The two source files say so themselves (test_crawler_reachability_unit.py:91, test_ipx_socket_unit.py:48).

    HAS_VENDOR_DEPS (41) is still dark, but for a narrower reason than "needs requests": it additionally wants html5lib, which only beautifulsoup4[html5lib] provides — i.e. the vendor or all extra. requests and bs4 are already installed. Note the one exception the guard also turned up: tests/vendor/test_vendor_dest_path_unit.py (4 methods, added by #741) gates on a VENDOR_DEPS tuple without html5lib, so those 4 already run.

    Both facts are now asserted rather than remembered — DependencyGateCoverageTests.test_the_crawler_dependencies_are_satisfied_by_the_test_extra fails if HAS_CRAWLER_DEPS ever goes dark again, and the HAS_VENDOR_DEPS allowlist entry fails if its missing-package set changes.

  10. JarryShaw commented on Sep 24, 2026

    @JarryShaw
    OwnerAuthor

    Independently confirmed, with the scopes named so the two figures do not read as contradicting.

    My AST sweep over tests/ on 5c0df9248 (class-level and method-level skipUnless):

    gate methods dark?
    HAS_VENDOR_DEPS 45 total — test_request_prompt_unit.py 16, test_user_agent_unit.py 11, test_ipx_packet_unit.py 9, test_ftp_return_code_unit.py 5, test_vendor_dest_path_unit.py 4 41 dark, the 4 in test_vendor_dest_path_unit.py already run
    HAS_CRAWLER_DEPS 14 — test_ipx_socket_unit.py 9, test_crawler_reachability_unit.py 5 none; both deps are in the test extra

    So 45 gated, 41 dark — the comment's 41 is the remaining-scope figure and mine is the total. Both
    right at their own scope, and it flagged the 4-method exception itself.

    The html5lib mechanism holds. VENDOR_DEPS = ('requests', 'bs4', 'html5lib') in four files, and
    html5lib ships only via beautifulsoup4[html5lib] — pyproject.toml:203 (vendor) and :226
    (all). :256 says it outright: the test extra is "Plain, without the vendor extra's [socks]
    and [html5lib]"
    . So one package, not three, is what stands between these 41 and a green leg.

    That narrows the ruling you owe this issue. The earlier framing was "55 methods, most of them
    hitting the network, needing a call given #518's Wikipedia 403s". It is now: 41 methods, one missing
    package.
    Whether they belong on a non-blocking leg is still yours — the network reachability concern
    is unchanged and real — but the cost of the (b) leg is smaller than this issue has been recording.

    One caveat on the guard's own assertion: it pins that HAS_CRAWLER_DEPS stays satisfied by the test
    extra, which is the right thing to pin. It does not evaluate environment markers, so it cannot see
    a gate that is dark only on some Python versions — the same limitation noted for HAS_PYPCAPFILE in #751.

  11. 3 remaining items

  12. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 25, 2026
  13. JarryShaw commented on Sep 25, 2026

    @JarryShaw
    OwnerAuthor

    Unblocked. #755 merged, so .github/workflows/unit-tests.yml and tests/test_tier_guard.py are free. Dispatching a worker now.

    Worth carrying across: #755 landed ambiguous_satisfactions() and contested_imports(), which derive the correct providing distribution from a flag's negated probe rather than from a hand-maintained table. So the guard can now tell PyPCAP from PCAP_CT, and a gate satisfied through the wrong half of an ambiguous import name is reported rather than passed through. Any fix here should use that machinery rather than adding a parallel exclusion list.

  14. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 25, 2026
  15. added this to the 1.5 milestone on Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)ciPull requests that change CI or workflow configuration (ci: subject prefix)testPull requests that add or correct tests (test: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions