diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 1dffdfb1e8..2238be4eb3 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -32,9 +32,10 @@ concurrency: cancel-in-progress: false # Every job below with an effect outside this run declares a deployment -# ``environment:``, which is what holds it at "Waiting" until a required reviewer -# approves. Four jobs qualify, and they get four environments rather than one -# shared ``release``: +# ``environment:``, but as of #887 only one of them -- ``github-release`` -- +# carries a required reviewer. Four jobs still get four environments rather than +# one shared ``release``, for credential and blast-radius separation, not for +# approval count: # # github -> github-release the public GitHub Release, and the ``v*`` tag that # ``action-gh-release``'s ``tag_name`` creates with it @@ -42,29 +43,44 @@ concurrency: # pypi -> pypi an upload to PyPI, and to TestPyPI # conda -> anaconda an upload to anaconda.org/jarryshaw # -# One environment per distinct credential and blast radius, because approval is -# granted to an *environment* and not to a job: a single ``release`` approved once -# would release PyPI and Anaconda together, and those are different credentials -# (OIDC trusted publishing versus ``ANACONDA_TOKEN``) reaching different indexes. -# PyPI is the one that has to be answerable alone -- a version number it has -# accepted cannot be reused, and ``skip-existing: true`` below means the re-upload -# then *succeeds* having published nothing, so an unintended publish consumes the -# number the intended release wanted. ``github`` and ``tag`` are split on the same -# principle: ``github`` spends the run's own ``GITHUB_TOKEN``, whereas ``tag`` -# spends a long-lived PAT and a deploy key to write to the default branch. +# Approving the GitHub Release is now the release's *only* approval. +# ``tag`` and ``pypi`` both carry ``needs: [ github, ... ]``, and ``conda`` needs +# ``tag`` and ``github`` too, so nothing downstream of ``github`` can start before +# that approval lands, and rejecting it leaves nothing tagged and nothing +# published. See :file:`docs/source/releasing.rst` for the full pipeline and the +# reasoning behind one approval being enough. # -# The cost is two approval waves, not four: ``github`` and ``tag`` need only -# ``version_check`` so they arrive together, and ``pypi`` and ``conda`` then arrive -# together too, each wave answerable in one visit to "Review pending deployments". +# The other three environments keep their names -- ``pypi``'s matters because +# PyPI trusted publishing matches its OIDC claim against it -- and keep their +# secret scoping, they just no longer hold a required reviewer of their own. Do +# not add one back to any of them without re-examining the ``needs:`` chain: a +# reviewer on ``conda-tag`` with its old ``needs: [ version_check ]`` is exactly +# the bug #887 fixed, because it let the commit-to-``main`` and ``conda-*`` tag +# run *before* the GitHub Release was ever approved. +# ``tests/project/test_release_gates.py``'s ``TestGatedJobsDependOnGithub`` +# asserts that every gated job's ``needs:`` closure includes ``github``, which +# is the checkable half of this paragraph -- reverting the line above to +# ``needs: [ version_check ]`` fails only that one test. +# +# One environment per distinct credential and blast radius regardless of who +# gates it: a single ``release`` approved once would release PyPI and Anaconda +# together, and those are different credentials (OIDC trusted publishing versus +# ``ANACONDA_TOKEN``) reaching different indexes. PyPI in particular cannot afford +# an unintended publish -- a version number it has accepted cannot be reused, and +# ``skip-existing: true`` below means a re-upload then *succeeds* having published +# nothing, so the number the intended release wanted is gone regardless. ``github`` +# and ``tag`` were split on the same principle: ``github`` spends the run's own +# ``GITHUB_TOKEN``, whereas ``tag`` spends a long-lived PAT and a deploy key to +# write to the default branch. # # Two things about this that are not self-enforcing, and #641 is the first of them: # # * An ``environment:`` naming an environment that does not exist is created # implicitly, with no protection rules, and the job proceeds unapproved. So -# these four lines are inert until each environment exists under -# Settings -> Environments with a required reviewer on it. This repository -# already ships the worked example of that failure: ``github-pages`` has -# existed since 2021 with ``"protection_rules": []``. +# the ``github-release`` line above is inert until that environment exists +# under Settings -> Environments with a required reviewer on it. This +# repository already ships the worked example of that failure: +# ``github-pages`` has existed since 2021 with ``"protection_rules": []``. # * Leave each one's "Deployment branches and tags" as "No restriction". This # workflow's ``push`` trigger is ``tags: v*``, and a branch policy that does # not list that tag pattern does not pause the job for approval -- it fails it @@ -132,7 +148,8 @@ jobs: name: GitHub Release runs-on: ubuntu-latest # Gated: publishes the Release, and creates the ``v*`` tag that every job - # below then checks out. See the note above ``jobs:``. + # below then checks out. Since #887 this is the release's only approval -- + # see the note above ``jobs:``. environment: github-release permissions: contents: write @@ -222,13 +239,16 @@ jobs: # two check rows reading `Conda Tag` with nothing to tell them apart. name: Conda Tag (release) runs-on: ubuntu-latest - # Gated: pushes a commit to ``main`` and the ``conda-*`` tag. ``permissions: {}`` + # Pushes a commit to ``main`` and the ``conda-*`` tag. ``permissions: {}`` # does not constrain it -- the write goes through the deploy key and PAT below, - # not the run's token -- which is why it needs an approval of its own. See the - # note above ``jobs:``. + # not the run's token. It carried its own required reviewer until #887; now it + # depends on ``github`` rather than on ``version_check`` alone, precisely so + # the single ``github-release`` approval still gates it -- without that + # ``needs:``, this job would start the moment ``version_check`` finished, + # ahead of any approval. See the note above ``jobs:``. environment: conda-tag permissions: {} - needs: [ version_check ] + needs: [ github, version_check ] if: ${{ startsWith(github.ref_name, 'v') || needs.version_check.outputs.PCAPKIT_TAG_EXISTS == 'false' }} steps: - name: Load maintenance deploy key @@ -283,11 +303,14 @@ jobs: pypi: name: PyPI distribution for Python ${{ matrix.python-version }} runs-on: ubuntu-latest - # Gated, and named ``pypi`` rather than the upstream snippet's ``release``: see - # the note above ``jobs:`` for why each target gets its own. #641 was that this - # line stayed commented while the rest of the same snippet did not -- the + # Named ``pypi`` rather than the upstream snippet's ``release``: see the note + # above ``jobs:`` for why each target gets its own. #641 was that this line + # stayed commented while the rest of the same snippet did not -- the # ``id-token: write`` that *enables* trusted publishing was re-added below, so - # the block read as disabled as a unit when only its gate still was. + # the block read as disabled as a unit when only its gate still was. #887 + # removed this environment's own required reviewer; it is gated only + # transitively now, through ``needs: [ github, version_check ]`` below, and the + # name still matters because PyPI's trusted-publishing claim match is keyed on it. environment: pypi permissions: contents: write @@ -372,10 +395,11 @@ jobs: conda: name: Conda deployment (release) on ${{ matrix.os }} with Python ${{ matrix.python-version }} runs-on: ${{ matrix.os }} - # Gated: uploads to anaconda.org/jarryshaw with ``ANACONDA_TOKEN``. This job - # never had an ``environment:`` at all, not even a commented one, so it was the - # half of #641 that was not written down as a thing to turn on. See the note - # above ``jobs:``. + # Uploads to anaconda.org/jarryshaw with ``ANACONDA_TOKEN``. This job never had + # an ``environment:`` at all, not even a commented one, so it was the half of + # #641 that was not written down as a thing to turn on. #887 later removed + # this environment's own required reviewer; gating is transitive now, through + # ``needs: [ tag, github, version_check ]`` below. See the note above ``jobs:``. environment: anaconda permissions: contents: write diff --git a/docs/source/index.rst b/docs/source/index.rst index 4c6eaa5f08..6c13ad2528 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -31,6 +31,7 @@ construction and analysis library. demo testing conventions + releasing pep changelog diff --git a/docs/source/releasing.rst b/docs/source/releasing.rst new file mode 100644 index 0000000000..c9e58473a0 --- /dev/null +++ b/docs/source/releasing.rst @@ -0,0 +1,236 @@ +Release Process +================ + +.. important:: + + This page records the **operational contract** for cutting a release -- + what a maintainer is expected to touch by hand, what the workflow is + expected to do unattended, and the precautions that follow from the two + being different. Settled on + `#887 `__, which also + collapsed what used to be four separate approvals into one. + + The short version: **the only manual edit is bumping** + ``pcapkit.__version__``. Everything else -- the ``v*`` tag, the GitHub + Release, the PyPI and Anaconda uploads, the ``conda-*`` tag and its commit + to ``main`` -- is produced by + ``.github/workflows/create-release.yml`` and is never hand-made. + +The one manual step +-------------------- + +Bumping ``pcapkit.__version__`` is done by :file:`util/bump_version.py`, whose +own docstring is the authority on what it does; summarised here rather than +duplicated. Given the current version, it works out the next one under +:pep:`440` -- a ``devN`` counter, a pre-release counter, a post-release +counter, or a fresh ``.post1`` for a final release -- and then: + +* rewrites the ``__version__`` assignment in ``pcapkit/__init__.py``; +* moves :file:`CITATION.cff`'s ``version`` and ``date-released`` fields to + match, line-oriented so the file's own quoting and comments survive; +* resets :file:`conda/build` to ``0``. + +Its one scheduled caller is the ``Bump Version`` step of +``.github/workflows/cron-vendor.yml``, and only when a weekly registry crawl +actually changed something (``steps.verify-changed-files.outputs.files_changed +== 'true'``) -- a crawl that finds nothing does not bump. That commit is what +starts the automated path described below. + +The same script is also the manual entry point: run +``python util/bump_version.py`` and commit the result to ``main`` for a +release that is not a vendor-registry refresh -- a feature or a fix that has +landed and is ready to go out. + +Editing ``__version__`` by hand instead, without also moving +:file:`CITATION.cff`'s ``version`` field to match, is **not** a safe +shortcut -- it does not drift through silently, it blocks the release. +``tests/project/test_bump_version.py``'s +``RepositoryCitationTests.test_the_citation_file_names_the_packaged_version`` +(``:493-513``) asserts the two agree, and that assertion is not optional on +the release path: ``create-release.yml``'s ``unit-tests`` job +(``:96-103``) calls ``unit-tests.yml`` with ``gate-only: true``, which runs +the **full** suite rather than the tiered subset an ordinary push runs +(its ``gate`` job, ``unit-tests.yml:734-803``). ``version_check`` itself +depends on that job (``needs: [ unit-tests ]``, ``:108``), so a citation left +behind fails the gate before ``version_check`` ever runs, and well before any +approval is requested. Either run the script, or move both fields by hand in +the same commit. + +This is the **only** edit a person makes to get a release started -- the tag, +the Release, and every upload are the workflow's job from here. Two other +checks run unconditionally on the same path and are worth knowing about +rather than being surprised by, though neither is a step in *this* process: +the ``changelog`` job (``unit-tests.yml:707-728``) fails outright if +``CHANGELOG.md`` has drifted from its source entry under +:file:`docs/source/changelog/`, and the release-body step warns, without +failing, if that entry's heading still reads "unreleased" +(``create-release.yml:210-211``). Both are enforcing work the changelog +convention already asks for, not extra work this process adds. + +Two ways to start a release +---------------------------- + +``create-release.yml`` has two triggers +(``.github/workflows/create-release.yml:2-9``), and both are first-class -- +neither is a workaround for the other: + +``push: tags: ['v*']`` + Pushing a tag matching ``v*`` yourself. This chooses **which commit** to + release, not which version: ``version_check``'s checkout + (``:112-115``) carries no explicit ``ref:``, so it checks out whatever + commit the pushed tag points at, and reads ``pcapkit.__version__`` out of + *that* tree (``:132``). Every downstream tag and release name is built + from that output -- ``tag_name: "v${{ needs.version_check.outputs.PCAPKIT_VERSION }}"`` + appears at ``:233``, ``:392`` and ``:549``, always from + ``PCAPKIT_VERSION``, never from ``github.ref_name`` -- so the version + actually released is whatever ``__version__`` says on the tagged commit, + not the name of the tag that was pushed. The two agree only if the tag + was named to match. Push ``v1.6.0`` at a commit whose ``__version__`` is + still ``1.5.0b5`` and ``softprops/action-gh-release`` creates a *new* + ``v1.5.0b5`` tag and names the Release after it; the ``v1.6.0`` tag that + was actually pushed is left dangling, attached to nothing. + :doc:`pep` calls this workflow "version-driven rather than tag-driven" + for the same reason -- name the tag to match the version you intend, and + the two conveniently coincide, but nothing enforces that they do. + +``workflow_run`` after **Vendor Update** completes + The path a version bump on ``main`` takes without anyone pushing a tag: + a push to ``main`` completes **Unit Tests**, whose completion triggers + **Vendor Update** (``cron-vendor.yml``), whose completion -- bump step run + or not -- triggers this workflow. A ``workflow_run``-triggered run always + evaluates the tip of the default branch (documented GitHub Actions + behaviour, not measured here), so ``github.ref_name`` reads ``main``, + never a tag. + +Both paths converge on the same job graph from ``unit-tests`` onward; only +the trigger and ``github.ref_name`` differ, which is what the guard below +reads. + +Every job past ``version_check`` shares: + +.. code-block:: yaml + + if: ${{ startsWith(github.ref_name, 'v') || needs.version_check.outputs.PCAPKIT_TAG_EXISTS == 'false' }} + +and the two triggers hit it differently: + +* On the tag-push path ``github.ref_name`` already starts with ``v``, so the + guard is **unconditionally bypassed** -- its second half, the actual check + against ``PCAPKIT_TAG_EXISTS``, can never carry the run, because + ``startsWith`` alone already satisfied the ``||``. Nothing here checks + whether the version read out of the tagged commit was already published, + and pushing a tag is also a ref **update**, not only a creation, so a tag + force-moved onto a new commit takes this same path with the same bypass. + The protection on this path is the operator's deliberate act of choosing + what to tag, not an absence of risk -- see `Precautions`_ below for what + that risk actually is. +* On the ``workflow_run`` path ``github.ref_name`` is ``main``, so the guard + falls through to ``PCAPKIT_TAG_EXISTS``, and *that* is what stops the same + version being published twice, including the sibling race described in the + comment above ``concurrency:`` where two Vendor Update completions land + close together. + +The pipeline, and why one approval is enough +---------------------------------------------- + +.. mermaid:: + + flowchart TD + T1["push: tags v*
ref_name starts with v"] --> UT + T2["workflow_run: Vendor Update completed
ref_name = main"] --> UT + UT["unit-tests -- Release test gate"] --> VC + VC["version_check -- reads pcapkit.__version__
checks whether v<version> already exists
no gate, read-only"] + VC --> GH + GH["github -- GitHub Release
creates/attaches the v* tag
THE ONLY APPROVAL"] + GH --> TG["tag -- Conda Tag
commit to main + conda-<version>+0 tag"] + GH --> PY["pypi -- build + publish
OIDC trusted publishing"] + TG --> CD["conda -- build + upload
matrix: 2 OS"] + GH --> CD + classDef gate stroke-dasharray:6 3,stroke-width:2px + class GH gate + +A transitive reduction: ``version_check`` is also a direct ``needs:`` of +``tag``, ``pypi`` and ``conda`` in the file itself, alongside the edges drawn +above -- omitted here because ``github`` already implies it (``github`` +itself depends on ``version_check``), and drawing it would add three more +lines without changing which job can start before which. + +Only ``github-release`` carries a required reviewer today; ``conda-tag``, +``pypi`` and ``anaconda`` kept their environments but had their reviewers +removed on `#887 `__. +What still makes one approval mean *the whole release* is the ``needs:`` +graph: ``tag`` and ``pypi`` both depend on ``github`` +(``.github/workflows/create-release.yml:251,319``), and ``conda`` depends on +``tag`` and ``github`` (``:406``) -- so nothing downstream of ``github`` can +start before it is approved, and rejecting it leaves nothing tagged and +nothing published. Before this change ``tag`` depended on ``version_check`` +alone, so removing its reviewer without moving this dependency would have let +it push a commit to ``main`` and cut a ``conda-*`` tag *ahead of* the +approval it was supposed to wait for. + +Precautions +----------- + +.. warning:: + + **A half-finished release leaves a ``v*`` tag that makes every retry skip + silently.** ``github`` creates the tag; if ``pypi`` or ``conda`` then fails + (or is rejected), the tag now exists with the upload incomplete. The next + ``workflow_run``-triggered attempt reads ``PCAPKIT_TAG_EXISTS=true`` with + ``ref_name=main``, so the shared guard evaluates false and **every release + job is skipped** -- the run finishes green, because skipped is not failed, + and nothing in the UI says a release did not happen. This is + `#888 `__, open at the + time of writing. Recognise it by a green **Create Release** run that + published nothing, rather than trusting the checkmark. + +**Do not hand-make a tag to route around a stuck release.** This is a +narrower rule than "never tag by hand" -- pushing a *fresh* ``v`` +tag to start a release, as in `Two ways to start a release`_ above, is fine +and unchanged by any of this. What is not sanctioned is deleting and +re-pushing a tag that a stuck run already created, to force a retry. A +``git push`` of a tag ref that already points at the same commit produces no +new event (documented GitHub Actions behaviour, not measured here), so the +only way to make that push fire again is to delete the tag first -- which +both touches a ref the release automation owns, and lands the retry back on +the tag-push path, where the guard is unconditionally bypassed (see +`Two ways to start a release`_ above) regardless of what has already gone +out. Neither is worth the risk of a double upload to an index that cannot +take one back. + +**``environment: pypi`` stays even though its reviewer is gone.** ``pypi`` +publishes through PyPI's OIDC trusted publishing (``environment: pypi`` at +``:314``, ``id-token: write`` at ``:318``), and PyPI's trusted-publisher +configuration can be scoped to a GitHub Actions environment name. If this +project's publisher on PyPI is scoped that way, removing the ``pypi`` name +-- not just its reviewer -- would break the upload with a claim mismatch; +this is standard OIDC trusted-publishing behaviour, not something the +publisher's actual PyPI-side configuration was read to confirm here. Keep +the name regardless, since there is no upside to removing it. + +**``conda-tag`` writes to ``main``, so a release is not read-only on the +branch.** The ``tag`` job resets :file:`conda/build` to ``0``, commits that, +and pushes straight to ``main`` +(``.github/workflows/create-release.yml:266-286``) before cutting the +``conda-+0`` tag. Approving ``github-release`` therefore also +approves a commit landing on the default branch, not only the artefacts that +sound like they are the point. + +Recovery +-------- + +The sanctioned recovery from a failed or partial release run is re-running +the workflow -- nothing more elaborate, and specifically not re-tagging by +hand, for the reasons above. Be aware it can be a **silent no-op** on the +``workflow_run`` path, exactly as +`#888 `__ describes: a +green re-run is not evidence that anything was actually published, so check +the PyPI and Anaconda listings themselves rather than the checkmark. + +Beyond that: there is currently **no sanctioned manual recovery** for a +release that is already stranded -- a ``v*`` tag created with the publish +incomplete. That gap is +`#888 `__, and it stays +open rather than being papered over here with a procedure that does not +actually work; the previous suggestion of pushing the tag by hand does not +apply once the tag already exists -- see `Precautions`_ above. diff --git a/tests/project/test_release_gates.py b/tests/project/test_release_gates.py index f67906e823..519b8fd5e2 100644 --- a/tests/project/test_release_gates.py +++ b/tests/project/test_release_gates.py @@ -42,8 +42,10 @@ tier runs on a fresh clone with no network and no token. It is written down in the workflow's own comment and in #641 instead. * **Whether GitHub requests one approval per environment or one per matrix leg.** - ``pypi`` is a 7-leg matrix and ``conda`` a 10-leg one. That is a property of the - Actions runner, observable only from a real run. + ``pypi`` is a 7-leg matrix and ``conda`` a 10-leg one. That is a property of + the Actions runner, observable only from a real run -- and now moot for both, + since #887 removed their required reviewers. The question would resurface + only if a matrix job were gated by its own environment again. :mod:`yaml` is not in the ``test`` extra, so the scanner in this module is hand-rolled and dependency-free, and :class:`TestYAMLAgreesWithTheScanner` checks @@ -99,6 +101,12 @@ _ENVIRONMENT = re.compile(r'^ {4}environment:[ \t]*(.*)$') #: The ``name:`` of an ``environment:`` written in its mapping form. _ENVIRONMENT_NAME = re.compile(r'^ {6}name:[ \t]*(\S+)') +#: A job-level ``needs:`` key written as a single-line inline list, i.e. +#: ``needs: [ a, b ]``. The only form this workflow uses -- confirmed by +#: :class:`TestYAMLAgreesWithTheScanner` -- so, like :data:`PUBLISHING_MARKERS`, +#: extending it to the multi-line ``needs:\n - a`` form is a maintenance +#: obligation for whoever adds one, not something this scan infers. +_NEEDS = re.compile(r'^ {4}needs:[ \t]*\[([^\]]*)\][ \t]*(?:#.*)?$') def strip_comments(text: 'str') -> 'str': @@ -175,6 +183,42 @@ def declared_environment(block: 'str') -> 'Optional[str]': return None +def declared_needs(block: 'str') -> 'list[str]': + """The job names in this job's ``needs: [ ... ]`` list, empty if it has none. + + ``unit-tests`` has no ``needs:`` at all and correctly returns ``[]`` rather + than raising, since it is the root of the graph this module walks. + + """ + for line in block.splitlines(): + match = _NEEDS.match(line) + if match is not None: + return [name.strip() for name in match.group(1).split(',') if name.strip()] + return [] + + +def needs_closure(jobs: 'dict[str, str]', name: str) -> 'set[str]': + """Every job ``name`` depends on, directly or transitively. + + A plain graph walk over :func:`declared_needs`, not a topological sort -- + nothing here needs an order, only membership. A dependency that is not a + key of ``jobs`` (there is none in this workflow, but a rename could produce + one) is walked as if it declared no ``needs:`` of its own, rather than + raising, so a scanner bug surfaces as a missing closure member instead of + an exception unrelated to what the caller is checking. + + """ + seen = set() # type: set[str] + stack = list(declared_needs(jobs[name])) + while stack: + dep = stack.pop() + if dep in seen: + continue + seen.add(dep) + stack.extend(declared_needs(jobs.get(dep, ''))) + return seen + + def publishing_reasons(block: 'str') -> 'list[str]': """Why this job has an effect outside the run, empty when it has none.""" body = strip_comments(block) @@ -205,8 +249,14 @@ def test_the_scan_found_the_jobs(self) -> None: def test_every_publishing_job_declares_an_environment(self) -> None: """No job may reach PyPI, Anaconda, a Release or a git push unapproved. - An ``environment:`` is the only thing in Actions that will hold a job for - a human. Without one the job runs the moment its ``needs:`` are met, which + An ``environment:`` is what lets Actions hold a job for a human, but + since #887 only ``github-release``'s actually does -- ``conda-tag``, + ``pypi`` and ``anaconda`` kept their environments with no reviewer on + them, and are held instead by depending on ``github`` through + ``needs:``. This test only checks that the line is present, not that + it still carries a reviewer or that the dependency exists; + :class:`TestGatedJobsDependOnGithub` below is what checks the latter. + Without either, the job runs the moment its ``needs:`` are met, which for this workflow means "whenever a version bump lands". """ @@ -238,11 +288,16 @@ class TestEnvironmentsAreNotShared(WorkflowMixin, unittest.TestCase): def test_each_gated_job_has_its_own_environment(self) -> None: """Approval is granted to an environment, so sharing one merges two gates. - A single ``release`` environment approved once releases PyPI *and* - Anaconda, which is the outcome #641's fix is supposed to prevent rather - than rename. Different credentials reach the two indexes -- OIDC trusted - publishing and ``ANACONDA_TOKEN`` -- and ``tag``'s write to ``main`` uses a - third, so each gets an approval that can be refused on its own. + A single ``release`` environment approved once would release PyPI + *and* Anaconda together, which is the outcome #641's fix was meant to + prevent rather than rename. Different credentials reach the two + indexes -- OIDC trusted publishing and ``ANACONDA_TOKEN`` -- and + ``tag``'s write to ``main`` uses a third, so sharing one environment + would still merge those blast radii even though, since #887, only + ``github-release`` carries a reviewer of its own; the other three are + held by depending on it instead. That each *does* depend on it is + :class:`TestGatedJobsDependOnGithub`'s check, not this one -- this + test only stops the four gates from being collapsed back into one. """ environments = {} # type: dict[str, list[str]] @@ -267,6 +322,40 @@ def test_pypi_and_anaconda_are_separate(self) -> None: self.assertNotEqual(pypi, anaconda) +class TestGatedJobsDependOnGithub(WorkflowMixin, unittest.TestCase): + """#887: with only ``github-release`` still holding a required reviewer, + the other three gated jobs have to reach it through ``needs:`` instead, + or removing their own reviewer left them ungated in every sense that + matters. + + Nothing above this class reads ``needs:`` at all -- :class:`WorkflowMixin` + aside, every test in :class:`TestPublishingJobsAreGated` and + :class:`TestEnvironmentsAreNotShared` passes identically if ``tag``'s + ``needs:`` is reverted to ``[ version_check ]``, which is the exact + regression #887 fixed. This is the one test in the module that would + catch it. + + """ + + def test_every_gated_job_depends_on_github(self) -> None: + """Every ``environment:``-declaring job but ``github`` itself must + have ``github`` in its ``needs:`` closure, directly or transitively. + + """ + for name, block in sorted(self.jobs.items()): + if name == 'github' or declared_environment(block) is None: + continue + with self.subTest(job=name): + closure = needs_closure(self.jobs, name) + self.assertIn( + 'github', closure, + f'`{name}` declares an environment but its `needs:` closure ' + f'{sorted(closure)} does not include `github` -- removing ' + f'its own required reviewer (#887) would let it run before ' + f'the one approval left to gate it', + ) + + class TestMarkersStillMatch(WorkflowMixin, unittest.TestCase): """:data:`PUBLISHING_MARKERS` has to keep matching the file it describes.""" @@ -405,6 +494,25 @@ def test_the_same_job_to_environment_mapping(self) -> None: 'the indentation-based scanner and yaml.safe_load disagree ' 'about which jobs are gated') + def test_the_same_job_to_needs_mapping(self) -> None: + """:func:`declared_needs` against the same real parse, for the same reason.""" + try: + import yaml + except ImportError: # pragma: no cover + self.skipTest('PyYAML is not in the test extra; the textual scan is ' + 'asserted by TestGatedJobsDependOnGithub') + + parsed = yaml.safe_load(self.text) + self.assertIsInstance(parsed, dict, 'the workflow is not a YAML mapping') + + expected = {name: list(job.get('needs', [])) + for name, job in parsed['jobs'].items()} + scanned = {name: declared_needs(block) for name, block in self.jobs.items()} + + self.assertEqual(scanned, expected, + 'the indentation-based `needs:` scanner and yaml.safe_load ' + 'disagree about the dependency graph') + if __name__ == '__main__': unittest.main()