From da93856b09690519ca01e0c69470ae914b3ea4df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Tue, 27 Oct 2026 15:31:14 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix(validate-news):=20read=20the?= =?UTF-8?q?=20current=20PR=20labels?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/news.yaml | 1 + README.md | 7 ++- changelog.d/+live-labels.change.md | 3 ++ tests/conftest.py | 4 +- tests/validate_news/test_news.py | 68 +++++++++++++++++++++++------- validate-news/action.yml | 15 ++++++- 6 files changed, 78 insertions(+), 20 deletions(-) create mode 100644 changelog.d/+live-labels.change.md diff --git a/.github/workflows/news.yaml b/.github/workflows/news.yaml index 94fc4e2..2ecc75f 100644 --- a/.github/workflows/news.yaml +++ b/.github/workflows/news.yaml @@ -4,6 +4,7 @@ on: types: [opened, reopened, synchronize, labeled, unlabeled] permissions: contents: read + pull-requests: read jobs: changelog: name: 📝 News fragment diff --git a/README.md b/README.md index a3c644e..f464b1b 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ caller's `pylock.toml`. configured directory against the configured categories, filename format, and nonempty content. A valid fragment runs `towncrier check` against the PR base. A human PR without a fragment needs `skip news`; a bot PR without a fragment does not. A bot's supplied fragment is checked and included in the normal release flow. The Action reads repository files and -Git history; it does not add labels or write to GitHub. +Git history and reads the PR's current labels; it does not add labels or write to GitHub. Each check runs only when the pull request touches what it validates, compared with the merge base: @@ -85,6 +85,7 @@ changelog: runs-on: ubuntu-26.04 permissions: contents: read + pull-requests: read steps: - uses: actions/checkout@CHECKOUT_SHA with: @@ -102,7 +103,9 @@ changelog: ``` The Action expects a pull request base ref and a Markdown or reStructuredText changelog. The caller should also run on -`labeled` and `unlabeled` events, so a label added after opening the PR updates the required check. +`labeled` and `unlabeled` events, so a label added after opening the PR updates the required check. The Action reads the +PR's labels from the API rather than from the event, so re-running an older run sees the labels the PR has now; the job +needs `pull-requests: read` for that. ## Refresh release tools diff --git a/changelog.d/+live-labels.change.md b/changelog.d/+live-labels.change.md new file mode 100644 index 0000000..2dc3e9e --- /dev/null +++ b/changelog.d/+live-labels.change.md @@ -0,0 +1,3 @@ +`validate-news` reads the pull request's current labels from the API, so re-running an older Changelog run after adding +`skip news` passes instead of failing on the labels the original event carried. Callers grant `pull-requests: read` to +the job. diff --git a/tests/conftest.py b/tests/conftest.py index e7d396d..4290b36 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -471,8 +471,8 @@ def _expression( return environment.get("GH_TOKEN", environment.get("GH_ENTERPRISE_TOKEN", "")) if expression == "github.event.pull_request.user.type": return "Bot" if environment.get("BOT_AUTHOR") == "true" else "User" - if expression == "contains(github.event.pull_request.labels.*.name, 'skip news')": - return environment.get("SKIP_NEWS", "false") + if expression == "github.event.pull_request.number": + return environment.get("PULL_NUMBER", "7") raise ValueError(f"unsupported GitHub step environment: {expression}") # pragma: no cover - new syntax only diff --git a/tests/validate_news/test_news.py b/tests/validate_news/test_news.py index 854571e..41c7e23 100644 --- a/tests/validate_news/test_news.py +++ b/tests/validate_news/test_news.py @@ -8,12 +8,20 @@ import pytest import yaml from conftest import action, run +from pytest_httpserver import HTTPServer _STEPS: Final = yaml.safe_load((Path(__file__).resolve().parents[2] / "validate-news/action.yml").read_text())["runs"][ "steps" ] +def _current_labels(github_api: tuple[HTTPServer, dict[str, str]], skip: str) -> dict[str, str]: + server, environment = github_api + labels = [{"name": "documentation"}, *([{"name": "skip news"}] if skip == "true" else [])] + server.expect_request("/api/v3/repos/tox-dev/fixture/pulls/7", method="GET").respond_with_json({"labels": labels}) + return environment + + def _script(action_scripts: Path, step_id: str) -> Path: return action_scripts / f"validate-news-{next(i for i, step in enumerate(_STEPS) if step.get('id') == step_id)}.sh" @@ -27,7 +35,13 @@ def _script(action_scripts: Path, step_id: str) -> Path: ], ) def test_validate_news_without_fragment( - repository: Path, tmp_path: Path, action_scripts: Path, bot: str, skip: str, accepted: bool + repository: Path, + tmp_path: Path, + action_scripts: Path, + bot: str, + skip: str, + accepted: bool, + github_api: tuple[HTTPServer, dict[str, str]], ) -> None: result = action( repository, @@ -37,7 +51,7 @@ def test_validate_news_without_fragment( version="0.2.0", BASE_REF="main", BOT_AUTHOR=bot, - SKIP_NEWS=skip, + **_current_labels(github_api, skip), ) assert (result.returncode == 0) == accepted if not accepted: @@ -52,7 +66,9 @@ def test_validate_news_without_fragment( pytest.param("123.feature.2.md", id="numbered-multiple"), ], ) -def test_validate_news_checks_bot_fragment(repository: Path, tmp_path: Path, action_scripts: Path, name: str) -> None: +def test_validate_news_checks_bot_fragment( + repository: Path, tmp_path: Path, action_scripts: Path, name: str, github_api: tuple[HTTPServer, dict[str, str]] +) -> None: (repository / "changelog.d" / name).write_text("Add another feature.\n") run(repository, "git", "add", ".") run(repository, "git", "commit", "-m", "Add news") @@ -64,12 +80,14 @@ def test_validate_news_checks_bot_fragment(repository: Path, tmp_path: Path, act version="0.2.0", BASE_REF="main", BOT_AUTHOR="true", - SKIP_NEWS="true", + **_current_labels(github_api, "true"), ) assert result.returncode == 0, result.stderr -def test_validate_news_reads_rst_categories(repository: Path, tmp_path: Path, action_scripts: Path) -> None: +def test_validate_news_reads_rst_categories( + repository: Path, tmp_path: Path, action_scripts: Path, github_api: tuple[HTTPServer, dict[str, str]] +) -> None: project = repository / "pyproject.toml" project.write_text( project.read_text() @@ -90,7 +108,7 @@ def test_validate_news_reads_rst_categories(repository: Path, tmp_path: Path, ac version="0.2.0", BASE_REF="main", BOT_AUTHOR="false", - SKIP_NEWS="false", + **_current_labels(github_api, "false"), ) assert result.returncode == 0, result.stderr @@ -105,7 +123,13 @@ def test_validate_news_reads_rst_categories(repository: Path, tmp_path: Path, ac ], ) def test_validate_news_rejects_bad_fragment( - repository: Path, tmp_path: Path, action_scripts: Path, name: str, content: str, error: str + repository: Path, + tmp_path: Path, + action_scripts: Path, + name: str, + content: str, + error: str, + github_api: tuple[HTTPServer, dict[str, str]], ) -> None: (repository / "changelog.d" / name).write_text(content) run(repository, "git", "add", ".") @@ -118,13 +142,15 @@ def test_validate_news_rejects_bad_fragment( version="0.2.0", BASE_REF="main", BOT_AUTHOR="true", - SKIP_NEWS="false", + **_current_labels(github_api, "false"), ) assert result.returncode != 0 assert error in result.stderr -def test_validate_news_requires_pr_base(repository: Path, tmp_path: Path, action_scripts: Path) -> None: +def test_validate_news_requires_pr_base( + repository: Path, tmp_path: Path, action_scripts: Path, github_api: tuple[HTTPServer, dict[str, str]] +) -> None: result = action( repository, tmp_path, @@ -133,7 +159,7 @@ def test_validate_news_requires_pr_base(repository: Path, tmp_path: Path, action version="0.2.0", BASE_REF="", BOT_AUTHOR="true", - SKIP_NEWS="false", + **_current_labels(github_api, "false"), ) assert result.returncode != 0 assert "origin/" in result.stderr @@ -264,6 +290,7 @@ def test_validate_news_checks_changed_release_inputs_against_the_lock( monkeypatch: pytest.MonkeyPatch, change: str, accepted: bool, + github_api: tuple[HTTPServer, dict[str, str]], ) -> None: monkeypatch.delenv("SSL_CERT_FILE") project = repository / "pyproject.toml" @@ -287,7 +314,7 @@ def test_validate_news_checks_changed_release_inputs_against_the_lock( "validate-news", version="0.2.0", BASE_REF="main", - SKIP_NEWS="true", + **_current_labels(github_api, "true"), ) assert (result.returncode == 0) is accepted, result.stderr @@ -371,7 +398,12 @@ def test_validate_news_detects_news_input_change( ], ) def test_validate_news_checks_towncrier_config_only_when_touched( - repository: Path, tmp_path: Path, action_scripts: Path, touched: str, accepted: bool + repository: Path, + tmp_path: Path, + action_scripts: Path, + touched: str, + accepted: bool, + github_api: tuple[HTTPServer, dict[str, str]], ) -> None: project = repository / "pyproject.toml" project.write_text(project.read_text().replace('directory = "changelog.d"\n', 'package = "fixture"\n')) @@ -392,7 +424,7 @@ def test_validate_news_checks_towncrier_config_only_when_touched( "validate-news", version="0.2.0", BASE_REF="main", - SKIP_NEWS="true", + **_current_labels(github_api, "true"), ) assert (result.returncode == 0) is accepted, result.stderr @@ -409,7 +441,13 @@ def test_validate_news_checks_towncrier_config_only_when_touched( ], ) def test_validate_news_explains_choices( - repository: Path, tmp_path: Path, action_scripts: Path, bot: str, skip: str, explanation: str + repository: Path, + tmp_path: Path, + action_scripts: Path, + bot: str, + skip: str, + explanation: str, + github_api: tuple[HTTPServer, dict[str, str]], ) -> None: result = action( repository, @@ -419,7 +457,7 @@ def test_validate_news_explains_choices( version="0.2.0", BASE_REF="main", BOT_AUTHOR=bot, - SKIP_NEWS=skip, + **_current_labels(github_api, skip), ) assert explanation in result.stdout diff --git a/validate-news/action.yml b/validate-news/action.yml index e936b4d..fe6de80 100644 --- a/validate-news/action.yml +++ b/validate-news/action.yml @@ -89,10 +89,23 @@ runs: source "$GITHUB_ACTION_PATH/../tools/plural.sh" release-creator validate-news --base "$BASE_REF" >"$RUNNER_TEMP/news-fragment-count" echo "Changed fragments: $(plural "$(<"$RUNNER_TEMP/news-fragment-count")" fragment) valid against origin/$BASE_REF" + - id: labels + name: 🏷️ Read the current labels + shell: bash + env: + GH_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + PULL: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + # A re-run replays its original event, whose label list can predate a skip news label added since. + skip=$(gh api "repos/$REPOSITORY/pulls/$PULL" --jq 'any(.labels[]; .name == "skip news")') + echo "Current labels of #$PULL: skip news $([[ "$skip" == true ]] && echo present || echo absent)" + echo "skip=$skip" >>"$GITHUB_OUTPUT" - name: 🔍 Require news for human changes shell: bash env: - SKIP_NEWS: ${{ contains(github.event.pull_request.labels.*.name, 'skip news') }} + SKIP_NEWS: ${{ steps.labels.outputs.skip }} BOT_AUTHOR: ${{ github.event.pull_request.user.type == 'Bot' }} run: | # shellcheck disable=SC1091 # GitHub supplies this Action path at runtime.