diff --git a/tests/foundation/engines/test_engine_agreement_retry_unit.py b/tests/foundation/engines/test_engine_agreement_retry_unit.py new file mode 100644 index 0000000000..d42b0502ea --- /dev/null +++ b/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() diff --git a/tests/foundation/engines/test_engine_agreement_runtime.py b/tests/foundation/engines/test_engine_agreement_runtime.py index 2acb389412..d7d2bcc7ab 100644 --- a/tests/foundation/engines/test_engine_agreement_runtime.py +++ b/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]