diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 2916000de..18df57d62 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -804,9 +804,17 @@ commits it did not make: when the branch holds some and proposes other tiers, ``detect`` fails before asking for the approval. Once that pull request is merged or closed, the next run with something to propose recreates the branch from ``main``, a closed proposal's tiers included. It reports a pre-release -instead of writing it. The experimental entries are hand-maintained: no source it -reads rates them, so every run keeps their lines as written, and a hand edit to -one changes no proposal. +instead of writing it. Nor does it retire a job (`#1747 +`__): when the tiers leave +a matrix job with no leg, as 3.11 reaching end of life leaves +``pypcap-parity``, it writes them as computed, warns, and puts the job first in +the body as a manual step: retire or remove it, and the ``needs:`` that list it, +such as the one in ``required-checks``, before merging. A proposal is still +identified by its tiers alone, so retiring the job on ``main`` does not propose +them again. The +experimental entries are hand-maintained: no source it reads rates them, so +every run keeps their lines as written, and a hand edit to one changes no +proposal. Deployment ``environment:`` ---------------------------- diff --git a/tests/project/test_python_versions_empty_job_1747.py b/tests/project/test_python_versions_empty_job_1747.py new file mode 100644 index 000000000..08e6a7264 --- /dev/null +++ b/tests/project/test_python_versions_empty_job_1747.py @@ -0,0 +1,90 @@ +# -*- coding: utf-8 -*- +"""#1747: the version updater names a job its update would leave with no leg. + +Since #1746, ``pypcap-parity`` holds the supported versions below 3.12 only, so +3.11 reaching end of life leaves its matrix empty while ``required-checks`` +still needs it. The owner's ruling on #1747: the updater retires nothing; its +proposal flags the job as a manual step, for him to retire or remove it, rather +than quietly writing a zero-leg matrix. The tier file is still written as +computed, and the fingerprint is still the tiers alone. + +The scenarios are :mod:`tests.project.test_python_versions_update_1660`'s. + +""" + +from __future__ import annotations + +import unittest +from typing import TYPE_CHECKING + +from tests.project.test_python_versions_update_1660 import (FROZEN, ROOT, as_of_the_file, changes, drop_build, + drop_image, notes, plan_for, set_status, snapshot, + three_eleven_ends_and_three_sixteen_ships, updater, + with_experimental) + +if TYPE_CHECKING: + from typing import Any + +#: What 3.11 reaching end of life asks of the owner. +PARITY = ('unit-tests.yml: `pypcap-parity` would have no leg left once 3.11 moves to KTLO; retire or remove the ' + 'job (and its `needs:` in `required-checks`) before merging this update') +#: What 3.10 losing its ubuntu-26.04 build asks: no job needs a KTLO twin (#1738). +PARITY_KTLO = ('unit-tests.yml: `pypcap-parity-ktlo` would have no leg left once 3.10 runs on ubuntu-24.04; ' + 'retire or remove the job before merging this update') + + +def describe(plan: 'dict[str, Any]') -> 'str': + return updater().describe_plan(plan, ('success', []), 'https://example.invalid/run') + + +class TestAnEmptiedJobIsAManualStep(unittest.TestCase): + """Each job a tier move leaves with no leg is named, with what to retire; the tiers are written regardless.""" + + def test_three_eleven_ending_names_pypcap_parity(self) -> None: + for label, sources in (('3.11 ends', set_status(as_of_the_file(snapshot()), '3.11', 'end-of-life')), + ('and 3.16 ships', three_eleven_ends_and_three_sixteen_ships(snapshot()))): + with self.subTest(scenario=label): + _, plan, texts = plan_for(self, sources) + self.assertEqual(plan['retire'], [PARITY]) + self.assertIn(PARITY, notes(plan, 'manual')) + # The tier move is still proposed as computed. + self.assertEqual(changes(plan)['3.11'], ({'tier': 'supported'}, {'tier': 'ktlo'})) + self.assertIn('"3.11" = { tier = "ktlo" }\n', texts['.github/python-versions.toml']) + + def test_the_body_puts_it_first_and_once(self) -> None: + _, plan, _ = plan_for(self, set_status(as_of_the_file(snapshot()), '3.11', 'end-of-life')) + description = describe(plan) + self.assertEqual(description.count(PARITY), 1) + block = description.index('**Before merging, retire or remove these jobs by hand**') + self.assertLess(block, description.index(f'- {PARITY}')) + self.assertLess(description.index(f'- {PARITY}'), description.index('**Before merging, edit ruleset')) + self.assertLess(description.index(f'- {PARITY}'), description.index('Not done here:')) + + def test_no_note_when_no_job_empties(self) -> None: + """PyPy 3.12 joining, and 3.7 leaving ``test-ktlo`` with three legs; ``experimental`` was empty already.""" + for label, sources in (('pypy3.12 joins', snapshot()), + ('3.7 drops', drop_image(as_of_the_file(snapshot()), 'ubuntu-22.04'))): + with self.subTest(scenario=label): + _, plan, _ = plan_for(self, sources) + self.assertTrue(plan['changes']) + self.assertEqual(plan['retire'], []) + self.assertFalse([text for text in notes(plan, 'manual') if 'no leg left' in text]) + self.assertNotIn('retire or remove', describe(plan)) + + def test_a_ktlo_twin_emptying_is_flagged(self) -> None: + """3.10, ``pypcap-parity-ktlo``'s one leg, needs ubuntu-24.04, which that job does not run on.""" + _, plan, _ = plan_for(self, drop_build(as_of_the_file(snapshot()), '3.10', '26.04')) + self.assertEqual(plan['retire'], [PARITY_KTLO]) + self.assertIn(PARITY_KTLO, notes(plan, 'manual')) + self.assertIn(f'- {PARITY_KTLO}', describe(plan)) + + def test_experimental_yml_is_covered_too(self) -> None: + """Its entries are hand-kept, so no run empties it; the check reads every matrix job all the same.""" + module = updater() + workflows = module.load_workflows(ROOT) + before = module.pv.outputs(module.validate(with_experimental(FROZEN.read_text(encoding='utf-8')))) + after = module.pv.outputs(module.validate(FROZEN.read_text(encoding='utf-8'))) + self.assertEqual(module.emptied_jobs(workflows, before, after, {}), [ + 'experimental.yml: `experimental` would have no leg left once 3.14t leaves it and graalpy-25.4 leaves ' + 'it; retire or remove the job before merging this update']) + self.assertEqual(module.emptied_jobs(workflows, after, after, {}), []) diff --git a/tests/project/test_python_versions_update_1660.py b/tests/project/test_python_versions_update_1660.py index c714aadf1..30ab0e44a 100644 --- a/tests/project/test_python_versions_update_1660.py +++ b/tests/project/test_python_versions_update_1660.py @@ -527,7 +527,9 @@ def test_the_regenerated_tree_passes_the_audit(self) -> None: self.assertEqual(result.testsRun, stale.testsRun) self.assertTrue(result.wasSuccessful(), '\n'.join(text for _, text in result.failures + result.errors)) - self.assertEqual([note for note in notes(plan, 'manual') if 'ktlo_311' not in note], []) + # #1747: and `pypcap-parity`, which 3.11 leaving empties, for the owner to retire. + self.assertEqual([note for note in notes(plan, 'manual') + if 'ktlo_311' not in note and note not in plan['retire']], []) def test_every_claim_and_rewriter_matches_once_on_main(self) -> None: """So a reworded sentence fails here, not as a silent skip in a weekly run.""" diff --git a/util/update_python_versions.py b/util/update_python_versions.py index 30b71800f..322118db2 100644 --- a/util/update_python_versions.py +++ b/util/update_python_versions.py @@ -62,7 +62,9 @@ is regenerated with it (:data:`CLAIMS`, :data:`REWRITERS`), but nothing under :file:`.github/workflows/`: an App installation token needs the ``workflows`` permission to push a change there, and the job's token has contents and pull -requests only. Workflow comments name the tiers instead of restating them. +requests only. Workflow comments name the tiers instead of restating them. Nor +does it retire a job whose matrix the new tiers leave empty: the proposal names +the job as a manual step, for the owner to retire or remove first (#1747). Three commands: @@ -468,6 +470,40 @@ def required_contexts(workflows: 'dict[str, Any]', values: 'dict[str, str]') -> if leg.get('ktlo') is not True and leg.get('experimental') is not True] +def needed_by(workflow: 'dict[str, Any]', job_id: 'str') -> 'list[str]': + """The jobs of ``workflow`` whose ``needs:`` lists ``job_id``.""" + found = [] + for other, job in (workflow.get('jobs') or {}).items(): + needs = job.get('needs') or [] + if job_id in ([needs] if isinstance(needs, str) else needs): + found.append(other) + return found + + +def emptied_jobs(workflows: 'dict[str, Any]', before: 'dict[str, str]', after: 'dict[str, str]', + why: 'dict[str, str]') -> 'list[str]': + """A manual step for each job whose matrix the tiers ``after`` leave with no leg (#1747). + + ``pypcap-parity`` holds supported versions below 3.12 only, so 3.11 moving to + KTLO empties it. The updater retires nothing: dropping a job is a workflow + edit, which ``pr``'s token cannot push, and the owner's call. So the tiers are + written as computed, and the job is named here, with each version ``why`` + says leaves it and the jobs that need it, for the owner to retire first. + A matrix that was empty already is not one this update empties. + + """ + old, new = matrix_legs(workflows, before), matrix_legs(workflows, after) + found = [] + for name, job_id in sorted(job for job, legs in old.items() if legs and not new.get(job)): + versions = dict.fromkeys(str(leg.get('python-version')) for leg in old[(name, job_id)]) + reasons = ' and '.join(why.get(version, f'{version} leaves it') for version in versions) + needs = needed_by(workflows[name], job_id) + needed = f' (and its `needs:` in {", ".join(f"`{job}`" for job in needs)})' if needs else '' + found.append(f'{name}: `{job_id}` would have no leg left once {reasons}; retire or remove the ' + f'job{needed} before merging this update') + return found + + def setup_steps(job: 'dict[str, Any]') -> 'list[dict[str, Any]]': return [step for step in job.get('steps', []) if str(step.get('uses', '')).startswith('actions/setup-python@')] @@ -1145,6 +1181,13 @@ def compute_plan(root: 'pathlib.Path', sources: 'dict[str, Any]') -> 'tuple[dict f'its own `{key}` output for `release_status`')) changes = tier_changes(before, after, reasons) + # #1747: in the plan's notes and its own key, which the body puts first. + # Not in the fingerprint: like the renames, it follows from the tiers and + # main's workflows, and the owner retiring the job on main must not make + # the same tiers a new proposal, which would recreate the branch. + retire = emptied_jobs(workflows, before_values, after_values, + {change['version']: phrase(change) for change in changes}) + notes.extend(Note('manual', text) for text in retire) plan = { 'fingerprint': fingerprint(latest, versions), 'title': title_of(changes, (before['latest'], latest)), @@ -1156,6 +1199,7 @@ def compute_plan(root: 'pathlib.Path', sources: 'dict[str, Any]') -> 'tuple[dict 'renamed': renamed, 'added': added, 'removed': removed, + 'retire': retire, 'notes': [note._asdict() for note in notes], } return plan, {path: texts[path] for path in changed} @@ -1194,6 +1238,11 @@ def describe_plan(plan: 'dict[str, Any]', audit: 'tuple[str, list[str]]', run_ur if plan['latest'][0] != plan['latest'][1]: out.append(f'| `latest` | {plan["latest"][0]} | {plan["latest"][1]} | the newest bugfix branch |') out.append('') + if plan['retire']: + out.append('**Before merging, retire or remove these jobs by hand** -- the workflow retires nothing and ' + 'never edits a workflow file, and these tiers leave their matrices empty (#1747):') + out += [f'- {text}' for text in plan['retire']] + out.append('') required = plan['required'] if required['remove'] or required['add']: out.append('**Before merging, edit ruleset 23497679 (`main`)** -- the workflow never edits it:') @@ -1208,8 +1257,9 @@ def describe_plan(plan: 'dict[str, Any]', audit: 'tuple[str, list[str]]', run_ur out.append('New check runs: ' + ', '.join(f'`{name}`' for name in plan['added']) + '.') if plan['removed']: out.append('Removed check runs: ' + ', '.join(f'`{name}`' for name in plan['removed']) + '.') - if plan['notes']: - out += ['', 'Not done here:'] + [f'- {note["text"]}' for note in plan['notes']] + rest = [note['text'] for note in plan['notes'] if note['text'] not in plan['retire']] + if rest: + out += ['', 'Not done here:'] + [f'- {text}' for text in rest] outcome, failures = audit names = ', '.join(f'`{module.rsplit(".", 1)[-1]}`' for module in AUDIT) if outcome == 'success':