From e17dbe864c65f981fc832ea5c44b5189beb52e94 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 15 Aug 2026 03:20:31 +0000 Subject: [PATCH 1/5] fix(release): surface subprocess stdout and stderr in backport errors Subprocess failures during `process-backports` swallowed stderr and stdout details because top-level error handling only printed the exception message without notes or traceback. Print tracebacks on unexpected errors and add a `format_exception` helper to include attached PEP 678 notes. --- .../private/release/process_backports_test.py | 64 +++++++++++++++++++ tests/tools/private/release/utils_test.py | 20 ++++++ tools/private/release/process_backports.py | 21 ++++-- tools/private/release/release.py | 6 +- tools/private/release/utils.py | 9 +++ 5 files changed, 109 insertions(+), 11 deletions(-) diff --git a/tests/tools/private/release/process_backports_test.py b/tests/tools/private/release/process_backports_test.py index 5c37e63a97..5e67c9932d 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -1,5 +1,6 @@ import argparse import datetime +import subprocess from unittest.mock import ANY, call from tools.private.release.process_backports import ProcessBackports @@ -491,3 +492,66 @@ def test_process_backports_version_sync_failure(mocker, mock_git, mock_gh): updated_body = mock_gh.get_issue_body(123) assert "- [ ] Sync Changelog #124 | status=pending pr=#1001" in updated_body assert "- [ ] Sync Changelog #125 | status=pending pr=#1001" in updated_body + + +def test_process_backports_sync_changelog_create_pr_failure( + mocker, mock_git, mock_gh, capsys +): + mocker.patch("tools.private.release.process_backports.changelog_news") + mocker.patch("tools.private.release.process_backports.replace_version_next") + mock_datetime = mocker.patch("tools.private.release.process_backports.datetime") + mock_datetime.date.today.return_value = datetime.date(2026, 7, 1) + + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + add=None, + triggering_comment=5297050431, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": """ +## Checklist +- [ ] Prepare Release +- [ ] Create Release branch +- [ ] Sync Changelog #124 +- [ ] Tag Final + +## Backports +- [ ] #124 | status=pending +""", + "labels": ["type: release"], + } + mock_gh.prs[124] = { + "state": "MERGED", + "mergeCommit": {"oid": "abcdef12"}, + } + mock_git.get_remote_tags.return_value = [] + mock_git.sort_commits_chronologically.return_value = ["abcdef12"] + mock_git.get_commit_sha.side_effect = ["12345678", "12345678", "main_sha"] + mock_git.get_commit_message.return_value = 'Cherry-pick "fix bug"' + mock_git.get_modified_files.return_value = ["news/124.fixed.md"] + mock_git.diff.return_value = "version diff for 124" + mock_git.apply_check.return_value = True + + # Make create_pr raise CalledProcessError with note attached (as shell.py does) + err = subprocess.CalledProcessError(1, ["gh", "pr", "create"]) + err.add_note( + "Error running command: gh pr create ...\nStdout: \nStderr: pull request" + " already exists" + ) + mocker.patch.object(mock_gh, "create_pr", side_effect=err) + + result = ProcessBackports(args, mock_git, mock_gh).run() + + assert result == 1 + assert mock_gh.reactions.get(5297050431) == ["-1"] + + captured = capsys.readouterr() + assert ( + "Unexpected error: Command '['gh', 'pr', 'create']' returned non-zero" + " exit status 1." in captured.out + ) + assert "Error running command: gh pr create ..." in captured.err + assert "Stderr: pull request already exists" in captured.err diff --git a/tests/tools/private/release/utils_test.py b/tests/tools/private/release/utils_test.py index 48abbad8d4..83faa7c00b 100644 --- a/tests/tools/private/release/utils_test.py +++ b/tests/tools/private/release/utils_test.py @@ -326,3 +326,23 @@ def test_determine_next_version_on_main_with_is_patch(mocker, release_tool_env): assert utils.determine_next_version(is_patch=False) == "1.3.0" # With is_patch=True, it produces a patch bump assert utils.determine_next_version(is_patch=True) == "1.2.4" + + +def test_format_exception_no_notes(): + e = ValueError("something went wrong") + assert utils.format_exception(e) == "something went wrong" + + +def test_format_exception_with_notes(): + e = RuntimeError("failed to execute") + e.add_note("Note 1: additional details") + e.add_note("Note 2: more info") + assert utils.format_exception(e) == ( + "failed to execute\nNote 1: additional details\nNote 2: more info" + ) + + +def test_format_exception_empty_message_with_notes(): + e = Exception() + e.add_note("Note only") + assert utils.format_exception(e) == "Note only" diff --git a/tools/private/release/process_backports.py b/tools/private/release/process_backports.py index 4ad846c0ac..0d8594eb8a 100644 --- a/tools/private/release/process_backports.py +++ b/tools/private/release/process_backports.py @@ -5,6 +5,7 @@ import hashlib import os import tempfile +import traceback from dataclasses import dataclass from typing import Any @@ -21,6 +22,7 @@ update_task_in_body, ) from tools.private.release.utils import ( + format_exception, get_latest_rc_tag, parse_pr_list, replace_version_next, @@ -88,7 +90,7 @@ def _process_pr_commit_infos( except Exception as e: print( f"ERROR: Failed to update tracking issue for" - f" unresolved PR {item.pr_ref}: {e}" + f" unresolved PR {item.pr_ref}: {format_exception(e)}" ) return shas, sha_to_item, failed_prs, ignored_prs, body @@ -149,7 +151,7 @@ def _cherry_pick_and_update_prs( successful_pr_nums.append(pr_num) except Exception as e: print( - f"Warning: Failed to resolve PR number for {item.pr_ref}: {e}" + f"Warning: Failed to resolve PR number for {item.pr_ref}: {format_exception(e)}" ) if not dry_run: @@ -171,7 +173,7 @@ def _cherry_pick_and_update_prs( except Exception as e: print( f"ERROR: Failed to update tracking issue for PR" - f" {item.pr_ref}: {e}" + f" {item.pr_ref}: {format_exception(e)}" ) print(f"Success: backported {item.pr_ref} / {sha} to {branch_name}") else: @@ -184,7 +186,9 @@ def _cherry_pick_and_update_prs( f" PR {item.pr_ref} to status=done" ) except Exception as e: - print(f"ERROR: Conflict or error on {sha}: {e}. Aborting.") + print( + f"ERROR: Conflict or error on {sha}: {format_exception(e)}. Aborting." + ) try: self.git.cherry_pick_abort() except Exception: @@ -216,7 +220,7 @@ def _cherry_pick_and_update_prs( except Exception as e: print( f"ERROR: Failed to update tracking issue for" - f" failed PR {item.pr_ref}: {e}" + f" failed PR {item.pr_ref}: {format_exception(e)}" ) return CherryPickAndUpdatePrsResult( failed_prs=failed_prs, @@ -349,7 +353,7 @@ def _sync_changelog_to_main( except Exception as e: print( f"Warning: Failed to update tracking issue or enable" - f" auto-merge: {e}" + f" auto-merge: {format_exception(e)}" ) finally: if args.dry_run: @@ -401,6 +405,7 @@ def run(self) -> int: exit_code = self._run_internal() except Exception as e: print(f"Unexpected error: {e}") + traceback.print_exc() exit_code = 1 if exit_code != 0 and args.triggering_comment: @@ -426,7 +431,9 @@ def _run_internal(self) -> int: pr_num = self.gh.resolve_pr_number(pr_ref) items_to_add.append({"ref": f"#{pr_num}"}) except Exception as e: - print(f"Warning: PR ref '{pr_ref}' is invalid: {e}") + print( + f"Warning: PR ref '{pr_ref}' is invalid: {format_exception(e)}" + ) items_to_add.append( { "ref": pr_ref, diff --git a/tools/private/release/release.py b/tools/private/release/release.py index 7f1896ecf4..9f868f5849 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -17,6 +17,7 @@ from tools.private.release.prepare import Prepare from tools.private.release.process_backports import ProcessBackports from tools.private.release.promote import Promote +from tools.private.release.utils import format_exception cmds = [ DetermineNextVersion, @@ -65,10 +66,7 @@ def main(): exit_code = args.command(args) except Exception as e: sys.stdout.flush() - print(f"Fatal error: {e}", file=sys.stderr) - if hasattr(e, "__notes__"): - for note in e.__notes__: - print(note, file=sys.stderr) + print(f"Fatal error: {format_exception(e)}", file=sys.stderr) sys.exit(1) sys.exit(exit_code if exit_code is not None else 0) diff --git a/tools/private/release/utils.py b/tools/private/release/utils.py index 244138062a..b0573d2af7 100644 --- a/tools/private/release/utils.py +++ b/tools/private/release/utils.py @@ -194,3 +194,12 @@ def set_github_output(name: str, value: str) -> None: if github_output := os.environ.get("GITHUB_OUTPUT"): with open(github_output, "a", encoding="utf-8") as f: f.write(f"{name}={value}\n") + + +def format_exception(e: BaseException) -> str: + """Formats an exception to a string, including any attached PEP 678 notes.""" + msg = str(e) + notes = getattr(e, "__notes__", None) + if not notes: + return msg + return "\n".join(filter(None, [msg] + [str(note) for note in notes])) From 69bf0040ee658afcb0847894e4f7d2b59f6077e6 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 15 Aug 2026 03:53:24 +0000 Subject: [PATCH 2/5] refactor(release): use logging instead of print in process_backports Release operations rely on structured logs for diagnostics and debugging. Configure logging to stderr with line numbers in the release CLI entry point and replace print calls in process_backports with logger methods. --- .../private/release/process_backports_test.py | 29 +- tests/tools/private/release/release_test.py | 11 + tools/private/release/process_backports.py | 264 +++++++++++------- tools/private/release/release.py | 6 + 4 files changed, 209 insertions(+), 101 deletions(-) diff --git a/tests/tools/private/release/process_backports_test.py b/tests/tools/private/release/process_backports_test.py index 5e67c9932d..96fee13e8c 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -1,5 +1,6 @@ import argparse import datetime +import logging import subprocess from unittest.mock import ANY, call @@ -495,7 +496,7 @@ def test_process_backports_version_sync_failure(mocker, mock_git, mock_gh): def test_process_backports_sync_changelog_create_pr_failure( - mocker, mock_git, mock_gh, capsys + mocker, mock_git, mock_gh, capsys, caplog ): mocker.patch("tools.private.release.process_backports.changelog_news") mocker.patch("tools.private.release.process_backports.replace_version_next") @@ -543,7 +544,8 @@ def test_process_backports_sync_changelog_create_pr_failure( ) mocker.patch.object(mock_gh, "create_pr", side_effect=err) - result = ProcessBackports(args, mock_git, mock_gh).run() + with caplog.at_level(logging.ERROR): + result = ProcessBackports(args, mock_git, mock_gh).run() assert result == 1 assert mock_gh.reactions.get(5297050431) == ["-1"] @@ -551,7 +553,28 @@ def test_process_backports_sync_changelog_create_pr_failure( captured = capsys.readouterr() assert ( "Unexpected error: Command '['gh', 'pr', 'create']' returned non-zero" - " exit status 1." in captured.out + " exit status 1." in caplog.text ) assert "Error running command: gh pr create ..." in captured.err assert "Stderr: pull request already exists" in captured.err + + +def test_process_backports_logs_no_pending(mock_git, mock_gh, caplog): + args = argparse.Namespace( + issue=123, + remote="origin", + dry_run=False, + add=None, + triggering_comment=None, + ) + mock_gh.issues[123] = { + "title": "Release 2.0.0", + "body": "No backports here", + "labels": ["type: release"], + } + + with caplog.at_level(logging.INFO): + result = ProcessBackports(args, mock_git, mock_gh).run() + + assert result == 0 + assert "No pending backports found." in caplog.text diff --git a/tests/tools/private/release/release_test.py b/tests/tools/private/release/release_test.py index 651af24c2f..f383c856fd 100644 --- a/tests/tools/private/release/release_test.py +++ b/tests/tools/private/release/release_test.py @@ -17,3 +17,14 @@ def test_invalid_version(): releaser.create_parser().parse_args(["prepare", "0.28"]) with pytest.raises(SystemExit): releaser.create_parser().parse_args(["prepare", "a.b.c"]) + + +def test_main_runs_command(mocker): + mocker.patch("sys.argv", ["release", "prepare", "0.28.0"]) + mock_cmd = mocker.patch( + "tools.private.release.prepare.Prepare.run_from_args", return_value=0 + ) + with pytest.raises(SystemExit) as exc_info: + releaser.main() + assert exc_info.value.code == 0 + mock_cmd.assert_called_once() diff --git a/tools/private/release/process_backports.py b/tools/private/release/process_backports.py index 0d8594eb8a..920e87d8ce 100644 --- a/tools/private/release/process_backports.py +++ b/tools/private/release/process_backports.py @@ -3,6 +3,7 @@ import argparse import datetime import hashlib +import logging import os import tempfile import traceback @@ -28,6 +29,8 @@ replace_version_next, ) +logger = logging.getLogger(__name__) + @dataclass class CherryPickAndUpdatePrsResult: @@ -64,20 +67,22 @@ def _process_pr_commit_infos( sha_to_item[sha] = item shas.append(sha) elif item.status in ("open-pr", "draft-pr"): - print(f"PR {item.pr_ref} is open or draft. Ignoring.") + logger.info("PR %s is open or draft. Ignoring.", item.pr_ref) ignored_prs.append(item.pr_ref) else: failed_prs.append(item.pr_ref) status_to_set = item.status or "error-unmerged-pr" if dry_run: - print( - f"[DRY RUN] Would update tracking issue checklist for" - f" unresolved PR {item.pr_ref} to status={status_to_set}" + logger.info( + "[DRY RUN] Would update tracking issue checklist for" + " unresolved PR %s to status=%s", + item.pr_ref, + status_to_set, ) else: - print( - f"Updating tracking issue checklist for unresolved PR" - f" {item.pr_ref}..." + logger.info( + "Updating tracking issue checklist for unresolved PR %s...", + item.pr_ref, ) try: body = update_task_in_body( @@ -88,9 +93,10 @@ def _process_pr_commit_infos( ) self.gh.update_issue_body(issue, body) except Exception as e: - print( - f"ERROR: Failed to update tracking issue for" - f" unresolved PR {item.pr_ref}: {format_exception(e)}" + logger.error( + "Failed to update tracking issue for unresolved PR %s: %s", + item.pr_ref, + format_exception(e), ) return shas, sha_to_item, failed_prs, ignored_prs, body @@ -112,7 +118,7 @@ def _cherry_pick_and_update_prs( collected_diffs = [] for sha in sorted_shas: item = sha_to_item[sha] - print(f"Cherry-picking {item.pr_ref} / {sha}...") + logger.info("Cherry-picking %s / %s...", item.pr_ref, sha) try: self.git.cherry_pick(sha) @@ -123,14 +129,17 @@ def _cherry_pick_and_update_prs( collected_news_files.append(f) # Replace version markers FIRST to isolate diff - print(f"Replacing version markers for PR {item.pr_ref}...") + logger.info("Replacing version markers for PR %s...", item.pr_ref) replace_version_next(version) # Get diff of unstaged changes (version marker replacement) diff_content = self.git.diff() # Perform news processing (merging news/ files into the changelog) - print(f"Merging news fragments into changelog for PR {item.pr_ref}...") + logger.info( + "Merging news fragments into changelog for PR %s...", + item.pr_ref, + ) release_date = datetime.date.today().strftime("%Y-%m-%d") changelog_news.update_changelog(version, release_date) @@ -139,7 +148,7 @@ def _cherry_pick_and_update_prs( # Amend cherry-pick commit to include news merging and deletions, # and reference the release tracking issue. - print(f"Amending cherry-pick commit for PR {item.pr_ref}...") + logger.info("Amending cherry-pick commit for PR %s...", item.pr_ref) current_msg = self.git.get_commit_message("HEAD") new_msg = f"{current_msg.strip()}\n\nWork towards #{issue}" self.git.commit(new_msg, amend=True) @@ -150,8 +159,10 @@ def _cherry_pick_and_update_prs( collected_diffs.append((pr_num, diff_content)) successful_pr_nums.append(pr_num) except Exception as e: - print( - f"Warning: Failed to resolve PR number for {item.pr_ref}: {format_exception(e)}" + logger.warning( + "Failed to resolve PR number for %s: %s", + item.pr_ref, + format_exception(e), ) if not dry_run: @@ -164,30 +175,43 @@ def _cherry_pick_and_update_prs( "rc": next_rc_suffix, "commit": new_sha, } - print(f"Updating tracking issue checklist for PR {item.pr_ref}...") + logger.info( + "Updating tracking issue checklist for PR %s...", + item.pr_ref, + ) try: body = update_task_in_body( body, item.pr_ref, checked=True, metadata=metadata ) self.gh.update_issue_body(issue, body) except Exception as e: - print( - f"ERROR: Failed to update tracking issue for PR" - f" {item.pr_ref}: {format_exception(e)}" + logger.error( + "Failed to update tracking issue for PR %s: %s", + item.pr_ref, + format_exception(e), ) - print(f"Success: backported {item.pr_ref} / {sha} to {branch_name}") + logger.info( + "Success: backported %s / %s to %s", + item.pr_ref, + sha, + branch_name, + ) else: - print( - f"[DRY RUN] Success: {item.pr_ref} / {sha} can be" - f" backported without error." + logger.info( + "[DRY RUN] Success: %s / %s can be backported without error.", + item.pr_ref, + sha, ) - print( - f"[DRY RUN] Would update tracking issue checklist for" - f" PR {item.pr_ref} to status=done" + logger.info( + "[DRY RUN] Would update tracking issue checklist for" + " PR %s to status=done", + item.pr_ref, ) except Exception as e: - print( - f"ERROR: Conflict or error on {sha}: {format_exception(e)}. Aborting." + logger.error( + "Conflict or error on %s: %s. Aborting.", + sha, + format_exception(e), ) try: self.git.cherry_pick_abort() @@ -196,14 +220,15 @@ def _cherry_pick_and_update_prs( failed_prs.append(item.pr_ref) if dry_run: - print( - f"[DRY RUN] Would update tracking issue checklist for" - f" failed PR {item.pr_ref} to status=error-merge-conflict" + logger.info( + "[DRY RUN] Would update tracking issue checklist for" + " failed PR %s to status=error-merge-conflict", + item.pr_ref, ) else: - print( - f"Updating tracking issue checklist for failed PR" - f" {item.pr_ref}..." + logger.info( + "Updating tracking issue checklist for failed PR %s...", + item.pr_ref, ) try: body = update_task_in_body( @@ -213,14 +238,16 @@ def _cherry_pick_and_update_prs( metadata={"status": "error-merge-conflict"}, ) self.gh.update_issue_body(issue, body) - print( - f"Updated back port of {item.pr_ref} to" - f" status=error-merge-conflict (unchecked)" + logger.info( + "Updated back port of %s to" + " status=error-merge-conflict (unchecked)", + item.pr_ref, ) except Exception as e: - print( - f"ERROR: Failed to update tracking issue for" - f" failed PR {item.pr_ref}: {format_exception(e)}" + logger.error( + "Failed to update tracking issue for failed PR %s: %s", + item.pr_ref, + format_exception(e), ) return CherryPickAndUpdatePrsResult( failed_prs=failed_prs, @@ -246,7 +273,11 @@ def _sync_changelog_to_main( main_branch = "main" backport_branch = f"prepare-{version}-backports-{prs_hash}" - print(f"Syncing changelog to {main_branch} via branch {backport_branch}...") + logger.info( + "Syncing changelog to %s via branch %s...", + main_branch, + backport_branch, + ) self.git.fetch(args.remote, refspec=main_branch) self.git.checkout(main_branch, track_remote=args.remote) @@ -255,8 +286,10 @@ def _sync_changelog_to_main( failed_version_sync_prs = [] try: if args.dry_run: - print( - f"[DRY RUN] Would create and checkout branch {backport_branch} from {main_branch}" + logger.info( + "[DRY RUN] Would create and checkout branch %s from %s", + backport_branch, + main_branch, ) else: if self.git.branch_exists(backport_branch): @@ -265,8 +298,9 @@ def _sync_changelog_to_main( else: self.git.checkout(backport_branch, create_branch=True) - print( - f"Updating CHANGELOG.md and removing news files on {backport_branch}..." + logger.info( + "Updating CHANGELOG.md and removing news files on %s...", + backport_branch, ) release_date = datetime.date.today().strftime("%Y-%m-%d") changelog_news.update_changelog( @@ -280,18 +314,26 @@ def _sync_changelog_to_main( failed_version_sync_prs = self._apply_version_marker_diffs(collected_diffs) if args.dry_run: - print( - f"[DRY RUN] Would commit: 'chore(release): sync changelog for v{version} backports'" + logger.info( + "[DRY RUN] Would commit: 'chore(release): sync changelog" + " for v%s backports'", + version, + ) + logger.info( + "[DRY RUN] Would push %s to %s", + backport_branch, + args.remote, ) - print(f"[DRY RUN] Would push {backport_branch} to {args.remote}") - print( - f"[DRY RUN] Would create PR to {main_branch} with label 'type: sync-changelog'" + logger.info( + "[DRY RUN] Would create PR to %s with label 'type: sync-changelog'", + main_branch, ) - print( - f"[DRY RUN] Would update tracking issue #{args.issue} checklist tasks 'Sync Changelog #' to PENDING" + logger.info( + "[DRY RUN] Would update tracking issue #%s checklist tasks" + " 'Sync Changelog #' to PENDING", + args.issue, ) - print("[DRY RUN] Diff of changes:") - print(self.git.status()) + logger.info("[DRY RUN] Diff of changes:\n%s", self.git.status()) else: self.git.add_modified_and_deleted() self.git.commit( @@ -321,23 +363,24 @@ def _sync_changelog_to_main( pr_body_lines.append(f"Release-Tracking-Issue: #{args.issue}") pr_body = "\n".join(pr_body_lines) - print(f"Creating PR to {main_branch}...") + logger.info("Creating PR to %s...", main_branch) pr_url = self.gh.create_pr( title=pr_title, body=pr_body, base=main_branch, labels=["type: sync-changelog"], ) - print(f"Created PR: {pr_url}") + logger.info("Created PR: %s", pr_url) try: pr_num = int(pr_url.split("/")[-1]) - print(f"Enabling auto-merge for PR #{pr_num}...") + logger.info("Enabling auto-merge for PR #%s...", pr_num) self.gh.enable_auto_merge(pr_num) - print( - f"Updating tracking issue #{args.issue} checklist with" - " Sync Changelog tasks..." + logger.info( + "Updating tracking issue #%s checklist with" + " Sync Changelog tasks...", + args.issue, ) issue_body = self.gh.get_issue_body(args.issue) for pr in successful_pr_nums: @@ -351,9 +394,9 @@ def _sync_changelog_to_main( ) self.gh.update_issue_body(args.issue, issue_body) except Exception as e: - print( - f"Warning: Failed to update tracking issue or enable" - f" auto-merge: {format_exception(e)}" + logger.warning( + "Failed to update tracking issue or enable auto-merge: %s", + format_exception(e), ) finally: if args.dry_run: @@ -371,11 +414,13 @@ def _apply_version_marker_diffs( return failed_version_sync_prs with tempfile.TemporaryDirectory() as temp_dir: - print(f"Applying {len(collected_diffs)} version marker patches...") + logger.info("Applying %d version marker patches...", len(collected_diffs)) for pr_num, diff_content in collected_diffs: if args.dry_run: - print( - f"[DRY RUN] Would check and apply version marker patch for PR #{pr_num}" + logger.info( + "[DRY RUN] Would check and apply version marker patch" + " for PR #%s", + pr_num, ) patch_filepath = os.path.join(temp_dir, f"{pr_num}.patch") @@ -384,15 +429,22 @@ def _apply_version_marker_diffs( if self.git.apply_check(patch_filepath): if args.dry_run: - print( - f"[DRY RUN] Version marker patch for PR #{pr_num} applies cleanly." + logger.info( + "[DRY RUN] Version marker patch for PR #%s applies" + " cleanly.", + pr_num, ) else: - print(f"Applying version marker patch for PR #{pr_num}...") + logger.info( + "Applying version marker patch for PR #%s...", + pr_num, + ) self.git.apply(patch_filepath) else: - print( - f"Warning: Version marker patch for PR #{pr_num} could not be applied cleanly to main. Skipping." + logger.warning( + "Version marker patch for PR #%s could not be applied" + " cleanly to main. Skipping.", + pr_num, ) failed_version_sync_prs.append(pr_num) return failed_version_sync_prs @@ -404,18 +456,21 @@ def run(self) -> int: try: exit_code = self._run_internal() except Exception as e: - print(f"Unexpected error: {e}") + logger.error("Unexpected error: %s", e) traceback.print_exc() exit_code = 1 if exit_code != 0 and args.triggering_comment: - print(f"Reacting with thumbs-down to comment {args.triggering_comment}...") + logger.info( + "Reacting with thumbs-down to comment %s...", + args.triggering_comment, + ) try: self.gh.add_comment_reaction( args.triggering_comment, GH_REACTION_THUMBS_DOWN ) except Exception as e: - print(f"Failed to add reaction to comment: {e}") + logger.error("Failed to add reaction to comment: %s", e) return exit_code @@ -431,8 +486,10 @@ def _run_internal(self) -> int: pr_num = self.gh.resolve_pr_number(pr_ref) items_to_add.append({"ref": f"#{pr_num}"}) except Exception as e: - print( - f"Warning: PR ref '{pr_ref}' is invalid: {format_exception(e)}" + logger.warning( + "PR ref '%s' is invalid: %s", + pr_ref, + format_exception(e), ) items_to_add.append( { @@ -441,7 +498,11 @@ def _run_internal(self) -> int: } ) - print(f"Adding backports {items_to_add} to tracking issue #{args.issue}...") + logger.info( + "Adding backports %s to tracking issue #%s...", + items_to_add, + args.issue, + ) try: body = add_backports_to_body(body, items_to_add) for item in items_to_add: @@ -460,25 +521,28 @@ def _run_internal(self) -> int: ) next_rc_num = max(rc_tags.keys()) + 1 if rc_tags else 0 if not has_pending_rc: - print( - f"No pending RC task found. Adding 'Tag" - f" RC{next_rc_num}' to checklist..." + logger.info( + "No pending RC task found. Adding 'Tag RC%s' to checklist...", + next_rc_num, ) body = add_rc_task_to_body(body, next_rc_num) except ValueError as e: - print(f"Error: {e}") + logger.error("Error: %s", e) return 1 if not args.dry_run: self.gh.update_issue_body(args.issue, body) - print("Successfully updated tracking issue checklist.") + logger.info("Successfully updated tracking issue checklist.") else: - print( + logger.info( "[DRY RUN] Would update tracking issue checklist with new" " backports." ) if not has_pending_rc: - print(f"[DRY RUN] Would add 'Tag RC{next_rc_num}' to checklist.") + logger.info( + "[DRY RUN] Would add 'Tag RC%s' to checklist.", + next_rc_num, + ) items = parse_backports(body) @@ -489,16 +553,16 @@ def _run_internal(self) -> int: ] if not pending_items: - print("No pending backports found.") + logger.info("No pending backports found.") return 0 - print(f"Found {len(pending_items)} pending backports to process.") + logger.info("Found %d pending backports to process.", len(pending_items)) # Determine branch name from issue title issue_title = self.gh.get_issue_title(args.issue) version_match = RELEASE_TITLE_RE.search(issue_title) if not version_match: - print(f"Error: Could not parse version from issue title: {issue_title}") + logger.error("Could not parse version from issue title: %s", issue_title) return 1 version = version_match.group(1) @@ -524,18 +588,18 @@ def _run_internal(self) -> int: ) if not shas: - print("No valid merge commits to process.") + logger.info("No valid merge commits to process.") if failed_prs: - print("Failed PRs:") + logger.error("Failed PRs:") for pr in failed_prs: - print(f"- {pr}") + logger.error("- %s", pr) return 1 return 0 # Verify workspace is clean before proceeding if self.git.status(): - print( - "ERROR: Git workspace is dirty. Please commit or stash changes" + logger.error( + "Git workspace is dirty. Please commit or stash changes" " before running backports." ) return 1 @@ -569,7 +633,11 @@ def _run_internal(self) -> int: body = result.body finally: if args.dry_run: - print(f"[DRY RUN] Resetting branch {branch_name} to {start_sha}") + logger.info( + "[DRY RUN] Resetting branch %s to %s", + branch_name, + start_sha, + ) self.git.reset_hard(reset_to=start_sha) if successful_pr_nums: @@ -582,15 +650,15 @@ def _run_internal(self) -> int: ) if failed_prs: - print("ERROR: One or more cherry-picks/resolutions failed:") + logger.error("One or more cherry-picks/resolutions failed:") for pr in failed_prs: - print(f"- {pr}") + logger.error("- %s", pr) return 1 if args.dry_run: - print("Dry run completed successfully. No errors found.") + logger.info("Dry run completed successfully. No errors found.") else: - print("All backports successfully processed!") + logger.info("All backports successfully processed!") return 0 @classmethod diff --git a/tools/private/release/release.py b/tools/private/release/release.py index 9f868f5849..416d9b54fd 100644 --- a/tools/private/release/release.py +++ b/tools/private/release/release.py @@ -1,6 +1,7 @@ """A tool to perform release steps.""" import argparse +import logging import os import sys @@ -53,6 +54,11 @@ def create_parser(): def main(): + logging.basicConfig( + format="%(levelname)s:%(filename)s:%(lineno)d: %(message)s", + level=logging.INFO, + stream=sys.stderr, + ) print(f"sys.argv: {sys.argv}") if "BUILD_WORKSPACE_DIRECTORY" in os.environ: os.chdir(os.environ["BUILD_WORKSPACE_DIRECTORY"]) From a88d5af6e9ba088ee3c43355638036cc2285dca2 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 15 Aug 2026 03:56:33 +0000 Subject: [PATCH 3/5] refactor(release): log cleanup actions in sync changelog finally block Branch checkouts and resets during error handling or dry runs in sync changelog were unlogged. Add info logging for resetting main and restoring the release branch. --- tools/private/release/process_backports.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/private/release/process_backports.py b/tools/private/release/process_backports.py index 920e87d8ce..ce1d54ee34 100644 --- a/tools/private/release/process_backports.py +++ b/tools/private/release/process_backports.py @@ -400,7 +400,16 @@ def _sync_changelog_to_main( ) finally: if args.dry_run: + logger.info( + "[DRY RUN] Resetting branch %s to %s after changelog sync dry run", + main_branch, + main_start_sha, + ) self.git.reset_hard(reset_to=main_start_sha) + logger.info( + "Restoring checkout of release branch %s after syncing changelog to main", + release_branch, + ) self.git.checkout(release_branch) def _apply_version_marker_diffs( From 255aa5f6b20c9d4ce520276edecf15d8127b5077 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 15 Aug 2026 04:10:00 +0000 Subject: [PATCH 4/5] feat(release): raise CreatePrError with stdout/stderr on PR creation failure Define a custom CreatePrError exception raised when GitHub PR creation fails, capturing the command, error details, and formatted stdout/stderr blocks. --- tests/tools/private/release/gh_test.py | 76 ++++++++++++++++++- .../private/release/process_backports_test.py | 16 ++-- tools/private/release/gh.py | 36 ++++++++- 3 files changed, 117 insertions(+), 11 deletions(-) diff --git a/tests/tools/private/release/gh_test.py b/tests/tools/private/release/gh_test.py index 7e19b2232c..4027edd6bd 100644 --- a/tests/tools/private/release/gh_test.py +++ b/tests/tools/private/release/gh_test.py @@ -1,7 +1,7 @@ import pytest from tools.private.release import shell -from tools.private.release.gh import GitHub +from tools.private.release.gh import CreatePrError, GitHub from tools.private.release.git import Git pytest_plugins = ["tests.tools.private.release.release_test_helper"] @@ -94,3 +94,77 @@ def mock_run(*args, **kwargs): gh.update_issue_body(123, "new body content") auto_patch_cmd_helpers.run_gh.assert_called_once() assert captured_body["content"] == "new body content" + + +def test_create_pr_success(gh, auto_patch_cmd_helpers): + auto_patch_cmd_helpers.run_gh.return_value = ( + "https://github.com/my-owner/my-repo/pull/123" + ) + url = gh.create_pr( + title="feat: my feature", + body="PR body", + base="main", + labels=["type: sync-changelog"], + ) + assert url == "https://github.com/my-owner/my-repo/pull/123" + auto_patch_cmd_helpers.run_gh.assert_called_with( + "pr", + "create", + "--title=feat: my feature", + "--body=PR body", + "--base=main", + "--label=type: sync-changelog", + "--repo=my-owner/my-repo", + check=True, + capture_output=True, + ) + + +def test_create_pr_failure_raises_create_pr_error(gh, auto_patch_cmd_helpers): + import subprocess + + err = subprocess.CalledProcessError( + 1, + ["gh", "pr", "create"], + output="my stdout", + stderr="pull request already exists", + ) + auto_patch_cmd_helpers.run_gh.side_effect = err + + with pytest.raises(CreatePrError) as exc_info: + gh.create_pr(title="feat: my feature", body="PR body") + + assert ( + "Failed to create PR 'feat: my feature': Command '['gh', 'pr'," + " 'create']' returned non-zero exit status 1." in str(exc_info.value) + ) + assert "==================== STDOUT BEGIN ====================" in str( + exc_info.value + ) + assert "my stdout" in str(exc_info.value) + assert "==================== STDOUT END ====================" in str(exc_info.value) + assert "==================== STDERR BEGIN ====================" in str( + exc_info.value + ) + assert "pull request already exists" in str(exc_info.value) + assert "==================== STDERR END ====================" in str(exc_info.value) + assert exc_info.value.__cause__ is err + + +def test_create_pr_empty_output_raises_create_pr_error(gh, auto_patch_cmd_helpers): + auto_patch_cmd_helpers.run_gh.return_value = "" + with pytest.raises(CreatePrError, match="gh pr create returned no output"): + gh.create_pr(title="feat: my feature", body="PR body") + + +def test_create_pr_generic_exception_raises_create_pr_error(gh, auto_patch_cmd_helpers): + err = RuntimeError("network disconnected") + auto_patch_cmd_helpers.run_gh.side_effect = err + + with pytest.raises(CreatePrError) as exc_info: + gh.create_pr(title="feat: my feature", body="PR body") + + assert "Failed to create PR 'feat: my feature': network disconnected" in str( + exc_info.value + ) + assert exc_info.value.__cause__ is err diff --git a/tests/tools/private/release/process_backports_test.py b/tests/tools/private/release/process_backports_test.py index 96fee13e8c..dd768400ba 100644 --- a/tests/tools/private/release/process_backports_test.py +++ b/tests/tools/private/release/process_backports_test.py @@ -1,9 +1,9 @@ import argparse import datetime import logging -import subprocess from unittest.mock import ANY, call +from tools.private.release.gh import CreatePrError from tools.private.release.process_backports import ProcessBackports pytest_plugins = ["tests.tools.private.release.release_test_helper"] @@ -536,11 +536,11 @@ def test_process_backports_sync_changelog_create_pr_failure( mock_git.diff.return_value = "version diff for 124" mock_git.apply_check.return_value = True - # Make create_pr raise CalledProcessError with note attached (as shell.py does) - err = subprocess.CalledProcessError(1, ["gh", "pr", "create"]) - err.add_note( - "Error running command: gh pr create ...\nStdout: \nStderr: pull request" - " already exists" + # Make create_pr raise CreatePrError + err = CreatePrError( + "Failed to create PR 'chore(release): sync changelog for v2.0.0 backports': " + "Command '['gh', 'pr', 'create']' returned non-zero exit status 1.\n" + "Error running command: gh pr create ...\nStdout: \nStderr: pull request already exists" ) mocker.patch.object(mock_gh, "create_pr", side_effect=err) @@ -552,8 +552,8 @@ def test_process_backports_sync_changelog_create_pr_failure( captured = capsys.readouterr() assert ( - "Unexpected error: Command '['gh', 'pr', 'create']' returned non-zero" - " exit status 1." in caplog.text + "Unexpected error: Failed to create PR 'chore(release): sync changelog for v2.0.0 backports'" + in caplog.text ) assert "Error running command: gh pr create ..." in captured.err assert "Stderr: pull request already exists" in captured.err diff --git a/tools/private/release/gh.py b/tools/private/release/gh.py index c21041384e..dbe644c29f 100644 --- a/tools/private/release/gh.py +++ b/tools/private/release/gh.py @@ -4,6 +4,7 @@ import json import os import re +import subprocess import tempfile from typing import TypedDict @@ -104,6 +105,12 @@ class NoTrackingIssueError(ValueError): pass +class CreatePrError(Exception): + """Raised when creating a pull request fails.""" + + pass + + class GitHub: """GitHub CLI helper class for the release tool.""" @@ -392,6 +399,9 @@ def create_pr( Returns: The URL of the created PR. + + Raises: + CreatePrError: If creating the pull request fails. """ cmd = [ "create", @@ -402,8 +412,30 @@ def create_pr( if labels: for label in labels: cmd.append(f"--label={label}") - output = self._gh_pr(*cmd) - return output if output else "" + try: + output = self._gh_pr(*cmd) + except subprocess.CalledProcessError as e: + msg = f"Failed to create PR '{title}': {e}" + if e.stdout: + msg += ( + f"\n{'=' * 20} STDOUT BEGIN {'=' * 20}\n" + f"{e.stdout}\n" + f"{'=' * 20} STDOUT END {'=' * 20}" + ) + if e.stderr: + msg += ( + f"\n{'=' * 20} STDERR BEGIN {'=' * 20}\n" + f"{e.stderr}\n" + f"{'=' * 20} STDERR END {'=' * 20}" + ) + raise CreatePrError(msg) from e + except Exception as e: + raise CreatePrError(f"Failed to create PR '{title}': {e}") from e + if not output: + raise CreatePrError( + f"Failed to create PR '{title}': gh pr create returned no output" + ) + return output def enable_auto_merge(self, pr_num: int, method: str = "squash") -> None: """Enables auto-merge for a PR. From 60fe86fedbf4ff5e4d66e1b76efa06a88a8ff3c5 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 15 Aug 2026 18:44:30 +0000 Subject: [PATCH 5/5] fix(release): check runfiles object for None in release_test_helper Ensure runfiles.Create() result is not None before accessing Rlocation, resolving Pyrefly type checking errors in CI. --- tests/tools/private/release/release_test_helper.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/tools/private/release/release_test_helper.py b/tests/tools/private/release/release_test_helper.py index c1d314360f..d7a8b3ada7 100644 --- a/tests/tools/private/release/release_test_helper.py +++ b/tests/tools/private/release/release_test_helper.py @@ -24,6 +24,8 @@ class ReleaseToolEnv: def _find_real_template_path() -> Path: r = runfiles.Create() + if r is None: + raise RuntimeError("Unable to create runfiles object") path = r.Rlocation( "rules_python/.github/ISSUE_TEMPLATE/release_tracking_template.md" )