ci(deps): install the vendor extra on engine-tests, closing HAS_VENDOR_DEPS (#738)

[JarryShaw]

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change
• Added a changelog entry — N/A, changelog centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

• fix
• feat
• perf
• refactor
• test
• docs
• ci
• chore

---

Description

Closes #738's remaining scope: HAS_VENDOR_DEPS' 41 methods (tests/vendor/test_request_prompt_unit.py, test_user_agent_unit.py, test_ipx_packet_unit.py, test_ftp_return_code_unit.py) were one requirement extra short of the test extra's own requests+bs4 (#507) -- html5lib, which only beautifulsoup4[html5lib] (the vendor extra) provides.

#738's ruling was to land this on a non-blocking leg rather than add it to test. engine-tests (#751) already is that leg, and is the only per-pull-request job that both reaches these gates and sits outside ruleset 23497679's 15 required checks -- gate reaches them too but never runs on a pull request, and integration's fixture-tier selection never reaches tests/vendor/. So adding vendor to engine-tests' install line was the forced choice, not merely a convenient one.

Updated tests/_dependency_gates.py's HAS_VENDOR_DEPS exclusion to drop engine-tests, per dependency_gate_gaps()'s own before/after, and to state gate's own substantive reason for staying dark instead of "also non-blocking" (which would otherwise permit adding vendor there too).

None of the 41 methods make a live network call, but not for one reason across all four files: test_user_agent_unit.py (11) and test_request_prompt_unit.py (16) gate on html5lib only as an import precondition and replace requests.get with a recorder; test_ipx_packet_unit.py (9) is a regression suite for a retired scrape (Packet.LINK is None); test_ftp_return_code_unit.py (5) is not retired -- its crawler's LINK is a live Wikipedia URL -- and is the one file where html5lib is a functional dependency: it parses an inline HTML fixture with the real parser rather than fetching anything. An independent cross-review confirmed the no-network claim directly, with socket.connect/connect_ex/getaddrinfo/create_connection all patched to raise across all 41 methods -- zero failures. Swept the same stale "skips in CI" prose this correction touches, in the four files themselves, in test_vendor_dest_path_unit.py's cross-reference to them, and in two related claims in pyproject.toml.

Added a guard test pinning the closure (shown to fail against the pre-fix workflow), and updated two falsifiability tests' hardcoded install-line strings to match the new line.

Fixes #738.

[JarryShaw]

Cross-review: NEEDS CHANGES (opus, a different model from the author), verified at 9502488b5 and re-confirmed on the merge with 83b58ebda. The head has since rebased to 6d407e87c with an identical diff, so the findings stand. Every finding is prose — the mechanics are correct and I could not break them.

Fixes #738 is defensible. It closes one gate, but it is the last one. Re-derived per flag rather than trusting the issue: HAS_CRYPTO, HAS_PYCRATE (#740), HAS_EMOJI (#740), HAS_CRAWLER_DEPS (never dark, #507), HAS_PYSHARK (#755), HAS_PYPCAPFILE (closed but marker-limited), and HAS_VENDOR_DEPS here. No flag runs nowhere.

Verified: gaps 16 → 15, exactly the one tuple removed and every other byte-identical, no new gap. Skip delta 41 → 0 on tests/vendor, matching the PR's figure at the right scope. All 41 pass under pytest and plain unittest. And the PR's central new claim was falsified directly rather than accepted — with socket.connect, connect_ex, getaddrinfo and create_connection all patched to raise, Ran 41 tests … OK, 0 failures. None of the 41 touches the network. engine-tests is also the unique job that both reaches these gates and is absent from ruleset 23497679's 15 required checks, so the choice was forced rather than merely sensible. Added weight is ~1.4 MB of pure Python.

My own brief was wrong on one figure, and both the author and I measure the same correction: the guard is 96 passed / 511 subtests at 1a852698b, not the 105/519 I gave — that was the combined test_tier_guard.py + test_pyshark_engine.py total. After this PR: 97/512, the +1 fully attributed to test_every_extra_any_pytest_job_installs_is_declared gaining the engine-tests/vendor pair.

Six prose changes required, four of them false claims in the PR's own new text:

1. The two new texts contradict each other on gate — the exclusion reason says it "is not part of the blocking matrix either way", the test docstring says "both remain part of it". The ruleset settles it: the exclusion is right. That also leaves the decision unjustified, so the stated reason needs to be the real one (the release path should not gain a network-parser dependency).
2. test_ftp_return_code_unit.py is not a retired scrape — pcapkit/vendor/ftp/return_code.py:182 still carries the live Wikipedia LINK; only ipx/packet.py is retired.
3. It mocks nothing. It builds the fixture inline and calls ReturnCode.request(text), which is bs4.BeautifulSoup(text, 'html5lib'). For those 5 methods html5lib is a functional dependency, not an import precondition. The conclusion holds; the reason given does not.
4. A quotation attributed to two files exists in one — that exact string is only in test_user_agent_unit.py:31; the ... elision splices two different sentences.
5. "one of the four methods" has no referent — the two files named hold 14 methods.
6. Six stale sites left unswept, four in the very files this PR un-darks, each still telling the reader these tests skip in CI: test_user_agent_unit.py:65-67, test_request_prompt_unit.py:101-103, test_ipx_packet_unit.py:55-57, test_ftp_return_code_unit.py:50-53, test_vendor_dest_path_unit.py:96-98, plus pyproject.toml:247.

One sentence worth adding when #738 closes: HAS_PYPCAPFILE is dark on 3.12-3.14 regardless, because pypcapfile; python_version < '3.12' resolves to nothing there — a property of the dependency the guard deliberately does not model, and not something this PR could close.

[JarryShaw]

Final cross-review at c9be8726d: NEEDS CHANGES — one prose correction across five lines. Everything else verified green, and all six earlier findings are confirmed fixed.

Verified good. The gate contradiction is gone with one statement surviving and the docstring deferring to it. "Retired" is now correct, and the reviewer checked the mechanism rather than the prose: Vendor.LINK = None at default.py:241, 'LINK' in Packet.__dict__ is False, and _request() tests if self.LINK is None: at :569 before the requests.get at :584 — while ftp.ReturnCode.LINK is a live Wikipedia URL. test_request_makes_no_network_call exists at test_ipx_packet_unit.py:159 and replaces both requests.get and requests.Session.request, so the referent is real. The per-file counts re-derive by AST at gate scope: 11 / 16 / 9 / 5, 41 dark of 45 gated. No regression anywhere — gaps still 15 byte-identical, guard 97/512 under both runners, tests/vendor 62 passed / 186 subtests / 0 skipped. And CI now settles what the last round could not: all five Engines Python 3.10-3.14 are SUCCESS. Fixes #738. can land as-is — #776 disposed of the only caveat.

The one change. The sweep made all five un-darked files name integration as a job where these tests "skip". It never collects them. Measured:

test             reaches 7 of 7 tests/vendor modules
integration      reaches 0 of 7
engine-tests     reaches 7 of 7
pypcap-parity    reaches 0 of 7
gate             reaches 7 of 7

So four sites — test_ftp_return_code_unit.py:54, test_ipx_packet_unit.py:57, test_request_prompt_unit.py:103, test_user_agent_unit.py:67 — should drop integration. Note pypcap-parity is the same fixture-tier shape and is correctly omitted from all four, so listing one and not the other is internally inconsistent as well.

And test_vendor_dest_path_unit.py:96 now overstates: "both classes below run on every job that runs pytest at all". They run on three of five. The text it replaced — "run in CI as things stand" — was true; the replacement is not. :98 repeats the integration error in the same sentence.

This matters more than ordinary imprecision, which is why it is a change request rather than a note: "skipped" versus "never collected" is the exact distinction the #738/#745 subsystem is built on — dependency_gate_gaps() reports a gap only where a job reaches the gate, for precisely this reason. And the same PR now asserts both sides: _dependency_gates.py says "integration's fixture-tier selection never reaches tests/vendor/ in the first place", while four test files say these tests skip there.

Nothing functional argues against merging, and landing this with a follow-up sweep would be defensible. But the fix-up exists to make these comments accurate, and the correction is one word in four places plus one reworded clause.

[JarryShaw]

Final cross-review at 3d45a3f62 (sonnet, a different model from the author): GOOD TO GO. Every claim re-derived independently matched, and nothing was taken on trust.

Correcting my own figure first. The verification table I posted earlier said test/engine-tests/gate reach "7 of 7" tests/vendor modules. There are 8, not 7 — git ls-tree origin/main tests/vendor/ lists test_crawler_reachability_unit.py, test_ftp_return_code_unit.py, test_ipx_packet_unit.py, test_ipx_socket_unit.py, test_request_prompt_unit.py, test_user_agent_unit.py, test_vendor_dest_path_unit.py, test_vendor_reg_apptype_generator_unit.py. The denominator was wrong; the conclusion is not. pytest --collect-only tests/vendor returns 62 tests across all 8 under both the test/engine-tests ignore shape and the bare gate shape — identical sets — and fixture_tier_paths() returns zero entries mentioning "vendor", so integration and pypcap-parity reach 0 of 8. No text in this PR states "7", so the prose is unaffected.

Also correcting my own brief. I warned that the four dark files "may name a different flag set" and to verify per file. They do not: all four declare a byte-identical VENDOR_DEPS = ('requests', 'bs4', 'html5lib'), which I checked myself. So the four rewritten comments are interchangeable in mechanism, not merely in wording.

And the suspicion I raised was well placed, with the opposite answer. I said a dependency_gate_gaps() report that came back identical after un-darkening 41 methods deserved scrutiny. It is not identical: base tree 16 gaps including ('HAS_VENDOR_DEPS', 'engine-tests'), PR head 15 with that entry correctly gone. That is the right outcome rather than a red flag, and test_the_undoctored_workflow_produces_no_unexplained_gap plus test_each_exclusion_still_describes_a_gap_that_is_really_there enforce it and pass.

The new prose is true, clause by clause. Monkeypatching find_spec('html5lib') to None flips HAS_VENDOR_DEPS to False for the four dark files and leaves it True for test_vendor_dest_path_unit.py, which is exactly the skip/run split the comments claim. pyproject.toml:243 is where the vendor-extra comment begins, with requests/beautifulsoup4 plain at :264-265. The six bs4.BeautifulSoup(text, 'html5lib') call sites are exactly six files — ftp/return_code.py, pcapng/{block_type,option_type,record_type}.py, reg/linktype.py, vlan/priority_level.py — and no test in test_vendor_dest_path_unit.py constructs a Vendor or calls .context(): every instance is cls.__new__(cls) with only ._dest_path() invoked.

Counts reproduced under both runners. Per-file collection 5/9/16/11/4 sums to 41 + 4 = 45, matching the PR's breakdown; pytest 45 passed / 159 subtests, unittest 45 OK. Re-run with socket.socket.connect/connect_ex, getaddrinfo and create_connection all patched to raise: 0 failures, which independently confirms the no-network claim. tests/test_tier_guard.py 97 passed / 512 subtests under both, in agreement.

No conflict from the extra. vendor = ["requests[socks]", "beautifulsoup4[html5lib]"] is html5lib's only declared provider; dpkt/scapy/pyshark/pycrate/cryptography declare no transitive dependency on requests/bs4/html5lib/PySocks; MUTUALLY_EXCLUSIVE_IMPORTS = frozenset({'pcap'}) is unrelated. Only engine-tests' install line gained ,vendor — the other four pip install -e lines are untouched, and the step runs once per matrix leg so it survives 3.10-3.14.

Nothing went stale when #773 merged, and no comment names a PR or agent as owning a file — I asked specifically because that defect appeared twice on #777. cron-vendor.yml's post-#773 install line is still pip install -e .[all] with no pytest invocation, so the pyproject.toml comment referring to it remains true.

CI 27 success / 3 expected skips, mergeStateStatus: CLEAN, one commit, +185/−40 matching git diff --stat. Coverage: this PR touches zero pcapkit/ lines, and the applicable evidence is the new guard test shown to fail pre-fix plus CI un-darkening 41 never-executed methods across all five Engines legs.