diff --git a/.github/workflows/experimental.yml b/.github/workflows/experimental.yml index 3f021d3cb..6249ad3be 100644 --- a/.github/workflows/experimental.yml +++ b/.github/workflows/experimental.yml @@ -25,12 +25,17 @@ on: permissions: contents: read -# Only pull-request runs may be superseded, as in unit-tests.yml: every other -# event gets a group of its own, by run id, so a push is never cancelled while -# pending behind the next one. +# #1903, the owner's option 1: a run is superseded by the next one on its ref, +# whether a pull request's or a push to main's. Nothing waits on these legs, so +# main's newest commit is the one worth testing, and a merge wave no longer +# queues a two-hour GraalPy leg per push. unit-tests.yml still gives each push +# a run of its own, since its `unittest-ordering` leg runs only on main. Any +# other event -- a push to another branch, should `on:` ever allow one, or a +# trigger added later -- keeps a group of its own, by run id, and is never +# cancelled. concurrency: - group: experimental-tests-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + group: experimental-tests-${{ (github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == 'refs/heads/main')) && github.ref || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == 'refs/heads/main') }} jobs: # unit-tests.yml's job of the same name, reading the one output this file needs. diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 9b5b6a8f7..ed56ffdba 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -751,7 +751,11 @@ Tests run completes without it, and with it ``Required checks passed``, (``coverage-comment.yml``, ``deploy-pages.yml``, ``cron-vendor.yml`` and ``cron-conda.yml``). It runs on Unit Tests' ``push`` and ``pull_request`` events, skips a docs-only pull request by the same ``changes`` script, and cancels a -pull request's superseded run. The legs never +superseded run, a pull request's or, unlike Unit Tests, a push to ``main``'s +(`#1903 `__): since nothing +waits on the legs, ``main``'s newest commit is the one worth testing, whereas Unit +Tests keeps a run per push for ``unittest-ordering``, which runs only there. Any +other event keeps a concurrency group of its own, by run id. The legs never block, by the KTLO mechanics above: every step that can fail has ``continue-on-error: true``, their annotations are warnings, and ``Experimental summary``, the job's last step, reports each failed step. A failure no step diff --git a/tests/project/test_experimental_concurrency_1903.py b/tests/project/test_experimental_concurrency_1903.py new file mode 100644 index 000000000..4be394b3d --- /dev/null +++ b/tests/project/test_experimental_concurrency_1903.py @@ -0,0 +1,139 @@ +# -*- coding: utf-8 -*- +"""#1903: a newer run on ``main`` supersedes Experimental Tests' older one. + +Every push to ``main`` used to get a concurrency group of its own, by run id, so +a merge wave queued a GraalPy leg of up to two hours per commit and held runners +that the open pull requests' CI then waited behind. The owner chose option 1: in +experimental.yml only, a push to ``main`` joins one group per ref and cancels the +run in progress, as a pull request's run already did. Nothing waits on these legs +(#1732), so the newest commit's run is enough. + +unit-tests.yml keeps a group per push, because its ``unittest-ordering`` leg runs +only on ``main`` and a cancelled run there would leave a commit with no ordering +check at all: :class:`TestUnitTestsKeepsARunPerCommit`. + +The expressions are evaluated, with the evaluator +:mod:`tests.project.test_release_gates` pins, rather than matched as text: what +is checked is the group each kind of run lands in and whether it cancels, which +a reworded expression can keep or break without any substring changing. + +""" + +from __future__ import annotations + +import re +import unittest + +import yaml + +from tests import _tiers +from tests.project.test_release_gates import evaluate_condition, evaluate_expression + +WORKFLOWS = _tiers.ROOT / '.github' / 'workflows' + +MAIN = 'refs/heads/main' +PULL = 'refs/pull/1903/merge' + +#: One ``${{ }}`` interpolation in a ``group:`` value, its body captured. +_INTERPOLATION = re.compile(r'\$\{\{(.*?)\}\}', re.S) + + +def concurrency(name: 'str') -> 'dict[str, str]': + """The workflow-level ``concurrency:`` mapping of ``name``.""" + with open(WORKFLOWS / name, encoding='utf-8') as file: + return yaml.safe_load(file)['concurrency'] + + +def context(event_name: 'str', ref: 'str', run_id: 'str', workflow: 'str' = 'Experimental Tests') -> 'dict[str, object]': + """One run's ``github`` context, as far as a ``concurrency:`` block reads it.""" + return {'github': {'event_name': event_name, 'ref': ref, 'run_id': run_id, 'workflow': workflow}} + + +class Concurrency: + """A ``concurrency:`` block, evaluated per run.""" + + def __init__(self, name: 'str') -> None: + self.raw = concurrency(name) + + def group(self, run: 'dict[str, object]') -> 'str': + """The group key: literal text kept, each interpolation evaluated in turn. + + unit-tests.yml's interpolates ``github.workflow`` too, so a key may hold + more than one; the evaluator takes one whole expression at a time. + + """ + parts = _INTERPOLATION.split(self.raw['group']) + return ''.join(part if index % 2 == 0 else str(evaluate_expression('${{' + part + '}}', run)) + for index, part in enumerate(parts)) + + def cancels(self, run: 'dict[str, object]') -> 'bool': + value = self.raw['cancel-in-progress'] + if isinstance(value, bool): + return value + return evaluate_condition(value, run) + + +class TestExperimentalSupersedesOnMain(unittest.TestCase): + """experimental.yml: a pull request's and ``main``'s runs are superseded per ref.""" + + def setUp(self) -> None: + self.own = Concurrency('experimental.yml') + + def test_the_cases_below_cover_every_trigger(self) -> None: + # Pushes to `main` and pull requests are all experimental.yml runs on + # today; the other events below are the guard for a trigger added later. + with open(WORKFLOWS / 'experimental.yml', encoding='utf-8') as file: + spec = yaml.safe_load(file) + on = spec.get('on', spec.get(True)) + self.assertEqual(set(on), {'push', 'pull_request'}) + self.assertEqual(on['push'], {'branches': ['main']}) + + def test_pushes_to_main_share_one_cancelling_group(self) -> None: + """#1903: the next push's run cancels the one in progress.""" + first, second = context('push', MAIN, '1'), context('push', MAIN, '2') + self.assertEqual(self.own.group(first), f'experimental-tests-{MAIN}') + self.assertEqual(self.own.group(first), self.own.group(second)) + self.assertTrue(self.own.cancels(second)) + + def test_a_pull_request_still_shares_one_per_ref(self) -> None: + first, second = context('pull_request', PULL, '1'), context('pull_request', PULL, '2') + self.assertEqual(self.own.group(first), f'experimental-tests-{PULL}') + self.assertEqual(self.own.group(first), self.own.group(second)) + self.assertTrue(self.own.cancels(second)) + other = context('pull_request', 'refs/pull/1904/merge', '3') + self.assertNotEqual(self.own.group(other), self.own.group(first)) + self.assertNotEqual(self.own.group(first), self.own.group(context('push', MAIN, '1'))) + + def test_any_other_run_gets_a_group_of_its_own_and_is_never_cancelled(self) -> None: + """Including a dispatch or schedule on ``main``, which must not join the pushes' group.""" + for event_name, ref in (('push', 'refs/heads/feature'), ('schedule', MAIN), + ('workflow_dispatch', MAIN), ('workflow_run', MAIN)): + with self.subTest(event_name=event_name, ref=ref): + run = context(event_name, ref, '38497299656') + self.assertEqual(self.own.group(run), 'experimental-tests-38497299656') + self.assertFalse(self.own.cancels(run)) + + +class TestUnitTestsKeepsARunPerCommit(unittest.TestCase): + """unit-tests.yml is unchanged: only a pull request's run is superseded.""" + + def setUp(self) -> None: + self.unit = Concurrency('unit-tests.yml') + + def test_the_block_is_unchanged(self) -> None: + self.assertEqual(self.unit.raw, { + 'group': "unit-tests-${{ github.workflow }}-" + "${{ github.event_name == 'pull_request' && github.ref || github.run_id }}", + 'cancel-in-progress': "${{ github.event_name == 'pull_request' }}", + }) + + def test_each_push_to_main_gets_its_own_group(self) -> None: + first = context('push', MAIN, '1', workflow='Unit Tests') + second = context('push', MAIN, '2', workflow='Unit Tests') + self.assertEqual(self.unit.group(first), 'unit-tests-Unit Tests-1') + self.assertNotEqual(self.unit.group(first), self.unit.group(second)) + self.assertFalse(self.unit.cancels(second)) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/project/test_sweeps_on_latest_only_1732.py b/tests/project/test_sweeps_on_latest_only_1732.py index a87c480c6..443765c29 100644 --- a/tests/project/test_sweeps_on_latest_only_1732.py +++ b/tests/project/test_sweeps_on_latest_only_1732.py @@ -295,15 +295,17 @@ def test_the_workflow_holds_every_experimental_leg_and_unit_tests_none(self) -> self.assertNotIn('experimental', str(spec.get('name'))) def test_the_same_events_and_its_own_concurrency(self) -> None: - """unit-tests.yml's push and pull_request, no ``workflow_call``, read-only, PR runs superseded.""" + """unit-tests.yml's push and pull_request, no ``workflow_call``, read-only, a group of its own. + + Which runs it supersedes is #1903's, evaluated in + :mod:`tests.project.test_experimental_concurrency_1903`. + + """ events = triggers(self.own) self.assertEqual(events, {key: value for key, value in triggers(self.unit).items() if key != 'workflow_call'}) self.assertEqual(set(events), {'push', 'pull_request'}) self.assertEqual(self.own['permissions'], {'contents': 'read'}) - concurrency = self.own['concurrency'] - self.assertEqual(concurrency['cancel-in-progress'], "${{ github.event_name == 'pull_request' }}") - self.assertIn("github.event_name == 'pull_request' && github.ref || github.run_id", concurrency['group']) - self.assertNotEqual(concurrency['group'], self.unit['concurrency']['group']) + self.assertNotEqual(self.own['concurrency']['group'], self.unit['concurrency']['group']) def test_it_skips_a_docs_only_diff_as_test_does(self) -> None: """unit-tests.yml's ``changes`` script, byte for byte, gating the job as it gates ``test``."""