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

.github/workflows/unit-tests.yml

@@ -274,6 +274,59 @@ jobs:
   # 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.
+  #
+  # `vendor` joins the install line for #738's remaining gap, HAS_VENDOR_DEPS
+  # (41 methods across tests/vendor/test_request_prompt_unit.py,
+  # test_user_agent_unit.py, test_ipx_packet_unit.py and
+  # test_ftp_return_code_unit.py) -- one requirement extra short of the `test`
+  # extra's own requests+bs4 (#507): html5lib, which only
+  # beautifulsoup4[html5lib] provides. #738's ruling was to land this on a
+  # non-blocking leg rather than add it to `test`, following the precedent of
+  # 3.15 in python-compatibility.yml (b7f51401b) -- a job that exists and
+  # reports without being one of ruleset 23497679's 15 required checks. This
+  # job already is exactly that leg: "Engines Python X" is not among the 15
+  # (Python/Integration Python/Compat Python 3.10-3.14), so adding the extra
+  # here, rather than opening a new job, closes the gap without a sixth matrix
+  # to maintain. Its selection mirrors `test`'s ignore-shape, which is what
+  # already reaches these gates dark -- see this job's own comment above and
+  # tests/_dependency_gates.py's HAS_VENDOR_DEPS exclusion. It is also the
+  # only *per-pull-request* job with that property: `gate` reaches these
+  # gates too but never runs on a pull request at all (only via a release's
+  # `gate-only: true` call), and `integration`'s fixture-tier selection never
+  # reaches tests/vendor/ in the first place -- so this was the forced
+  # choice, not merely a convenient one.
+  #
+  # Worth recording since it bears on the ruling: none of the 41 methods this
+  # closes make a live network call, though not for one reason across the
+  # four files.
+  #
+  # test_user_agent_unit.py (11 methods) and test_request_prompt_unit.py (16)
+  # gate on HAS_VENDOR_DEPS only because pcapkit.vendor's package import
+  # chain needs requests/bs4/html5lib importable -- html5lib itself is never
+  # used by either file's test bodies, both of which replace requests.get
+  # with a recorder for the whole of every case (each file's own module
+  # docstring says so in its own words, not a shared one).
+  #
+  # test_ipx_packet_unit.py (9 methods) is a regression suite for a
+  # *retired* scrape: pcapkit.vendor.ipx.packet.Packet.LINK is None, so
+  # _request() short-circuits before ever calling requests.get, and
+  # test_request_makes_no_network_call asserts exactly that by making
+  # requests.get and requests.Session.request raise if reached at all.
+  # html5lib is an import precondition here too, not something the test
+  # bodies use.
+  #
+  # test_ftp_return_code_unit.py (5 methods) is not retired --
+  # pcapkit.vendor.ftp.return_code.ReturnCode.LINK is a live Wikipedia URL --
+  # and is the one file where html5lib is a *functional* dependency rather
+  # than an import precondition: its tests build the HTML fixture inline and
+  # hand it straight to ReturnCode.request(), which is
+  # bs4.BeautifulSoup(text, 'html5lib'). What keeps it off the network is the
+  # fixture being inline, not an absence of html5lib use.
+  #
+  # The #518 Wikipedia-403 flakiness the ruling was guarding against belongs
+  # to the crawlers HAS_CRAWLER_DEPS gates (live, unconditionally, since
+  # #507), not to any of the above -- but the leg placement follows the
+  # ruling as recorded rather than re-opening it here.
   engine-tests:
     name: Engines Python ${{ matrix.python-version }}
     if: ${{ inputs.gate-only != true }}
@@ -332,7 +385,7 @@ jobs:
       - 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]'
+          python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,PCAP_CT,vendor]'
 
       # See the `test` job above for why this step exists.
       - name: Report available parallelism

pyproject.toml

