This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch backport-ci-20261025-v3-4-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 3af8f8bafdc0a66e5966633c09219b91f7ecab4a Author: Jarek Potiuk <[email protected]> AuthorDate: Sun Oct 25 19:30:14 2026 +0100 Check newsfragment PR numbers in static checks instead of a separate workflow (#75612) * Check newsfragment PR numbers in static checks instead of a separate workflow The separate workflow ran outside the fork CI gate (AIP-120), needed its own required check, and needed its own approval for first-time contributors. As part of the existing newsfragment hook it runs wherever static checks run, in project CI and in a contributor's fork, and fails CI passed like any other static check. A newsfragment is usually written before its pull request exists, so a local run cannot know the right number and accepts any. Generated-by: Claude Opus 5 * Address review of the newsfragment PR number check A pull request inside a fork carries the fork's number, so only apache/airflow pull requests are checked. A GitHub API error is reported without a traceback, and the instructions say who re-runs the checks when CI runs in the contributor's fork. Generated-by: Claude Opus 5 (cherry picked from commit 757898c95e52d52fe357e091de99a3c3b4bca79f) The documentation conflicts were resolved by keeping the release branch's text and updating only its sentences about the newsfragment check. --- .asf.yaml | 7 +- .github/workflows/basic-tests.yml | 7 ++ .github/workflows/check-newsfragment-pr-number.yml | 60 ---------- .github/workflows/ci-amd.yml | 29 +++++ .github/workflows/ci-arm.yml | 29 +++++ .github/workflows/ci-image-checks.yml | 6 + contributing-docs/18_contribution_workflow.rst | 7 +- dev/README_AIRFLOW3_DEV.md | 6 +- scripts/ci/prek/newsfragments.py | 69 +++++++++++ scripts/tests/ci/prek/test_newsfragments.py | 128 ++++++++++++++++++++- 10 files changed, 277 insertions(+), 71 deletions(-) diff --git a/.asf.yaml b/.asf.yaml index 1992824fb1f..8c62e4183ba 100644 --- a/.asf.yaml +++ b/.asf.yaml @@ -136,10 +136,9 @@ github: strict_required_status_checks_policy: false required_status_checks: - context: "CI passed" - - context: "check-newsfragment-pr-number" - # The branches `ci-amd.yml` runs pull request CI for, apart from `main`. The newsfragment check - # runs only on `main`, so these require "CI passed" alone. A release branch gets the "CI passed" - # job only when it is backported there; until then its PRs need a committer to bypass the rule. + # The branches `ci-amd.yml` runs pull request CI for, apart from `main`. A release branch gets + # the "CI passed" job only when it is backported there; until then its PRs need a committer to + # bypass the rule. - name: "Require CI on release branches" target: branch enforcement: active diff --git a/.github/workflows/basic-tests.yml b/.github/workflows/basic-tests.yml index 88740c02c46..1ecf344ef6c 100644 --- a/.github/workflows/basic-tests.yml +++ b/.github/workflows/basic-tests.yml @@ -20,6 +20,11 @@ name: Basic tests on: # yamllint disable-line rule:truthy workflow_call: inputs: + newsfragment-pr-number: + description: "Pull request the newsfragments are checked against; empty to accept any number" + required: false + type: string + default: "" runners: description: "The array of labels (in json form) determining runners." required: true @@ -316,6 +321,8 @@ jobs: PUSH_BASE_COMMIT: ${{ inputs.push-base-commit }} VERBOSE: "false" SKIP_BREEZE_PREK_HOOKS: "true" + PR_NUMBER: ${{ inputs.newsfragment-pr-number }} + GITHUB_TOKEN: ${{ github.token }} SKIP: ${{ inputs.skip-prek-hooks }} COLUMNS: "202" diff --git a/.github/workflows/check-newsfragment-pr-number.yml b/.github/workflows/check-newsfragment-pr-number.yml deleted file mode 100644 index 80dd07e33dc..00000000000 --- a/.github/workflows/check-newsfragment-pr-number.yml +++ /dev/null @@ -1,60 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# ---- -name: Check newsfragment PR number -on: # yamllint disable-line rule:truthy - pull_request: - branches: - - main - types: [opened, reopened, synchronize, labeled, unlabeled] - -permissions: - contents: read - pull-requests: read - -concurrency: - group: check-newsfragment-${{ github.event.pull_request.number }} - cancel-in-progress: false - -jobs: - check-newsfragment-pr-number: - if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip newsfragment check') }} - runs-on: ubuntu-slim - timeout-minutes: 5 - steps: - - name: Check newsfragment PR number - env: - GH_TOKEN: ${{ github.token }} - PR_NUMBER: "${{ github.event.pull_request.number }}" - run: | - # Find newsfragment files whose PR number doesn't match this PR. - # Use the REST API to get file statuses so we can exclude deleted files. - bad=$(gh api "repos/${{ github.repository }}/pulls/${PR_NUMBER}/files" \ - --paginate --jq '.[] | select(.status != "removed") | .filename' \ - | grep '/newsfragments/.*\.rst$' \ - | grep -v "/newsfragments/${PR_NUMBER}\." || true) - - if [ -n "$bad" ]; then - msg="Newsfragment PR number mismatch." - msg+=" Expected ${PR_NUMBER} but found: ${bad}." - msg+=" Rename the newsfragment to use the correct" - msg+=" PR number, or add the 'skip newsfragment" - msg+=" check' label to the PR to skip this check." - echo "::error::${msg}" - exit 1 - fi diff --git a/.github/workflows/ci-amd.yml b/.github/workflows/ci-amd.yml index 2de3f4f30ad..818659ee187 100644 --- a/.github/workflows/ci-amd.yml +++ b/.github/workflows/ci-amd.yml @@ -201,6 +201,7 @@ jobs: latest-versions-only: ${{ steps.selective-checks.outputs.latest-versions-only }} mysql-exclude: ${{ steps.selective-checks.outputs.mysql-exclude }} mysql-versions: ${{ steps.selective-checks.outputs.mysql-versions }} + newsfragment-pr-number: ${{ steps.newsfragment-pr.outputs.pr-number }} # platform is hardcoded per workflow file (this is the # AMD copy). The matching ARM copy lives in ci-arm.yml. platform: "linux/amd64" @@ -318,6 +319,32 @@ jobs: env: SKIP_BREEZE_SELF_UPGRADE_CHECK: "true" GITHUB_CONTEXT_INPUT: "${{ runner.temp }}/github_context.json" + # The newsfragment check of static checks fails a newsfragment of the pull request that is + # named after another number. A push in a fork (AIP-120) has no pull request in its event, + # so the open pull request of the pushed branch is looked up; without one, any number passes. + - name: "Find the pull request for the newsfragment check" + id: newsfragment-pr + env: + GH_TOKEN: ${{ github.token }} + EVENT_NAME: ${{ github.event_name }} + BASE_REF: ${{ github.base_ref }} + EVENT_PR_NUMBER: ${{ github.event.pull_request.number }} + REPOSITORY: ${{ github.repository }} + OWNER: ${{ github.repository_owner }} + REF_NAME: ${{ github.ref_name }} + run: | + number="" + # A pull request inside a fork has a number of the fork's, not of apache/airflow. + if [[ "${EVENT_NAME}" == "pull_request" && "${BASE_REF}" == "main" \ + && "${REPOSITORY}" == "apache/airflow" ]]; then + number="${EVENT_PR_NUMBER}" + elif [[ "${EVENT_NAME}" == "push" && "${REPOSITORY}" != "apache/airflow" ]]; then + number=$(gh api -X GET repos/apache/airflow/pulls -f head="${OWNER}:${REF_NAME}" \ + -f state=open -f base=main --jq '.[0].number // empty' || true) + fi + if [[ "${number}" =~ ^[0-9]+$ ]]; then + echo "pr-number=${number}" >> "${GITHUB_OUTPUT}" + fi - name: Selective checks id: selective-checks env: @@ -349,6 +376,7 @@ jobs: needs: [build-info, check-run-needed] uses: ./.github/workflows/basic-tests.yml with: + newsfragment-pr-number: ${{ needs.build-info.outputs.newsfragment-pr-number }} runners: ${{ needs.build-info.outputs.runner-type }} run-ui-tests: ${{needs.build-info.outputs.run-ui-tests}} run-www-tests: ${{needs.build-info.outputs.run-www-tests}} @@ -434,6 +462,7 @@ jobs: id-token: write contents: read with: + newsfragment-pr-number: ${{ needs.build-info.outputs.newsfragment-pr-number }} runners: ${{ needs.build-info.outputs.runner-type }} platform: ${{ needs.build-info.outputs.platform }} python-versions-list-as-string: ${{ needs.build-info.outputs.python-versions-list-as-string }} diff --git a/.github/workflows/ci-arm.yml b/.github/workflows/ci-arm.yml index 69ceccc2f3f..caea75528d4 100644 --- a/.github/workflows/ci-arm.yml +++ b/.github/workflows/ci-arm.yml @@ -180,6 +180,7 @@ jobs: latest-versions-only: ${{ steps.selective-checks.outputs.latest-versions-only }} mysql-exclude: ${{ steps.selective-checks.outputs.mysql-exclude }} mysql-versions: ${{ steps.selective-checks.outputs.mysql-versions }} + newsfragment-pr-number: ${{ steps.newsfragment-pr.outputs.pr-number }} # platform is hardcoded per workflow file (this is the # ARM copy). The matching AMD copy lives in ci-amd.yml. platform: "linux/arm64" @@ -297,6 +298,32 @@ jobs: env: SKIP_BREEZE_SELF_UPGRADE_CHECK: "true" GITHUB_CONTEXT_INPUT: "${{ runner.temp }}/github_context.json" + # The newsfragment check of static checks fails a newsfragment of the pull request that is + # named after another number. A push in a fork (AIP-120) has no pull request in its event, + # so the open pull request of the pushed branch is looked up; without one, any number passes. + - name: "Find the pull request for the newsfragment check" + id: newsfragment-pr + env: + GH_TOKEN: ${{ github.token }} + EVENT_NAME: ${{ github.event_name }} + BASE_REF: ${{ github.base_ref }} + EVENT_PR_NUMBER: ${{ github.event.pull_request.number }} + REPOSITORY: ${{ github.repository }} + OWNER: ${{ github.repository_owner }} + REF_NAME: ${{ github.ref_name }} + run: | + number="" + # A pull request inside a fork has a number of the fork's, not of apache/airflow. + if [[ "${EVENT_NAME}" == "pull_request" && "${BASE_REF}" == "main" \ + && "${REPOSITORY}" == "apache/airflow" ]]; then + number="${EVENT_PR_NUMBER}" + elif [[ "${EVENT_NAME}" == "push" && "${REPOSITORY}" != "apache/airflow" ]]; then + number=$(gh api -X GET repos/apache/airflow/pulls -f head="${OWNER}:${REF_NAME}" \ + -f state=open -f base=main --jq '.[0].number // empty' || true) + fi + if [[ "${number}" =~ ^[0-9]+$ ]]; then + echo "pr-number=${number}" >> "${GITHUB_OUTPUT}" + fi - name: Selective checks id: selective-checks env: @@ -328,6 +355,7 @@ jobs: needs: [build-info, check-run-needed] uses: ./.github/workflows/basic-tests.yml with: + newsfragment-pr-number: ${{ needs.build-info.outputs.newsfragment-pr-number }} runners: ${{ needs.build-info.outputs.runner-type }} run-ui-tests: ${{needs.build-info.outputs.run-ui-tests}} run-www-tests: ${{needs.build-info.outputs.run-www-tests}} @@ -413,6 +441,7 @@ jobs: id-token: write contents: read with: + newsfragment-pr-number: ${{ needs.build-info.outputs.newsfragment-pr-number }} runners: ${{ needs.build-info.outputs.runner-type }} platform: ${{ needs.build-info.outputs.platform }} python-versions-list-as-string: ${{ needs.build-info.outputs.python-versions-list-as-string }} diff --git a/.github/workflows/ci-image-checks.yml b/.github/workflows/ci-image-checks.yml index 1a1c06c6804..cca6c1826b4 100644 --- a/.github/workflows/ci-image-checks.yml +++ b/.github/workflows/ci-image-checks.yml @@ -20,6 +20,11 @@ name: CI Image Checks on: # yamllint disable-line rule:truthy workflow_call: inputs: + newsfragment-pr-number: + description: "Pull request the newsfragments are checked against; empty to accept any number" + required: false + type: string + default: "" runners: description: "The array of labels (in json form) determining runners." required: true @@ -154,6 +159,7 @@ jobs: SKIP: ${{ inputs.skip-prek-hooks }} COLUMNS: "202" SKIP_GROUP_OUTPUT: "true" + PR_NUMBER: ${{ inputs.newsfragment-pr-number }} DEFAULT_BRANCH: ${{ inputs.branch }} RUFF_FORMAT: "github" - name: "Show prek log on failure" diff --git a/contributing-docs/18_contribution_workflow.rst b/contributing-docs/18_contribution_workflow.rst index 86181a11043..0ce8075e9f0 100644 --- a/contributing-docs/18_contribution_workflow.rst +++ b/contributing-docs/18_contribution_workflow.rst @@ -204,9 +204,10 @@ Step 4: Prepare PR In general newsfragments must be one line. For newsfragment type ``significant``, you may include summary and body separated by a blank line, similar to ``git`` commit messages. One thing to note here is that a ``significant`` newsfragment doesn't have to be a breaking change, it can be something that is notable but not breaking. - A CI check validates that newsfragment filenames use the correct PR number. If you need to skip - this check (e.g. when cherry-picking a newsfragment from another PR), add the - ``skip newsfragment check`` label to your PR. + Static checks in CI validate that newsfragment filenames use the correct PR number. Run locally, + before the PR exists, they accept any number. If you need to skip this check (e.g. when + cherry-picking a newsfragment from another PR), ask a maintainer to add the + ``skip newsfragment check`` label to your PR, then re-run the failed static checks. 2. Rebase your fork, squash commits, and resolve all conflicts. See `How to rebase PR <10_working_with_git.rst#how-to-rebase-pr>`_ if you need help with rebasing your change. Remember to rebase often if your PR takes a lot of time to diff --git a/dev/README_AIRFLOW3_DEV.md b/dev/README_AIRFLOW3_DEV.md index 4ebd6069272..fb33ba862a7 100644 --- a/dev/README_AIRFLOW3_DEV.md +++ b/dev/README_AIRFLOW3_DEV.md @@ -195,15 +195,15 @@ Auto-merge is enabled for the whole repository, so every branch that PRs target `airflow-ctl` branches. A release branch gets the **CI passed** job only once it is backported there; until then, its PRs never report the check, and a committer merges them by bypassing the rule. -The `main` ruleset requires two checks. **CI passed** is a job in the `Tests (AMD)` workflow that always +The `main` ruleset requires one check, **CI passed**. It is a job in the `Tests (AMD)` workflow that always runs and fails when any other CI job failed or was cancelled; jobs that selective checks skip count as passed, so PRs that run only part of the tests still get a result. The `check-ci-gate-needs` prek hook makes sure every new CI job is added to it. `Tests (ARM)`, which runs on pushes and on a schedule rather than on PRs, has the same job, kept identical by the `check-ci-workflows-in-sync` hook. Once the fork CI gate (AIP-120) is in place, a PR that it leaves to CI in the contributor's fork does not run project CI, so the job reports as **CI passed (project CI not run)** instead, and the fork CI reconciler posts **CI -passed** once the fork run for the PR's latest commit is green and trusted. -**check-newsfragment-pr-number** runs on every PR too. +passed** once the fork run for the PR's latest commit is green and trusted. Static checks also fail a +newsfragment of a PR to `main` that is named after another PR number. Auto-merge does not wait for checks from other workflows: **CodeQL**, **Dependency review** and **Registry tests**. The last two only run when dependency or registry files change. Look at those diff --git a/scripts/ci/prek/newsfragments.py b/scripts/ci/prek/newsfragments.py index 41704cd4bb0..69a2f0883cc 100755 --- a/scripts/ci/prek/newsfragments.py +++ b/scripts/ci/prek/newsfragments.py @@ -19,14 +19,24 @@ """ Check things about newsfragments: - Only a single line, except for `significant` changes which can have a blank line, then the body + - In CI, where ``PR_NUMBER`` is set, the newsfragments the pull request adds are named after it. + Locally the pull request may not exist yet, so any number is accepted. """ from __future__ import annotations +import json +import os +import re import sys +import urllib.error +import urllib.request from pathlib import Path VALID_CHANGE_TYPES = {"significant", "feature", "improvement", "bugfix", "doc", "misc"} +SKIP_PR_NUMBER_LABEL = "skip newsfragment check" +PR_REPOSITORY = "apache/airflow" +NEWSFRAGMENT_PATH = re.compile(r"/newsfragments/[^/]+\.rst$") def validate_newsfragment(filename: str, lines: list[str]) -> list[str]: @@ -63,6 +73,53 @@ def validate_newsfragment(filename: str, lines: list[str]) -> list[str]: return errors +def find_misnumbered_newsfragments(pr_number: str, filenames: list[str]) -> list[str]: + return [ + filename + for filename in filenames + if NEWSFRAGMENT_PATH.search(filename) and not Path(filename).name.startswith(f"{pr_number}.") + ] + + +def get_github_items(path: str) -> list[dict]: + """Return every item of a paginated GitHub REST API listing.""" + headers = {"Accept": "application/vnd.github+json"} + if token := os.environ.get("GITHUB_TOKEN"): + headers["Authorization"] = f"Bearer {token}" + items: list[dict] = [] + page = 1 + while True: + request = urllib.request.Request( + f"https://api.github.com/{path}?per_page=100&page={page}";, headers=headers + ) + with urllib.request.urlopen(request, timeout=30) as response: + batch = json.load(response) + items.extend(batch) + if len(batch) < 100: + return items + page += 1 + + +def check_pr_number(pr_number: str) -> list[str]: + """Return the errors for the newsfragments the pull request adds under another number.""" + labels = get_github_items(f"repos/{PR_REPOSITORY}/issues/{pr_number}/labels") + if any(label["name"] == SKIP_PR_NUMBER_LABEL for label in labels): + print(f"Newsfragment PR number not checked: the pull request has the {SKIP_PR_NUMBER_LABEL!r} label.") + return [] + # Removed files are left out, so deleting a misnumbered newsfragment fixes the check. + files = [ + f["filename"] + for f in get_github_items(f"repos/{PR_REPOSITORY}/pulls/{pr_number}/files") + if f["status"] != "removed" + ] + return [ + f"Newsfragment {filename} is not named after pull request {pr_number}. Rename it to " + f"{pr_number}.<type>.rst, or have a maintainer add the {SKIP_PR_NUMBER_LABEL!r} label and " + "re-run the failed static checks (with CI in your fork, re-run them there)." + for filename in find_misnumbered_newsfragments(pr_number, files) + ] + + def main(filenames: list[str]) -> int: failed = False for filename in filenames: @@ -73,6 +130,18 @@ def main(filenames: list[str]) -> int: print(error) if errors: failed = True + pr_number = os.environ.get("PR_NUMBER") + if pr_number: + try: + errors = check_pr_number(pr_number) + except (urllib.error.HTTPError, urllib.error.URLError) as e: + errors = [ + f"Could not read pull request {pr_number} from GitHub to check newsfragment numbers: {e}" + ] + for error in errors: + print(error) + if errors: + failed = True return 1 if failed else 0 diff --git a/scripts/tests/ci/prek/test_newsfragments.py b/scripts/tests/ci/prek/test_newsfragments.py index bd249e45405..c43b522352e 100644 --- a/scripts/tests/ci/prek/test_newsfragments.py +++ b/scripts/tests/ci/prek/test_newsfragments.py @@ -16,8 +16,20 @@ # under the License. from __future__ import annotations +import json +import urllib.error +from email.message import Message +from unittest import mock + import pytest -from ci.prek.newsfragments import VALID_CHANGE_TYPES, validate_newsfragment +from ci.prek import newsfragments +from ci.prek.newsfragments import ( + VALID_CHANGE_TYPES, + check_pr_number, + find_misnumbered_newsfragments, + main, + validate_newsfragment, +) ALL_CHANGE_TYPES = sorted(VALID_CHANGE_TYPES) NON_SIGNIFICANT_CHANGE_TYPES = sorted(VALID_CHANGE_TYPES - {"significant"}) @@ -79,3 +91,117 @@ class TestNewsfragmentContentValidation: lines = ["Big change", "", "Details here", "More details", "Even more"] errors = validate_newsfragment("123.significant.rst", lines) assert errors == [] + + +class TestNewsfragmentPrNumber: + @pytest.mark.parametrize( + ("filenames", "expected"), + [ + pytest.param(["airflow-core/newsfragments/123.bugfix.rst"], [], id="matching"), + pytest.param( + ["airflow-core/newsfragments/124.bugfix.rst"], + ["airflow-core/newsfragments/124.bugfix.rst"], + id="other-number", + ), + pytest.param( + ["airflow-core/newsfragments/1234.bugfix.rst"], + ["airflow-core/newsfragments/1234.bugfix.rst"], + id="number-with-the-same-prefix", + ), + pytest.param(["airflow-core/docs/124.bugfix.rst", "README.md"], [], id="not-a-newsfragment"), + ], + ) + def test_find_misnumbered_newsfragments(self, filenames, expected): + assert find_misnumbered_newsfragments("123", filenames) == expected + + @mock.patch.object(newsfragments, "get_github_items", autospec=True) + def test_check_pr_number_ignores_removed_files(self, mock_get_items): + mock_get_items.side_effect = [ + [{"name": "area:core"}], + [ + {"filename": "airflow-core/newsfragments/124.bugfix.rst", "status": "added"}, + {"filename": "airflow-core/newsfragments/99.bugfix.rst", "status": "removed"}, + ], + ] + + errors = check_pr_number("123") + + assert len(errors) == 1 + assert "airflow-core/newsfragments/124.bugfix.rst" in errors[0] + assert mock_get_items.call_args_list == [ + mock.call("repos/apache/airflow/issues/123/labels"), + mock.call("repos/apache/airflow/pulls/123/files"), + ] + + @mock.patch.object(newsfragments, "get_github_items", autospec=True) + def test_check_pr_number_is_skipped_by_label(self, mock_get_items): + mock_get_items.return_value = [{"name": "skip newsfragment check"}] + + assert check_pr_number("123") == [] + mock_get_items.assert_called_once() + + @pytest.mark.parametrize( + ("pr_number", "expected_calls", "expected_exit"), + [ + pytest.param(None, [], 0, id="local-run-accepts-any-number"), + pytest.param("123", [mock.call("123")], 1, id="ci"), + ], + ) + @mock.patch.object(newsfragments, "check_pr_number", autospec=True, return_value=["wrong number"]) + def test_main_checks_pr_number_only_in_ci( + self, mock_check, monkeypatch, tmp_path, pr_number, expected_calls, expected_exit + ): + if pr_number: + monkeypatch.setenv("PR_NUMBER", pr_number) + else: + monkeypatch.delenv("PR_NUMBER", raising=False) + newsfragment = tmp_path / "newsfragments" / "124.bugfix.rst" + newsfragment.parent.mkdir() + newsfragment.write_text("A change\n") + + assert main([str(newsfragment)]) == expected_exit + assert mock_check.call_args_list == expected_calls + + @pytest.mark.parametrize( + "error", + [ + pytest.param(urllib.error.URLError("no network"), id="url-error"), + pytest.param( + urllib.error.HTTPError("https://api.github.com";, 403, "rate limited", Message(), None), + id="http-error", + ), + ], + ) + @mock.patch.object(newsfragments, "check_pr_number", autospec=True) + def test_main_reports_an_unreadable_pull_request(self, mock_check, monkeypatch, capsys, error): + monkeypatch.setenv("PR_NUMBER", "123") + mock_check.side_effect = error + + assert main([]) == 1 + assert "Could not read pull request 123 from GitHub" in capsys.readouterr().out + + @pytest.mark.parametrize( + ("token", "expected_authorization"), + [pytest.param(None, None, id="anonymous"), pytest.param("secret", "Bearer secret", id="token")], + ) + @mock.patch.object(newsfragments.urllib.request, "urlopen", autospec=True) + def test_get_github_items_reads_every_page( + self, mock_urlopen, monkeypatch, token, expected_authorization + ): + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + if token: + monkeypatch.setenv("GITHUB_TOKEN", token) + pages = [[{"n": i} for i in range(100)], [{"n": 100}]] + mock_urlopen.return_value.__enter__.side_effect = [ + mock.Mock(read=mock.Mock(return_value=json.dumps(page).encode())) for page in pages + ] + + items = newsfragments.get_github_items("repos/apache/airflow/pulls/1/files") + + assert len(items) == 101 + requests = [call.args[0] for call in mock_urlopen.call_args_list] + assert [r.full_url for r in requests] == [ + "https://api.github.com/repos/apache/airflow/pulls/1/files?per_page=100&page=1";, + "https://api.github.com/repos/apache/airflow/pulls/1/files?per_page=100&page=2";, + ] + assert requests[0].get_header("Authorization") == expected_authorization
