fix(generators): make_samples builds fixtures from the shared checkout, not the tree it runs in

examples/generators/dispatch.py

@@ -75,7 +75,9 @@
 from __future__ import annotations
 
 import os
+import pathlib
 import struct
+import sys
 import tempfile
 from typing import TYPE_CHECKING, NamedTuple
 
@@ -85,6 +87,14 @@
 __all__ = ['cases', 'probe', 'outcomes', 'Case', 'Outcome', 'Family', 'FAMILIES',
           'KNOWN_DEGRADED', 'code_name']
 
+#: Repository root, i.e. the grandparent of the directory holding this file.
+ROOT = pathlib.Path(__file__).resolve().parents[2]
+
+# Resolve pcapkit from this tree rather than from the install -- see
+# make_samples.py (#1343).
+if str(ROOT) not in sys.path:
+    sys.path.insert(0, str(ROOT))
+
 #: Fixed capture start time, kept for parity with :mod:`examples.generators.options`
 #: even though nothing here is written to disk.
 EPOCH = 1500000000

examples/generators/make_samples.py

@@ -43,6 +43,15 @@
 
 #: Repository root, i.e. the grandparent of the directory holding this script.
 ROOT = pathlib.Path(__file__).resolve().parents[2]
+
+# Run as a script, sys.path[0] is this directory, not the tree root, so pcapkit
+# would resolve to whatever is installed -- an editable install pins one
+# checkout -- rather than to this tree (#1343). PathFinder precedes the
+# editable finder in sys.meta_path, so putting the root first wins. Every
+# sibling that imports pcapkit repeats this, since each may run on its own.
+if str(ROOT) not in sys.path:
+    sys.path.insert(0, str(ROOT))
+
 #: Directory holding this script and its sibling generator modules.
 HERE = pathlib.Path(__file__).resolve().parent
 #: Destination directory for every generated capture.

examples/generators/options.py

@@ -30,10 +30,11 @@
 ``_read_*`` against ``_make_*``.
 
 Only the codes that construct *successfully* can appear in a capture -- there
-are no octets for the ones that do not. Roughly a fifth of the space does not,
-and that shortfall is not swept up: it is recorded case by case in
-:file:`tests/protocols/test_option_roundtrip_unit.py`, against the defect that
-causes it.
+are no octets for the ones that do not. A failing case in a captured family
+makes :func:`generate` raise once the rest are written, so a broken or foreign
+library cannot pass for a smaller fixture set. Every other failure is recorded
+case by case in :file:`tests/protocols/test_option_roundtrip_unit.py`, against
+the defect that causes it.
 
 Why the case table is not a list of options
 -------------------------------------------
@@ -80,7 +81,7 @@
 Two deliberate deviations from the sibling generators
 ----------------------------------------------------
 
-Almost every import of :mod:`pcapkit` and of :mod:`scapy` in this module is
+Every import of :mod:`pcapkit` and of :mod:`scapy` in this module is
 inside the function that needs it, which is why :mod:`pylint`'s
 ``import-outside-toplevel`` is switched off for the file rather than argued with
 forty times. Both are load-bearing. :mod:`scapy` is needed only by
@@ -111,18 +112,23 @@
 import datetime
 import pathlib
 import signal
+import sys
 import warnings
 from typing import TYPE_CHECKING, NamedTuple
 
-from pcapkit.protocols.protocol import ProtocolBase
-
 if TYPE_CHECKING:
     from typing import Any, Callable, Iterator, Optional
 
 __all__ = ['generate', 'cases', 'roundtrip', 'outcomes', 'Case', 'Outcome', 'FAMILIES']
 
 #: Repository root, i.e. the grandparent of the directory holding this file.
 ROOT = pathlib.Path(__file__).resolve().parents[2]
+
+# Resolve pcapkit from this tree rather than from the install -- see
+# make_samples.py (#1343).
+if str(ROOT) not in sys.path:
+    sys.path.insert(0, str(ROOT))
+
 #: Default destination directory for the generated captures.
 SAMPLE = ROOT / 'examples' / 'captures'
 
