diff --git a/examples/generators/options.py b/examples/generators/options.py index 89256ac33a..a6bdc08e29 100644 --- a/examples/generators/options.py +++ b/examples/generators/options.py @@ -99,7 +99,9 @@ and without a deadline ``make samples`` does not fail -- it *hangs*, taking the whole build with it. A signal is what interrupts it, because the loop is pure Python and holds the GIL for the whole of an iteration, so no watchdog thread -would ever get to run. +would ever get to run. The deadline is wall-clock time, so a stalled runner +can trip it on a case that normally takes milliseconds (#1428): +:data:`TIMEOUT_SCALE_ENV` stretches it without disarming it. """ @@ -110,6 +112,8 @@ import collections import datetime +import math +import os import pathlib import signal import sys @@ -149,9 +153,23 @@ DST_IP6 = '2001:db8::2' #: Seconds a single case may take before :func:`roundtrip` gives up on it and -#: records ``'TIMEOUT'``. Generous next to a working case, which takes low -#: single-digit milliseconds, and short enough that a whole sweep still ends. -DEADLINE = 5 +#: records ``'TIMEOUT'``, before :data:`TIMEOUT_SCALE_ENV` stretches it. A working +#: case takes low single-digit milliseconds, so this is headroom for a stalled +#: runner rather than for slow code (#1428), and it matches the per-case budget +#: :file:`tests/protocols/test_option_roundtrip_unit.py` gives the same cycle. A +#: case that never finishes still trips it, so a sweep still ends. +DEADLINE = 30 + +#: Environment variable multiplying every :func:`roundtrip` deadline -- the same +#: one :func:`tests._support.scale_timeout` reads, so one setting stretches the +#: suite's hang guards and this one together. Read here rather than imported, +#: because this module runs as a standalone script with no :mod:`tests` package. +TIMEOUT_SCALE_ENV = 'PCAPKIT_TEST_TIMEOUT_SCALE' + +#: Longest deadline :func:`scale_deadline` hands out: one day, as in +#: :mod:`tests._support`. :func:`signal.alarm` takes a C ``int`` and raises +#: :exc:`OverflowError` past it, after the handler would already be installed. +MAX_DEADLINE = 86400 #: Fixed instant handed to any constructor that would otherwise read the clock. #: Timezone-aware and in the past, so it is stable and unambiguous. @@ -224,7 +242,8 @@ class Outcome(NamedTuple): #: * ``'RECONSTRUCT'`` -- ``_make_*`` could not consume what ``_read_*`` #: produced. This is the step a construct-then-parse test cannot see. #: * ``'MISMATCH'`` -- both directions worked and the octets differed. -#: * ``'TIMEOUT'`` -- the case did not finish inside :data:`DEADLINE`. +#: * ``'TIMEOUT'`` -- the case did not finish inside :data:`DEADLINE`, as +#: stretched by :data:`TIMEOUT_SCALE_ENV`. STATUSES = ('OK', 'CONSTRUCT', 'PARSE', 'RECONSTRUCT', 'MISMATCH', 'TIMEOUT') @@ -1685,8 +1704,9 @@ def roundtrip(case: 'Case', deadline: 'int' = DEADLINE) -> 'Outcome': Args: case: The case to exercise. - deadline: Whole seconds to allow, or ``0`` for no deadline. See the - module docstring for why there is one at all. + deadline: Whole seconds to allow, or ``0`` for no deadline. A non-zero + deadline is stretched by :data:`TIMEOUT_SCALE_ENV`. See the module + docstring for why there is one at all. Returns: An :class:`Outcome` naming the step that failed, or ``'OK'``. @@ -1752,9 +1772,52 @@ def seen() -> 'tuple[str, ...]': return Outcome(case, 'OK', '', octets, warned) +def scale_deadline(seconds: 'int') -> 'int': + """Stretch a per-case deadline by :data:`TIMEOUT_SCALE_ENV`. + + The same contract as :func:`tests._support.scale_timeout`, restated because + this module cannot import from :mod:`tests`. Read on every call, so a test + can set the variable for itself. Unset or blank, the deadline applies as + written. A value that is not a positive finite number is refused rather than + ignored: falling back to ``1`` would quietly undo a scale CI asked for, and a + zero would disarm the guard. A scale stretching the deadline past + :data:`MAX_DEADLINE` is refused the same way. + + Args: + seconds: The deadline as given to :func:`roundtrip`. + + Returns: + Whole seconds, at least one -- :func:`signal.alarm` counts in whole + seconds, and an alarm of zero is no alarm at all. + + Raises: + ValueError: If :data:`TIMEOUT_SCALE_ENV` is malformed, or scales + ``seconds`` past :data:`MAX_DEADLINE`. + + """ + raw = os.environ.get(TIMEOUT_SCALE_ENV, '').strip() + try: + scale = float(raw) if raw else 1.0 + except ValueError: + scale = math.nan + if not (math.isfinite(scale) and scale > 0): + raise ValueError(f'{TIMEOUT_SCALE_ENV} must be a positive number, not {raw!r}') + # Compared before rounding: the product of two finite floats can still be + # infinite, and ``math.ceil`` raises on that rather than returning. + scaled = seconds * scale + if not scaled <= MAX_DEADLINE: + raise ValueError(f'{TIMEOUT_SCALE_ENV}={raw!r} stretches a {seconds}s deadline ' + f'past the {MAX_DEADLINE}s ceiling') + return max(1, math.ceil(scaled)) + + class _deadline: # pylint: disable=invalid-name """Raise :exc:`TimeoutError` in the body after ``seconds`` seconds. + ``seconds`` is stretched by :func:`scale_deadline` on entry, and the + :exc:`TimeoutError` names the stretched value -- the seconds actually + allowed. + Written as a class rather than a :func:`contextlib.contextmanager` so that it can be entered alongside :func:`warnings.catch_warnings` in one ``with`` statement without the generator machinery swallowing the alarm. @@ -1767,8 +1830,9 @@ class _deadline: # pylint: disable=invalid-name """ def __init__(self, seconds: 'int') -> 'None': - #: Whole seconds to allow. :func:`signal.alarm` counts in whole - #: seconds, so this cannot usefully be fractional. + #: Whole seconds to allow: as given, then as scaled once entered. + #: :func:`signal.alarm` counts in whole seconds, so this cannot usefully + #: be fractional. self.seconds = seconds #: Whether the alarm was actually armed, and so needs disarming. self.armed = False @@ -1778,6 +1842,9 @@ def __init__(self, seconds: 'int') -> 'None': def __enter__(self) -> '_deadline': if self.seconds <= 0 or not hasattr(signal, 'SIGALRM'): return self + # Scaled before the handler goes in, so a malformed scale raises with + # nothing installed that would need restoring. + self.seconds = scale_deadline(self.seconds) def expire(signum: 'int', frame: 'Any') -> 'None': raise TimeoutError(f'did not finish within {self.seconds}s') @@ -1802,7 +1869,8 @@ def outcomes(families: 'Optional[tuple[Family, ...]]' = None, Args: families: Families to enumerate; :data:`FAMILIES` if not given. - deadline: Whole seconds to allow each case. + deadline: Whole seconds to allow each case, before + :data:`TIMEOUT_SCALE_ENV` stretches it. Yields: One :class:`Outcome` per case, in case order. diff --git a/tests/protocols/test_option_generator_deadline_scale_1428_unit.py b/tests/protocols/test_option_generator_deadline_scale_1428_unit.py new file mode 100644 index 0000000000..351beee5b8 --- /dev/null +++ b/tests/protocols/test_option_generator_deadline_scale_1428_unit.py @@ -0,0 +1,155 @@ +# -*- coding: utf-8 -*- +"""The option generator's per-case deadline honours the suite's timeout scale. C.f. #1428. + +:func:`roundtrip` in :file:`examples/generators/options.py` arms a +:func:`signal.alarm` around each case and records ``'TIMEOUT'`` when it fires. +The budget was a flat 5 seconds of wall-clock time, so a stalled CI runner failed +:mod:`tests.protocols.test_option_generator_httpv2_stream_unit` on a case that +takes about 1.3 ms. The default is now 30 seconds, the budget +:mod:`tests.protocols.test_option_roundtrip_unit` already gives the same cycle, +and ``PCAPKIT_TEST_TIMEOUT_SCALE`` stretches it exactly as it stretches +:func:`tests._support.time_limit`. + +This module is unit tier: it loads the generator by path and builds nothing, so +it imports no :mod:`pcapkit` and reads no capture. + +""" + +from __future__ import annotations + +import importlib.util +import os +import signal +import sys +import types +import unittest +from typing import TYPE_CHECKING +from unittest import mock + +from tests._tiers import ROOT + +if TYPE_CHECKING: + from typing import Any + +#: The variable :mod:`tests._support` reads, spelled out so a generator reading a +#: different name fails here rather than agreeing with itself. +SCALE_ENV = 'PCAPKIT_TEST_TIMEOUT_SCALE' + + +def _load_generator() -> 'types.ModuleType': + """Load :file:`examples/generators/options.py` by path, under a name of its own. + + Returns: + The generator module. + + Raises: + RuntimeError: If the module cannot be found or loaded. + + """ + path = ROOT / 'examples' / 'generators' / 'options.py' + spec = importlib.util.spec_from_file_location( + 'pcapkit_samples_options_deadline_scale', path) + if spec is None or spec.loader is None: # pragma: no cover + raise RuntimeError(f'cannot load the option generator from {path}') + + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +class _Spin: # pylint: disable=too-few-public-methods + """A family whose constructor never returns, standing in for a real hang.""" + + def build(self, code: 'Any', kwargs: 'Any') -> bytes: # pylint: disable=unused-argument + """Loop until the deadline interrupts it.""" + while True: + pass + + +@unittest.skipUnless(hasattr(signal, 'SIGALRM'), 'needs signal.SIGALRM') +class GeneratorDeadlineScaleTests(unittest.TestCase): + """:data:`DEADLINE`, as armed under each value of the scale.""" + + options: 'types.ModuleType' + + @classmethod + def setUpClass(cls) -> None: + cls.options = _load_generator() + + def setUp(self) -> None: + # Start each test from no pending alarm and a handler this class owns, and + # from no scale: one set for the whole run would stretch every value below. + self.handler = lambda signum, frame: None + previous = signal.signal(signal.SIGALRM, self.handler) + self.addCleanup(signal.signal, signal.SIGALRM, previous) + self.addCleanup(signal.alarm, 0) + environ = mock.patch.dict(os.environ) + environ.start() + self.addCleanup(environ.stop) + os.environ.pop(SCALE_ENV, None) + + def armed(self, seconds: int) -> int: + """Seconds :class:`_deadline` arms for ``seconds``, read back from the alarm.""" + with self.options._deadline(seconds): # pylint: disable=protected-access + return signal.alarm(0) + + def test_the_default_is_thirty_seconds_unscaled(self) -> None: + """Unset, the default applies as written -- and it is no longer 5.""" + self.assertEqual(self.options.DEADLINE, 30) + self.assertEqual(self.options.TIMEOUT_SCALE_ENV, SCALE_ENV) + self.assertIn(self.armed(self.options.DEADLINE), (29, 30)) + + def test_the_scale_multiplies_what_is_armed(self) -> None: + """The scale stretches the alarm itself, rounding up to whole seconds.""" + for raw, seconds, expected in (('3', 5, 15), ('1.5', 5, 8), ('0.01', 5, 1), + (' 2 ', 30, 60)): + with self.subTest(scale=raw, seconds=seconds): + os.environ[SCALE_ENV] = raw + self.assertEqual(self.options.scale_deadline(seconds), expected) + self.assertIn(self.armed(seconds), (expected - 1, expected)) + + def test_a_blank_scale_leaves_the_deadline_alone(self) -> None: + """Empty or whitespace reads as unset, as in :mod:`tests._support`.""" + for raw in ('', ' '): + with self.subTest(scale=raw): + os.environ[SCALE_ENV] = raw + self.assertEqual(self.options.scale_deadline(30), 30) + + def test_a_malformed_scale_is_refused_before_anything_is_armed(self) -> None: + """Ignoring it would undo a scale CI asked for; zero would disarm the guard.""" + for raw in ('fast', '0', '-1', 'nan', 'inf'): + with self.subTest(scale=raw): + os.environ[SCALE_ENV] = raw + with self.assertRaisesRegex(ValueError, f'{SCALE_ENV} must be a positive number'): + self.armed(5) + self.assertIs(signal.getsignal(signal.SIGALRM), self.handler) + self.assertEqual(signal.alarm(0), 0) + + def test_an_oversized_scale_is_refused(self) -> None: + """Past a day is refused, not handed to :func:`signal.alarm` to overflow.""" + for raw in ('1e9', '1e308'): + with self.subTest(scale=raw): + os.environ[SCALE_ENV] = raw + with self.assertRaisesRegex(ValueError, 'past the 86400s ceiling'): + self.armed(30) + self.assertIs(signal.getsignal(signal.SIGALRM), self.handler) + + def test_a_zero_deadline_stays_disarmed_under_any_scale(self) -> None: + """``deadline=0`` is the opt-out the round-trip suite uses; scaling must not undo it.""" + os.environ[SCALE_ENV] = '3' + self.assertEqual(self.armed(0), 0) + + def test_a_hang_is_still_recorded_as_timeout_naming_the_scaled_deadline(self) -> None: + """The ``'TIMEOUT'`` contract holds, and its detail names the seconds allowed.""" + os.environ[SCALE_ENV] = '0.5' + case = self.options.Case('spin', 0, 'Spin', {}) + with mock.patch.dict(self.options.FAMILY_MAP, {'spin': _Spin()}): + outcome = self.options.roundtrip(case, deadline=2) + self.assertEqual(outcome.status, 'TIMEOUT') + self.assertEqual(outcome.detail, 'did not finish within 1s') + self.assertIs(signal.getsignal(signal.SIGALRM), self.handler) + + +if __name__ == '__main__': + unittest.main()