From badfc34fd4e1bc32fdd70e294960b62ef0817c9f Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 25 Oct 2026 03:31:09 -0400 Subject: [PATCH] fix(ci): give project-tests' unittest step a stall dump under its own cap (#2164) Problem: util/run_unittest_leg.py's default stall dump (1080s) is sized for unittest-ordering's 20-minute step cap. project-tests' step "Run tests/project and the root-level modules under plain unittest" is capped at 15 minutes, so a hang there was killed before the dump naming where it stopped could fire. Fix: that step passes --stall-dump 840, a minute under its cap; the script's docstring says a step with a shorter cap passes its own. Test: TestEveryStepDumpsBeforeItsCap reads every workflow step running the script and checks its effective dump (flag, env, else default) is below its timeout-minutes. It fails on origin/main with 1080 not less than 900. --- .github/workflows/unit-tests.yml | 6 +- .../test_run_unittest_leg_stall_dump.py | 60 +++++++++++++++++++ util/run_unittest_leg.py | 3 +- 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index cf2ee19286..33ce04454e 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -1345,10 +1345,14 @@ jobs: python util/cap_file_size.py python -m pytest -q -n auto --dist load \ --durations=30 --durations-min=1.0 --junitxml="$RUNNER_TEMP/junit.xml" "${pins[@]}" + # #2164: the script's default stall dump sits under `unittest-ordering`'s + # 20-minute step cap, so this step passes its own, a minute under its + # 15. tests/project/test_run_unittest_leg_stall_dump.py pins every step + # running the script to a dump inside its cap. - name: Run tests/project and the root-level modules under plain unittest if: ${{ needs.changes.outputs.code == 'false' }} timeout-minutes: 15 - run: python util/cap_file_size.py python util/run_unittest_leg.py project --verbose + run: python util/cap_file_size.py python util/run_unittest_leg.py project --verbose --stall-dump 840 - name: Run the other tests that read docs/ or Markdown if: ${{ needs.changes.outputs.code == 'false' }} diff --git a/tests/project/test_run_unittest_leg_stall_dump.py b/tests/project/test_run_unittest_leg_stall_dump.py index 74ca694162..cc5780eaed 100644 --- a/tests/project/test_run_unittest_leg_stall_dump.py +++ b/tests/project/test_run_unittest_leg_stall_dump.py @@ -6,6 +6,9 @@ builds the suite and cancels it on the way out. The in-process tests mock :mod:`faulthandler`, because this module itself runs inside the ``project`` leg, whose own dump is armed; the subprocess test lets a real one fire. +:class:`TestEveryStepDumpsBeforeItsCap` reads every workflow step that runs the +script, as :func:`tests._workflows.load` parses it: each one's dump has to fire +before its own ``timeout-minutes`` kills it (#2164). The last class pins the decision #1052 recorded against restoring :data:`sys.modules` between tests: a sibling's write to the ``pcapkit`` region @@ -17,12 +20,19 @@ import os import pathlib +import shlex import subprocess import sys import textwrap import unittest +from typing import TYPE_CHECKING from unittest import mock +from tests import _workflows + +if TYPE_CHECKING: + from typing import Any, Optional + ROOT = pathlib.Path(__file__).resolve().parents[2] if str(ROOT / 'util') not in sys.path: @@ -106,6 +116,56 @@ def test_sleeps_past_the_dump(self): self.assertIn('1 test(s), 0 failure(s), 0 error(s)', proc.stdout) +class TestEveryStepDumpsBeforeItsCap(unittest.TestCase): + """A workflow step running the leg dumps its stack before its cap kills it (#2164). + + The dump is what :func:`run_unittest_leg.main` arms: the step's + ``--stall-dump``, else :data:`~run_unittest_leg.STALL_DUMP_ENV` from the + step's, job's or workflow's ``env``, else + :data:`~run_unittest_leg.DEFAULT_STALL_DUMP`. The cap is the step's + ``timeout-minutes``, else its job's. + + """ + + @staticmethod + def _flag(run: 'str') -> 'Optional[float]': + words = shlex.split(run.replace('\\\n', ' '), comments=True) + for i, word in enumerate(words): + if word == '--stall-dump': + return float(words[i + 1]) + if word.startswith('--stall-dump='): + return float(word.partition('=')[2]) + return None + + @staticmethod + def _steps() -> 'list[tuple[str, Any, Any, Any, Any]]': + found = [] + for path in sorted(_workflows.WORKFLOWS.glob('*.yml')): + workflow = _workflows.load(path.name) + for job_id, job in (workflow.get('jobs') or {}).items(): + for step in job.get('steps') or []: + if 'run_unittest_leg.py' in str(step.get('run', '')): + found.append((f'{path.name} {job_id} {step.get("name")!r}', + step, job, workflow, str(step['run']))) + return found + + def test_every_step_dumps_before_its_cap(self): + found = self._steps() + self.assertTrue(found, 'no workflow step runs util/run_unittest_leg.py') + for where, step, job, workflow, run in found: + with self.subTest(step=where): + dump = self._flag(run) + if dump is None: + env = {**(workflow.get('env') or {}), **(job.get('env') or {}), + **(step.get('env') or {})} + dump = float(env.get(leg.STALL_DUMP_ENV) or leg.DEFAULT_STALL_DUMP) + cap = step.get('timeout-minutes', job.get('timeout-minutes')) + self.assertIsNotNone(cap, 'the step has no cap to dump before') + self.assertGreater(dump, 0, 'the dump is disabled') + self.assertLess(dump, float(cap) * 60, + f'the dump at {dump:g}s fires after the {cap}-minute cap') + + class TestPeriodicCollect(unittest.TestCase): """``--gc-every N`` frees unreachable re-import generations every N tests (#1052).""" diff --git a/util/run_unittest_leg.py b/util/run_unittest_leg.py index 2827b16940..adc21ea210 100644 --- a/util/run_unittest_leg.py +++ b/util/run_unittest_leg.py @@ -102,7 +102,8 @@ arms :func:`faulthandler.dump_traceback_later`, so a leg still running after that long writes every thread's stack to stderr once, then carries on. The default, 1080s, is just under the ``unittest-ordering`` step's 20-minute cap, -so a leg about to be killed says where it was first; ``0`` disables it. +so a leg about to be killed says where it was first; a step with a shorter cap +passes its own (#2164). ``0`` disables it. Memory: every purge-then-import leaves the previous generation as cyclic garbage, and the interpreter's own collector intermittently falls behind --