@@ -303,6 +309,7 @@ def handler(registry: 'Any', code: 'Any') -> 'Any':
         The handler registered for ``code``, or the registry's fallback.
 
     """
+    from pcapkit.protocols.protocol import ProtocolBase
     return ProtocolBase._lookup_registry(registry, code)  # pylint: disable=protected-access
 
 
@@ -1859,6 +1866,10 @@ def generate(dest: 'pathlib.Path | None' = None) -> 'list[pathlib.Path]':
     Returns:
         The paths written, in the order they were written.
 
+    Raises:
+        RuntimeError: If any captured case was left out, after the captures
+            that could be built have been written.
+
     """
     from scapy.all import wrpcap  # pylint: disable=no-name-in-module
 
@@ -1903,6 +1914,14 @@ def generate(dest: 'pathlib.Path | None' = None) -> 'list[pathlib.Path]':
         print(f'  [left out]  {label}: {unreachable[label]}')
     for label in sorted(warned):
         print(f'  [warned]    {label}: {"; ".join(warned[label])}')
+
+    # On a correct tree every captured case constructs and parses, so a case
+    # left out means the library is broken or is not this tree's (#1343). Fail
+    # rather than hand back a silently smaller fixture set.
+    if unreachable:
+        import pcapkit
+        raise RuntimeError(f'options: {len(unreachable)} case(s) left out of the captures, '
+                           f'built with pcapkit from {pathlib.Path(pcapkit.__file__).parent}')
     return written
 
 

examples/generators/pcapng.py

@@ -167,6 +167,12 @@
 
 #: Repository root, i.e. the grandparent of the directory holding this script.
 ROOT = pathlib.Path(__file__).resolve().parents[2]
+
+# Resolve pcapkit from this tree rather than from the install -- see
+# make_samples.py (#1343).
+if str(ROOT) not in sys.path:
+    sys.path.insert(0, str(ROOT))
+
 #: Default destination directory for the fixtures.
 SAMPLE_DIR = ROOT / 'examples' / 'captures'
 #: Committed fixture the synthesised DHCP payloads are lifted from.

tests/project/test_generators_import_their_own_tree.py

