From 5dd520c0648de9d598f03070d296fcf1fe067282 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 1 Nov 2026 08:03:32 -0500 Subject: [PATCH 1/2] ci(timing): run the unit legs at -n logical and read latest's JUnit timings (#2571) - unit-tests.yml `Run unit tests`: `-n logical`, not `auto`, which counts physical cores once psutil is installed (the `test` extra, CPython), half the runner's CPUs. PyPy has no psutil; there the two agree. `test-ktlo` shares the step by alias. - The `test` matrix comment drops its stale figures and says why the coverage leg is slower: the `LATEST` tier too, under coverage. - util/ci_timing.py: JUNIT_LEGS adds `latest`, read from the tier file with COVERAGE_JOB, so the report names the coverage leg's modules. - test-coverage.rst names the new flag; process.rst's monthly-refresh bullet no longer lists the dropped headroom figure. - Tests: a per-leg `-n logical` pin, a JUNIT_LEGS-has-latest check, and the Windows leg's flags pin re-pinned to each side's `-n` value. --- .github/workflows/unit-tests.yml | 13 +++++++++---- .../contributing/conventions/process.rst | 4 ++-- .../conventions/test-coverage.rst | 2 +- tests/project/test_ci_timing.py | 7 +++++++ .../test_sweeps_on_latest_only_1732.py | 13 +++++++++++++ tests/project/test_windows_test_leg_2060.py | 7 ++++--- util/ci_timing.py | 19 ++++++++++--------- 7 files changed, 46 insertions(+), 19 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 5d8aa5e0b2..127c85ddc0 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -101,8 +101,9 @@ jobs: # legs run the same `ALL` tier as every CPython leg but `latest`. # # `include` marks one leg `coverage: true`, `latest` (#1661), so coverage - # adds no leg to the queue. Uncovered, 3.15's tests step took a median - # 8.1 minutes and coverage cost about 1.8x, which fits the job's 45. + # adds no leg to the queue. That leg runs slower than the rest: it runs the + # `LATEST` tier as well as `ALL`, under coverage. ci-timing.yml's W3 tracks + # the gap. # # #2243, the owner's option B: a test-only pull request runs the # `latest` leg alone, the coverage leg, which runs the whole suite with @@ -166,7 +167,7 @@ jobs: if: ${{ matrix.coverage }} run: python -m pip install 'coverage>=7.10' - # Nothing else in the log says what `-n auto` resolved to on this runner. + # Nothing else in the log says what `-n logical` resolved to on this runner. - name: Report available parallelism id: parallelism if: ${{ steps.install.outcome == 'success' }} @@ -183,6 +184,10 @@ jobs: # cannot silently change how this suite is distributed. The durations # table and JUnit file are #1538's per-test timings. # + # #2571: `-n logical`, as `auto` counts physical cores, half the runner's + # CPUs, wherever psutil is installed: the `test` extra installs it on + # CPython. On PyPy, which has none, the two agree. + # # #1958, the test tiers: tests/_tiers.py's module_tier() puts every test # module in `ALL`, a curated list each stating `__tier__ = 'ALL'` (#1963), # or else in `LATEST`, and `LATEST` runs on `latest` @@ -215,7 +220,7 @@ jobs: echo "::${ANNOTATION_LEVEL} title=Could not compute the LATEST tier::tests._tiers.latest_tier_paths() failed -- see the traceback above." exit 1 fi - set -- -q -n auto --dist load \ + set -- -q -n logical --dist load \ --durations=30 --durations-min=1.0 --junitxml="$RUNNER_TEMP/junit.xml" \ --ignore=tests/integration \ --ignore-glob='*_runtime.py' \ diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index 5fbf5a626b..c2b902152d 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -428,8 +428,8 @@ thresholds are on :doc:`../workflows`. job's shape or selection -- a leg added, split or moved -- dispatch the workflow, so the next week compares like with like. * **Monthly, refresh the measured figures** quoted in :file:`unit-tests.yml`'s - comments that the report shows to be stale -- the coverage leg's headroom, the - ordering cells' timings and their step cap's rationale. + comments that the report shows to be stale -- the ordering cells' timings and + their step cap's rationale. The PEP Discussion, Kept Current diff --git a/docs/source/contributing/conventions/test-coverage.rst b/docs/source/contributing/conventions/test-coverage.rst index 00951743a5..56525c956c 100644 --- a/docs/source/contributing/conventions/test-coverage.rst +++ b/docs/source/contributing/conventions/test-coverage.rst @@ -123,7 +123,7 @@ Running the Tests * **One module per process, never the whole suite.** Check a change by running the modules it touches, each as its own ``python -m pytest -q``, and leave the - whole tier to CI, which spreads it over workers with ``-n auto --dist load``. Every + whole tier to CI, which spreads it over workers with ``-n logical --dist load``. Every purge-then-import leaves the previous generation of :mod:`pcapkit` behind as garbage, and the whole suite in one plain-unittest process runs out of memory -- :file:`util/run_unittest_leg.py` records the measurement. diff --git a/tests/project/test_ci_timing.py b/tests/project/test_ci_timing.py index 51ac899b65..22ab3edacb 100644 --- a/tests/project/test_ci_timing.py +++ b/tests/project/test_ci_timing.py @@ -1178,6 +1178,13 @@ def test_the_legs_the_review_reads_exist(self) -> 'None': coverage = [entry['python-version'] for entry in found if entry.get('coverage')] self.assertEqual([f'Python {version}' for version in coverage], [ci_timing.COVERAGE_JOB]) + def test_the_review_reads_the_coverage_legs_timings(self) -> 'None': + """#2571: the coverage leg, ``latest``, alone runs the ``LATEST`` tier, so its file is read too.""" + coverage = [entry['python-version'] for entry in matrix_legs(matrix('unit-tests.yml', 'test')) + if entry.get('coverage')] + self.assertEqual(len(coverage), 1) + self.assertIn(coverage[0], ci_timing.JUNIT_LEGS) + def test_the_reusable_workflow_asks_for_no_write_permission(self) -> 'None': """Its callers grant ``contents: read``, so a write scope would break them.""" code = '\n'.join(line.split('#', 1)[0] for line in self.text.splitlines()) diff --git a/tests/project/test_sweeps_on_latest_only_1732.py b/tests/project/test_sweeps_on_latest_only_1732.py index 807312189e..fec9005ce2 100644 --- a/tests/project/test_sweeps_on_latest_only_1732.py +++ b/tests/project/test_sweeps_on_latest_only_1732.py @@ -293,6 +293,19 @@ def test_every_other_leg_runs_the_rest(self) -> None: self.assertEqual(argv[:2], ['-m', 'pytest']) self.assertEqual(pytest_args(argv), want) + def test_every_test_leg_runs_a_worker_per_logical_cpu(self) -> None: + """#2571: ``-n logical``, as ``auto`` counts physical cores wherever psutil is installed. + + PyPy's legs share the flag, and with no psutil there resolve it as ``auto`` would. + + """ + for version, (leg, argv) in self.runs.items(): + if is_experimental(leg): + continue + with self.subTest(python=version): + args = pytest_args(argv) + self.assertEqual(args[args.index('-n') + 1], 'logical') + def test_a_step_that_ignores_the_pins_alone_off_latest_is_caught(self) -> None: """The check above fails when a leg off ``latest`` leaves out the pins rather than the tier.""" script = step(JOB)['run'] diff --git a/tests/project/test_windows_test_leg_2060.py b/tests/project/test_windows_test_leg_2060.py index ce216936b0..b731fc67f6 100644 --- a/tests/project/test_windows_test_leg_2060.py +++ b/tests/project/test_windows_test_leg_2060.py @@ -287,14 +287,15 @@ def test_it_runs_the_all_tier_with_tests_flags(self) -> None: *(f'--ignore={path}' for path in latest)]) def test_its_flags_are_tests(self) -> None: - """The flags as unit-tests.yml ``test``'s step writes them.""" - flags = re.compile(r"set -- -q -n auto --dist load .*?--ignore-glob='\*_regression\.py'", re.S) + """The flags as unit-tests.yml ``test``'s step writes them, but for ``-n``, which #2571 moved there alone.""" + flags = re.compile(r"set -- -q -n (\w+) (--dist load .*?--ignore-glob='\*_regression\.py')", re.S) own = flags.search(step('Run unit tests')['run']) tests = [entry for entry in _workflows.steps('unit-tests.yml', 'test') if entry.get('name') == 'Run unit tests'] theirs = flags.search(tests[0]['run']) self.assertIsNotNone(own) self.assertIsNotNone(theirs) - self.assertEqual(own.group(0).split(), theirs.group(0).split()) # type: ignore[union-attr] + self.assertEqual((own.group(1), theirs.group(1)), ('auto', 'logical')) # type: ignore[union-attr] + self.assertEqual(own.group(2).split(), theirs.group(2).split()) # type: ignore[union-attr] def test_a_tier_it_cannot_list_fails_the_step_with_a_warning(self) -> None: result, argv, _ = self.run_step(FAIL_C='1') diff --git a/util/ci_timing.py b/util/ci_timing.py index 0767b37f00..fb11fe2580 100644 --- a/util/ci_timing.py +++ b/util/ci_timing.py @@ -124,26 +124,27 @@ TIERS = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), '.github', 'python-versions.toml') -def _coverage_legs() -> 'tuple[str, tuple[str, ...]]': - """The ``latest`` leg's job name, and the other supported legs', from :data:`TIERS`.""" +def _coverage_legs() -> 'tuple[str, str, tuple[str, ...]]': + """``latest``, its leg's job name, and the other supported legs', from :data:`TIERS`.""" with open(TIERS, 'rb') as file: data = tomllib.load(file) paired = tuple(f'Python {version}' for version, entry in data['versions']['cpython'].items() if entry['tier'] == 'supported' and version != data['latest']) - return f"Python {data['latest']}", paired + return data['latest'], f"Python {data['latest']}", paired -#: The leg running under coverage, ``latest``, and the legs its tests step is paired -#: against: the rest of the supported tier (#1633), not the KTLO legs, which are -#: named ``Python 3.x (KTLO)`` and may stop early without failing. -COVERAGE_JOB, PAIRED_JOBS = _coverage_legs() +#: ``latest``, the job of its leg, which runs under coverage, and the legs that job's +#: tests step is paired against: the rest of the supported tier (#1633), not the KTLO +#: legs, which are named ``Python 3.x (KTLO)`` and may stop early without failing. +LATEST, COVERAGE_JOB, PAIRED_JOBS = _coverage_legs() TESTS_STEP = 'Run unit tests' #: The leg ms/test is measured on: a supported version not under coverage, kept on #: 3.13 -- the newest such leg when W4's baseline was measured -- so that W4 compares #: one interpreter with itself week over week. MS_PER_TEST_LEG = '3.13' -#: The JUnit artifacts read, by leg, and from how many push runs. -JUNIT_LEGS = ('3.13', '3.14') +#: The JUnit artifacts read, by leg, and from how many push runs. ``latest``'s holds +#: the ``LATEST`` tier, which no other leg runs (#2571). +JUNIT_LEGS = ('3.13', '3.14', LATEST) JUNIT_ARTIFACT = 'junit-unit-{leg}' JUNIT_RUNS = 5 From fba120819b765d5cd378a4495170991bb379c569 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 1 Nov 2026 08:30:49 -0500 Subject: [PATCH 2/2] ci(timing): fold in the review's accuracy fixes (#2571) - test_windows_test_leg_2060.py: the flags docstrings state what is true now, all but `-n`, which only `test`'s step sets to `logical`. - unit-tests.yml: W3 is util/ci_timing.py's, not ci-timing.yml's; the `-n logical` comment is two lines. - test-coverage.rst: `-n logical` is the `test` job's, the unit tier's. - util/ci_timing.py: LATEST_VERSION, so it is not mistaken for the `LATEST` tier; JUNIT_LEGS drops a duplicate leg, which the test pins. --- .github/workflows/unit-tests.yml | 9 ++++----- docs/source/contributing/conventions/test-coverage.rst | 9 +++++---- tests/project/test_ci_timing.py | 2 ++ tests/project/test_windows_test_leg_2060.py | 8 ++++---- util/ci_timing.py | 4 ++-- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 127c85ddc0..86748b46de 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -102,8 +102,8 @@ jobs: # # `include` marks one leg `coverage: true`, `latest` (#1661), so coverage # adds no leg to the queue. That leg runs slower than the rest: it runs the - # `LATEST` tier as well as `ALL`, under coverage. ci-timing.yml's W3 tracks - # the gap. + # `LATEST` tier as well as `ALL`, under coverage. util/ci_timing.py's W3 + # tracks the gap. # # #2243, the owner's option B: a test-only pull request runs the # `latest` leg alone, the coverage leg, which runs the whole suite with @@ -184,9 +184,8 @@ jobs: # cannot silently change how this suite is distributed. The durations # table and JUnit file are #1538's per-test timings. # - # #2571: `-n logical`, as `auto` counts physical cores, half the runner's - # CPUs, wherever psutil is installed: the `test` extra installs it on - # CPython. On PyPy, which has none, the two agree. + # #2571: `-n logical`, as with psutil (the `test` extra's, CPython only) + # `auto` counts physical cores, half the runner's CPUs; on PyPy they agree. # # #1958, the test tiers: tests/_tiers.py's module_tier() puts every test # module in `ALL`, a curated list each stating `__tier__ = 'ALL'` (#1963), diff --git a/docs/source/contributing/conventions/test-coverage.rst b/docs/source/contributing/conventions/test-coverage.rst index 56525c956c..038c0301cc 100644 --- a/docs/source/contributing/conventions/test-coverage.rst +++ b/docs/source/contributing/conventions/test-coverage.rst @@ -123,10 +123,11 @@ Running the Tests * **One module per process, never the whole suite.** Check a change by running the modules it touches, each as its own ``python -m pytest -q``, and leave the - whole tier to CI, which spreads it over workers with ``-n logical --dist load``. Every - purge-then-import leaves the previous generation of :mod:`pcapkit` behind as - garbage, and the whole suite in one plain-unittest process runs out of memory -- - :file:`util/run_unittest_leg.py` records the measurement. + whole tier to CI, whose ``test`` job spreads the unit tier over workers with + ``-n logical --dist load``. Every purge-then-import leaves the previous generation + of :mod:`pcapkit` behind as garbage, and the whole suite in one plain-unittest + process runs out of memory -- :file:`util/run_unittest_leg.py` records the + measurement. * **Read the summary line, not the exit code.** pytest's last line (``N passed, M failed``) and unittest's ``OK`` or ``FAILED (...)`` are the result; an exit code read through a pipe or a wrapper is the pipe's or the wrapper's. diff --git a/tests/project/test_ci_timing.py b/tests/project/test_ci_timing.py index 22ab3edacb..a8d514d775 100644 --- a/tests/project/test_ci_timing.py +++ b/tests/project/test_ci_timing.py @@ -1184,6 +1184,8 @@ def test_the_review_reads_the_coverage_legs_timings(self) -> 'None': if entry.get('coverage')] self.assertEqual(len(coverage), 1) self.assertIn(coverage[0], ci_timing.JUNIT_LEGS) + # Each leg once, or its artifact is read and counted twice. + self.assertEqual(len(set(ci_timing.JUNIT_LEGS)), len(ci_timing.JUNIT_LEGS)) def test_the_reusable_workflow_asks_for_no_write_permission(self) -> 'None': """Its callers grant ``contents: read``, so a write scope would break them.""" diff --git a/tests/project/test_windows_test_leg_2060.py b/tests/project/test_windows_test_leg_2060.py index b731fc67f6..3a3e2faeb2 100644 --- a/tests/project/test_windows_test_leg_2060.py +++ b/tests/project/test_windows_test_leg_2060.py @@ -15,9 +15,9 @@ an error. * :class:`TestItRunsOnWindows`: every ``run:`` is Git Bash's, none calls a Linux-only tool, and the checkout keeps LF line endings. -* :class:`TestTheTestsStep`: run as bash runs it, the step hands pytest ``test``'s - flags and ignores the ``LATEST`` tier, and a tier it cannot list fails the step - with a warning. +* :class:`TestTheTestsStep`: run as bash runs it, the step hands pytest all but + ``-n`` of ``test``'s flags and ignores the ``LATEST`` tier, and a tier it cannot + list fails the step with a warning. * :class:`TestItHoldsLatestBackNowhere`: the weekly tier job does not wait for a Windows build before it moves ``latest``. * :class:`TestTheReporters`: CI Reports and the CI Timing review read the job as a @@ -287,7 +287,7 @@ def test_it_runs_the_all_tier_with_tests_flags(self) -> None: *(f'--ignore={path}' for path in latest)]) def test_its_flags_are_tests(self) -> None: - """The flags as unit-tests.yml ``test``'s step writes them, but for ``-n``, which #2571 moved there alone.""" + """``test``'s flags, but for ``-n``, which only ``test``'s step sets to ``logical``.""" flags = re.compile(r"set -- -q -n (\w+) (--dist load .*?--ignore-glob='\*_regression\.py')", re.S) own = flags.search(step('Run unit tests')['run']) tests = [entry for entry in _workflows.steps('unit-tests.yml', 'test') if entry.get('name') == 'Run unit tests'] diff --git a/util/ci_timing.py b/util/ci_timing.py index fb11fe2580..b368fc3690 100644 --- a/util/ci_timing.py +++ b/util/ci_timing.py @@ -136,7 +136,7 @@ def _coverage_legs() -> 'tuple[str, str, tuple[str, ...]]': #: ``latest``, the job of its leg, which runs under coverage, and the legs that job's #: tests step is paired against: the rest of the supported tier (#1633), not the KTLO #: legs, which are named ``Python 3.x (KTLO)`` and may stop early without failing. -LATEST, COVERAGE_JOB, PAIRED_JOBS = _coverage_legs() +LATEST_VERSION, COVERAGE_JOB, PAIRED_JOBS = _coverage_legs() TESTS_STEP = 'Run unit tests' #: The leg ms/test is measured on: a supported version not under coverage, kept on #: 3.13 -- the newest such leg when W4's baseline was measured -- so that W4 compares @@ -144,7 +144,7 @@ def _coverage_legs() -> 'tuple[str, str, tuple[str, ...]]': MS_PER_TEST_LEG = '3.13' #: The JUnit artifacts read, by leg, and from how many push runs. ``latest``'s holds #: the ``LATEST`` tier, which no other leg runs (#2571). -JUNIT_LEGS = ('3.13', '3.14', LATEST) +JUNIT_LEGS = tuple(dict.fromkeys(('3.13', '3.14', LATEST_VERSION))) JUNIT_ARTIFACT = 'junit-unit-{leg}' JUNIT_RUNS = 5