diff --git a/.github/workflows/cron-conda.yml b/.github/workflows/cron-conda.yml index 6e66db5ffe..49e8c54322 100644 --- a/.github/workflows/cron-conda.yml +++ b/.github/workflows/cron-conda.yml @@ -23,11 +23,18 @@ on: # publishes to Anaconda, and letting two runs upload the same version # concurrently is worse than dropping a superseded duplicate. # +# A `workflow_run` is keyed on the commit `Unit Tests` verified instead (#1643). +# Each `Unit Tests` completion raises a run here, and one whose verdict is for an +# older commit only runs `verdict` and skips -- but in a shared group it would +# still displace a pending run whose verdict matches, since "any existing pending +# job or workflow in the same concurrency group will be canceled" (GitHub, +# "Control the concurrency of workflows and jobs"). +# # Note this group is not what serialises the repository writes -- the # `conda-update` job below carries its own `repository-maintenance` group, shared # with cron-vendor, and that is what actually stops two jobs committing at once. concurrency: - group: conda-update-${{ github.ref }} + group: conda-update-${{ github.event.workflow_run.head_sha || github.ref }} cancel-in-progress: false jobs: @@ -44,18 +51,54 @@ jobs: with: gate-only: true + # #1643: `github.sha`, which every checkout here takes, is main's tip when the + # `workflow_run` was raised -- not the commit the completed `Unit Tests` run + # verified, and in a merge wave the two differ. This job compares them with + # util/verdict_covers_checkout.py, whose docstring gives the rule, and the job + # below acts on a `workflow_run` only when `verified` is 'true'. Its `if:` is + # that job's `workflow_run` clause, so a run raised by a pull request's or a + # failed `Unit Tests` run still concludes `skipped`, as before. + verdict: + name: Verdict covers this commit + if: >- + github.event_name == 'workflow_run' && + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'push' && + github.event.workflow_run.head_branch == 'main' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + outputs: + verified: ${{ steps.verdict.outputs.verified }} + steps: + # The default ref, as the job below uses, so both see the same `github.sha`. + # Twenty commits leaves room for the bot commits the rule accepts; a verdict + # further back is not found, and so not accepted. + - uses: actions/checkout@v7 + with: + fetch-depth: 20 + persist-credentials: false + + - name: Compare the verified commit with this checkout + id: verdict + env: + VERDICT_SHA: ${{ github.event.workflow_run.head_sha }} + run: python3 util/verdict_covers_checkout.py + conda-update: name: Update requirements.txt runs-on: ubuntu-latest - needs: [ unit-tests ] + needs: [ unit-tests, verdict ] # `needs` cannot itself be conditional, so a skipped gate (`workflow_run`) # has to be accepted explicitly -- by default this job would inherit the # skip. `!cancelled()` rather than `always()` so that cancelling the run # still cancels the update; a gate that actually failed still blocks. # # The `workflow_run` clause is the real gate for a push to main now: it - # reads the verdict `Unit Tests` already reached for this exact commit - # instead of re-running it. `.event == 'push'` and `.head_branch == 'main'` + # reads the verdict `Unit Tests` already reached instead of re-running it, + # and `verdict` says whether that verdict covers the commit checked out + # below (#1643). `.event == 'push'` and `.head_branch == 'main'` # both matter -- `workflow_run` fires for *every* completion of that # workflow, including one triggered by a pull request, and without this # check a green PR would upload a conda distribution built from that PR's @@ -65,7 +108,8 @@ jobs: (github.event_name != 'workflow_run' || ( github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'push' && - github.event.workflow_run.head_branch == 'main' + github.event.workflow_run.head_branch == 'main' && + needs.verdict.outputs.verified == 'true' )) concurrency: group: repository-maintenance @@ -90,7 +134,8 @@ jobs: # default-branch commit removes the pattern outright rather than # relying on CodeQL tracing through the guard. The push below rebases # onto main's actual tip before writing regardless, so pinning to the - # exact tested sha would have bought nothing here either. + # exact tested sha would have bought nothing here either. Whether this + # commit is one that verdict covers is `verdict`'s question (#1643). # # The push below re-points `origin` at an SSH remote and authenticates # with the loaded deploy key, not with this checkout's credential -- diff --git a/.github/workflows/cron-vendor.yml b/.github/workflows/cron-vendor.yml index 14ad201eba..d4c03676a9 100644 --- a/.github/workflows/cron-vendor.yml +++ b/.github/workflows/cron-vendor.yml @@ -29,11 +29,18 @@ permissions: {} # picks the registries up again, whereas a dropped validation run means a commit # nobody ever checked. # +# A `workflow_run` is keyed on the commit `Unit Tests` verified instead (#1643). +# Each `Unit Tests` completion raises a run here, and one whose verdict is for an +# older commit only runs `verdict` and skips -- but in a shared group it would +# still displace a pending run whose verdict matches, since "any existing pending +# job or workflow in the same concurrency group will be canceled" (GitHub, +# "Control the concurrency of workflows and jobs"). +# # Note this group is not what serialises the repository writes -- the # `vendor-update` job below carries its own `repository-maintenance` group, shared # with cron-conda, and that is what actually stops two jobs committing at once. concurrency: - group: vendor-update-${{ github.ref }} + group: vendor-update-${{ github.event.workflow_run.head_sha || github.ref }} cancel-in-progress: false jobs: @@ -50,17 +57,53 @@ jobs: with: gate-only: true + # #1643: `github.sha`, which every checkout here takes, is main's tip when the + # `workflow_run` was raised -- not the commit the completed `Unit Tests` run + # verified, and in a merge wave the two differ. This job compares them with + # util/verdict_covers_checkout.py, whose docstring gives the rule, and the job + # below acts on a `workflow_run` only when `verified` is 'true'. Its `if:` is + # that job's `workflow_run` clause, so a run raised by a pull request's or a + # failed `Unit Tests` run still concludes `skipped`, as before. + verdict: + name: Verdict covers this commit + if: >- + github.event_name == 'workflow_run' && + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'push' && + github.event.workflow_run.head_branch == 'main' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + outputs: + verified: ${{ steps.verdict.outputs.verified }} + steps: + # The default ref, as the job below uses, so both see the same `github.sha`. + # Twenty commits leaves room for the bot commits the rule accepts; a verdict + # further back is not found, and so not accepted. + - uses: actions/checkout@v7 + with: + fetch-depth: 20 + persist-credentials: false + + - name: Compare the verified commit with this checkout + id: verdict + env: + VERDICT_SHA: ${{ github.event.workflow_run.head_sha }} + run: python3 util/verdict_covers_checkout.py + vendor-update: runs-on: macos-latest - needs: [ unit-tests ] + needs: [ unit-tests, verdict ] # `needs` cannot itself be conditional, so a skipped gate (`workflow_run`) # has to be accepted explicitly -- by default this job would inherit the # skip. `!cancelled()` rather than `always()` so that cancelling the run # still cancels the update; a gate that actually failed still blocks. # # The `workflow_run` clause is the real gate for a push to main now: it - # reads the verdict `Unit Tests` already reached for this exact commit - # instead of re-running it. `.event == 'push'` and `.head_branch == 'main'` + # reads the verdict `Unit Tests` already reached instead of re-running it, + # and `verdict` says whether that verdict covers the commit checked out + # below (#1643). `.event == 'push'` and `.head_branch == 'main'` # both matter -- `workflow_run` fires for *every* completion of that # workflow, including one triggered by a pull request, and without this # check a green PR would push a vendor-registry commit from that PR's tree. @@ -69,7 +112,8 @@ jobs: (github.event_name != 'workflow_run' || ( github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'push' && - github.event.workflow_run.head_branch == 'main' + github.event.workflow_run.head_branch == 'main' && + needs.verdict.outputs.verified == 'true' )) concurrency: group: repository-maintenance @@ -94,7 +138,8 @@ jobs: # default-branch commit removes the pattern outright rather than # relying on CodeQL tracing through the guard. The push below rebases # onto main's actual tip before writing regardless, so pinning to the - # exact tested sha would have bought nothing here either. + # exact tested sha would have bought nothing here either. Whether this + # commit is one that verdict covers is `verdict`'s question (#1643). # The push below re-points `origin` at an SSH remote and authenticates # with the loaded deploy key, not with this checkout's credential -- # so there is nothing here for a persisted token to do, and one less diff --git a/.github/workflows/deploy-pages.yml b/.github/workflows/deploy-pages.yml index 993bbbb24e..d449e5cc38 100644 --- a/.github/workflows/deploy-pages.yml +++ b/.github/workflows/deploy-pages.yml @@ -29,8 +29,16 @@ permissions: # the newer one publishes, and deploying the older commit's docs after the newer # ones would actively be wrong. GitHub Pages also permits only one live # deployment, so serialising here is a requirement rather than a saving. +# +# A `workflow_run` is keyed on the commit `Unit Tests` verified instead (#1643). +# One whose verdict is for an older commit only runs `verdict` and skips, and in +# the shared group it would still displace a pending run whose verdict matches, +# since "any existing pending job or workflow in the same concurrency group will +# be canceled" (GitHub, "Control the concurrency of workflows and jobs"). The +# coalescing and serialising above then happen in `deploy-pages`' own `ci-` +# group, which only a run whose verdict covers its checkout reaches. concurrency: - group: pages-${{ github.ref }} + group: pages-${{ github.event.workflow_run.head_sha || github.ref }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: @@ -48,8 +56,43 @@ jobs: with: gate-only: true + # #1643: `github.sha`, which every checkout here takes, is main's tip when the + # `workflow_run` was raised -- not the commit the completed `Unit Tests` run + # verified, and in a merge wave the two differ. This job compares them with + # util/verdict_covers_checkout.py, whose docstring gives the rule, and the job + # below acts on a `workflow_run` only when `verified` is 'true'. Its `if:` is + # that job's `workflow_run` clause, so a run raised by a pull request's or a + # failed `Unit Tests` run still concludes `skipped`, as before. + verdict: + name: Verdict covers this commit + if: >- + github.event_name == 'workflow_run' && + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'push' && + github.event.workflow_run.head_branch == 'main' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + outputs: + verified: ${{ steps.verdict.outputs.verified }} + steps: + # The default ref, as the job below uses, so both see the same `github.sha`. + # Twenty commits leaves room for the bot commits the rule accepts; a verdict + # further back is not found, and so not accepted. + - uses: actions/checkout@v7 + with: + fetch-depth: 20 + persist-credentials: false + + - name: Compare the verified commit with this checkout + id: verdict + env: + VERDICT_SHA: ${{ github.event.workflow_run.head_sha }} + run: python3 util/verdict_covers_checkout.py + deploy-pages: - needs: [ unit-tests ] + needs: [ unit-tests, verdict ] # `needs` cannot itself be conditional, so a skipped gate (pull requests, # and now `workflow_run`) has to be accepted explicitly -- by default this # job would inherit the skip. `!cancelled()` rather than `always()` so that @@ -57,8 +100,9 @@ jobs: # failed still blocks. # # The `workflow_run` clause is the real gate for a push to main now: it - # reads the verdict `Unit Tests` already reached for this exact commit - # instead of re-running it. `.event == 'push'` and `.head_branch == 'main'` + # reads the verdict `Unit Tests` already reached instead of re-running it, + # and `verdict` says whether that verdict covers the commit checked out + # below (#1643). `.event == 'push'` and `.head_branch == 'main'` # both matter -- `workflow_run` fires for *every* completion of that # workflow, including one triggered by a pull request, and without this # check a green PR would publish docs from that PR's tree. @@ -67,7 +111,8 @@ jobs: (github.event_name != 'workflow_run' || ( github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'push' && - github.event.workflow_run.head_branch == 'main' + github.event.workflow_run.head_branch == 'main' && + needs.verdict.outputs.verified == 'true' )) concurrency: ci-${{ github.ref }} # Recommended if you intend to make multiple deployments in quick succession. runs-on: macos-latest @@ -86,6 +131,8 @@ jobs: # relying on CodeQL tracing through the guard. `main`'s tip is also # the *right* commit for docs, which should reflect current main, # not a slightly older one -- so this is not a compromise here. + # Whether this commit is one that verdict covers is `verdict`'s + # question (#1643). persist-credentials: false - uses: actions/setup-python@v7 diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 8b05d16ba4..5b3f3c83bb 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -196,8 +196,10 @@ itself (dotted, above) **only when its own trigger is not** ``workflow_run`` (their gate jobs' ``if:`` guards this explicitly) -- i.e. only on the Saturday ``schedule`` path, where no completed Unit Tests run exists yet to read a verdict from. On the ``workflow_run`` path (thick arrows), each reads -the verdict Unit Tests already reached for that same commit instead of -re-running the gate a second time. +the verdict Unit Tests already reached instead of re-running the gate a second +time. Vendor Update, Conda Update and GitHub Pages act on it only once their +``verdict`` job, described under the edge list below, says it is a verdict on +the commit they build. ``workflow_run`` Edges ---------------------- @@ -224,6 +226,19 @@ and ``.head_branch`` (or, for Create Release and Coverage Comment, ``.conclusion doing anything with an outward effect; none of the filtering happens in the ``on:`` block. +Those checks say nothing about *which* commit was verified. A ``workflow_run`` +checks out ``github.sha``, ``main``'s tip when the event is raised, not +``github.event.workflow_run.head_sha``, the commit the completed run tested, and +in a merge wave the two differ: Conda Update 38325136710 bumped 2e941bbb7 under +the verdict for 9083163f5 (`#1643 `__). +So Vendor Update, Conda Update and GitHub Pages each run a ``verdict`` job first, +and act only when the tip is the verified commit, or that commit plus bot commits +to ``conda/build`` and ``conda/requirements.txt`` alone; otherwise they skip, and +the tip's own Unit Tests run raises them again. The rule and its evidence are in +``util/verdict_covers_checkout.py``. It covers the checkout only: Conda Update's +push rebases onto ``main`` again, and Conda Tag tags whatever ``git pull`` brings +in, so a commit that lands after the event can still be tagged. + Reusable-Workflow Calls (``uses:``) ----------------------------------- @@ -234,11 +249,11 @@ repository target the same reusable workflow, with the same input: * ``.github/workflows/create-release.yml:374`` -- job ``unit-tests`` calls ``./.github/workflows/unit-tests.yml`` with ``gate-only: true``. -* ``.github/workflows/cron-conda.yml:43`` -- job ``unit-tests`` calls +* ``.github/workflows/cron-conda.yml:50`` -- job ``unit-tests`` calls ``./.github/workflows/unit-tests.yml`` with ``gate-only: true``. -* ``.github/workflows/cron-vendor.yml:49`` -- job ``unit-tests`` calls +* ``.github/workflows/cron-vendor.yml:56`` -- job ``unit-tests`` calls ``./.github/workflows/unit-tests.yml`` with ``gate-only: true``. -* ``.github/workflows/deploy-pages.yml:47`` -- job ``unit-tests`` calls +* ``.github/workflows/deploy-pages.yml:55`` -- job ``unit-tests`` calls ``./.github/workflows/unit-tests.yml`` with ``gate-only: true``. ``gate-only: true`` selects **two** of ``unit-tests.yml``'s **ten** jobs, diff --git a/tests/project/test_workflow_run_verdict.py b/tests/project/test_workflow_run_verdict.py new file mode 100644 index 0000000000..88762090db --- /dev/null +++ b/tests/project/test_workflow_run_verdict.py @@ -0,0 +1,504 @@ +# -*- coding: utf-8 -*- +"""Tests that Conda Update, Vendor Update and GitHub Pages act only under a verdict +for the commit they build (#1643). + +All three chain off ``Unit Tests`` completing and check out ``github.sha``, which +for a ``workflow_run`` is ``main``'s tip when the event is raised -- not the commit +the completed run verified. Conda Update 38325136710 was raised 2 s after the run +for 9083163f5 succeeded, ran on 2e941bbb7 (three merged pull requests later), and +pushed f4f94bcad at 00:03:38Z; 2e941bbb7's own run 38323130015 finished at +00:13:35Z. + +Two layers, as in :mod:`tests.project.test_release_gates`: + +* :class:`TestTheRuleOnRealRunShapes` runs :file:`util/verdict_covers_checkout.py` + in throwaway repositories rebuilt from that merge wave, so ancestry, authorship + and paths are git's answers rather than a model's; +* :class:`TestEachWorkflowGatesOnTheVerdict` evaluates each workflow's job + conditions and concurrency group over the scenarios, with the evaluator + :mod:`tests.project.test_release_gates` pins. + +""" + +from __future__ import annotations + +import importlib.util +import os +import pathlib +import re +import shutil +import subprocess +import sys +import tempfile +import unittest +from typing import TYPE_CHECKING + +from tests._support import scale_timeout +from tests.project.test_release_gates import (NULL, STATUS, _evaluate, declared_if, declared_needs, + evaluate_condition, job_blocks, parse_condition) + +if TYPE_CHECKING: + from typing import Optional + +ROOT = pathlib.Path(__file__).resolve().parents[2] +SCRIPT = ROOT / 'util' / 'verdict_covers_checkout.py' +WORKFLOWS = ROOT / '.github' / 'workflows' + +#: ``{workflow file: the job the verdict gates}``. +ACTING_JOBS = { + 'cron-conda.yml': 'conda-update', + 'cron-vendor.yml': 'vendor-update', + 'deploy-pages.yml': 'deploy-pages', +} + + +def _load_script(): + """:file:`util/verdict_covers_checkout.py` as a module; it has no import-time effects.""" + spec = importlib.util.spec_from_file_location('verdict_covers_checkout', SCRIPT) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +verdict_covers_checkout = _load_script() + +#: The identities on the 2026-10-13 commits: a squash merge is authored by its PR's +#: author and committed by GitHub; pyup's weekly update the same way. +HUMAN = ('Jarry Shaw', 'jarryshaw@icloud.com') +GITHUB = ('GitHub', 'noreply@github.com') +PYUP = ('pyup.io bot', 'github-bot@pyup.io') +BOT = ('github-actions[bot]', verdict_covers_checkout.BOT_EMAIL) + + +class TestTheRuleOnRealRunShapes(unittest.TestCase): + """The script against a repository shaped like ``main`` on 2026-10-13. + + Each commit carries the real one's identities and paths, keyed by its real + abbreviated sha in :attr:`sha`, and the content is placeholder text. The root, + ee324e1dc, also seeds every file the later commits touch. + + """ + + def setUp(self) -> None: + if shutil.which('git') is None: + raise unittest.SkipTest('git not found on PATH') + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.root = pathlib.Path(tmp.name) + self.repo = self.root / 'repo' + + # Isolated from the caller's git configuration, which may sign commits or + # rewrite URLs of its own. No automatic maintenance either, whose detached + # repack can outlive the test and race its cleanup: maintenance.auto is + # what commit, merge and fetch consult, gc.auto what they fall back to + # (actions/checkout sets it to 0 in every checkout), and autoDetach keeps + # anything that still runs in the foreground. See + # test_the_harness_git_runs_no_maintenance. + home = self.root / 'home' + home.mkdir() + (home / '.gitconfig').write_text( + '[init]\n\tdefaultBranch = main\n[commit]\n\tgpgsign = false\n' + '[maintenance]\n\tauto = false\n\tautoDetach = false\n' + '[gc]\n\tauto = 0\n\tautoDetach = false\n', encoding='utf-8') + self.env = { + 'PATH': os.environ.get('PATH', ''), + 'HOME': str(home), + 'XDG_CONFIG_HOME': str(home / '.config'), + 'GIT_CONFIG_NOSYSTEM': '1', + 'LC_ALL': 'C', + } + self._git('init', str(self.repo), cwd=self.root) + + conda = ['conda/build', 'conda/requirements.txt'] + dpkt, scapy = 'pcapkit/toolkit/dpkt.py', 'pcapkit/toolkit/scapy.py' + self.sha = {} # type: dict[str, str] + self.sha['ee324e1dc'] = self._commit( + 'Bumped build to 2', + conda + ['docs/requirements.txt', 'pcapkit/protocols/protocol.py', dpkt, scapy], BOT, BOT) + self.sha['01151b13f'] = self._commit('fix(protocols): bound a frame (#1620)', + ['pcapkit/protocols/protocol.py']) + self.sha['9083163f5'] = self._commit('fix(toolkit): dpkt reads SYN (#1621)', [dpkt]) + self.sha['bbcabf1dd'] = self._commit('fix(toolkit): dpkt hands IPv4 (#1622)', [dpkt]) + self.sha['e5f3af373'] = self._commit('fix(toolkit): dpkt and scapy (#1624)', [dpkt, scapy]) + self.sha['2e941bbb7'] = self._commit('Scheduled weekly dependency update (#1629)', + ['docs/requirements.txt'], PYUP) + self.sha['f4f94bcad'] = self._commit('Bumped build to 3', conda, BOT, BOT) + + def _git(self, *args: 'str', cwd: 'Optional[pathlib.Path]' = None, + env: 'Optional[dict[str, str]]' = None) -> 'str': + result = subprocess.run(['git', *args], cwd=cwd or self.repo, env=env or self.env, + capture_output=True, text=True, timeout=scale_timeout(30)) + self.assertEqual(result.returncode, 0, f'git {" ".join(args)}: {result.stderr}') + return result.stdout.strip() + + def _commit(self, message: 'str', paths: 'list[str]', author: 'tuple[str, str]' = HUMAN, + committer: 'tuple[str, str]' = GITHUB) -> 'str': + """Commit a change to each of ``paths`` as ``author``/``committer``; the new sha.""" + for path in paths: + file = self.repo / path + file.parent.mkdir(parents=True, exist_ok=True) + with file.open('a', encoding='utf-8') as stream: + stream.write(f'{message}\n') + self._git('add', '--', *paths) + env = dict(self.env, GIT_AUTHOR_NAME=author[0], GIT_AUTHOR_EMAIL=author[1], + GIT_COMMITTER_NAME=committer[0], GIT_COMMITTER_EMAIL=committer[1]) + self._git('commit', '-q', '-m', message, env=env) + return self._git('rev-parse', 'HEAD') + + def _run(self, verdict: 'str', head: 'str', repo: 'Optional[pathlib.Path]' = None, + ) -> 'tuple[subprocess.CompletedProcess[str], Optional[str]]': + """Check ``head`` out and run the script under ``verdict``: the result and ``verified``.""" + repo = repo or self.repo + self._git('checkout', '-q', '--detach', head, cwd=repo) + output = self.root / 'output' + output.write_text('', encoding='utf-8') + result = subprocess.run([sys.executable, str(SCRIPT)], cwd=repo, + env=dict(self.env, VERDICT_SHA=verdict, GITHUB_OUTPUT=str(output)), + capture_output=True, text=True, timeout=scale_timeout(60)) + written = re.findall(r'^verified=(.*)$', output.read_text(encoding='utf-8'), re.M) + self.assertLessEqual(len(written), 1, written) + return result, written[0] if written else None + + def assertVerdict(self, verdict: 'str', head: 'str', expected: 'str', + repo: 'Optional[pathlib.Path]' = None) -> 'str': + """Assert the script answers ``expected`` and exits 0; its stdout.""" + result, verified = self._run(verdict, head, repo) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(verified, expected, result.stdout) + return result.stdout + + def test_run_38325136710_a_verdict_under_three_human_commits_is_refused(self) -> None: + """The issue: 9083163f5's verdict, 2e941bbb7 checked out.""" + stdout = self.assertVerdict(self.sha['9083163f5'], self.sha['2e941bbb7'], 'false') + self.assertIn('::notice title=Skipped', stdout) + self.assertIn(self.sha['bbcabf1dd'], stdout, 'the notice names the first unverified commit') + + def test_the_tips_own_completion_is_accepted(self) -> None: + """2e941bbb7's own run, had nothing landed on it yet.""" + self.assertVerdict(self.sha['2e941bbb7'], self.sha['2e941bbb7'], 'true') + + def test_run_38326184463_a_conda_bump_on_the_verified_tip_is_accepted(self) -> None: + """2e941bbb7's own run completing with f4f94bcad already on top.""" + self.assertVerdict(self.sha['2e941bbb7'], self.sha['f4f94bcad'], 'true') + + def test_several_bot_commits_to_conda_on_top_are_accepted(self) -> None: + """A bump and then Create Release's reset, both bot-managed paths only.""" + reset = self._commit('Reset conda build number', ['conda/build'], BOT, BOT) + self.assertVerdict(self.sha['2e941bbb7'], reset, 'true') + + def test_run_38326990571_the_bot_commits_own_completion_is_accepted(self) -> None: + self.assertVerdict(self.sha['f4f94bcad'], self.sha['f4f94bcad'], 'true') + + def test_a_human_commit_on_top_is_refused(self) -> None: + human = self._commit('fix(toolkit): another merge', ['pcapkit/toolkit/scapy.py']) + self.assertVerdict(self.sha['f4f94bcad'], human, 'false') + + def test_a_human_commit_under_a_bot_commit_is_refused(self) -> None: + """e5f3af373's verdict: pyup's 2e941bbb7 sits between it and the bump.""" + stdout = self.assertVerdict(self.sha['e5f3af373'], self.sha['f4f94bcad'], 'false') + self.assertIn(PYUP[1], stdout) + + def test_a_human_commit_between_bot_commits_is_refused(self) -> None: + self._commit('fix(toolkit): another merge', ['pcapkit/toolkit/scapy.py']) + reset = self._commit('Reset conda build number', ['conda/build'], BOT, BOT) + self.assertVerdict(self.sha['2e941bbb7'], reset, 'false') + + def test_a_vendor_bump_on_top_is_refused(self) -> None: + """e58618bdf's paths: its own run failed 12 tests its parent's had passed.""" + bump = self._commit('Bumped version to 1.5.0b5', [ + 'CITATION.cff', 'conda/build', 'pcapkit/__init__.py', + 'pcapkit/const/pcapng/record_type.py', 'pcapkit/const/pcapng/secrets_type.py', + ], BOT, BOT) + stdout = self.assertVerdict(self.sha['f4f94bcad'], bump, 'false') + self.assertIn('touches CITATION.cff, pcapkit/__init__.py, pcapkit/const/pcapng/record_type.py, ' + 'pcapkit/const/pcapng/secrets_type.py, which', stdout) + + def test_a_bot_authored_commit_someone_else_committed_is_refused(self) -> None: + """A cherry-pick of a bump, say: the bot did not push it.""" + picked = self._commit('Bumped build to 4', ['conda/build'], BOT, HUMAN) + self.assertVerdict(self.sha['f4f94bcad'], picked, 'false') + + def test_a_merge_on_top_is_refused(self) -> None: + base = self.sha['f4f94bcad'] + self._git('checkout', '-q', '-b', 'side', base) + side = self._commit('Bumped build to 4', ['conda/build'], BOT, BOT) + self._git('checkout', '-q', '--detach', base) + bump = self._commit('Bumped build to 4', ['conda/requirements.txt'], BOT, BOT) + env = dict(self.env, GIT_AUTHOR_NAME=BOT[0], GIT_AUTHOR_EMAIL=BOT[1], + GIT_COMMITTER_NAME=BOT[0], GIT_COMMITTER_EMAIL=BOT[1]) + self._git('merge', '-q', '--no-ff', '-m', 'Merge', side, env=env) + merge = self._git('rev-parse', 'HEAD') + self.assertNotEqual(merge, bump) + self.assertVerdict(base, merge, 'false') + + def test_a_verdict_that_is_not_an_ancestor_is_refused(self) -> None: + """A verdict for a commit no longer on ``main``: a sibling bump of + f4f94bcad's, as 644b9c197 was. Only the ancestry check refuses it, since + what lies between the two is a bot commit to bot-managed paths. + + """ + self._git('checkout', '-q', '--detach', self.sha['2e941bbb7']) + sibling = self._commit('Bumped build to 3, again', ['conda/build', 'conda/requirements.txt'], + BOT, BOT) + stdout = self.assertVerdict(sibling, self.sha['f4f94bcad'], 'false') + self.assertIn('not an ancestor', stdout) + + def test_a_verdict_beyond_a_shallow_checkout_is_refused_not_failed(self) -> None: + """What the ``fetch-depth`` bound means: too far back is not covered.""" + clone = self.root / 'clone' + self._git('clone', '-q', '--depth', '2', '--branch', 'main', self.repo.as_uri(), str(clone), + cwd=self.root) + self.assertEqual(self._git('rev-parse', 'HEAD', cwd=clone), self.sha['f4f94bcad']) + stdout = self.assertVerdict(self.sha['9083163f5'], self.sha['f4f94bcad'], 'false', repo=clone) + self.assertIn('not in the history', stdout) + self.assertVerdict(self.sha['2e941bbb7'], self.sha['f4f94bcad'], 'true', repo=clone) + + def test_a_merge_deeper_than_the_checkout_is_refused_not_failed(self) -> None: + """A merged side branch longer than the verdict job's ``fetch-depth: 20`` + puts the clone's boundary commit, parentless there, between the verdict and + ``HEAD``. Two merges on ``main`` since 2026-08-15 brought in that many: + e23817bdb 76 commits, 4a151c0ac 43. + + """ + base = self.sha['f4f94bcad'] + self._git('checkout', '-q', '-b', 'side', base) + for index in range(30): + self._commit(f'fix(toolkit): step {index}', ['pcapkit/toolkit/scapy.py']) + self._git('checkout', '-q', 'main') + env = dict(self.env, GIT_AUTHOR_NAME=HUMAN[0], GIT_AUTHOR_EMAIL=HUMAN[1], + GIT_COMMITTER_NAME=GITHUB[0], GIT_COMMITTER_EMAIL=GITHUB[1]) + self._git('merge', '-q', '--no-ff', '-m', 'Merge pull request #7 from side', 'side', env=env) + merge = self._git('rev-parse', 'HEAD') + + clone = self.root / 'clone' + self._git('clone', '-q', '--depth', '20', '--branch', 'main', self.repo.as_uri(), str(clone), + cwd=self.root) + self.assertEqual(self._git('rev-parse', 'HEAD', cwd=clone), merge) + boundary = (clone / '.git' / 'shallow').read_text(encoding='utf-8').split() + self.assertEqual(len(boundary), 1, 'the clone is cut on the side branch only') + self.assertEqual(self._git('show', '-s', '--format=%P', boundary[0], cwd=clone), '', + 'the boundary commit shows no parents, the case under test') + + stdout = self.assertVerdict(base, merge, 'false', repo=clone) + self.assertIn('::notice title=Skipped', stdout) + + def test_a_missing_verdict_fails_loudly_and_writes_nothing(self) -> None: + """No sha is a broken payload, not a stale verdict.""" + for verdict in ('', 'main', self.sha['f4f94bcad'][:9]): + with self.subTest(verdict=verdict): + result, verified = self._run(verdict, self.sha['f4f94bcad']) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn('::error', result.stdout) + self.assertIsNone(verified) + + def test_the_harness_git_runs_no_maintenance(self) -> None: + """No git process outlives the command that started it. + + A commit, merge or fetch runs ``git maintenance run --auto``, detached by + default, and since git 2.54 that repacks once ``objects/17`` holds two + loose objects. :meth:`test_a_merge_on_top_is_refused` gets there at its + second bump every run: setUp's ``protocol.py`` blob 17a8c7ab3 and the + bump's root tree 17d5a5af3, both content-addressed. On git 2.55.0, the + runner's, the repack can still be writing into ``.git`` when the temporary + directory is removed: "OSError: [Errno 39] Directory not empty: 'objects'" + in run 38349290407 (Python 3.10). Here a commit-graph write is due on + every commit and runs in the foreground, so any maintenance at all leaves + a file behind. + + """ + for key, value in (('maintenance.commit-graph.enabled', 'true'), + ('maintenance.commit-graph.auto', '-1'), + ('maintenance.autoDetach', 'false')): + self._git('config', key, value) + self._commit('fix(toolkit): another merge', ['pcapkit/toolkit/scapy.py']) + written = sorted(path.name for path in (self.repo / '.git' / 'objects' / 'info').glob('*')) + self.assertEqual(written, [], 'a commit ran maintenance in the test repository') + + +def _context(event_name: 'str' = 'workflow_run', conclusion: 'str' = 'success', + event: 'str' = 'push', head_branch: 'str' = 'main', head_sha: 'str' = 'f' * 40, + unit_tests: 'str' = 'skipped', verdict: 'str' = 'success', + verified: 'object' = 'true', cancelled: bool = False) -> 'dict[str, object]': + """One run's ``github``/``needs`` context; the defaults are a verdict that covers its checkout. + + ``verified=NULL`` is a ``verdict`` job that wrote no output, which is what a + skipped or failed one leaves. + + """ + outputs = {} if verified is NULL else {'verified': verified} + github = { + 'event_name': event_name, + 'ref': 'refs/pull/7/merge' if event_name == 'pull_request' else 'refs/heads/main', + 'event': {}, + } # type: dict[str, object] + if event_name == 'workflow_run': + github['event'] = {'workflow_run': {'conclusion': conclusion, 'event': event, + 'head_branch': head_branch, 'head_sha': head_sha}} + return { + 'github': github, + 'needs': {'unit-tests': {'result': unit_tests}, + 'verdict': {'result': verdict, 'outputs': outputs}}, + STATUS: {'cancelled': cancelled}, + } + + +#: ``[(row, context, whether verdict's if: holds, whether the acting job's does)]``, +#: for all three workflows; :data:`None` asserts nothing about that job. +ROWS = [ + ('run 38325136710: the verdict is for an older commit', + _context(verified='false'), True, False), + ("run 38326990571: the verdict covers the checkout", + _context(), True, True), + ('verdict failed before writing its answer', + _context(verdict='failure', verified=NULL), True, False), + ('the Saturday schedule, the gate green', + _context(event_name='schedule', unit_tests='success', verdict='skipped', verified=NULL), False, True), + ('the Saturday schedule, the gate failed', + _context(event_name='schedule', unit_tests='failure', verdict='skipped', verified=NULL), False, False), + ("a pull request's Unit Tests completed", + _context(event='pull_request', verdict='skipped', verified=NULL), False, False), + ('Unit Tests failed on main', + _context(conclusion='failure', verdict='skipped', verified=NULL), False, False), + ('Unit Tests completed on a branch that is not main', + _context(head_branch='dependabot/pip/scapy-2.6.2', verdict='skipped', verified=NULL), False, False), + ('the run was cancelled', + _context(cancelled=True), None, False), +] + +#: GitHub Pages also builds, without deploying, for a pull request against ``main``. +PAGES_ROWS = [ + ('a pull request against main', + _context(event_name='pull_request', verdict='skipped', verified=NULL), False, True), +] + +_GROUP = re.compile(r'^concurrency:\n group: (?P[^$\n]*)\$\{\{(?P[^}]*)\}\}\n', re.M) + + +class TestEachWorkflowGatesOnTheVerdict(unittest.TestCase): + """The ``verdict`` job, and the job it gates, in each of the three workflows.""" + + @classmethod + def setUpClass(cls) -> None: + cls.texts = {name: (WORKFLOWS / name).read_text(encoding='utf-8') for name in ACTING_JOBS} + cls.jobs = {name: job_blocks(text) for name, text in cls.texts.items()} + + def _jobs(self, workflow: 'str') -> 'dict[str, str]': + jobs = self.jobs[workflow] + if 'verdict' not in jobs: + self.fail(f'{workflow} has no `verdict` job, so it acts on whatever commit it checks out') + return jobs + + def test_each_acting_job_needs_the_verdict(self) -> None: + for workflow, job in ACTING_JOBS.items(): + with self.subTest(workflow=workflow): + self.assertIn('verdict', declared_needs(self._jobs(workflow)[job])) + + def test_every_scenario_decides_correctly(self) -> None: + """The table above, for both jobs, in all three workflows.""" + violations = [] # type: list[str] + for workflow, job in ACTING_JOBS.items(): + jobs = self.jobs[workflow] + rows = ROWS + (PAGES_ROWS if workflow == 'deploy-pages.yml' else []) + for name, context, verdict_runs, acting_runs in rows: + for target, expected in (('verdict', verdict_runs), (job, acting_runs)): + if expected is None: + continue + if target not in jobs: + violations.append(f'{workflow}: {name}: no `{target}` job') + continue + if evaluate_condition(declared_if(jobs[target]), context) != expected: + violations.append(f'{workflow}: {name}: `{target}` should ' + f'{"run" if expected else "skip"}, it does not') + if violations: + self.fail(f'{len(violations)} scenario(s) decided wrongly:\n' + '\n'.join(violations)) + + def test_a_skipped_verdict_cannot_cascade_into_the_acting_job(self) -> None: + """``verdict`` skips on the schedule, so without a status-check function + the implicit ``success()`` would skip the job behind it there too. + + """ + for workflow, job in ACTING_JOBS.items(): + with self.subTest(workflow=workflow): + self.assertTrue(declared_if(self._jobs(workflow)[job]).startswith('!cancelled() && ')) + + def test_the_verdict_job_is_one_job_in_three_places(self) -> None: + """Identical text, so a fix to one cannot leave the other two behind.""" + blocks = {workflow: self._jobs(workflow)['verdict'] for workflow in ACTING_JOBS} + self.assertEqual(len(set(blocks.values())), 1, sorted(blocks)) + + def test_the_verdict_job_reads_the_payload_as_data(self) -> None: + """The sha arrives through ``env:``, and the script it runs exists.""" + block = self._jobs('cron-conda.yml')['verdict'] + self.assertIn('VERDICT_SHA: ${{ github.event.workflow_run.head_sha }}', block) + command = re.search(r'(?m)^ +run: python3 (\S+)$', block) + self.assertIsNotNone(command, block) + self.assertEqual(ROOT / command.group(1), SCRIPT) # type: ignore[union-attr] + self.assertEqual(block.count('${{'), 2, 'the only expressions are the output and the env value') + + def test_the_verdict_job_checks_out_what_the_acting_job_builds(self) -> None: + """Neither checkout names a ``ref:``, so both are the default ``github.sha``.""" + for workflow, job in ACTING_JOBS.items(): + jobs = self._jobs(workflow) + for target in ('verdict', job): + with self.subTest(workflow=workflow, job=target): + self.assertIn('uses: actions/checkout@', jobs[target]) + self.assertNotRegex(jobs[target], r'(?m)^ +ref:') + + def test_the_verdict_checkout_reaches_past_a_bot_commit(self) -> None: + """At ``fetch-depth: 1`` only ``HEAD`` is fetched, so with a bot commit on + top the verified commit is never found and the bot-commit allowance is dead: + a fail-safe skip, but every tip with a bump on it waits for its own run. + ``0`` fetches all history. + + """ + for workflow in ACTING_JOBS: + with self.subTest(workflow=workflow): + depths = re.findall(r'(?m)^ +fetch-depth: *(\S+)$', self._jobs(workflow)['verdict']) + self.assertEqual(len(depths), 1, 'one checkout, with an explicit fetch-depth') + self.assertRegex(depths[0], r'^\d+$') + depth = int(depths[0]) + self.assertTrue(depth == 0 or depth >= 2, + f'fetch-depth {depth} cannot hold the verdict under a bot commit') + + def test_a_stale_run_cannot_share_a_concurrency_group_with_a_fresh_one(self) -> None: + """GitHub cancels a pending run when a newer one joins its group, so a run + that will skip must not join the group of one that will not. Every other + event keeps the group it had. + + """ + for workflow in ACTING_JOBS: + with self.subTest(workflow=workflow): + match = _GROUP.search(self.texts[workflow]) + self.assertIsNotNone(match, 'no workflow-level concurrency group') + prefix, body = match.group('prefix'), match.group('body') # type: ignore[union-attr] + + def group(context: 'dict[str, object]') -> 'str': + return prefix + str(_evaluate(parse_condition('${{' + body + '}}'), context)) + + stale, fresh = _context(head_sha='9' * 40), _context(head_sha='2' * 40) + self.assertNotEqual(group(stale), group(fresh), + 'a run for an older verdict joins the group of the run whose ' + 'verdict matches, so it can cancel that run while it is pending') + self.assertEqual(group(fresh), group(_context(head_sha='2' * 40, verified='false'))) + self.assertEqual(group(_context(event_name='schedule')), prefix + 'refs/heads/main') + if workflow == 'deploy-pages.yml': + self.assertEqual(group(_context(event_name='pull_request')), prefix + 'refs/pull/7/merge') + + def test_the_scanner_agrees_with_yaml(self) -> None: + """What :func:`job_blocks`/:func:`declared_if` read is what a YAML parse reads.""" + try: + import yaml # pylint: disable=import-outside-toplevel + except ImportError: + self.skipTest('PyYAML is only installed with the test extra') + for workflow, job in ACTING_JOBS.items(): + parsed = yaml.safe_load(self.texts[workflow])['jobs'] + for target in ('verdict', job): + with self.subTest(workflow=workflow, job=target): + self.assertIn(target, sorted(parsed)) + self.assertEqual(' '.join(parsed[target]['if'].split()), + ' '.join(declared_if(self._jobs(workflow)[target]).split())) + + +if __name__ == '__main__': + unittest.main() diff --git a/util/verdict_covers_checkout.py b/util/verdict_covers_checkout.py new file mode 100644 index 0000000000..f6c7292244 --- /dev/null +++ b/util/verdict_covers_checkout.py @@ -0,0 +1,135 @@ +# -*- coding: utf-8 -*- +"""Whether the ``Unit Tests`` verdict a ``workflow_run`` carries covers this checkout. + +Conda Update, Vendor Update and GitHub Pages chain off ``Unit Tests`` completing +on ``main``. Their checkouts take ``github.sha``, which for a ``workflow_run`` is +the "Last commit on default branch" when the event is raised (GitHub, "Events +that trigger workflows"), not the commit the completed run tested -- +``github.event.workflow_run.head_sha``. In a merge wave the two differ (#1643): +Conda Update 38325136710 ran on 2e941bbb7 under the verdict for 9083163f5, three +merged pull requests older, and pushed f4f94bcad on top at 00:03:38Z; 2e941bbb7's +own run 38323130015 finished at 00:13:35Z. + +The verdict covers ``HEAD`` when + +* it *is* ``HEAD``; or +* every commit after it up to ``HEAD`` is a single-parent commit authored and + committed as :data:`BOT_EMAIL` that touches only :data:`BOT_MANAGED_PATHS`. + +Anything else is not covered, and the job gated on this one skips. Nothing is +lost by that: ``HEAD``'s own ``Unit Tests`` run raises the workflow again when it +completes, bot commits included -- since #401 (bb900906b, 2026-09-15) they are +pushed with a deploy key, not with ``GITHUB_TOKEN``, whose events "will not create +a new workflow run" (GitHub, "Triggering a workflow from a workflow"). All 17 bot +commits on ``main`` since have their own ``push`` run of ``Unit Tests``, +f4f94bcad's being 38325349953; the three before #401, 5d5d8af4d, 48bc11384 and +e4d0823db, have none. The second case only saves waiting for that run where it +cannot change the answer. + +Run with ``VERDICT_SHA`` set, from the checkout. Writes ``verified=true`` or +``verified=false`` to ``$GITHUB_OUTPUT``, and exits non-zero only when it cannot +answer at all. + +""" + +from __future__ import annotations + +import os +import re +import subprocess # nosec B404 +import sys +from typing import TYPE_CHECKING, NamedTuple + +if TYPE_CHECKING: + from typing import Optional + +#: The identity Conda Update, Vendor Update and Create Release commit as. All +#: 275 commits on ``main`` authored by it were committed by it too, none of them +#: a merge (measured on b375656e1). +BOT_EMAIL = 'github-actions[bot]@users.noreply.github.com' + +#: Files a bot commit may touch and still be built under its parent's verdict. +#: These are what Conda Update ("Bumped build to N", 137 commits on ``main``) and +#: Create Release's ``tag`` job ("Reset conda build number", 9) commit, and no +#: test reads either. Vendor Update's "Bumped version to N" commits are left out +#: on purpose: all 123 rewrite ``pcapkit/``, and e58618bdf's Unit Tests run +#: 36355214959 reported "12 failed" on each Python leg, where its parent +#: 70fa92010's run 36354135989 passed every job. +BOT_MANAGED_PATHS = frozenset({'conda/build', 'conda/requirements.txt'}) + +#: A full commit sha, which is all ``head_sha`` ever holds for a push run. +_SHA = re.compile(r'^[0-9a-f]{40}$') + + +class Decision(NamedTuple): + """Whether the verdict covers ``HEAD``, and why, in one line for the log.""" + + verified: bool + reason: str + + +def _git(*args: 'str', cwd: 'Optional[str]' = None, + check: bool = True) -> 'subprocess.CompletedProcess[str]': + return subprocess.run(['git', *args], cwd=cwd, check=check, # nosec B603 B607 + capture_output=True, text=True) + + +def decide(verdict: 'str', head: 'str', cwd: 'Optional[str]' = None) -> 'Decision': + """Whether the commit ``verdict`` (a full sha) covers ``head``, in ``cwd``'s repository.""" + if verdict == head: + return Decision(True, f'Unit Tests verified {head}, the commit this run checked out.') + + if _git('cat-file', '-e', f'{verdict}^{{commit}}', cwd=cwd, check=False).returncode != 0: + return Decision(False, f'Unit Tests verified {verdict}, which is not in the history ' + f'fetched for {head}, so it is too far back to cover it.') + if _git('merge-base', '--is-ancestor', verdict, head, cwd=cwd, check=False).returncode != 0: + return Decision(False, f'Unit Tests verified {verdict}, which is not an ancestor of {head}.') + + commits = _git('rev-list', '--reverse', f'{verdict}..{head}', cwd=cwd).stdout.split() + for commit in commits: + # Only the newline goes: ``%P`` is empty for a parentless commit, which a + # shallow checkout's boundary is, and ``strip()`` would take its tab too. + parents, author, committer = _git('show', '-s', '--format=%P%x09%ae%x09%ce', + commit, cwd=cwd).stdout.strip('\n').split('\t') + if len(parents.split()) != 1: + return Decision(False, f'{commit} after the verified {verdict} has ' + f'{len(parents.split())} parents.') + if author != BOT_EMAIL or committer != BOT_EMAIL: + return Decision(False, f'{commit} after the verified {verdict} was authored by ' + f'{author} and committed by {committer}, not by the bot.') + paths = _git('diff-tree', '--no-commit-id', '--name-only', '-r', '-z', '--no-renames', + commit, cwd=cwd).stdout.split('\0') + outside = sorted(path for path in paths if path and path not in BOT_MANAGED_PATHS) + if outside: + return Decision(False, f'bot commit {commit} after the verified {verdict} touches ' + f'{", ".join(outside)}, which is not bot-managed.') + + managed = ', '.join(sorted(BOT_MANAGED_PATHS)) + return Decision(True, f'Unit Tests verified {verdict}; the {len(commits)} commit(s) after it ' + f'up to {head} are bot commits to {managed} only.') + + +def main() -> int: + """Decide for the checkout in the working directory and report it.""" + verdict = os.environ.get('VERDICT_SHA', '') + if _SHA.match(verdict) is None: + print(f'::error title=No verdict to compare::VERDICT_SHA is {verdict!r}, not a commit sha') + return 1 + head = _git('rev-parse', 'HEAD').stdout.strip() + + decision = decide(verdict, head) + if decision.verified: + print(decision.reason) + else: + print(f'::notice title=Skipped as the verdict is for another commit::{decision.reason} ' + f'{head} gets its own Unit Tests run, which triggers this workflow again.') + + output = os.environ.get('GITHUB_OUTPUT') + if output: + with open(output, 'a', encoding='utf-8') as file: + print(f'verified={str(decision.verified).lower()}', file=file) + return 0 + + +if __name__ == '__main__': + sys.exit(main())