From 5f55841982e023531dacc04ebf476cc065ef0f86 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 28 Jun 2026 21:08:26 +0000 Subject: [PATCH 1/5] fix(release): support tags without 'v' prefix in promote-rc and rc lookup Update promote-rc and get_latest_rc_tag to work with the new tag format that lacks a 'v' prefix. Add corresponding unit tests to verify the behavior. --- tests/tools/private/release/release_test.py | 135 +++++++++++++++++++- tools/private/release/release.py | 18 ++- 2 files changed, 142 insertions(+), 11 deletions(-) diff --git a/tests/tools/private/release/release_test.py b/tests/tools/private/release/release_test.py index 9f9cd2e801..121a8fc8e1 100644 --- a/tests/tools/private/release/release_test.py +++ b/tests/tools/private/release/release_test.py @@ -3,7 +3,7 @@ import shutil import tempfile import unittest -from unittest.mock import patch +from unittest.mock import MagicMock, patch from tools.private.release import changelog_news, release as releaser @@ -528,6 +528,33 @@ def test_get_latest_version_only_rc_tags(self, mock_get_tags): releaser.get_latest_version() +class GetLatestRcTagTest(unittest.TestCase): + @patch("tools.private.release.release.git.get_tags") + def test_get_latest_rc_tag_no_tags(self, mock_get_tags): + mock_get_tags.return_value = [] + self.assertIsNone(releaser.get_latest_rc_tag("2.0.0")) + + @patch("tools.private.release.release.git.get_tags") + def test_get_latest_rc_tag_no_matching_tags(self, mock_get_tags): + mock_get_tags.return_value = ["1.0.0", "2.0.0", "v2.0.0-rc0", "2.1.0-rc0"] + self.assertIsNone(releaser.get_latest_rc_tag("2.0.0")) + + @patch("tools.private.release.release.git.get_tags") + def test_get_latest_rc_tag_success(self, mock_get_tags): + mock_get_tags.return_value = [ + "2.0.0-rc0", + "2.0.0-rc2", + "2.0.0-rc1", + "2.1.0-rc0", + ] + self.assertEqual(releaser.get_latest_rc_tag("2.0.0"), "2.0.0-rc2") + + @patch("tools.private.release.release.git.get_tags") + def test_get_latest_rc_tag_ignores_v_prefix(self, mock_get_tags): + mock_get_tags.return_value = ["v2.0.0-rc0", "2.0.0-rc1"] + self.assertEqual(releaser.get_latest_rc_tag("2.0.0"), "2.0.0-rc1") + + class DetermineNextVersionTest(unittest.TestCase): def setUp(self): self.tmpdir = pathlib.Path(tempfile.mkdtemp()) @@ -648,5 +675,111 @@ def test_determine_next_version_on_main_branch_fallback(self, mock_get_branch): self.assertEqual(next_version, "1.2.4") +class CmdPromoteRcTest(unittest.TestCase): + def setUp(self): + self.mock_git = patch("tools.private.release.release.git").start() + self.mock_gh = patch("tools.private.release.release.gh").start() + self.addCleanup(patch.stopall) + + def test_promote_rc_success(self): + # Arrange + args = MagicMock(version="2.0.0", issue=123) + self.mock_git.get_tags.return_value = ["2.0.0-rc0", "2.0.0-rc1"] + self.mock_git.get_commit_sha.return_value = "abcdef123456" + self.mock_git.tag_exists.return_value = False + + initial_body = "- [ ] Tag Final" + self.mock_gh.get_issue_body.return_value = initial_body + + # Act + result = releaser.cmd_promote_rc(args) + + # Assert + self.assertEqual(result, 0) + self.mock_git.fetch.assert_called_once_with("--tags", "--force") + self.mock_git.checkout.assert_called_once_with("2.0.0-rc1") + self.mock_git.tag_exists.assert_called_once_with("2.0.0") + self.mock_git.tag.assert_called_once_with("2.0.0") + self.mock_git.push.assert_called_once_with("origin", "2.0.0") + + # Verify issue update + self.mock_gh.get_issue_body.assert_called_once_with(123) + expected_updated_body = ( + "- [x] Tag Final | status=done tag=2.0.0 commit=abcdef12" + ) + self.mock_gh.update_issue_body.assert_called_once_with( + 123, expected_updated_body + ) + + def test_promote_rc_defaults_to_determine_next_version(self): + # Arrange + args = MagicMock(version=None, issue=123) + self.mock_git.get_current_branch.return_value = "release/2.0" + self.mock_git.get_tags.return_value = ["2.0.0", "2.0.1-rc0"] + self.mock_git.get_commit_sha.return_value = "12345678" + self.mock_git.tag_exists.return_value = False + + initial_body = "- [ ] Tag Final" + self.mock_gh.get_issue_body.return_value = initial_body + + # Act + result = releaser.cmd_promote_rc(args) + + # Assert + self.assertEqual(result, 0) + self.mock_git.get_current_branch.assert_called_once() + self.assertTrue(self.mock_git.get_tags.call_count >= 2) + + self.mock_git.checkout.assert_called_once_with("2.0.1-rc0") + self.mock_git.tag.assert_called_once_with("2.0.1") + self.mock_git.push.assert_called_once_with("origin", "2.0.1") + + expected_updated_body = ( + "- [x] Tag Final | status=done tag=2.0.1 commit=12345678" + ) + self.mock_gh.update_issue_body.assert_called_once_with( + 123, expected_updated_body + ) + + def test_promote_rc_tag_already_exists(self): + # Arrange + args = MagicMock(version="2.0.0", issue=123) + self.mock_git.get_tags.return_value = ["2.0.0-rc1"] + self.mock_git.get_commit_sha.return_value = "abcdef123456" + self.mock_git.tag_exists.return_value = True + + initial_body = "- [ ] Tag Final" + self.mock_gh.get_issue_body.return_value = initial_body + + # Act + result = releaser.cmd_promote_rc(args) + + # Assert + self.assertEqual(result, 0) + self.mock_git.tag.assert_not_called() + self.mock_git.push.assert_not_called() + self.mock_gh.get_issue_body.assert_called_once_with(123) + expected_updated_body = ( + "- [x] Tag Final | status=done tag=2.0.0 commit=abcdef12" + ) + self.mock_gh.update_issue_body.assert_called_once_with( + 123, expected_updated_body + ) + + def test_promote_rc_no_rc_found(self): + # Arrange + args = MagicMock(version="2.0.0", issue=123) + self.mock_git.get_tags.return_value = [] + + # Act + result = releaser.cmd_promote_rc(args) + + # Assert + self.assertEqual(result, 1) + self.mock_git.checkout.assert_not_called() + self.mock_git.tag.assert_not_called() + self.mock_gh.get_issue_body.assert_not_called() + + if __name__ == "__main__": unittest.main() diff --git a/tools/private/release/release.py b/tools/private/release/release.py index 0e509f1bf3..ce40d11c65 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -76,7 +76,7 @@ def get_latest_version(): def get_latest_rc_tag(version): """Queries git tags and returns the highest RC tag for the version.""" tags = git.get_tags() - pattern = rf"^v{re.escape(version)}-rc\d+$" + pattern = rf"^{re.escape(version)}-rc\d+$" rc_tags = [tag.strip() for tag in tags if re.match(pattern, tag.strip())] if not rc_tags: return None @@ -770,25 +770,23 @@ def cmd_promote_rc(args): version = args.version if version is None: version = determine_next_version() - version = version.replace("v", "") - final_tag = f"v{version}" git.fetch("--tags", "--force") latest_rc = get_latest_rc_tag(version) if not latest_rc: - print(f"Error: No release candidate tags found matching v{version}-rc*") + print(f"Error: No release candidate tags found matching {version}-rc*") return 1 - print(f"Promoting {latest_rc} to final release {final_tag}...") + print(f"Promoting {latest_rc} to final release {version}...") git.checkout(latest_rc) commit_sha = git.get_commit_sha("HEAD") - if not git.tag_exists(final_tag): - git.tag(final_tag) - git.push("origin", final_tag) + if not git.tag_exists(version): + git.tag(version) + git.push("origin", version) else: - print(f"Final tag {final_tag} already exists.") + print(f"Final tag {version} already exists.") # Resolve issue number issue_num = args.issue @@ -801,7 +799,7 @@ def cmd_promote_rc(args): if issue_num: print(f"Updating tracking issue #{issue_num} checklist...") body = gh.get_issue_body(issue_num) - metadata = {"status": "done", "tag": final_tag, "commit": commit_sha[:8]} + metadata = {"status": "done", "tag": version, "commit": commit_sha[:8]} updated_body = update_task_in_body( body, "Tag Final", checked=True, metadata=metadata ) From 72a9a3f3dc7459f7a56bd9a17f4b08dc82e9d2c5 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 28 Jun 2026 21:26:42 +0000 Subject: [PATCH 2/5] refactor(release): tag commit SHA directly and enforce commit_ref in git.tag Update git.tag to require a commit_ref. Restructure promote-rc to verify pre-conditions before making modifications, and point the final tag directly to the RC's commit SHA. Update tests accordingly. --- tests/tools/private/release/release_test.py | 90 ++++++++++++++++----- tools/private/release/git.py | 6 +- tools/private/release/release.py | 62 ++++++++------ 3 files changed, 111 insertions(+), 47 deletions(-) diff --git a/tests/tools/private/release/release_test.py b/tests/tools/private/release/release_test.py index 121a8fc8e1..1057dea3af 100644 --- a/tests/tools/private/release/release_test.py +++ b/tests/tools/private/release/release_test.py @@ -687,7 +687,6 @@ def test_promote_rc_success(self): self.mock_git.get_tags.return_value = ["2.0.0-rc0", "2.0.0-rc1"] self.mock_git.get_commit_sha.return_value = "abcdef123456" self.mock_git.tag_exists.return_value = False - initial_body = "- [ ] Tag Final" self.mock_gh.get_issue_body.return_value = initial_body @@ -696,11 +695,12 @@ def test_promote_rc_success(self): # Assert self.assertEqual(result, 0) - self.mock_git.fetch.assert_called_once_with("--tags", "--force") - self.mock_git.checkout.assert_called_once_with("2.0.0-rc1") + self.mock_git.fetch.assert_called_once_with("upstream", tags=True, force=True) + self.mock_git.get_commit_sha.assert_called_once_with("2.0.0-rc1") + self.mock_git.checkout.assert_not_called() self.mock_git.tag_exists.assert_called_once_with("2.0.0") - self.mock_git.tag.assert_called_once_with("2.0.0") - self.mock_git.push.assert_called_once_with("origin", "2.0.0") + self.mock_git.tag.assert_called_once_with("2.0.0", "abcdef123456") + self.mock_git.push.assert_called_once_with("upstream", "2.0.0") # Verify issue update self.mock_gh.get_issue_body.assert_called_once_with(123) @@ -711,6 +711,28 @@ def test_promote_rc_success(self): 123, expected_updated_body ) + def test_promote_rc_resolve_issue_success(self): + # Arrange + args = MagicMock(version="2.0.0", issue=None) + self.mock_git.get_tags.return_value = ["2.0.0-rc1"] + self.mock_git.tag_exists.return_value = False + self.mock_gh.resolve_issue_number.return_value = 123 + self.mock_git.get_commit_sha.return_value = "abcdef123456" + initial_body = "- [ ] Tag Final" + self.mock_gh.get_issue_body.return_value = initial_body + + # Act + result = releaser.cmd_promote_rc(args) + + # Assert + self.assertEqual(result, 0) + self.mock_gh.resolve_issue_number.assert_called_once_with("2.0.0") + self.mock_git.get_commit_sha.assert_called_once_with("2.0.0-rc1") + self.mock_git.checkout.assert_not_called() + self.mock_git.tag.assert_called_once_with("2.0.0", "abcdef123456") + self.mock_git.push.assert_called_once_with("upstream", "2.0.0") + self.mock_gh.get_issue_body.assert_called_once_with(123) + def test_promote_rc_defaults_to_determine_next_version(self): # Arrange args = MagicMock(version=None, issue=123) @@ -718,7 +740,6 @@ def test_promote_rc_defaults_to_determine_next_version(self): self.mock_git.get_tags.return_value = ["2.0.0", "2.0.1-rc0"] self.mock_git.get_commit_sha.return_value = "12345678" self.mock_git.tag_exists.return_value = False - initial_body = "- [ ] Tag Final" self.mock_gh.get_issue_body.return_value = initial_body @@ -730,9 +751,10 @@ def test_promote_rc_defaults_to_determine_next_version(self): self.mock_git.get_current_branch.assert_called_once() self.assertTrue(self.mock_git.get_tags.call_count >= 2) - self.mock_git.checkout.assert_called_once_with("2.0.1-rc0") - self.mock_git.tag.assert_called_once_with("2.0.1") - self.mock_git.push.assert_called_once_with("origin", "2.0.1") + self.mock_git.checkout.assert_not_called() + self.mock_git.get_commit_sha.assert_called_once_with("2.0.1-rc0") + self.mock_git.tag.assert_called_once_with("2.0.1", "12345678") + self.mock_git.push.assert_called_once_with("upstream", "2.0.1") expected_updated_body = ( "- [x] Tag Final | status=done tag=2.0.1 commit=12345678" @@ -745,26 +767,56 @@ def test_promote_rc_tag_already_exists(self): # Arrange args = MagicMock(version="2.0.0", issue=123) self.mock_git.get_tags.return_value = ["2.0.0-rc1"] - self.mock_git.get_commit_sha.return_value = "abcdef123456" self.mock_git.tag_exists.return_value = True - initial_body = "- [ ] Tag Final" - self.mock_gh.get_issue_body.return_value = initial_body + # Act + result = releaser.cmd_promote_rc(args) + + # Assert + self.assertEqual(result, 1) + self.mock_git.checkout.assert_not_called() + self.mock_git.tag.assert_not_called() + self.mock_git.push.assert_not_called() + self.mock_gh.get_issue_body.assert_not_called() + self.mock_gh.update_issue_body.assert_not_called() + + def test_promote_rc_issue_not_found(self): + # Arrange + args = MagicMock(version="2.0.0", issue=None) + self.mock_git.get_tags.return_value = ["2.0.0-rc1"] + self.mock_git.tag_exists.return_value = False + self.mock_gh.resolve_issue_number.return_value = None # Act result = releaser.cmd_promote_rc(args) # Assert - self.assertEqual(result, 0) + self.assertEqual(result, 1) + self.mock_gh.resolve_issue_number.assert_called_once_with("2.0.0") + self.mock_git.checkout.assert_not_called() self.mock_git.tag.assert_not_called() self.mock_git.push.assert_not_called() + self.mock_gh.get_issue_body.assert_not_called() + + def test_promote_rc_issue_malformed(self): + # Arrange + args = MagicMock(version="2.0.0", issue=123) + self.mock_git.get_tags.return_value = ["2.0.0-rc1"] + self.mock_git.tag_exists.return_value = False + self.mock_git.get_commit_sha.return_value = "abcdef123456" + initial_body = "malformed body" + self.mock_gh.get_issue_body.return_value = initial_body + + # Act + result = releaser.cmd_promote_rc(args) + + # Assert + self.assertEqual(result, 1) self.mock_gh.get_issue_body.assert_called_once_with(123) - expected_updated_body = ( - "- [x] Tag Final | status=done tag=2.0.0 commit=abcdef12" - ) - self.mock_gh.update_issue_body.assert_called_once_with( - 123, expected_updated_body - ) + self.mock_git.checkout.assert_not_called() + self.mock_git.tag.assert_not_called() + self.mock_git.push.assert_not_called() + self.mock_gh.update_issue_body.assert_not_called() def test_promote_rc_no_rc_found(self): # Arrange diff --git a/tools/private/release/git.py b/tools/private/release/git.py index 9bfd905109..4e623e2f7a 100644 --- a/tools/private/release/git.py +++ b/tools/private/release/git.py @@ -59,9 +59,9 @@ def merge(commit_ref, ff_only=True): run_cmd(*cmd, capture_output=False) -def tag(tag_name): - """Creates a local tag pointing to HEAD.""" - run_cmd("git", "tag", tag_name, capture_output=False) +def tag(tag_name, commit_ref): + """Creates a local tag pointing to a specific commit.""" + run_cmd("git", "tag", tag_name, commit_ref, capture_output=False) def cherry_pick(sha): diff --git a/tools/private/release/release.py b/tools/private/release/release.py index ce40d11c65..e5846f5a3d 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -740,7 +740,7 @@ def cmd_create_rc(args): return 0 print(f"Tagging and pushing next RC: {next_rc}...") - git.tag(next_rc) + git.tag(next_rc, "HEAD") git.push("origin", next_rc) commit_sha = git.get_commit_sha("HEAD") @@ -771,47 +771,59 @@ def cmd_promote_rc(args): if version is None: version = determine_next_version() - git.fetch("--tags", "--force") + # Fetch from upstream to ensure we have the latest tags + git.fetch("upstream", tags=True, force=True) latest_rc = get_latest_rc_tag(version) if not latest_rc: print(f"Error: No release candidate tags found matching {version}-rc*") return 1 - print(f"Promoting {latest_rc} to final release {version}...") - git.checkout(latest_rc) - - commit_sha = git.get_commit_sha("HEAD") - - if not git.tag_exists(version): - git.tag(version) - git.push("origin", version) - else: - print(f"Final tag {version} already exists.") + # Verify final tag doesn't already exist + if git.tag_exists(version): + print(f"Error: Final tag {version} already exists.") + return 1 - # Resolve issue number + # Verify issue can be found issue_num = args.issue if not issue_num: try: issue_num = gh.resolve_issue_number(version) except Exception as e: - print(f"Warning: Could not query GitHub to find tracking issue: {e}") + print(f"Error: Could not query GitHub to find tracking issue: {e}") + return 1 + if not issue_num: + print(f"Error: Could not find tracking issue for version {version}.") + return 1 - if issue_num: - print(f"Updating tracking issue #{issue_num} checklist...") - body = gh.get_issue_body(issue_num) - metadata = {"status": "done", "tag": version, "commit": commit_sha[:8]} + # Get commit SHA of the RC tag (which will be the same for the final tag) + commit_sha = git.get_commit_sha(latest_rc) + + # Verify issue is in the right format by trying to prepare the update + print(f"Verifying tracking issue #{issue_num} format...") + body = gh.get_issue_body(issue_num) + metadata = {"status": "done", "tag": version, "commit": commit_sha[:8]} + try: updated_body = update_task_in_body( body, "Tag Final", checked=True, metadata=metadata ) - gh.update_issue_body(issue_num, updated_body) - print("Checklist updated successfully.") - return 0 - else: - print( - "Error: No active tracking issue found or specified. Checklist was not updated." - ) + except ValueError as e: + print(f"Error: Tracking issue #{issue_num} is malformed: {e}") return 1 + # All pre-conditions met, perform modifications + print( + f"Promoting {latest_rc} to final release {version} (commit" + f" {commit_sha[:8]}) using tracking issue #{issue_num}..." + ) + + # Tag the specific commit without checkout, and push to upstream + git.tag(version, commit_sha) + git.push("upstream", version) + + print(f"Updating tracking issue #{issue_num} checklist...") + gh.update_issue_body(issue_num, updated_body) + return 0 + def create_parser(): """Creates the argument parser with subcommands.""" From 6002678236e615e17d5cd377d949d601bba9d964 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sun, 28 Jun 2026 22:02:22 +0000 Subject: [PATCH 3/5] refactor(release): improve tracking issue resolution and add post-promotion comment Rename and optimize `resolve_issue_number` to search by title via `gh` using corrected repo/label globals. Post a comment on the tracking issue with release and BCR search links upon successful promotion, and use `shlex.join` for copy-paste friendly logging. --- tests/tools/private/release/release_test.py | 32 ++++++++++++-- tools/private/release/gh.py | 46 +++++++++++++-------- tools/private/release/release.py | 21 +++++++++- tools/private/release/utils.py | 5 ++- 4 files changed, 78 insertions(+), 26 deletions(-) diff --git a/tests/tools/private/release/release_test.py b/tests/tools/private/release/release_test.py index 1057dea3af..17a0504323 100644 --- a/tests/tools/private/release/release_test.py +++ b/tests/tools/private/release/release_test.py @@ -710,13 +710,19 @@ def test_promote_rc_success(self): self.mock_gh.update_issue_body.assert_called_once_with( 123, expected_updated_body ) + expected_comment = ( + "Version 2.0.0 has been tagged.\n\n" + "- **Release Page**: https://github.com/bazel-contrib/rules_python/releases/tag/2.0.0\n" + '- **BCR PR Search**: [is:pr ("bazel-contrib/rules_python" in:title) ("@2.0.0" in:title)](https://github.com/bazelbuild/bazel-central-registry/pulls?q=is%3Apr%20%28%22bazel-contrib/rules_python%22%20in%3Atitle%29%20%28%22%402.0.0%22%20in%3Atitle%29)' + ) + self.mock_gh.post_issue_comment.assert_called_once_with(123, expected_comment) def test_promote_rc_resolve_issue_success(self): # Arrange args = MagicMock(version="2.0.0", issue=None) self.mock_git.get_tags.return_value = ["2.0.0-rc1"] self.mock_git.tag_exists.return_value = False - self.mock_gh.resolve_issue_number.return_value = 123 + self.mock_gh.find_release_tracking_issue.return_value = 123 self.mock_git.get_commit_sha.return_value = "abcdef123456" initial_body = "- [ ] Tag Final" self.mock_gh.get_issue_body.return_value = initial_body @@ -726,12 +732,24 @@ def test_promote_rc_resolve_issue_success(self): # Assert self.assertEqual(result, 0) - self.mock_gh.resolve_issue_number.assert_called_once_with("2.0.0") + self.mock_gh.find_release_tracking_issue.assert_called_once_with("2.0.0") self.mock_git.get_commit_sha.assert_called_once_with("2.0.0-rc1") self.mock_git.checkout.assert_not_called() self.mock_git.tag.assert_called_once_with("2.0.0", "abcdef123456") self.mock_git.push.assert_called_once_with("upstream", "2.0.0") self.mock_gh.get_issue_body.assert_called_once_with(123) + expected_updated_body = ( + "- [x] Tag Final | status=done tag=2.0.0 commit=abcdef12" + ) + self.mock_gh.update_issue_body.assert_called_once_with( + 123, expected_updated_body + ) + expected_comment = ( + "Version 2.0.0 has been tagged.\n\n" + "- **Release Page**: https://github.com/bazel-contrib/rules_python/releases/tag/2.0.0\n" + '- **BCR PR Search**: [is:pr ("bazel-contrib/rules_python" in:title) ("@2.0.0" in:title)](https://github.com/bazelbuild/bazel-central-registry/pulls?q=is%3Apr%20%28%22bazel-contrib/rules_python%22%20in%3Atitle%29%20%28%22%402.0.0%22%20in%3Atitle%29)' + ) + self.mock_gh.post_issue_comment.assert_called_once_with(123, expected_comment) def test_promote_rc_defaults_to_determine_next_version(self): # Arrange @@ -762,6 +780,12 @@ def test_promote_rc_defaults_to_determine_next_version(self): self.mock_gh.update_issue_body.assert_called_once_with( 123, expected_updated_body ) + expected_comment = ( + "Version 2.0.1 has been tagged.\n\n" + "- **Release Page**: https://github.com/bazel-contrib/rules_python/releases/tag/2.0.1\n" + '- **BCR PR Search**: [is:pr ("bazel-contrib/rules_python" in:title) ("@2.0.1" in:title)](https://github.com/bazelbuild/bazel-central-registry/pulls?q=is%3Apr%20%28%22bazel-contrib/rules_python%22%20in%3Atitle%29%20%28%22%402.0.1%22%20in%3Atitle%29)' + ) + self.mock_gh.post_issue_comment.assert_called_once_with(123, expected_comment) def test_promote_rc_tag_already_exists(self): # Arrange @@ -785,14 +809,14 @@ def test_promote_rc_issue_not_found(self): args = MagicMock(version="2.0.0", issue=None) self.mock_git.get_tags.return_value = ["2.0.0-rc1"] self.mock_git.tag_exists.return_value = False - self.mock_gh.resolve_issue_number.return_value = None + self.mock_gh.find_release_tracking_issue.return_value = None # Act result = releaser.cmd_promote_rc(args) # Assert self.assertEqual(result, 1) - self.mock_gh.resolve_issue_number.assert_called_once_with("2.0.0") + self.mock_gh.find_release_tracking_issue.assert_called_once_with("2.0.0") self.mock_git.checkout.assert_not_called() self.mock_git.tag.assert_not_called() self.mock_git.push.assert_not_called() diff --git a/tools/private/release/gh.py b/tools/private/release/gh.py index 9fa94eee20..6721eda0d7 100644 --- a/tools/private/release/gh.py +++ b/tools/private/release/gh.py @@ -6,41 +6,51 @@ from tools.private.release.utils import run_cmd +_REPO = "bazel-contrib/rules_python" +_LABEL = "type: release" -def get_open_tracking_issues(): - """Returns a list of open tracking issues with the 'type:release' label.""" + +def get_open_tracking_issues(version): + """Returns a list of open tracking issues with the 'type: release' label.""" output = run_cmd( "gh", "issue", "list", - "--label=type:release", + f"--repo={_REPO}", + f"--label={_LABEL}", "--state=open", + f'--search="Release {version}" in:title', "--json=number,title,url", ) return json.loads(output) if output else [] -def resolve_issue_number(version): +def find_release_tracking_issue(version): """Resolves the tracking issue number for a given version. - Searches for an open issue with label 'type:release' and 'Release ' in the title. + Searches for an open issue with label 'type: release' and 'Release ' in the title. Raises ValueError if 0 or multiple issues are found. """ - matching_issues = [] - for issue in get_open_tracking_issues(): - if f"Release {version}" in issue["title"]: - matching_issues.append(issue) - - if not matching_issues: - raise ValueError(f"No open tracking issue found matching 'Release {version}'") - if len(matching_issues) > 1: - urls = [issue["url"] for issue in matching_issues] + matching_issues = get_open_tracking_issues(version) + + exact_matches = [] + for issue in matching_issues: + if issue["title"] == f"Release {version}": + exact_matches.append(issue) + + if not exact_matches: + raise ValueError( + f"No open tracking issue found matching 'Release {version}' " + f"in repo {_REPO} with label '{_LABEL}'" + ) + if len(exact_matches) > 1: + urls = [issue["url"] for issue in exact_matches] raise ValueError( - f"Multiple open tracking issues found for version {version}:\n" - + "\n".join(urls) + f"Multiple open tracking issues found for version {version} " + f"in repo {_REPO} with label '{_LABEL}':\n" + "\n".join(urls) ) - return matching_issues[0]["number"] + return exact_matches[0]["number"] def create_tracking_issue(version, template_content): @@ -63,7 +73,7 @@ def create_tracking_issue(version, template_content): "issue", "create", f"--title=Release {version}", - "--label=type:release", + f"--label={_LABEL}", f"--body-file={temp_path}", ) issue_url = output.strip() diff --git a/tools/private/release/release.py b/tools/private/release/release.py index e5846f5a3d..0aa2e68c9f 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -227,7 +227,10 @@ def update_task_in_body(body, task_name, checked, metadata): updated_lines.append(line) if not found: - raise ValueError(f"Task '{task_name}' not found in issue body.") + raise ValueError( + f"Task '{task_name}' not found in issue body. " + f"Expected format: '- [ ] {task_name}' or '- [x] {task_name}' (optionally followed by '| key=value')" + ) return "\n".join(updated_lines) @@ -787,7 +790,7 @@ def cmd_promote_rc(args): issue_num = args.issue if not issue_num: try: - issue_num = gh.resolve_issue_number(version) + issue_num = gh.find_release_tracking_issue(version) except Exception as e: print(f"Error: Could not query GitHub to find tracking issue: {e}") return 1 @@ -822,6 +825,20 @@ def cmd_promote_rc(args): print(f"Updating tracking issue #{issue_num} checklist...") gh.update_issue_body(issue_num, updated_body) + + print(f"Posting comment to tracking issue #{issue_num}...") + import urllib.parse + + release_url = f"{_REPO_URL}/releases/tag/{version}" + bcr_query = f'is:pr ("bazel-contrib/rules_python" in:title) ("@{version}" in:title)' + bcr_search_url = f"https://github.com/bazelbuild/bazel-central-registry/pulls?q={urllib.parse.quote(bcr_query)}" + comment_body = ( + f"Version {version} has been tagged.\n\n" + f"- **Release Page**: {release_url}\n" + f"- **BCR PR Search**: [{bcr_query}]({bcr_search_url})" + ) + gh.post_issue_comment(issue_num, comment_body) + return 0 diff --git a/tools/private/release/utils.py b/tools/private/release/utils.py index 5a8411b657..5eb7c43a71 100644 --- a/tools/private/release/utils.py +++ b/tools/private/release/utils.py @@ -1,5 +1,6 @@ """Utility functions for the release tool.""" +import shlex import subprocess @@ -10,7 +11,7 @@ def run_cmd(*args, check=True, capture_output=True): a detailed note explaining the failure to preserve the stack trace. """ cmd = [str(arg) for arg in args] - print(f"Running: {' '.join(cmd)}") + print(f"Running: {shlex.join(cmd)}") try: result = subprocess.run( cmd, @@ -21,7 +22,7 @@ def run_cmd(*args, check=True, capture_output=True): ) return result.stdout.strip() if capture_output else None except subprocess.CalledProcessError as e: - note = f"Error running command: {' '.join(cmd)}" + note = f"Error running command: {shlex.join(cmd)}" if capture_output: note += f"\nStdout: {e.stdout}\nStderr: {e.stderr}" e.add_note(note) From b92a94ec48693f80a41c53ee0bbec1d94c057c11 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 29 Jun 2026 03:23:28 +0000 Subject: [PATCH 4/5] chore(release): address PR feedback and refactor issue resolution Rename to `get_release_tracking_issue` with strict error raising, refactor commands to use it, and remove the 'v' prefix from RC tags. Factor out `list_issues` helper in `gh.py` and add missing unit tests. --- tests/tools/private/release/release_test.py | 179 +++++++++++++++++++- tools/private/release/gh.py | 35 ++-- tools/private/release/release.py | 32 ++-- 3 files changed, 214 insertions(+), 32 deletions(-) diff --git a/tests/tools/private/release/release_test.py b/tests/tools/private/release/release_test.py index 17a0504323..8be378f1ba 100644 --- a/tests/tools/private/release/release_test.py +++ b/tests/tools/private/release/release_test.py @@ -675,6 +675,177 @@ def test_determine_next_version_on_main_branch_fallback(self, mock_get_branch): self.assertEqual(next_version, "1.2.4") +class CmdPrepareTest(unittest.TestCase): + def setUp(self): + self.mock_git = patch("tools.private.release.release.git").start() + self.mock_gh = patch("tools.private.release.release.gh").start() + self.addCleanup(patch.stopall) + + @patch("tools.private.release.release.pathlib.Path") + @patch("tools.private.release.release.changelog_news") + @patch("tools.private.release.release.replace_version_next") + def test_prepare_success_existing_issue( + self, mock_replace, mock_changelog, mock_path + ): + # Arrange + args = MagicMock(version="2.0.0", issue=None) + self.mock_git.status.side_effect = ["", "M foo"] + self.mock_git.branch_exists.return_value = False + self.mock_gh.get_release_tracking_issue.return_value = 123 + self.mock_gh.create_pr.return_value = "https://github.com/foo/bar/pull/456" + self.mock_gh.get_issue_body.return_value = "- [ ] Prepare Release" + + # Act + result = releaser.cmd_prepare(args) + + # Assert + self.assertEqual(result, 0) + self.mock_gh.get_release_tracking_issue.assert_called_once_with("2.0.0") + self.mock_gh.create_tracking_issue.assert_not_called() + self.mock_gh.create_pr.assert_called_once_with("2.0.0", "prepare-2.0.0", 123) + + @patch("tools.private.release.release.pathlib.Path") + @patch("tools.private.release.release.changelog_news") + @patch("tools.private.release.release.replace_version_next") + def test_prepare_success_create_issue( + self, mock_replace, mock_changelog, mock_path + ): + # Arrange + args = MagicMock(version="2.0.0", issue=None) + self.mock_git.status.side_effect = ["", "M foo"] + self.mock_git.branch_exists.return_value = False + self.mock_gh.get_release_tracking_issue.side_effect = ValueError("Not found") + self.mock_gh.create_tracking_issue.return_value = 123 + self.mock_gh.create_pr.return_value = "https://github.com/foo/bar/pull/456" + self.mock_gh.get_issue_body.return_value = "- [ ] Prepare Release" + + mock_template = MagicMock() + mock_template.exists.return_value = True + mock_template.read_text.return_value = "template content" + mock_path.return_value = mock_template + + # Act + result = releaser.cmd_prepare(args) + + # Assert + self.assertEqual(result, 0) + self.mock_gh.get_release_tracking_issue.assert_called_once_with("2.0.0") + self.mock_gh.create_tracking_issue.assert_called_once_with( + "2.0.0", "template content" + ) + self.mock_gh.create_pr.assert_called_once_with("2.0.0", "prepare-2.0.0", 123) + + @patch("tools.private.release.release.pathlib.Path") + @patch("tools.private.release.release.changelog_news") + @patch("tools.private.release.release.replace_version_next") + def test_prepare_ambiguous_issue(self, mock_replace, mock_changelog, mock_path): + # Arrange + args = MagicMock(version="2.0.0", issue=None) + self.mock_git.status.side_effect = ["", "M foo"] + self.mock_git.branch_exists.return_value = False + self.mock_gh.get_release_tracking_issue.side_effect = ValueError( + "Multiple open tracking issues" + ) + + # Act + result = releaser.cmd_prepare(args) + + # Assert + self.assertEqual(result, 1) + self.mock_gh.get_release_tracking_issue.assert_called_once_with("2.0.0") + self.mock_gh.create_tracking_issue.assert_not_called() + self.mock_gh.create_pr.assert_not_called() + + +class CmdCreateRcTest(unittest.TestCase): + def setUp(self): + self.mock_git = patch("tools.private.release.release.git").start() + self.mock_gh = patch("tools.private.release.release.gh").start() + self.addCleanup(patch.stopall) + + def test_create_rc_success_first_rc(self): + # Arrange + args = MagicMock(issue=123) + self.mock_gh.get_issue_title.return_value = "Release 2.0.0" + self.mock_gh.get_issue_body.return_value = """ +## Checklist +- [x] Prepare Release | status=done pr=#122 commit=abcdef12 +- [x] Create Release branch | status=done branch=release/2.0 commit=abcdef12 +- [ ] Tag RC0 | status=pending +""" + self.mock_git.get_tags.return_value = [] + self.mock_git.get_tags_at_head.return_value = [] + self.mock_git.get_commit_sha.return_value = "1234567890" + + # Act + result = releaser.cmd_create_rc(args) + + # Assert + self.assertEqual(result, 0) + self.mock_git.tag.assert_called_once_with("2.0.0-rc0", "HEAD") + self.mock_git.push.assert_called_once_with("origin", "2.0.0-rc0") + + self.mock_gh.update_issue_body.assert_called_once() + call_args = self.mock_gh.update_issue_body.call_args[0] + self.assertEqual(call_args[0], 123) + self.assertIn("tag=2.0.0-rc0", call_args[1]) + self.assertIn("commit=12345678", call_args[1]) + + self.mock_gh.post_issue_comment.assert_called_once() + + def test_create_rc_success_next_rc(self): + # Arrange + args = MagicMock(issue=123) + self.mock_gh.get_issue_title.return_value = "Release 2.0.0" + self.mock_gh.get_issue_body.return_value = """ +## Checklist +- [x] Prepare Release | status=done pr=#122 commit=abcdef12 +- [x] Create Release branch | status=done branch=release/2.0 commit=abcdef12 +- [x] Tag RC0 | status=done tag=2.0.0-rc0 commit=abcdef12 +- [ ] Tag RC1 | status=pending +""" + self.mock_git.get_tags.return_value = ["2.0.0-rc0"] + self.mock_git.get_tags_at_head.return_value = [] + self.mock_git.get_commit_sha.return_value = "1234567890" + + # Act + result = releaser.cmd_create_rc(args) + + # Assert + self.assertEqual(result, 0) + self.mock_git.tag.assert_called_once_with("2.0.0-rc1", "HEAD") + self.mock_git.push.assert_called_once_with("origin", "2.0.0-rc1") + + self.mock_gh.update_issue_body.assert_called_once() + call_args = self.mock_gh.update_issue_body.call_args[0] + self.assertEqual(call_args[0], 123) + self.assertIn("tag=2.0.0-rc1", call_args[1]) + + self.mock_gh.post_issue_comment.assert_called_once() + + def test_create_rc_already_tagged(self): + # Arrange + args = MagicMock(issue=123) + self.mock_gh.get_issue_title.return_value = "Release 2.0.0" + self.mock_gh.get_issue_body.return_value = """ +## Checklist +- [x] Prepare Release | status=done pr=#122 commit=abcdef12 +- [x] Create Release branch | status=done branch=release/2.0 commit=abcdef12 +- [ ] Tag RC0 | status=pending +""" + self.mock_git.get_tags.return_value = [] + self.mock_git.get_tags_at_head.return_value = ["2.0.0-rc0"] + + # Act + result = releaser.cmd_create_rc(args) + + # Assert + self.assertEqual(result, 0) + self.mock_git.tag.assert_not_called() + self.mock_git.push.assert_not_called() + self.mock_gh.update_issue_body.assert_not_called() + + class CmdPromoteRcTest(unittest.TestCase): def setUp(self): self.mock_git = patch("tools.private.release.release.git").start() @@ -722,7 +893,7 @@ def test_promote_rc_resolve_issue_success(self): args = MagicMock(version="2.0.0", issue=None) self.mock_git.get_tags.return_value = ["2.0.0-rc1"] self.mock_git.tag_exists.return_value = False - self.mock_gh.find_release_tracking_issue.return_value = 123 + self.mock_gh.get_release_tracking_issue.return_value = 123 self.mock_git.get_commit_sha.return_value = "abcdef123456" initial_body = "- [ ] Tag Final" self.mock_gh.get_issue_body.return_value = initial_body @@ -732,7 +903,7 @@ def test_promote_rc_resolve_issue_success(self): # Assert self.assertEqual(result, 0) - self.mock_gh.find_release_tracking_issue.assert_called_once_with("2.0.0") + self.mock_gh.get_release_tracking_issue.assert_called_once_with("2.0.0") self.mock_git.get_commit_sha.assert_called_once_with("2.0.0-rc1") self.mock_git.checkout.assert_not_called() self.mock_git.tag.assert_called_once_with("2.0.0", "abcdef123456") @@ -809,14 +980,14 @@ def test_promote_rc_issue_not_found(self): args = MagicMock(version="2.0.0", issue=None) self.mock_git.get_tags.return_value = ["2.0.0-rc1"] self.mock_git.tag_exists.return_value = False - self.mock_gh.find_release_tracking_issue.return_value = None + self.mock_gh.get_release_tracking_issue.side_effect = ValueError("Not found") # Act result = releaser.cmd_promote_rc(args) # Assert self.assertEqual(result, 1) - self.mock_gh.find_release_tracking_issue.assert_called_once_with("2.0.0") + self.mock_gh.get_release_tracking_issue.assert_called_once_with("2.0.0") self.mock_git.checkout.assert_not_called() self.mock_git.tag.assert_not_called() self.mock_git.push.assert_not_called() diff --git a/tools/private/release/gh.py b/tools/private/release/gh.py index 6721eda0d7..09a0d3ae47 100644 --- a/tools/private/release/gh.py +++ b/tools/private/release/gh.py @@ -10,22 +10,33 @@ _LABEL = "type: release" -def get_open_tracking_issues(version): +def list_issues(*, fields, label=None, state=None, search=None): + """Helper to list issues using gh CLI.""" + cmd = ["gh", "issue", "list", f"--repo={_REPO}"] + if label: + cmd.append(f"--label={label}") + if state: + cmd.append(f"--state={state}") + if search: + cmd.append(f"--search={search}") + cmd.append(f"--json={fields}") + + output = run_cmd(*cmd) + return json.loads(output) if output else [] + + +def get_open_tracking_issues(version=None): """Returns a list of open tracking issues with the 'type: release' label.""" - output = run_cmd( - "gh", - "issue", - "list", - f"--repo={_REPO}", - f"--label={_LABEL}", - "--state=open", - f'--search="Release {version}" in:title', - "--json=number,title,url", + search = f'"Release {version}" in:title' if version else None + return list_issues( + label=_LABEL, + state="open", + search=search, + fields="number,title,url", ) - return json.loads(output) if output else [] -def find_release_tracking_issue(version): +def get_release_tracking_issue(version): """Resolves the tracking issue number for a given version. Searches for an open issue with label 'type: release' and 'Release ' in the title. diff --git a/tools/private/release/release.py b/tools/private/release/release.py index 0aa2e68c9f..5eb9d082ec 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -416,13 +416,13 @@ def cmd_prepare(args): issue_num = args.issue if not issue_num: - open_issues = gh.get_open_tracking_issues() - for issue in open_issues: - if f"Release {version}" in issue["title"]: - issue_num = issue["number"] - break - - if not issue_num: + try: + issue_num = gh.get_release_tracking_issue(version) + print(f"Found active tracking issue #{issue_num} for v{version}") + except ValueError as e: + if "Multiple open tracking issues" in str(e): + print(f"Error: {e}") + return 1 print( f"No active tracking issue found for v{version}. Creating a new one..." ) @@ -713,18 +713,18 @@ def cmd_create_rc(args): if not latest_rc: next_rc_num = 0 - next_rc = f"v{version}-rc0" + next_rc = f"{version}-rc0" else: rc_num = int(latest_rc.split("-rc")[-1]) next_rc_num = rc_num + 1 - next_rc = f"v{version}-rc{next_rc_num}" + next_rc = f"{version}-rc{next_rc_num}" # Precheck: next RC number must exist and be unchecked in the checklist rc_tags = state.get("rc_tags", {}) if next_rc_num not in rc_tags: print( f"Error: Checklist is missing required task 'Tag RC{next_rc_num}'" - f" to cut v{version}-rc{next_rc_num}." + f" to cut {version}-rc{next_rc_num}." ) return 1 @@ -738,7 +738,7 @@ def cmd_create_rc(args): # Verify HEAD is not already tagged git.checkout(branch_name) head_tags = git.get_tags_at_head() - if any(tag.startswith(f"v{version}-rc") for tag in head_tags): + if any(tag.startswith(f"{version}-rc") for tag in head_tags): print(f"HEAD of {branch_name} is already tagged with an RC. Skipping.") return 0 @@ -790,12 +790,12 @@ def cmd_promote_rc(args): issue_num = args.issue if not issue_num: try: - issue_num = gh.find_release_tracking_issue(version) - except Exception as e: - print(f"Error: Could not query GitHub to find tracking issue: {e}") + issue_num = gh.get_release_tracking_issue(version) + except ValueError as e: + print(f"Error: {e}") return 1 - if not issue_num: - print(f"Error: Could not find tracking issue for version {version}.") + except Exception as e: + print(f"Error: Unexpected error finding tracking issue: {e}") return 1 # Get commit SHA of the RC tag (which will be the same for the final tag) From 847e40e9c2dcfe54041cc26aabd7747580069aa1 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 29 Jun 2026 03:41:19 +0000 Subject: [PATCH 5/5] chore(release): add --dry-run flag to promote-rc command Default to dry-run=True to prevent accidental promotions. Update existing tests and add a new dry-run success test. --- tests/tools/private/release/release_test.py | 30 ++++++++++++++++++--- tools/private/release/release.py | 16 +++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/tests/tools/private/release/release_test.py b/tests/tools/private/release/release_test.py index 8be378f1ba..4ec7de53ba 100644 --- a/tests/tools/private/release/release_test.py +++ b/tests/tools/private/release/release_test.py @@ -854,7 +854,7 @@ def setUp(self): def test_promote_rc_success(self): # Arrange - args = MagicMock(version="2.0.0", issue=123) + args = MagicMock(version="2.0.0", issue=123, dry_run=False) self.mock_git.get_tags.return_value = ["2.0.0-rc0", "2.0.0-rc1"] self.mock_git.get_commit_sha.return_value = "abcdef123456" self.mock_git.tag_exists.return_value = False @@ -890,7 +890,7 @@ def test_promote_rc_success(self): def test_promote_rc_resolve_issue_success(self): # Arrange - args = MagicMock(version="2.0.0", issue=None) + args = MagicMock(version="2.0.0", issue=None, dry_run=False) self.mock_git.get_tags.return_value = ["2.0.0-rc1"] self.mock_git.tag_exists.return_value = False self.mock_gh.get_release_tracking_issue.return_value = 123 @@ -924,7 +924,7 @@ def test_promote_rc_resolve_issue_success(self): def test_promote_rc_defaults_to_determine_next_version(self): # Arrange - args = MagicMock(version=None, issue=123) + args = MagicMock(version=None, issue=123, dry_run=False) self.mock_git.get_current_branch.return_value = "release/2.0" self.mock_git.get_tags.return_value = ["2.0.0", "2.0.1-rc0"] self.mock_git.get_commit_sha.return_value = "12345678" @@ -958,6 +958,30 @@ def test_promote_rc_defaults_to_determine_next_version(self): ) self.mock_gh.post_issue_comment.assert_called_once_with(123, expected_comment) + def test_promote_rc_dry_run_success(self): + # Arrange + args = MagicMock(version="2.0.0", issue=123, dry_run=True) + self.mock_git.get_tags.return_value = ["2.0.0-rc0", "2.0.0-rc1"] + self.mock_git.get_commit_sha.return_value = "abcdef123456" + self.mock_git.tag_exists.return_value = False + initial_body = "- [ ] Tag Final" + self.mock_gh.get_issue_body.return_value = initial_body + + # Act + result = releaser.cmd_promote_rc(args) + + # Assert + self.assertEqual(result, 0) + self.mock_git.fetch.assert_called_once_with("upstream", tags=True, force=True) + self.mock_git.get_commit_sha.assert_called_once_with("2.0.0-rc1") + self.mock_git.tag_exists.assert_called_once_with("2.0.0") + + # Core dry-run assertions: NO modifications + self.mock_git.tag.assert_not_called() + self.mock_git.push.assert_not_called() + self.mock_gh.update_issue_body.assert_not_called() + self.mock_gh.post_issue_comment.assert_not_called() + def test_promote_rc_tag_already_exists(self): # Arrange args = MagicMock(version="2.0.0", issue=123) diff --git a/tools/private/release/release.py b/tools/private/release/release.py index 5eb9d082ec..3c2eed82ff 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -814,6 +814,16 @@ def cmd_promote_rc(args): return 1 # All pre-conditions met, perform modifications + if args.dry_run: + print( + f"[DRY RUN] Pre-conditions passed successfully for promoting {latest_rc} to {version}." + ) + print(f"[DRY RUN] Would tag commit {commit_sha[:8]} as {version}") + print(f"[DRY RUN] Would push tag {version} to upstream") + print(f"[DRY RUN] Would update tracking issue #{issue_num} checklist") + print(f"[DRY RUN] Would post comment to tracking issue #{issue_num}") + return 0 + print( f"Promoting {latest_rc} to final release {version} (commit" f" {commit_sha[:8]}) using tracking issue #{issue_num}..." @@ -951,6 +961,12 @@ def create_parser(): type=int, help="The tracking issue number (optional).", ) + promote_parser.add_argument( + "--dry-run", + action=argparse.BooleanOptionalAction, + default=True, + help="Perform a dry run (default: True). Use --no-dry-run to actually execute.", + ) return parser