From 952d434d19e063668d41f5afb80b21654a09c2a8 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 26 Oct 2026 11:24:22 -0400 Subject: [PATCH] ci(ordering): shard the project ordering leg and cache the tier guard's workflow parse (#2254) Problem: the `Plain unittest ordering (project)` run step reached a p90 of 11.8 of its 20 minutes and was on course to hit the cap in about a week; `project-tests` runs the same leg on a docs-only diff under a 15-minute cap; and tests.test_tier_guard's WorkflowAgreementTests reran the same two tier selectors in a fresh interpreter seven times, about 7 s each. Fix: - B: util/run_unittest_leg.py takes --half 1|2, cut at the one name SHARD_BOUNDARIES holds for tests/project (test_q); the unittest-ordering matrix runs `project (half 1)` and `project (half 2)`, and project-tests runs its docs-only unittest step as two steps, one per half, in the same job. --before/--from cut at any other name by hand. - C: the tier guard's python stub replays a `-c` selector's output from a per-class cache once it has run it for real; a doctored step that asks something else, or a failing run, is not served from it. - Consolidation: no clear duplicate found; nothing removed. Tests: test_unittest_ordering_shards checks the cells and the project-tests steps cut at one boundary and cover the leg once; test_project_pins_once_1954 and test_unit_tests_queue read the new cells and steps; a new PythonStubTests case pins the cache. Workflow readers, tier guard and docstring contract: 1043 passed. actionlint clean. --- .github/workflows/unit-tests.yml | 36 +++- .../conventions/test-coverage.rst | 2 + docs/source/contributing/workflows.rst | 2 +- tests/project/test_project_pins_once_1954.py | 17 +- tests/project/test_unit_tests_queue.py | 4 +- .../project/test_unittest_ordering_shards.py | 158 +++++++++++++++--- tests/test_tier_guard.py | 88 ++++++++-- util/run_unittest_leg.py | 76 ++++++++- 8 files changed, 327 insertions(+), 56 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 8e796afadf..87ef9df095 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -1132,6 +1132,15 @@ jobs: # by a root-level module survives. The modules directly in tests/protocols # stay in the rest cell, as moving them would split more pairs than it joins. # + # #2254: the project cell's run step ran near its step cap, so tests/project, + # which has no subdirectories, is cut at a module name: `--half 1` and + # `--half 2`, at the one boundary SHARD_BOUNDARIES in util/run_unittest_leg.py + # holds, which `project-tests` cuts at too, so every module, a new one + # included, runs in exactly one half. The boundary is chosen so the halves + # take about the same CI time; both also run the root-level modules. Move + # SHARD_BOUNDARIES to rebalance. The same cost as #1029's: a pair split + # between the two cells does not share a process. + # # Every tests/ subdirectory is a leg except those below, and # tests/project/test_unittest_ordering_shards.py fails on a new one left out: # @@ -1140,7 +1149,7 @@ jobs: # files, so the leg would run the root-level modules alone -- green, # while covering none of it. # - # The `project` cell keeps the project pins (#1954): whether a module passes + # The `project` cells keep the project pins (#1954): whether a module passes # in one interpreter, in file order, with no xdist, is a question about the # module, so a pin can fail it as readily as any other. # @@ -1170,7 +1179,6 @@ jobs: - dumpkit - foundation - interface - - project - protocols/internet - toolkit - utilities @@ -1180,6 +1188,13 @@ jobs: - leg: protocols exclude: --exclude protocols/internet label: protocols (rest) + # tests/project, cut at one module name -- see the #2254 note above. + - leg: project + shard: --half 1 + label: project (half 1) + - leg: project + shard: --half 2 + label: project (half 2) steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -1213,7 +1228,7 @@ jobs: # under the job's, so a hang reports as this step timing out. - name: Run tests/${{ matrix.leg }} and the root-level modules under plain unittest timeout-minutes: 20 - run: python util/cap_file_size.py python util/run_unittest_leg.py ${{ matrix.leg }} ${{ matrix.exclude }} --verbose + run: python util/cap_file_size.py python util/run_unittest_leg.py ${{ matrix.leg }} ${{ matrix.exclude }} ${{ matrix.shard }} --verbose # ``CHANGELOG.md`` is generated from the newest entry under # ``docs/source/changelog/`` by ``util/changelog_md.py``, and ``Create @@ -1355,13 +1370,22 @@ jobs: --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 + # 20-minute step cap, so these steps pass their own, a minute under their # 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 + # + # #2254: the whole leg ran near this cap, so it runs as + # `unittest-ordering`'s two `project` halves do, one step each, at the + # same boundary. + - name: Run the first half of 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 --half 1 --verbose --stall-dump 840 + + - name: Run the second half of 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 --stall-dump 840 + run: python util/cap_file_size.py python util/run_unittest_leg.py project --half 2 --verbose --stall-dump 840 - name: Run the other tests that read docs/ or Markdown if: ${{ needs.changes.outputs.code == 'false' }} diff --git a/docs/source/contributing/conventions/test-coverage.rst b/docs/source/contributing/conventions/test-coverage.rst index 606114172b..1d3388e676 100644 --- a/docs/source/contributing/conventions/test-coverage.rst +++ b/docs/source/contributing/conventions/test-coverage.rst @@ -153,6 +153,8 @@ Running the Tests python util/run_unittest_leg.py protocols/internet python util/run_unittest_leg.py protocols --exclude protocols/internet + python util/run_unittest_leg.py project --half 1 + python util/run_unittest_leg.py project --half 2 The leg list lives in the workflow, not the script. Every :file:`tests/` subdirectory is a leg except :file:`tests/integration/`, which holds no unit-tier diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 7b362b3b2a..c46ccb2bad 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -1415,7 +1415,7 @@ Every ``test``, ``test-ktlo`` and ``experimental`` leg passes each pin as an selection, so no workflow lists the pins, and :file:`tests/test_tier_guard.py` runs the steps to check what each does with the answer. Under ``gate-only``, ``gate``'s full suite runs the pins with everything else. ``unittest-ordering``'s ``project`` -cell keeps them, as it asks how a module runs in one interpreter, in file order and +cells keep them, as they ask how a module runs in one interpreter, in file order and without xdist, which is a question about the module rather than the Python version. A pin's skip where ``tomllib`` is missing, before 3.11, runs on no leg: the scripts diff --git a/tests/project/test_project_pins_once_1954.py b/tests/project/test_project_pins_once_1954.py index 8a2a13bc20..350a6db459 100644 --- a/tests/project/test_project_pins_once_1954.py +++ b/tests/project/test_project_pins_once_1954.py @@ -7,7 +7,7 @@ ``pcapkit`` stay on every leg. :func:`tests._tiers.is_project_pin` decides which, and :file:`tests/test_tier_guard.py` checks that rule and runs the steps that use it. This module pins the rest: the job runs on every diff, on ``latest``, and -blocks; the release gate and the ``unittest-ordering`` cell keep every pin; the +blocks; the release gate and the ``unittest-ordering`` ``project`` cells keep every pin; the workflows page says so. """ @@ -67,15 +67,24 @@ def test_it_runs_under_the_caps(self) -> None: class TestEveryPinStillRunsWhereTheMatrixDoesNot(unittest.TestCase): - """The release gate's full suite and ``unittest-ordering``'s ``project`` cell keep every pin.""" + """The release gate's full suite and ``unittest-ordering``'s ``project`` cells keep every pin.""" def test_the_gate_runs_the_whole_suite(self) -> None: self.assertEqual(step(('unit-tests.yml', 'gate'), 'Run full test suite')['run'], 'python util/cap_file_size.py python -m pytest -q -n auto --dist load') def test_the_ordering_cell_runs_every_pin(self) -> None: - self.assertIn('project', job('unit-tests.yml', 'unittest-ordering')['strategy']['matrix']['leg']) - modules = set(run_unittest_leg.leg_modules('project')) + """#2254: the ``project`` cells between them, as they cut ``tests/project`` in two.""" + matrix = job('unit-tests.yml', 'unittest-ordering')['strategy']['matrix'] + cells = [{'leg': name} for name in matrix['leg']] + list(matrix.get('include') or []) + modules = set() + for cell in cells: + if cell['leg'] == 'project': + words = str(cell.get('shard', '')).split() + flags = dict(zip(words[::2], words[1::2])) + cut = run_unittest_leg.half_bounds('project', int(flags['--half'])) if '--half' in flags else {} + modules.update(run_unittest_leg.leg_modules('project', **cut)) + self.assertTrue(modules, 'no unittest-ordering cell runs tests/project') pins = {pin[:-len('.py')].replace('/', '.') for pin in _tiers.project_pin_paths()} self.assertTrue(pins) self.assertEqual(pins - modules, set()) diff --git a/tests/project/test_unit_tests_queue.py b/tests/project/test_unit_tests_queue.py index c48355f59b..3af9e3a076 100644 --- a/tests/project/test_unit_tests_queue.py +++ b/tests/project/test_unit_tests_queue.py @@ -304,7 +304,9 @@ def test_project_tests_runs_on_every_diff_and_never_duplicates_test(self) -> Non self.assertIn('if: ${{ inputs.gate-only != true }}', head) self.assertNotIn('needs.changes.outputs.code', head) for name, condition in (('Run the project pins', "needs.changes.outputs.code != 'false'"), - ('Run tests/project and the root-level modules under plain unittest', + ('Run the first half of tests/project and the root-level modules under plain unittest', + "needs.changes.outputs.code == 'false'"), + ('Run the second half of tests/project and the root-level modules under plain unittest', "needs.changes.outputs.code == 'false'"), ('Run the other tests that read docs/ or Markdown', "needs.changes.outputs.code == 'false'")): diff --git a/tests/project/test_unittest_ordering_shards.py b/tests/project/test_unittest_ordering_shards.py index 63edc708bf..938d8ae0b7 100644 --- a/tests/project/test_unittest_ordering_shards.py +++ b/tests/project/test_unittest_ordering_shards.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -"""Tests that the ``unittest-ordering`` matrix shards ``protocols`` without losing a module. +"""Tests that the ``unittest-ordering`` matrix shards ``protocols`` and ``project`` without losing a module. #1029: the ``protocols`` cell of ``unittest-ordering`` measured 30.4 minutes against the job's ``timeout-minutes: 45`` while every sibling leg stays under @@ -12,8 +12,13 @@ * the shards' module sets are disjoint and together equal the unsharded ``protocols`` leg, so a new ``tests/protocols//`` is either inside a shard or fails here; +* #2254: the ``project`` cells cut that flat directory at one module name, + ``--half 1`` in one cell and ``--half 2`` in the other, both read from + :data:`run_unittest_leg.SHARD_BOUNDARIES`, so they too are disjoint and + together equal the unsharded leg, a module added later included; and + ``project-tests``' docs-only steps cut it the same way; * every other matrix cell is a bare direct child of ``tests/`` with no - ``--exclude``, i.e. runs that whole directory; + ``--exclude`` or boundary, i.e. runs that whole directory; * every ``tests/`` subdirectory holding a test module is a leg, except the :data:`NOT_LEGS` names, each left out for the reason the workflow comment gives (#2115: ``corekit`` was missing, and nothing said so). @@ -26,10 +31,12 @@ from __future__ import annotations +import io import pathlib import re import sys import unittest +import unittest.mock ROOT = pathlib.Path(__file__).resolve().parents[2] WORKFLOW = ROOT / '.github' / 'workflows' / 'unit-tests.yml' @@ -46,6 +53,9 @@ 'integration', } +#: The legs the matrix shards, each into cells that must partition it. +SHARDED = ('protocols', 'project') + def _test_directories() -> 'set[str]': """Every direct child of ``tests/`` that holds a ``test_*.py`` module.""" @@ -53,61 +63,128 @@ def _test_directories() -> 'set[str]': if path.is_dir() and next(path.rglob('test_*.py'), None) is not None} -def _matrix() -> 'list[tuple[str, list[str]]]': - """``(leg, exclude)`` for every cell of the ``unittest-ordering`` matrix.""" +def _top(name: 'str') -> 'str': + """The direct child of ``tests/`` a leg name lies in.""" + return name.split('/', 1)[0] + + +def _job(name: 'str') -> 'str': + """The comment-stripped text of workflow job ``name``.""" lines = [] inside = False for raw in WORKFLOW.read_text(encoding='utf-8').splitlines(): code = raw.split('#', 1)[0].rstrip() - if re.match(r' unittest-ordering:\s*$', code): + if re.match(rf' {re.escape(name)}:\s*$', code): inside = True elif inside and re.match(r' \S', code): break elif inside: lines.append(code) - text = '\n'.join(lines) + return '\n'.join(lines) + + +def _boundary(name: 'str', flags: 'str') -> 'dict[str, str]': + """:func:`run_unittest_leg.leg_modules`'s ``before``/``start`` for a cell's or step's flags.""" + boundary = {'before' if flag == 'before' else 'start': value + for flag, value in re.findall(r'--(before|from)\s+(\S+)', flags)} + for half in re.findall(r'--half\s+(\S+)', flags): + boundary.update(leg.half_bounds(name, int(half))) + return boundary + + +def _halves(flags: 'str') -> 'list[str]': + return re.findall(r'--half\s+(\S+)', flags) + + +def _matrix() -> 'list[tuple[str, list[str], dict[str, str]]]': + """``(leg, exclude, boundary)`` for every cell of the ``unittest-ordering`` matrix. + + ``boundary`` maps the cell's ``--half``, ``--before`` and ``--from`` to + :func:`run_unittest_leg.leg_modules`'s ``before`` and ``start``. + + """ + text = _job('unittest-ordering') matrix = text.split('matrix:', 1)[1].split('steps:', 1)[0] plain, _, extra = matrix.partition('include:') - cells = [(name, []) for name in re.findall(r'^\s+- (\S+)\s*$', plain.split('leg:', 1)[1], re.M)] + cells = [(name, [], {}) for name in re.findall(r'^\s+- (\S+)\s*$', plain.split('leg:', 1)[1], re.M)] for block in re.split(r'^\s+- (?=leg:)', extra, flags=re.M)[1:]: name = re.search(r'leg:\s*(\S+)', block).group(1) flags = re.search(r'exclude:\s*"?([^"\n]*)"?', block) - cells.append((name, re.findall(r'--exclude\s+(\S+)', flags.group(1)) if flags else [])) + shard = re.search(r'shard:\s*"?([^"\n]*)"?', block) + boundary = _boundary(name, shard.group(1) if shard else '') + cells.append((name, re.findall(r'--exclude\s+(\S+)', flags.group(1)) if flags else [], boundary)) return cells +def _modules(name: 'str', exclude: 'list[str]', boundary: 'dict[str, str]') -> 'tuple[str, ...]': + """What the cell ``(name, exclude, boundary)`` runs from its own directory.""" + return leg.leg_modules(name, exclude, **boundary) + + class TestShardedProtocolsLeg(unittest.TestCase): def test_shards_partition_the_unsharded_leg(self): - shards = [leg.leg_modules(name, exclude) for name, exclude in _matrix() - if name == 'protocols' or name.startswith('protocols/')] - self.assertGreater(len(shards), 1, 'protocols is no longer sharded (#1029)') - - flat = [module for shard in shards for module in shard] - self.assertEqual(len(flat), len(set(flat)), 'a module runs in two shards') - self.assertEqual(set(flat), set(leg.leg_modules('protocols')), - 'a tests/protocols module falls between shards') + """#1029 for ``protocols``, #2254 for ``project``.""" + for top in SHARDED: + with self.subTest(leg=top): + shards = [_modules(*cell) for cell in _matrix() if _top(cell[0]) == top] + self.assertGreater(len(shards), 1, f'{top} runs as a single cell') + + flat = [module for shard in shards for module in shard] + self.assertEqual(len(flat), len(set(flat)), 'a module runs in two shards') + self.assertEqual(set(flat), set(leg.leg_modules(top)), + f'a tests/{top} module falls between shards') + + def test_the_project_cells_cut_at_one_name(self): + """#2254: ``--half 1`` in one cell and ``--half 2`` in the other, one name between + them, so any module name, one added later included, sorts into exactly one cell.""" + text = _job('unittest-ordering') + self.assertEqual(sorted(half for flags in re.findall(r'shard:\s*([^\n]*)', text) + for half in _halves(flags)), ['1', '2']) + boundaries = [boundary for name, _, boundary in _matrix() if name == 'project'] + self.assertEqual(sorted(sorted(b) for b in boundaries), [['before'], ['start']]) + self.assertEqual({value for b in boundaries for value in b.values()}, + {leg.SHARD_BOUNDARIES['project']}, + f'the project cells cut at {boundaries}, not all at ' + f"SHARD_BOUNDARIES' {leg.SHARD_BOUNDARIES['project']!r}") + edge = leg.SHARD_BOUNDARIES['project'] + for key in ('test_aaa_new.py', edge, f'{edge}.py', 'test_zzz_new.py'): + with self.subTest(key=key): + hits = [b for b in boundaries + if ('before' not in b or key < b['before']) and key >= b.get('start', '')] + self.assertEqual(len(hits), 1) + + def test_project_tests_cuts_where_the_matrix_does(self): + """#2254: ``project-tests``' docs-only steps run the two halves the ``project`` + cells do, one step each, and between them the whole leg, once.""" + runs = re.findall(r'run_unittest_leg\.py\s+project\b([^\n]*)', _job('project-tests')) + self.assertEqual(sorted(half for flags in runs for half in _halves(flags)), ['1', '2']) + steps = [leg.leg_modules('project', **_boundary('project', flags)) for flags in runs] + cells = [_modules(*cell) for cell in _matrix() if cell[0] == 'project'] + self.assertEqual(sorted(steps), sorted(cells)) + flat = [module for half in steps for module in half] + self.assertEqual(sorted(flat), sorted(leg.leg_modules('project'))) def test_every_other_cell_is_a_plain_direct_child(self): - others = [(name, exclude) for name, exclude in _matrix() - if name != 'protocols' and not name.startswith('protocols/')] - self.assertEqual(sorted(name for name, _ in others), - sorted(_test_directories() - NOT_LEGS - {'protocols'})) - for name, exclude in others: + others = [cell for cell in _matrix() if _top(cell[0]) not in SHARDED] + self.assertEqual(sorted(name for name, _, _ in others), + sorted(_test_directories() - NOT_LEGS - set(SHARDED))) + for name, exclude, boundary in others: self.assertEqual(exclude, [], name) + self.assertEqual(boundary, {}, name) self.assertEqual(leg.leg_modules(name), leg.leg_modules(name, ()), name) def test_every_tests_subdirectory_is_a_leg(self): """#2115: a ``tests/`` subdirectory left out of the matrix is never run under plain unittest, and the job stays green while covering less.""" - covered = {name.split('/', 1)[0] for name, _ in _matrix()} + covered = {_top(name) for name, _, _ in _matrix()} self.assertEqual(sorted(_test_directories() - NOT_LEGS - covered), [], 'a tests/ subdirectory is missing from unittest-ordering') def test_every_exception_is_real_and_still_left_out(self): - covered = {name.split('/', 1)[0] for name, _ in _matrix()} + covered = {_top(name) for name, _, _ in _matrix()} for name in sorted(NOT_LEGS): with self.subTest(name=name): self.assertIn(name, _test_directories(), 'stale NOT_LEGS entry') @@ -119,8 +196,8 @@ def test_integration_has_no_unit_tier_module(self): self.assertEqual(leg.leg_modules('integration'), ()) def test_no_shard_is_empty(self): - for name, exclude in _matrix(): - self.assertTrue(leg.leg_modules(name, exclude), name) + for cell in _matrix(): + self.assertTrue(_modules(*cell), cell[0]) class TestLegModulesSubpath(unittest.TestCase): @@ -137,6 +214,37 @@ def test_exclude_removes_only_that_subtree(self): self.assertEqual(set(full) - set(rest), set(leg.leg_modules('protocols/internet'))) self.assertEqual([m for m in full if m in rest], list(rest), 'order changed') + def test_before_and_from_split_at_one_name(self): + """#2254: the two sides of a cut are the leg, in order, with the boundary module in the second.""" + full = leg.leg_modules('project') + edge = full[len(full) // 2].rsplit('.', 1)[1] + low = leg.leg_modules('project', before=edge) + high = leg.leg_modules('project', start=edge) + self.assertTrue(low and high) + self.assertEqual(low + high, full) + self.assertEqual(high[0], f'tests.project.{edge}') + self.assertEqual(leg.leg_modules('project', before=edge, start=edge), ()) + + def test_a_half_is_the_boundary_from_shard_boundaries(self): + edge = leg.SHARD_BOUNDARIES['project'] + self.assertEqual(leg.half_bounds('project', 1), {'before': edge}) + self.assertEqual(leg.half_bounds('project', 2), {'start': edge}) + for directory, half in (('cli', 1), ('project', 3)): + with self.subTest(directory=directory, half=half), self.assertRaises(SystemExit): + leg.half_bounds(directory, half) + + def test_half_and_an_explicit_cut_are_refused_together(self): + with unittest.mock.patch('sys.stderr', new_callable=io.StringIO), self.assertRaises(SystemExit): + leg.main(['project', '--half', '1', '--from', 'test_a']) + + def test_the_tally_names_the_cut(self): + """The leg's closing line says which side ran, as it does for ``--exclude``.""" + with unittest.mock.patch.object(leg, 'build_suite', return_value=unittest.TestSuite()), \ + unittest.mock.patch('sys.stdout', new_callable=io.StringIO) as out, \ + unittest.mock.patch('sys.stderr', new_callable=io.StringIO): + self.assertEqual(leg.main(['project', '--half', '2', '--stall-dump', '0']), 0) + self.assertIn(f"tests/project (from {leg.SHARD_BOUNDARIES['project']}) + ", out.getvalue()) + def test_exclude_must_be_inside_the_leg(self): with self.assertRaises(SystemExit): leg.leg_modules('protocols', ['const']) diff --git a/tests/test_tier_guard.py b/tests/test_tier_guard.py index d8b4905977..6fd19d8d47 100644 --- a/tests/test_tier_guard.py +++ b/tests/test_tier_guard.py @@ -197,6 +197,17 @@ def test_absolute_and_relative_paths_agree(self) -> None: class WorkflowAgreementTests(unittest.TestCase): """The tier rule here matches the one the workflow actually runs.""" + #: :data:`PYTHON_STUB`'s ``$SELECTOR_CACHE`` for this class's steps, so each + #: selector runs once per class rather than once per step run (#2254). + selectors = '' + + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + temp = tempfile.TemporaryDirectory(prefix='pcapkit-selectors-') + cls.addClassCleanup(temp.cleanup) + cls.selectors = temp.name + def test_ignore_flags_match_the_fixture_tier_constants(self) -> None: """Every ignored path and glob is accounted for by a constant. @@ -278,9 +289,9 @@ def test_the_legs_leave_out_exactly_the_latest_tier_or_the_pins(self) -> None: latest = list(_tiers.latest_tier_paths()) self.assertTrue(pins) self.assertLess(len(pins), len(latest)) - self.assertEqual(leg_ignores(text, 'true'), pins) - self.assertEqual(leg_ignores(text, 'false'), latest) - self.assertEqual(project_pins_run(text), pins) + self.assertEqual(leg_ignores(text, 'true', self.selectors), pins) + self.assertEqual(leg_ignores(text, 'false', self.selectors), latest) + self.assertEqual(project_pins_run(text, self.selectors), pins) for workflow, job, step, selector in ( (WORKFLOW, 'test', 'Run unit tests', _tiers.PROJECT_PINS_SELECTOR), (WORKFLOW, 'test', 'Run unit tests', _tiers.LATEST_TIER_SELECTOR), @@ -306,7 +317,7 @@ def test_a_leg_that_stops_ignoring_the_latest_tier_is_caught(self) -> None: self.assertEqual(text.count(loop), 1, 'the loop this test removes has been reworded') for latest in ('true', 'false'): with self.subTest(latest=latest): - self.assertEqual(leg_ignores(text.replace(loop, ':'), latest), []) + self.assertEqual(leg_ignores(text.replace(loop, ':'), latest, self.selectors), []) @unittest.skipIf(BASH is None, 'the steps are bash scripts') def test_a_leg_that_asks_for_the_wrong_list_is_caught(self) -> None: @@ -318,9 +329,10 @@ def test_a_leg_that_asks_for_the_wrong_list_is_caught(self) -> None: pins = _tiers.PROJECT_PINS_SELECTOR.split('"', 1)[1] self.assertEqual(text.count(latest), 1) self.assertEqual(text.count(pins), 2) # the `test` step's and `project-tests`'s - self.assertNotEqual(leg_ignores(text.replace(latest, pins), 'false'), list(_tiers.latest_tier_paths())) + self.assertNotEqual(leg_ignores(text.replace(latest, pins), 'false', self.selectors), + list(_tiers.latest_tier_paths())) swapped = text.replace(pins, latest, 1) - self.assertNotEqual(leg_ignores(swapped, 'true'), list(_tiers.project_pin_paths())) + self.assertNotEqual(leg_ignores(swapped, 'true', self.selectors), list(_tiers.project_pin_paths())) #: The experimental legs' workflow (#1732), whose unit-tier step leaves the ``LATEST`` tier out too. @@ -330,7 +342,22 @@ def test_a_leg_that_asks_for_the_wrong_list_is_caught(self) -> None: #: command it execs, as the script does, with no real interpreter: under the coverage #: leg one would be measured, be logged as a second ``coverage run --rcfile`` #: controller, and exec before saving, which fails the #1387 guard (#1969). -PYTHON_STUB = ('#!/bin/sh\ncase "$1" in -c) exec "$REAL_PYTHON" "$@";; util/cap_file_size.py) shift; exec "$@";; esac\n' +#: +#: With ``$SELECTOR_CACHE`` set, a ``-c`` runs the real interpreter once per distinct +#: argument list and replays its output after that (#2254): each selector call rescans +#: the suite in a fresh interpreter, and :class:`WorkflowAgreementTests` asks the same +#: selectors repeatedly. A doctored step that asks something else is keyed apart and +#: runs for real; a failing run is not kept. +PYTHON_STUB = ('#!/bin/sh\n' + 'if [ "$1" = -c ] && [ -n "$SELECTOR_CACHE" ]; then\n' + ' answer="$SELECTOR_CACHE/$(printf \'%s\\n\' "$@" | cksum | tr \' \' _)"\n' + ' if [ ! -f "$answer" ]; then\n' + ' "$REAL_PYTHON" "$@" > "$answer.part" || { status=$?; cat "$answer.part"; exit "$status"; }\n' + ' mv "$answer.part" "$answer"\n' + ' fi\n' + ' exec cat "$answer"\n' + 'fi\n' + 'case "$1" in -c) exec "$REAL_PYTHON" "$@";; util/cap_file_size.py) shift; exec "$@";; esac\n' 'printf \'%s\\n\' "$@" > "$ARGS_FILE"\n') @@ -385,19 +412,54 @@ def test_the_capped_command_is_recorded_without_the_real_interpreter(self) -> No self.assertFalse(real.exists(), 'the real interpreter ran the capped command') self.assertEqual(record.read_text(encoding='utf-8').splitlines(), command) + @unittest.skipIf(os.name == 'nt', 'the stub is found on PATH by name, as Windows does not find a sh script') + def test_a_selector_runs_once_per_cache_and_a_failure_is_not_kept(self) -> None: + """#2254: with ``$SELECTOR_CACHE`` set, each distinct ``-c`` reaches the real interpreter once.""" + real_python = '#!/bin/sh\necho "$@" >> "$REAL_LOG"\necho "answer $2"\n[ "$2" != fail ]\n' + with tempfile.TemporaryDirectory() as temp: + for name, text in (('python', PYTHON_STUB), ('real', real_python)): + path = pathlib.Path(temp, name) + path.write_text(text, encoding='utf-8') + path.chmod(0o755) + cache, real = pathlib.Path(temp, 'cache'), pathlib.Path(temp, 'real.log') + cache.mkdir() + env = dict(os.environ, ARGS_FILE=str(pathlib.Path(temp, 'args')), SELECTOR_CACHE=str(cache), + REAL_PYTHON=str(pathlib.Path(temp, 'real')), REAL_LOG=str(real), + PATH=temp + os.pathsep + os.environ.get('PATH', '')) + + def ask(code: 'str') -> 'subprocess.CompletedProcess[str]': + return subprocess.run(['python', '-c', code], env=env, capture_output=True, text=True, + check=False, timeout=60) + + self.assertEqual([ask(code).stdout for code in ('a', 'a', 'b')], ['answer a\n', 'answer a\n', 'answer b\n']) + for _ in range(2): + failed = ask('fail') + self.assertEqual((failed.returncode, failed.stdout), (1, 'answer fail\n')) + self.assertEqual(real.read_text(encoding='utf-8').splitlines(), ['-c a', '-c b', '-c fail', '-c fail']) -def leg_ignores(text: 'str', latest: 'str') -> 'list[str]': - """The modules a ``test`` leg's ``Run unit tests`` step ``--ignore``\\ s, ``latest`` or not.""" + +def leg_ignores(text: 'str', latest: 'str', cache: 'str' = '') -> 'list[str]': + """The modules a ``test`` leg's ``Run unit tests`` step ``--ignore``\\ s, ``latest`` or not. + + ``cache`` is a directory for :data:`PYTHON_STUB`'s ``$SELECTOR_CACHE``; empty runs every selector. + + """ script = run_script(job_section(text, 'test'), 'Run unit tests') # #2005: a leg that runs pytest once, as a PyPy leg's own `SHARDS` would otherwise reach the stubbed step. - argv = stubbed_argv(script, {'ANNOTATION_LEVEL': 'error', 'COVERAGE': 'false', 'LATEST': latest, 'SHARDS': ''}) + argv = stubbed_argv(script, {'ANNOTATION_LEVEL': 'error', 'COVERAGE': 'false', 'LATEST': latest, 'SHARDS': '', + 'SELECTOR_CACHE': cache}) return [arg.split('=', 1)[1] for arg in argv if arg.startswith('--ignore=') and arg != f'--ignore=tests/{next(iter(_tiers.FIXTURE_TIER_DIRS))}'] -def project_pins_run(text: 'str') -> 'list[str]': - """The modules ``project-tests``'s ``Run the project pins`` step hands pytest.""" - argv = stubbed_argv(run_script(job_section(text, 'project-tests'), 'Run the project pins'), {}) +def project_pins_run(text: 'str', cache: 'str' = '') -> 'list[str]': + """The modules ``project-tests``'s ``Run the project pins`` step hands pytest. + + ``cache`` is as for :func:`leg_ignores`. + + """ + argv = stubbed_argv(run_script(job_section(text, 'project-tests'), 'Run the project pins'), + {'SELECTOR_CACHE': cache}) return [arg for arg in argv if arg.endswith('.py')] diff --git a/util/run_unittest_leg.py b/util/run_unittest_leg.py index baf51a2b57..1b8117ce63 100644 --- a/util/run_unittest_leg.py +++ b/util/run_unittest_leg.py @@ -86,6 +86,13 @@ pollution between them is invisible; the workflow comment records which pairs that loses. +GitHub issue #2254 -- sharding a flat leg: :mod:`tests.project` has no +subdirectories to exclude, so ``--half 1`` and ``--half 2`` cut it at the module +name :data:`SHARD_BOUNDARIES` holds for it: half 1 is every module sorting before +it, half 2 the rest, so a new module lands in exactly one half. Every caller asks +for a half rather than naming the boundary, so they cannot cut in different +places. ``--before NAME`` and ``--from NAME`` cut at any other name, by hand. + What this still does not catch: a defect running the *other* direction (a root module polluting a directory module), interference between two directories neither of which is bundled with the other in the same leg, and @@ -169,6 +176,10 @@ #: Environment override for :data:`DEFAULT_STALL_DUMP`; ``--stall-dump`` wins. STALL_DUMP_ENV = 'PCAPKIT_UNITTEST_STALL_DUMP' +#: The module name each flat leg ``--half`` splits is cut at (#2254). Move it to +#: rebalance; the workflow's cells and steps ask for a half and follow. +SHARD_BOUNDARIES = {'project': 'test_q'} + #: Run a full :func:`gc.collect` after every this many tests; see the #: module docstring for the measurements behind it. ``--gc-every 0`` disables. DEFAULT_GC_EVERY = 10 @@ -218,7 +229,8 @@ def _resolve(spec: 'str', what: 'str') -> 'pathlib.Path': return path -def leg_modules(directory: 'str', exclude: 'Sequence[str]' = ()) -> 'tuple[str, ...]': +def leg_modules(directory: 'str', exclude: 'Sequence[str]' = (), *, + before: 'str | None' = None, start: 'str | None' = None) -> 'tuple[str, ...]': """Every unit-tier ``test_*.py`` module under ``tests/``. Args: @@ -229,6 +241,12 @@ def leg_modules(directory: 'str', exclude: 'Sequence[str]' = ()) -> 'tuple[str, :data:`TESTS_ROOT`) whose modules are left out, so that one shard can be "everything in ``directory`` except its sibling shards". Empty by default, as for every unsharded leg. + before: keep only the modules whose path below ``tests/``, + in POSIX form, sorts before this string (#2254). + start: keep only the modules whose path sorts at or after it. One + boundary passed as ``before`` to one shard and as ``start`` to the + other splits a flat directory such as ``project`` in two, with every + module, a new one included, in exactly one of them. Returns: Dotted module names, in path-sorted order. @@ -244,21 +262,42 @@ def leg_modules(directory: 'str', exclude: 'Sequence[str]' = ()) -> 'tuple[str, if leg_root not in path.parents: raise SystemExit(f'--exclude is not a subdirectory of the leg: {path.relative_to(ROOT)}') + def in_range(path: 'pathlib.Path') -> 'bool': + key = path.relative_to(leg_root).as_posix() + return (before is None or key < before) and (start is None or key >= start) + return tuple( _dotted(path) for path in sorted(leg_root.rglob('test_*.py')) - if is_unit_tier(path) and not any(skip in path.parents for skip in skipped) + if is_unit_tier(path) and not any(skip in path.parents for skip in skipped) and in_range(path) ) -def build_suite(directory: 'str', exclude: 'Sequence[str]' = ()) -> 'unittest.TestSuite': +def half_bounds(directory: 'str', half: 'int') -> 'dict[str, str]': + """:func:`leg_modules`'s ``before`` or ``start`` for half ``half`` of ``directory``. + + Raises: + SystemExit: ``directory`` has no :data:`SHARD_BOUNDARIES` entry, or + ``half`` is neither 1 nor 2. + + """ + if directory not in SHARD_BOUNDARIES: + raise SystemExit(f'--half: tests/{directory} has no boundary in SHARD_BOUNDARIES') + if half not in (1, 2): + raise SystemExit(f'--half is 1 or 2, not {half}') + return {'before' if half == 1 else 'start': SHARD_BOUNDARIES[directory]} + + +def build_suite(directory: 'str', exclude: 'Sequence[str]' = (), *, + before: 'str | None' = None, start: 'str | None' = None) -> 'unittest.TestSuite': """The combined suite for one leg: ``directory``'s own tests, then root's. - See the module docstring for why that order, not the reverse. + See the module docstring for why that order, not the reverse; ``before`` and + ``start`` are :func:`leg_modules`'s. """ loader = unittest.TestLoader() suite = unittest.TestSuite() - for name in leg_modules(directory, exclude): + for name in leg_modules(directory, exclude, before=before, start=start): suite.addTests(loader.loadTestsFromName(name)) for name in root_modules(): suite.addTests(loader.loadTestsFromName(name)) @@ -277,6 +316,19 @@ def main(argv: 'list[str] | None' = None) -> 'int': help="a subdirectory of DIRECTORY (relative to tests/) to leave out; " "repeatable. Used to run the remainder of a sharded leg.", ) + parser.add_argument( + '--half', type=int, choices=(1, 2), + help='run half 1 or 2 of DIRECTORY, cut at its SHARD_BOUNDARIES name', + ) + parser.add_argument( + '--before', metavar='NAME', + help="keep only the modules whose path below DIRECTORY sorts before NAME, " + "e.g. 'test_q'. Used with --from to shard a flat leg in two.", + ) + parser.add_argument( + '--from', dest='start', metavar='NAME', + help='keep only the modules whose path below DIRECTORY sorts at or after NAME', + ) parser.add_argument( '-v', '--verbose', action='store_true', help='pass verbosity 2 to unittest.TextTestRunner instead of the default 1', @@ -291,6 +343,11 @@ def main(argv: 'list[str] | None' = None) -> 'int': help=f'run gc.collect() after every N tests (default {DEFAULT_GC_EVERY}); 0 disables', ) args = parser.parse_args(argv) + if args.half is not None: + if args.before is not None or args.start is not None: + parser.error('--half sets the boundary itself; drop --before and --from') + for key, value in half_bounds(args.directory, args.half).items(): + setattr(args, key, value) stall_dump = args.stall_dump if stall_dump is None: try: @@ -314,7 +371,10 @@ def main(argv: 'list[str] | None' = None) -> 'int': def _run(args: 'argparse.Namespace', start: 'float') -> 'int': """Build and run the leg, then print its one-line tally whatever happened.""" - suite = build_suite(args.directory, args.exclude) + # An unsharded leg passes no boundary, so its build_suite call matches what the stall-dump tests stub. + cut = {key: getattr(args, key, None) for key in ('before', 'start')} + suite = build_suite(args.directory, args.exclude, + **{key: value for key, value in cut.items() if value is not None}) result_class = type('_LegResult', (_CollectingResult,), {'gc_every': args.gc_every}) runner = unittest.TextTestRunner(verbosity=2 if args.verbose else 1, resultclass=result_class) result: 'unittest.TestResult | None' = None @@ -332,6 +392,10 @@ def _run(args: 'argparse.Namespace', start: 'float') -> 'int': f'{result.testsRun} test(s), {len(result.failures)} failure(s), ' f'{len(result.errors)} error(s)') left_out = ''.join(f' (without tests/{spec})' for spec in args.exclude) + if cut['before'] is not None: + left_out += f" (before {cut['before']})" + if cut['start'] is not None: + left_out += f" (from {cut['start']})" print(f'tests/{args.directory}{left_out} + {len(root_modules())} root module(s): ' f'{tally}, {elapsed:.1f}s elapsed')