@@ -0,0 +1,144 @@
+# -*- coding: utf-8 -*-
+"""The sample generators build from their own tree's :mod:`pcapkit`.
+
+GitHub issue #1343. Run as a script, a generator's ``sys.path[0]`` is
+:file:`examples/generators/`, not the tree root, so ``import pcapkit`` fell
+through to whatever was installed. An editable install pins one checkout, so a
+worktree's generators silently built their fixtures from *that* checkout's
+library: 257 captured cases instead of 266, with nothing failing.
+
+Each generator that imports :mod:`pcapkit` now puts its own tree root first on
+:data:`sys.path`. The probe below proves it from a scratch worktree, whose path
+no install points at, and with ``PYTHONPATH`` and ``PYTHONSAFEPATH`` stripped.
+The probe script lives outside the tree, so -- exactly as when a generator is run
+as a script -- nothing but the generator itself can put the tree on the path.
+
+The second half of the issue is that the shortfall was silent:
+``options.generate()`` recorded each failing case and carried on. It now raises
+once the captures it could build are written, which
+:meth:`OptionsGenerateTests.test_left_out_case_raises` pins.
+
+This module is unit-tier and reads no capture.
+
+"""
+from __future__ import annotations
+
+import importlib.util
+import os
+import pathlib
+import re
+import shutil
+import subprocess
+import sys
+import tempfile
+import unittest
+from unittest import mock
+
+#: Repository root.
+ROOT = pathlib.Path(__file__).resolve().parents[2]
+#: Directory holding the generators.
+GENERATORS = ROOT / 'examples' / 'generators'
+#: A top-level or function-local import of :mod:`pcapkit`.
+IMPORTS_PCAPKIT = re.compile(r'^\s*(?:from|import)\s+pcapkit\b', re.MULTILINE)
+
+#: Executes a generator's module body without its ``__main__`` block, then says
+#: where :mod:`pcapkit` resolves from. Kept outside the tree under test.
+PROBE = '''\
+import runpy, sys
+runpy.run_path(sys.argv[1], run_name='_probe_1343')
+import pcapkit
+print(pcapkit.__file__)
+'''
+
+
+def _git(*args: 'str') -> 'subprocess.CompletedProcess[str]':
+    return subprocess.run(['git', '-C', str(ROOT), *args], capture_output=True,
+                          text=True, check=False, timeout=120)
+
+
+def _generators() -> 'list[pathlib.Path]':
+    """:file:`make_samples.py`, plus every sibling that imports :mod:`pcapkit`."""
+    return sorted(path for path in GENERATORS.glob('*.py')
+                  if path.name == 'make_samples.py'
+                  or IMPORTS_PCAPKIT.search(path.read_text(encoding='utf-8')))
+
+
+class OwnTreeImportTests(unittest.TestCase):
+    """Run from a scratch worktree, every generator imports that worktree."""
+
+    scratch: 'pathlib.Path'
+    tree: 'pathlib.Path'
+
+    @classmethod
+    def setUpClass(cls) -> None:
+        if shutil.which('git') is None:
+            raise unittest.SkipTest('git is not installed')
+        if _git('rev-parse', '--is-inside-work-tree').stdout.strip() != 'true':
+            raise unittest.SkipTest('not a git work tree')
+
+        cls.scratch = pathlib.Path(tempfile.mkdtemp(prefix='pcapkit-1343-'))
+        cls.tree = cls.scratch / 'tree'
+        added = _git('worktree', 'add', '--detach', str(cls.tree), 'HEAD')
+        if added.returncode != 0:
+            shutil.rmtree(cls.scratch, ignore_errors=True)
+            raise unittest.SkipTest(f'git worktree add failed: {added.stderr.strip()}')
+
+        # Test the generators as they stand in this tree, committed or not.
+        for path in GENERATORS.glob('*.py'):
+            shutil.copy2(path, cls.tree / 'examples' / 'generators' / path.name)
+        (cls.scratch / 'probe.py').write_text(PROBE, encoding='utf-8')
+
+    @classmethod
+    def tearDownClass(cls) -> None:
+        # ``remove`` drops this worktree's own admin entry. No ``prune``: that
+        # would touch every other worktree of the repository too.
+        _git('worktree', 'remove', '--force', str(cls.tree))
+        shutil.rmtree(cls.scratch, ignore_errors=True)
+
+    def test_generators_resolve_pcapkit_inside_their_tree(self) -> None:
+        """``pcapkit.__file__`` lies in the scratch worktree, from any cwd."""
+        env = {key: value for key, value in os.environ.items()
+               if key not in ('PYTHONPATH', 'PYTHONSAFEPATH')}
+        expected = (self.tree / 'pcapkit').resolve()
+
+        generators = _generators()
+        self.assertIn('options.py', [path.name for path in generators])
+        for path in generators:
+            for cwd in (self.scratch, pathlib.Path('/')):
+                with self.subTest(generator=path.name, cwd=str(cwd)):
+                    result = subprocess.run(
+                        [sys.executable, str(self.scratch / 'probe.py'),
+                         str(self.tree / 'examples' / 'generators' / path.name)],
+                        cwd=cwd, env=env, capture_output=True, text=True,
+                        check=False, timeout=300)
+                    self.assertEqual(result.returncode, 0, result.stderr)
+                    resolved = pathlib.Path(result.stdout.strip().splitlines()[-1]).resolve()
+                    self.assertEqual(resolved.parent, expected,
+                                     f'{path.name} imported pcapkit from {resolved}')
+
+
+class OptionsGenerateTests(unittest.TestCase):
+    """``options.generate()`` fails rather than writing a smaller fixture set."""
+
+    def test_left_out_case_raises(self) -> None:
+        if importlib.util.find_spec('scapy') is None:
+            self.skipTest('options.generate() needs scapy')
+
+        spec = importlib.util.spec_from_file_location(
+            'pcapkit_samples_options_1343', GENERATORS / 'options.py')
+        assert spec is not None and spec.loader is not None
+        options = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(options)
+
+        family = next(family for family in options.FAMILIES if family.capture is not None)
+        case = options.Case(family.label, 0, 'Probe', {})
+        left_out = options.Outcome(case, 'CONSTRUCT', 'ValueError: probe', None, ())
+
+        with mock.patch.object(options, 'outcomes', lambda: iter([left_out])), \
+                tempfile.TemporaryDirectory(prefix='pcapkit-1343-') as dest:
+            with self.assertRaisesRegex(RuntimeError, r'1 case\(s\) left out'):
+                options.generate(pathlib.Path(dest))
+
+
+if __name__ == '__main__':
+    unittest.main()