diff --git a/dev/release/add_backports.py b/dev/release/add_backports.py index d36086b95c..8d357e8459 100644 --- a/dev/release/add_backports.py +++ b/dev/release/add_backports.py @@ -1,13 +1,12 @@ import os -from dev.release.gh import GitHub +from dev.release.gh import GitHub, format_complete_issue_warning from dev.release.git import Git from dev.release.release_issue import ( RELEASE_TITLE_RE, add_backports_to_body, add_rc_task_to_body, add_sync_changelog_task_to_body, - is_release_complete, load_release_tracking_template, parse_checklist_state, ) @@ -33,21 +32,11 @@ def run(self) -> int: " tracking issue..." ) try: - open_issues = self.gh.get_open_tracking_issues() - # An open issue whose release has already been tagged is - # finished; it just hasn't been closed. Don't add work to it. - active_issues = [] - for issue in open_issues: - body = self.gh.get_issue_body(issue["number"]) - if is_release_complete(body): - print( - "::warning::Ignoring open release tracking issue" - f" #{issue['number']} ({issue['title']}): its" - " 'Tag Final' task is done, so the release is" - " complete. Consider closing it." - ) - continue - active_issues.append(issue) + active_issues, complete_issues = ( + self.gh.partition_open_tracking_issues() + ) + for issue in complete_issues: + print(f"::warning::{format_complete_issue_warning(issue)}") if len(active_issues) > 1: print( diff --git a/dev/release/create_release_issue.py b/dev/release/create_release_issue.py index 1123cfd2f8..c987c8865a 100644 --- a/dev/release/create_release_issue.py +++ b/dev/release/create_release_issue.py @@ -1,6 +1,6 @@ """Subcommand to create a release tracking issue.""" -from dev.release.gh import GitHub +from dev.release.gh import GitHub, format_complete_issue_warning from dev.release.release_issue import load_release_tracking_template from dev.release.utils import determine_next_version, semver_type @@ -18,11 +18,13 @@ def run(self) -> int: if version is None: version = determine_next_version() - # Concurrency check - open_issues = self.gh.get_open_tracking_issues() - if open_issues: + # Concurrency check: only a release still in progress blocks a new one. + active_issues, complete_issues = self.gh.partition_open_tracking_issues() + for issue in complete_issues: + print(f"::warning::{format_complete_issue_warning(issue)}") + if active_issues: print("Error: A release is already in progress. Active tracking issues:") - for issue in open_issues: + for issue in active_issues: print(f"- {issue['title']}: {issue['url']}") return 1 diff --git a/dev/release/gh.py b/dev/release/gh.py index ec43662bd9..94ac2f3fc3 100644 --- a/dev/release/gh.py +++ b/dev/release/gh.py @@ -12,7 +12,7 @@ override, # pyrefly: ignore[missing-module-attribute] -- override available in Python 3.12+ ) -from dev.release.release_issue import BackportTask +from dev.release.release_issue import BackportTask, is_release_complete from dev.release.shell import run_cmd # GitHub label types @@ -230,6 +230,22 @@ class InvalidPrRefError(ValueError): pass +def format_complete_issue_warning(issue: IssueDict) -> str: + """Formats the warning for an open tracking issue whose release is complete. + + Args: + issue: The open release tracking issue being ignored. + + Returns: + A one-line message, without any `::warning::` prefix. + """ + return ( + f"Ignoring open release tracking issue #{issue['number']}" + f" ({issue['title']}): its 'Tag Final' task is done, so the release is" + " complete. Consider closing it." + ) + + class GitHubInterface(abc.ABC): """Abstract interface for GitHub operations.""" @@ -393,6 +409,31 @@ def get_open_tracking_issues(self, version: str | None = None) -> list[IssueDict List of matching open release tracking issue dictionaries. """ + def partition_open_tracking_issues( + self, + ) -> tuple[list[IssueDict], list[IssueDict]]: + """Splits open release tracking issues into active and complete ones. + + An open tracking issue whose release has already been tagged (its + "Tag Final" task is done) is complete; it just hasn't been closed. + Commands looking for the release currently in progress should only + consider the active issues, and may want to warn about the complete + ones so someone closes them. + + Returns: + A tuple of (active, complete) lists of open release tracking + issue dictionaries. + """ + active: list[IssueDict] = [] + complete: list[IssueDict] = [] + for issue in self.get_open_tracking_issues(): + body = self.get_issue_body(issue["number"]) + if is_release_complete(body): + complete.append(issue) + else: + active.append(issue) + return active, complete + @abc.abstractmethod def get_pr_info(self, pr_num: int) -> PrDict: """Gets info about a PR. diff --git a/dev/release/sync_changelog.py b/dev/release/sync_changelog.py index 229bf904b4..d79722f09d 100644 --- a/dev/release/sync_changelog.py +++ b/dev/release/sync_changelog.py @@ -10,6 +10,7 @@ SYNC_CHANGELOG_LABEL, GitHub, GitHubInterface, + format_complete_issue_warning, get_github_event_issue_number, ) from dev.release.git import Git @@ -86,15 +87,17 @@ def _run_internal(self) -> int: logger.info( "No issue specified. Auto-discovering open release tracking issue..." ) - open_issues = self.gh.get_open_tracking_issues() - if len(open_issues) > 1: + active_issues, complete_issues = self.gh.partition_open_tracking_issues() + for issue in complete_issues: + logger.warning("%s", format_complete_issue_warning(issue)) + if len(active_issues) > 1: logger.error( "Multiple open release tracking issues found: %s", - [f"#{i['number']}" for i in open_issues], + [f"#{i['number']}" for i in active_issues], ) return 1 - elif len(open_issues) == 1: - issue_num = open_issues[0]["number"] + elif len(active_issues) == 1: + issue_num = active_issues[0]["number"] logger.info("Discovered release tracking issue #%d", issue_num) else: logger.error("No open release tracking issues found.") diff --git a/tests/tools/private/release/BUILD.bazel b/tests/tools/private/release/BUILD.bazel index bc395a74a6..b4a41aff2a 100644 --- a/tests/tools/private/release/BUILD.bazel +++ b/tests/tools/private/release/BUILD.bazel @@ -100,6 +100,18 @@ pytest_test( ], ) +pytest_test( + name = "create_release_issue_test", + srcs = ["create_release_issue_test.py"], + python_version = "3.14", + target_compatible_with = NOT_WINDOWS, + deps = [ + ":conftest", + ":release_test_helper", + "//dev/release:release_lib", + ], +) + pytest_test( name = "git_test", srcs = ["git_test.py"], diff --git a/tests/tools/private/release/create_release_issue_test.py b/tests/tools/private/release/create_release_issue_test.py new file mode 100644 index 0000000000..3fbd4454c7 --- /dev/null +++ b/tests/tools/private/release/create_release_issue_test.py @@ -0,0 +1,55 @@ +import argparse + +from dev.release.create_release_issue import CreateReleaseIssue + +pytest_plugins = ["tests.tools.private.release.release_test_helper"] + + +def test_create_release_issue_blocked_by_active_release(mock_gh): + args = argparse.Namespace(version="2.1.0") + mock_gh.create_issue( + title="Release 2.0.0", + body=""" +## Checklist +- [x] Prepare Release | status=done +- [ ] Tag Final +""", + labels=["type: release"], + ) + + result = CreateReleaseIssue(args, mock_gh).run() + + assert result == 1 + assert [i["title"] for i in mock_gh.issues.values()] == ["Release 2.0.0"] + + +def test_create_release_issue_ignores_complete_release(mock_gh, release_tool_env): + # A tagged-but-unclosed release issue must not block the next release. + args = argparse.Namespace(version="2.1.0") + complete_body = "- [x] Tag Final | status=done tag=2.0.0 commit= abcdef12\n" + complete_issue_num = mock_gh.create_issue( + title="Release 2.0.0", + body=complete_body, + labels=["type: release"], + ) + + result = CreateReleaseIssue(args, mock_gh).run() + + assert result == 0 + assert mock_gh.get_issue_body(complete_issue_num) == complete_body + titles = sorted(i["title"] for i in mock_gh.issues.values()) + assert titles == ["Release 2.0.0", "Release 2.1.0"] + new_issue = next( + i for i in mock_gh.issues.values() if i["title"] == "Release 2.1.0" + ) + assert "- [ ] Tag Final" in new_issue["body"] + assert "type: release" in new_issue["labels"] + + +def test_create_release_issue_no_open_issues(mock_gh, release_tool_env): + args = argparse.Namespace(version="2.1.0") + + result = CreateReleaseIssue(args, mock_gh).run() + + assert result == 0 + assert [i["title"] for i in mock_gh.issues.values()] == ["Release 2.1.0"] diff --git a/tests/tools/private/release/gh_test.py b/tests/tools/private/release/gh_test.py index 814bedb3dc..0be2d01b25 100644 --- a/tests/tools/private/release/gh_test.py +++ b/tests/tools/private/release/gh_test.py @@ -8,6 +8,7 @@ GetPrError, GitHub, InvalidPrRefError, + format_complete_issue_warning, ) from dev.release.git import Git @@ -221,3 +222,41 @@ def test_create_pr_generic_exception_raises_create_pr_error(gh, auto_patch_cmd_h exc_info.value ) assert exc_info.value.__cause__ is err + + +def test_partition_open_tracking_issues(mock_gh): + active_num = mock_gh.create_issue( + title="Release 2.1.0", + body="## Checklist\n- [ ] Prepare Release\n- [ ] Tag Final\n", + labels=["type: release"], + ) + complete_num = mock_gh.create_issue( + title="Release 2.0.1", + body="- [x] Tag Final | status=done tag=2.0.1 commit= abcdef12\n", + labels=["type: release"], + ) + # Not a release tracking issue; must be ignored entirely. + mock_gh.create_issue( + title="Backport #42", + body="- [x] Tag Final\n", + labels=["type: backport-pr"], + ) + + active, complete = mock_gh.partition_open_tracking_issues() + + assert [i["number"] for i in active] == [active_num] + assert [i["number"] for i in complete] == [complete_num] + + +def test_partition_open_tracking_issues_none_open(mock_gh): + assert mock_gh.partition_open_tracking_issues() == ([], []) + + +def test_format_complete_issue_warning(): + msg = format_complete_issue_warning({"number": 123, "title": "Release 2.0.1"}) + assert msg == ( + "Ignoring open release tracking issue #123 (Release 2.0.1): its 'Tag" + " Final' task is done, so the release is complete. Consider closing it." + ) + # Callers add the annotation prefix themselves. + assert not msg.startswith("::") diff --git a/tests/tools/private/release/sync_changelog_test.py b/tests/tools/private/release/sync_changelog_test.py index ee438a6533..2cdf202695 100644 --- a/tests/tools/private/release/sync_changelog_test.py +++ b/tests/tools/private/release/sync_changelog_test.py @@ -226,6 +226,77 @@ def test_sync_changelog_multiple_open_issues_fails(mock_git, mock_gh): mock_git.fetch.assert_not_called() +def test_sync_changelog_auto_discover_ignores_complete_release( + mocker, mock_git, mock_gh +): + # A tagged-but-unclosed release issue beside the active one must not + # trigger the "multiple open issues" error; the active one is used. + mock_process_news_class = mocker.patch("dev.release.sync_changelog.ProcessNews") + mock_process_news_instance = MagicMock() + mock_process_news_instance.run.return_value = 0 + mock_process_news_class.return_value = mock_process_news_instance + + args = argparse.Namespace( + issue=None, + remote="origin", + prs=None, + release_date=None, + ) + complete_body = """ +## Checklist +- [x] Sync Changelog #100 | status=done +- [x] Tag Final | status=done tag=1.9.1 commit= abcdef12 +""" + mock_gh.issues[122] = { + "number": 122, + "title": "Release 1.9.1", + "body": complete_body, + "labels": ["type: release"], + } + mock_gh.issues[123] = { + "number": 123, + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Sync Changelog #124 +- [ ] Tag Final +""", + "labels": ["type: release"], + } + mock_git.status.side_effect = ["", "M CHANGELOG.md"] + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 0 + assert ( + "- [ ] Sync Changelog #124 | status=pending pr=#1001" + in mock_gh.get_issue_body(123) + ) + assert mock_gh.get_issue_body(122) == complete_body + assert 122 not in mock_gh.issue_comments + + +def test_sync_changelog_auto_discover_only_complete_release_fails(mock_git, mock_gh): + args = argparse.Namespace( + issue=None, + remote="origin", + prs=None, + release_date=None, + ) + mock_gh.issues[122] = { + "number": 122, + "title": "Release 1.9.1", + "body": "- [x] Tag Final | status=done tag=1.9.1 commit= abcdef12\n", + "labels": ["type: release"], + } + + result = SyncChangelog(args, mock_git, mock_gh).run() + + assert result == 1 + mock_git.fetch.assert_not_called() + assert 122 not in mock_gh.issue_comments + + def test_sync_changelog_specific_prs_arg(mocker, mock_git, mock_gh): mock_process_news_class = mocker.patch("dev.release.sync_changelog.ProcessNews") mock_process_news_instance = MagicMock()