diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index e1e4e58..4db267f 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -47,15 +47,18 @@ env: jobs: release: runs-on: ubuntu-latest - # t42: this job pushes the write-back commit itself, and a run starting from - # that commit would release the same version twice. Two layers guard it: the - # commit message carries the conventional `[skip ci]` marker, and this - # condition re-checks the pushed message, so the guard still holds if a Gitea - # version stops honouring the marker. workflow_dispatch has no head_commit, - # hence the first clause. + # t42/t44 R3: this job pushes the write-back commit itself, and a run starting + # from that commit would release the same version twice. The guard is anchored to + # the release commit's SUBJECT (`chore(release): …`), not to the `[skip ci]` + # marker anywhere in the message: a developer commit whose body happens to + # mention [skip ci] must still release — with contains() it silently did not. + # The marker stays in the commit message because it is the convention Gitea + # itself may honour to skip the run outright (README documents that platform + # layer, and that the job guard is a no-op there rather than the trigger). + # workflow_dispatch has no head_commit, hence the first clause. if: >- github.event_name == 'workflow_dispatch' || - !contains(github.event.head_commit.message || '', '[skip ci]') + !startsWith(github.event.head_commit.message || '', 'chore(release):') steps: - uses: actions/checkout@v4 with: @@ -151,18 +154,28 @@ jobs: env: VERSION: ${{ steps.version.outputs.VERSION }} TAG: ${{ steps.version.outputs.TAG }} + BUMP: ${{ steps.version.outputs.BUMP }} SERVER: ${{ github.server_url }} REPO: ${{ github.repository }} TOKEN: ${{ secrets.GITEA_TOKEN }} run: | set -euo pipefail - # Notes: subject lines of the commits in the released range. - PREV="$(git describe --tags --abbrev=0 "${TAG}^" 2>/dev/null || true)" - if [ -n "$PREV" ]; then - RANGE="${PREV}..${TAG}" + # Notes: subject lines of the commits in the released range — the + # commits a DEVELOPER wrote. With a write-back (BUMP=yes) the tag points + # at the bot's own commit, so the range has to stop at its parent or every + # release would open with "chore(release): vX [skip ci]". Without one the + # tag is the pushed commit itself and belongs in the notes (t44 R1). + if [ "$BUMP" = "yes" ]; then + NOTES_END="${TAG}^" else - RANGE="${TAG}" + NOTES_END="${TAG}" + fi + PREV="$(git describe --tags --abbrev=0 "${NOTES_END}^" 2>/dev/null || true)" + if [ -n "$PREV" ]; then + RANGE="${PREV}..${NOTES_END}" + else + RANGE="${NOTES_END}" fi { echo "Release ${TAG}" diff --git a/tests/test_release_versioning.py b/tests/test_release_versioning.py index 9b172a1..80e7305 100644 --- a/tests/test_release_versioning.py +++ b/tests/test_release_versioning.py @@ -333,6 +333,26 @@ def test_the_bot_commit_cannot_trigger_another_release(workflow_text): assert "workflow_dispatch" in workflow_text +def test_the_skip_guard_matches_the_subject_not_the_whole_message(workflow_text): + """t44 R3: `contains(message, '[skip ci]')` skipped a release silently. + + A developer commit whose BODY mentions the marker is not a release commit, so + the guard is anchored to the release bot's subject prefix instead. + """ + assert "!startsWith(github.event.head_commit.message || '', 'chore(release):')" in ( + workflow_text + ) + assert "!contains(github.event.head_commit.message" not in workflow_text + + +def test_the_release_notes_exclude_the_write_back_commit(workflow_text): + """t44 R1: with a write-back the tag IS the bot commit, so notes must skip it.""" + assert 'if [ "$BUMP" = "yes" ]; then' in workflow_text + assert 'NOTES_END="${TAG}^"' in workflow_text + assert 'RANGE="${PREV}..${NOTES_END}"' in workflow_text + assert 'BUMP: ${{ steps.version.outputs.BUMP }}' in workflow_text + + def test_the_preserved_release_behaviour_is_still_there(workflow_text): # one release at a time, never cancelled mid-flight assert "group: release-master" in workflow_text @@ -355,3 +375,280 @@ def test_the_script_reads_the_manifest_the_workflow_bumps(): manifest_version = json.loads(MANIFEST.read_text(encoding="utf-8"))["version"] # the script rejects anything else, so a typo here would fail every release assert re.fullmatch(r"[0-9]{4}\.[0-9]{2}\.[0-9]{2}(\.[0-9]+)?", manifest_version) + + +# ── t44 3c: real repositories, and the workflow's own steps by execution ───── + +MANIFEST_REL = "custom_components/nvidia_shield/manifest.json" + + +def _git(repo, *args) -> str: + return subprocess.run( + ["git", *args], cwd=repo, capture_output=True, text=True, check=True + ).stdout + + +def _repo(tmp_path, *, version="2026.10.09", commits=("a feature commit",)): + """A real repository: the resolver's tag lookups really are git commands.""" + repo = tmp_path / "repo" + repo.mkdir(parents=True) + _git(repo, "init", "-q", "-b", "master") + _git(repo, "config", "user.email", "dev@example.com") + _git(repo, "config", "user.name", "dev") + manifest = repo / MANIFEST_REL + manifest.parent.mkdir(parents=True, exist_ok=True) + for index, subject in enumerate(commits): + manifest.write_text( + json.dumps( + {"domain": "nvidia_shield", "version": version, "note": index} + ) + + "\n" + ) + _git(repo, "add", "-A") + _git(repo, "commit", "-qm", subject) + return repo + + +def _resolve_in(repo, *, today=TODAY): + """Run the resolver inside a real repo — no tag fixtures, the real git paths.""" + result = subprocess.run( + ["bash", str(SCRIPT), "--manifest", MANIFEST_REL, "--today", today], + cwd=repo, capture_output=True, text=True, check=False, + ) + values = dict( + line.split("=", 1) for line in result.stdout.splitlines() if "=" in line + ) + return result, values + + +class TestRealRepositories: + """The tagged-version branches against real git (t44 3c). + + The fixture-based tests above never exercise `refs/tags/X^{commit}`, and that + dereference is the whole reason an annotated tag is recognised: for one, + `git rev-parse refs/tags/X` is the TAG OBJECT, not the commit, so a comparison + that forgot to dereference would miss a released commit entirely. + """ + + def test_an_annotated_tag_on_head_is_already_released(self, tmp_path): + repo = _repo(tmp_path) + _git(repo, "tag", "-a", "v2026.10.09", "-m", "Release v2026.10.09") + + result, values = _resolve_in(repo) + + assert result.returncode == 0, result.stderr + assert values["SOURCE"] == "released" + assert values["RELEASE"] == "no" + assert "nothing to release" in result.stderr + # premise: the tag object really is not the commit + assert _git(repo, "rev-parse", "v2026.10.09").strip() != _git( + repo, "rev-parse", "HEAD" + ).strip() + + def test_a_lightweight_tag_on_head_is_also_already_released(self, tmp_path): + repo = _repo(tmp_path) + _git(repo, "tag", "v2026.10.09") + + _result, values = _resolve_in(repo) + + assert values["SOURCE"] == "released" + assert values["RELEASE"] == "no" + + def test_a_tag_on_an_older_commit_falls_back_to_the_date(self, tmp_path): + repo = _repo(tmp_path, commits=("the released work", "later work")) + _git(repo, "tag", "-a", "v2026.10.09", "-m", "Release v2026.10.09", "HEAD~1") + + _result, values = _resolve_in(repo) + + assert values["VERSION"] == TODAY + assert values["SOURCE"] == "date" + assert values["BUMP"] == "yes" + assert values["RELEASE"] == "yes" + + def test_no_tags_at_all_means_the_manifest_wins(self, tmp_path): + repo = _repo(tmp_path, version="2026.10.10") + + _result, values = _resolve_in(repo) + + assert values["VERSION"] == "2026.10.10" + assert values["SOURCE"] == "manifest" + assert values["BUMP"] == "no" + + def test_the_same_day_scan_walks_real_tags(self, tmp_path): + repo = _repo(tmp_path, commits=("released", "more")) + for tag in ("v2026.10.09", f"v{TODAY}", f"v{TODAY}.2"): + _git(repo, "tag", "-a", tag, "-m", f"Release {tag}", "HEAD~1") + + _result, values = _resolve_in(repo) + + assert values["VERSION"] == f"{TODAY}.3" + assert values["SOURCE"] == "same-day" + + def test_a_gap_in_the_suffixes_is_reused(self, tmp_path): + repo = _repo(tmp_path, commits=("released", "more")) + for tag in ("v2026.10.09", f"v{TODAY}", f"v{TODAY}.3"): + _git(repo, "tag", "-a", tag, "-m", f"Release {tag}", "HEAD~1") + + _result, values = _resolve_in(repo) + + assert values["VERSION"] == f"{TODAY}.2" + + def test_the_guard_has_a_script_side_layer(self, tmp_path): + """Even with the job guard bypassed, a released commit releases nothing. + + The job-level `if:` cannot be executed here (no Gitea runner), but the + decision it protects can: this is the layer that makes a bypass harmless. + """ + repo = _repo(tmp_path) + _git(repo, "tag", "-a", "v2026.10.09", "-m", "Release v2026.10.09") + # a re-run of the same push, guard or no guard + for _attempt in range(2): + _result, values = _resolve_in(repo) + assert values["RELEASE"] == "no" + assert values["BUMP"] == "no" + + +def _step_script(name: str) -> str: + """The `run:` body of one workflow step, read by hand. + + The fast suite has no YAML parser (the verify command parses the file with + pyyaml instead), and executing these steps is worth more than matching their + text, so the block is taken verbatim and dedented. + """ + lines = WORKFLOW.read_text(encoding="utf-8").splitlines() + start = next(i for i, line in enumerate(lines) if line.strip() == f"- name: {name}") + run_at = next( + i for i in range(start, len(lines)) if lines[i].strip() in {"run: |", "run: >-"} + ) + indent = len(lines[run_at + 1]) - len(lines[run_at + 1].lstrip()) + body = [] + for line in lines[run_at + 1 :]: + if line.strip() and not line.startswith(" " * indent): + break + body.append(line[indent:] if len(line) >= indent else "") + return "\n".join(body) + + +def _run_step(name: str, cwd, env): + return subprocess.run( + ["bash", "-euo", "pipefail", "-c", _step_script(name)], + cwd=cwd, capture_output=True, text=True, env={**os.environ, **env}, + ) + + +class TestWorkflowStepsByExecution: + """Run the workflow's own steps against a bare origin (t44 3c). + + Textual pins cannot tell whether the bump lands on the commit the tag names; + these do, and they are what the dry run in the task output was. + """ + + VERSION = "2026.10.11" + TAG = "v2026.10.11" + + def _clone(self, tmp_path, *, protect=False): + origin = tmp_path / "origin.git" + subprocess.run( + ["git", "init", "--bare", "-q", "-b", "master", str(origin)], check=True + ) + work = tmp_path / "work" + subprocess.run(["git", "clone", "-q", str(origin), str(work)], check=True) + _git(work, "config", "user.email", "dev@example.com") + _git(work, "config", "user.name", "dev") + manifest = work / MANIFEST_REL + manifest.parent.mkdir(parents=True, exist_ok=True) + manifest.write_text( + json.dumps({"domain": "nvidia_shield", "version": "2026.10.08"}) + "\n" + ) + _git(work, "add", "-A") + _git(work, "commit", "-qm", "a feature commit") + _git(work, "push", "-q", "origin", "master") + if protect: + hook = origin / "hooks" / "pre-receive" + hook.write_text("#!/bin/sh\necho 'protected branch' >&2\nexit 1\n") + hook.chmod(0o755) + return work + + def _env(self): + return { + "MANIFEST": MANIFEST_REL, + "VERSION": self.VERSION, + "TAG": self.TAG, + } + + def test_the_bump_verify_and_tag_steps_agree_on_one_commit(self, tmp_path): + work = self._clone(tmp_path) + + bump = _run_step("Write the version back into the manifest", work, self._env()) + assert bump.returncode == 0, bump.stderr + assert _git(work, "log", "-1", "--pretty=%s").strip() == ( + f"chore(release): {self.TAG} [skip ci]" + ) + + verify = _run_step("Verify the released manifest on master", work, self._env()) + assert verify.returncode == 0, verify.stderr + assert f"matching {self.TAG}" in verify.stdout + + tag = _run_step("Create tag", work, self._env()) + assert tag.returncode == 0, tag.stderr + + head = _git(work, "rev-parse", "HEAD").strip() + assert _git(work, "rev-parse", f"{self.TAG}^{{commit}}").strip() == head + tagged_manifest = json.loads(_git(work, "show", f"{self.TAG}:{MANIFEST_REL}")) + assert tagged_manifest["version"] == self.VERSION + assert _git(work, "ls-remote", "--tags", "origin", self.TAG).split()[1] == ( + f"refs/tags/{self.TAG}" + ) + + def test_the_verify_step_refuses_an_inconsistent_manifest(self, tmp_path): + work = self._clone(tmp_path) + _run_step("Write the version back into the manifest", work, self._env()) + # someone else's release lands on master before this one is verified + (work / MANIFEST_REL).write_text( + json.dumps({"domain": "nvidia_shield", "version": "2026.10.99"}) + "\n" + ) + _git(work, "add", "-A") + _git(work, "commit", "-qm", "a competing release") + _git(work, "push", "-q", "origin", "master") + + verify = _run_step("Verify the released manifest on master", work, self._env()) + + assert verify.returncode == 1 + assert "inconsistent" in verify.stderr + assert "2026.10.99" in verify.stderr + + def test_the_notes_range_describes_the_developers_commits(self, tmp_path): + """t44 R1, executed: the range the notes use must not open with the bot commit. + + With a write-back the tag IS the bot's commit, so `PREV..TAG` (the range + before this fix) published "- chore(release): vX [skip ci]" as the first + line of every release. The range is built from ``BUMP``, and this runs both + halves through real git. + """ + work = self._clone(tmp_path) + _git(work, "tag", "-a", "v2026.10.08", "-m", "previous release") + _git(work, "commit", "--allow-empty", "-qm", "feat: the real change") + _run_step("Write the version back into the manifest", work, self._env()) + _run_step("Create tag", work, self._env()) + + def subjects(rev_range): + return _git(work, "log", "--pretty=format:- %s", rev_range) + + notes_end = f"{self.TAG}^" # BUMP=yes: the write-back commit is skipped + prev = _git(work, "describe", "--tags", "--abbrev=0", f"{notes_end}^").strip() + + assert prev == "v2026.10.08" + assert subjects(f"{prev}..{notes_end}") == "- feat: the real change" + # ...and the range this replaced really did include the bot's own subject + assert "- chore(release):" in subjects(f"{prev}..{self.TAG}") + + def test_a_rejected_bump_push_fails_and_tags_nothing(self, tmp_path): + work = self._clone(tmp_path, protect=True) + + bump = _run_step("Write the version back into the manifest", work, self._env()) + + assert bump.returncode == 1 + assert "could not push" in bump.stderr + assert "manually" in bump.stderr + assert MANIFEST_REL in bump.stderr + assert _git(work, "tag", "-l").strip() == ""