test(foundation): retry a pyshark engine run once on a tshark crash (#1534)

tests/foundation/engines/test_engine_agreement_retry_unit.py

@@ -0,0 +1,91 @@
+# -*- coding: utf-8 -*-
+"""The engine agreement module retries a pyshark run once on a tshark crash.
+
+GitHub issue #1534: tshark now and then exits 255 on a capture it reads cleanly
+on a rerun, and pyshark raises ``TSharkCrashException``. The agreement module
+(:file:`test_engine_agreement_runtime.py`) caches one run per engine and
+capture, so that one crash failed all nine pyshark aspects of the capture.
+
+Each case replaces :func:`pcapkit.extract` with a stand-in, so nothing here
+reads a capture or needs pyshark; the exception is matched by class name, so a
+class of the same name stands in for pyshark's.
+
+"""
+
+import types
+import unittest
+import warnings
+from unittest import mock
+
+from tests._support import reimport_once_per_class
+from tests.foundation._roundtrip import Outcome
+from tests.foundation.engines import test_engine_agreement_runtime as agreement
+
+#: A capture git tracks, so the unit-tier guard lets its name through.
+CAPTURE = 'in.pcap'
+
+
+class TSharkCrashException(Exception):
+    """Stands in for ``pyshark.capture.capture.TSharkCrashException``."""
+
+
+@unittest.skipUnless(agreement.HAS_RUNTIME, 'runtime dependencies not installed')
+class TestTSharkCrashRetry(unittest.TestCase):
+    """Pin how the agreement module's runner handles a pyshark run that raises."""
+
+    def setUp(self) -> None:
+        reimport_once_per_class(self)
+
+    def _compare(self, *errors: 'Exception') -> 'tuple[Outcome, int, list[str]]':
+        """``compare`` the pyshark ``frames`` aspect, its runs raising ``errors`` in turn.
+
+        Returns the outcome, the number of pyshark runs, and every warning raised.
+
+        """
+        import pcapkit
+
+        pending = list(errors)
+        calls = []  # type: list[str]
+
+        def extract(**kwargs: 'object') -> 'object':
+            engine = kwargs['engine']
+            calls.append(engine)
+            if engine == 'pyshark' and pending:
+                raise pending.pop(0)
+            return types.SimpleNamespace(_exnam=engine, length=2, frame=(),
+                                         reassembly=None, trace=None)
+
+        with mock.patch.dict(agreement._RUNS, clear=True), \
+                mock.patch.object(pcapkit, 'extract', extract), \
+                warnings.catch_warnings(record=True) as caught:
+            warnings.simplefilter('always')
+            outcome = agreement.compare('pyshark', CAPTURE, 'frames')
+        return outcome, calls.count('pyshark'), [str(item.message) for item in caught]
+
+    def test_one_crash_is_retried_and_logged(self) -> None:
+        crash = TSharkCrashException('TShark (pid 1) seems to have crashed (retcode: 255).\n'
+                                     'Last error line: None')
+        outcome, runs, logged = self._compare(crash)
+        self.assertEqual(outcome, Outcome('OK'))
+        self.assertEqual(runs, 2)
+        self.assertEqual(logged, [f'pyshark on {CAPTURE}: TSharkCrashException: TShark (pid 1) '
+                                  'seems to have crashed (retcode: 255). Retrying once (#1534).'])
+
+    def test_a_crash_that_persists_is_an_error(self) -> None:
+        crashes = [TSharkCrashException(f'TShark (pid {pid}) seems to have crashed (retcode: 255).')
+                   for pid in (1, 2)]
+        outcome, runs, logged = self._compare(*crashes)
+        self.assertEqual(outcome, Outcome('ERROR', 'TSharkCrashException: TShark (pid 2) seems '
+                                                   'to have crashed (retcode: 255).'))
+        self.assertEqual(runs, 2)
+        self.assertEqual(len(logged), 1)
+
+    def test_any_other_error_is_not_retried(self) -> None:
+        outcome, runs, logged = self._compare(ValueError('invalid literal'))
+        self.assertEqual(outcome, Outcome('ERROR', 'ValueError: invalid literal'))
+        self.assertEqual(runs, 1)
+        self.assertEqual(logged, [])
+
+
+if __name__ == '__main__':
+    unittest.main()

tests/foundation/engines/test_engine_agreement_runtime.py

@@ -196,12 +196,23 @@ def _property(extractor: 'Any', name: 'str') -> 'Any':
 
 
 _RUNS = {}  # type: dict[tuple[str, str], _Run]
+#: What pyshark raises when tshark exits non-zero, by class name, so this module
+#: imports without pyshark.
+TSHARK_CRASH = 'TSharkCrashException'
 
 
 def _run(engine: 'str', capture: 'str') -> '_Run':
     key = (engine, capture)
     if key not in _RUNS:
-        _RUNS[key] = _Run(engine, capture)
+        run = _Run(engine, capture)
+        # tshark sometimes exits 255 on a capture it reads cleanly on a rerun
+        # (#1534), and the run is cached, so one crash fails every aspect. Retry
+        # once, visibly; a crash that persists stays an ERROR.
+        if run.error is not None and run.error.split(':', 1)[0] == TSHARK_CRASH:
+            warnings.warn(f'{engine} on {capture}: {run.error.splitlines()[0]} '
+                          'Retrying once (#1534).', RuntimeWarning)
+            run = _Run(engine, capture)
+        _RUNS[key] = run
     return _RUNS[key]