@@ -244,8 +244,8 @@ test = [
     # Without these, ``tests/vendor/test_ipx_socket_unit.py`` reports "9 skipped"
     # rather than failing, so the byte-for-byte regeneration check that is the
     # only guard on the hand-maintained IPX socket registry never runs on any CI
-    # path: the ``unit-tests.yml`` jobs install ``.[test]``, and
-    # ``cron-vendor.yml`` installs ``.[all]`` but never invokes pytest.
+    # path: every ``unit-tests.yml`` job's install line builds on ``.[test]``,
+    # and ``cron-vendor.yml`` installs ``.[all]`` but never invokes pytest.
     #
     # Both are needed, not just ``requests``. ``pcapkit.vendor.default`` imports
     # ``requests`` at module scope, and ``pcapkit.vendor.__init__`` then imports
@@ -254,9 +254,11 @@ test = [
     # ``ModuleNotFoundError`` that the crawler imports below it raise anyway.
     #
     # Plain, without the ``vendor`` extra's ``[socks]`` and ``[html5lib]``:
-    # nothing in the test suite goes through a proxy, and no unit-tier test
-    # parses HTML (none of them may -- the tier makes no network call), so
-    # neither parser nor proxy support is reachable from here. Their absence
+    # nothing in the test suite goes through a proxy, and the one unit-tier
+    # test that parses HTML (``tests/vendor/test_ftp_return_code_unit.py``)
+    # does so from an inline fixture with the ``vendor`` extra's own
+    # ``html5lib``, not through anything this extra provides, so neither
+    # parser nor proxy support is reachable from here. Their absence
     # makes :mod:`pcapkit.vendor` log two ``VendorWarning``s at import, which
     # pytest captures and shows only on failure; they change no result. See #507.
     "requests",

tests/_dependency_gates.py

@@ -313,25 +313,69 @@ class Exclusion(NamedTuple):
         ),
     ),
     'HAS_VENDOR_DEPS': Exclusion(
-        dark={'test': ('html5lib',), 'gate': ('html5lib',), 'engine-tests': ('html5lib',)},
+        dark={'test': ('html5lib',), 'gate': ('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 '
-            'dead IETF URL -- so a blocking leg would go red on upstream availability '
-            'rather than on this code. That leg is #738\'s own remaining scope and does '
-            'not exist yet. Note what is actually missing, which #738 gets wrong: '
-            '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.\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.'
+            "Ruled onto a non-blocking leg by #738's option (b), on the premise that the "
+            'crawlers fetch from IANA and Wikipedia and #518 records four Wikipedia 403s '
+            'and a dead IETF URL -- so a blocking leg would go red on upstream '
+            'availability rather than on this code. Note what is actually missing, which '
+            "#738's own inventory gets wrong: requests and bs4 have shipped in the test "
+            'extra since #507 and are installed on every job, so these classes are one '
+            '*requirement extra* short -- html5lib, which only beautifulsoup4[html5lib] '
+            'provides, i.e. the vendor and all extras.\n\n'
+            'engine-tests (#751) is that leg. It already mirrors test\'s ignore-shape '
+            'exactly (same --ignore flags), so it already reached these unit-tier '
+            'HAS_VENDOR_DEPS gates; adding vendor to its install line is what closes '
+            'them, and #738\'s own precedent for a job that "exists and reports" without '
+            "gating a merge -- 3.15 in python-compatibility.yml (b7f51401b) -- already "
+            'describes this job: "Engines Python X" has never been one of ruleset '
+            "23497679's 15 required checks, so it was already the non-blocking leg the "
+            'ruling asked for, and no new job was needed to get one. It is also the only '
+            '*per-pull-request* job with that property: gate reaches these gates too but '
+            "never runs on a pull request at all (only via a release's "
+            "gate-only: true call), and integration's fixture-tier selection never "
+            'reaches tests/vendor/ in the first place -- so engine-tests was the forced '
+            'choice, not merely a convenient one.\n\n'
+            'test and gate stay dark for different reasons, not the same one. test '
+            'declines html5lib per the non-blocking-leg ruling itself -- that is the '
+            'whole reason it is dark. gate is not part of the blocking matrix at all -- '
+            "it only runs on the release path (workflow_call's gate-only: true), never "
+            'on a pull request -- so the ruling does not require it dark; nothing about '
+            '"non-blocking" would stop vendor being added there too. It stays dark on a '
+            'separate, substantive ground instead: by the time a commit reaches the '
+            'release path it has already run test, integration and engine-tests, and '
+            'engine-tests already carries vendor, so gate would be re-verifying coverage '
+            'that already ran rather than adding any. Gaining a network-and-parser '
+            'extra on the one job that gates an actual release, for coverage the release '
+            'path already has by the time it runs, is not worth the footprint.\n\n'
+            'Worth recording since it bears on the ruling itself, though this exclusion '
+            'is not the place to relitigate it: none of the 41 methods this closes make '
+            'a live network call, though not for one reason across all four files. '
+            'test_user_agent_unit.py (11 methods) and test_request_prompt_unit.py (16) '
+            'gate on HAS_VENDOR_DEPS only because the pcapkit.vendor package import '
+            'chain needs requests/bs4/html5lib importable -- html5lib itself is never '
+            "used by either file's test bodies, both of which replace requests.get with "
+            "a recorder for the whole of every case (each file's own module docstring "
+            'says so in its own words, not a shared one). test_ipx_packet_unit.py (9 '
+            'methods) is a regression suite for a *retired* scrape -- Packet.LINK is '
+            'None, so _request() short-circuits before ever calling requests.get, and '
+            'test_request_makes_no_network_call asserts exactly that by making '
+            'requests.get and requests.Session.request raise if reached at all; '
+            'html5lib is an import precondition there too, nothing in the file uses it. '
+            'test_ftp_return_code_unit.py (5 methods) is not retired -- '
+            'pcapkit.vendor.ftp.return_code.ReturnCode.LINK is a live Wikipedia URL -- '
+            'and is the one file where html5lib is a *functional* dependency rather '
+            'than an import precondition: its tests build the HTML fixture inline and '
+            "hand it straight to ReturnCode.request(), which is "
+            "bs4.BeautifulSoup(text, 'html5lib'); what keeps it off the network is the "
+            'fixture being inline, not an absence of html5lib use. '
+            "#518's flakiness is real for HAS_CRAWLER_DEPS's live crawlers, which run "
+            'unconditionally since #507; it does not describe any of the above. '
+            'pypcap-parity does not appear here even though it also declines html5lib, '
+            'because its fixture-tier selection never reaches '
+            'tests/vendor/test_ipx_packet_unit.py or its three siblings in the first '
+            'place -- same shape as integration above it, which is why integration is '
+            'not listed either.'
         ),
     ),
     'HAS_SCAPY': Exclusion(

tests/test_tier_guard.py

@@ -1483,6 +1483,36 @@ def test_the_crawler_dependencies_are_satisfied_by_the_test_extra(self) -> None:
         self.assertNotIn('HAS_CRAWLER_DEPS', _dependency_gates.DEPENDENCY_GATE_EXCLUSIONS)
         self.assertIn('HAS_VENDOR_DEPS', _dependency_gates.DEPENDENCY_GATE_EXCLUSIONS)
 
+    def test_the_vendor_extra_closes_engine_tests_but_not_test_or_gate(self) -> None:
+        """#738's remaining scope: an existing non-blocking leg absorbs it.
+
+        ``engine-tests`` (#751) already reaches these gates through the same
+        ignore-shape selection as ``test`` -- see this guard's own
+        ``HAS_VENDOR_DEPS`` exclusion -- and it has never been one of ruleset
+        23497679's 15 required checks, so it already was the "job that exists
+        and reports without gating a merge" #738's ruling asked for. Installing
+        ``vendor`` there, rather than opening a sixth job, is what closes the
+        41-method gap on it. It is also the only *per-pull-request* job with
+        that property: ``gate`` reaches these gates too but never runs on a
+        pull request at all (only via a release's ``gate-only: true`` call),
+        and ``integration``'s fixture-tier selection never reaches
+        :file:`tests/vendor/` in the first place -- so ``engine-tests`` was
+        the forced choice, not merely a convenient one.
+
+        ``test`` and ``gate`` stay dark for different reasons, not the same
+        one -- see the exclusion's own reason for why ``gate`` is not merely
+        "also non-blocking".
+
+        """
+        gaps = {(gap.flag, gap.job) for gap in _dependency_gates.dependency_gate_gaps()}
+        self.assertNotIn(('HAS_VENDOR_DEPS', 'engine-tests'), gaps)
+        self.assertIn(('HAS_VENDOR_DEPS', 'test'), gaps)
+        self.assertIn(('HAS_VENDOR_DEPS', 'gate'), gaps)
+
+        exclusion = _dependency_gates.DEPENDENCY_GATE_EXCLUSIONS['HAS_VENDOR_DEPS']
+        self.assertNotIn('engine-tests', exclusion.dark)
+        self.assertEqual(set(exclusion.dark), {'test', 'gate'})
+
     def test_each_exclusion_still_describes_a_gap_that_is_really_there(self) -> None:
         """The anti-rot half, and the reason this is an allowlist and not a skip list.
 
@@ -1804,8 +1834,10 @@ def test_swapping_engine_tests_to_pypcap_is_an_ambiguous_satisfaction(self) -> N
         """
         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]'",
+            "python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,"
+            "PCAP_CT,vendor]'",
+            "python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,"
+            "PyPCAP,vendor]'",
         )
 
         gaps = {(gap.flag, gap.job) for gap in _dependency_gates.dependency_gate_gaps(doctored)}
@@ -1873,8 +1905,10 @@ def test_removing_the_negation_or_the_exact_path_reopens_762(self) -> None:
         )
         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]'",
+            "python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,"
+            "PCAP_CT,vendor]'",
+            "python -m pip install -e '.[test,DPKT,crypto,NGAP,Scapy,PyShark,PyPCAPFile,"
+            "PyPCAP,vendor]'",
         )
 
         with self.subTest(mutation='flag_exclusions stubbed to report nothing'):

tests/vendor/test_ftp_return_code_unit.py

@@ -50,7 +50,8 @@
 #: Every distribution importing :mod:`pcapkit.vendor` needs -- see
 #: :mod:`tests.vendor.test_user_agent_unit` for why ``requests`` alone is not
 #: enough. They ship in the ``vendor`` extra (:file:`pyproject.toml`), not
-#: ``test``, and CI installs ``.[test]``.
+#: ``test`` -- so these run on ``engine-tests``, which installs ``vendor`` too
+#: (#738), and skip on the ``test`` and ``gate`` jobs, which do not.
 VENDOR_DEPS = ('requests', 'bs4', 'html5lib')
 
 #: Whether the crawlers are importable at all.

tests/vendor/test_ipx_packet_unit.py

@@ -53,8 +53,10 @@
 VENDOR_DEPS = ('requests', 'bs4', 'html5lib')
 
 #: Whether the crawlers are importable at all. They ship in the ``vendor`` extra
-#: (:file:`pyproject.toml`), not ``test``, and CI installs ``.[test]`` -- so these
-#: tests skip in CI as things stand. Guarded the same way
+#: (:file:`pyproject.toml`), not ``test`` -- so these tests skip on the ``test``
+#: and ``gate`` jobs of :file:`.github/workflows/unit-tests.yml`, which never
+#: install ``vendor``, but run for real on ``engine-tests``, which does (#738).
+#: Guarded the same way
 #: :file:`tests/protocols/test_dispatch_registry_unit.py` guards its own optional
 #: runtime dependencies, rather than making the whole unit tier depend on the
 #: crawlers' requirements. See #518.

tests/vendor/test_request_prompt_unit.py

@@ -99,10 +99,12 @@
 VENDOR_DEPS = ('requests', 'bs4', 'html5lib')
 
 #: Whether the crawlers are importable at all. They ship in the ``vendor`` extra
-#: (:file:`pyproject.toml`), not ``test``, and CI installs ``.[test]``, so these
-#: cases skip in CI as things stand. Guarded the way
-#: :file:`tests/protocols/test_dispatch_registry_unit.py` guards its own optional
-#: dependencies rather than making the whole unit tier depend on the crawlers'.
+#: (:file:`pyproject.toml`), not ``test`` -- so these cases skip on the ``test``
+#: and ``gate`` jobs of :file:`.github/workflows/unit-tests.yml`, but run for
+#: real on ``engine-tests``, which installs ``vendor`` too (#738).
+#: Guarded the way :file:`tests/protocols/test_dispatch_registry_unit.py` guards
+#: its own optional dependencies rather than making the whole unit tier depend on
+#: the crawlers'.
 HAS_VENDOR_DEPS = all(importlib.util.find_spec(name) is not None for name in VENDOR_DEPS)
 
 #: How many prompts :func:`_fake_input` will answer before it decides the loop is

tests/vendor/test_user_agent_unit.py

@@ -63,8 +63,10 @@
 VENDOR_DEPS = ('requests', 'bs4', 'html5lib')
 
 #: Whether the crawlers are importable at all. They ship in the ``vendor`` extra
-#: (:file:`pyproject.toml`), not ``test``, and CI installs ``.[test]`` -- so these
-#: tests skip in CI as things stand. Guarded the same way
+#: (:file:`pyproject.toml`), not ``test`` -- so these tests skip on the ``test``
+#: and ``gate`` jobs of :file:`.github/workflows/unit-tests.yml`, which never
+#: install ``vendor``, but run for real on ``engine-tests``, which does (#738).
+#: Guarded the same way
 #: :file:`tests/protocols/test_dispatch_registry_unit.py` guards its own optional
 #: runtime dependencies, rather than making the whole unit tier depend on the
 #: crawlers' requirements. See #518.