From 5703fdbbdc5de58b769c1748defba9501004c0e5 Mon Sep 17 00:00:00 2001 From: gaurav dhameeja Date: Tue, 17 Mar 2020 21:57:42 +0530 Subject: [PATCH] Fix-6911(pytest-bot): Added error from commands that are run Earlier pytest-bot would only print out the exception in cases of failure but did not provide context on failing command and error from command. This patch adds the errors from the command to the exception message. `Command` provides abstraction over the command to run and helps in collecting errors from the first failing command only. With this, we don't need to check `returncode` from each command that we run, we can run all the commands and will have access to the error from the first command that failed. This pattern was taken from Go. Please refer: https://blog.golang.org/errors-are-values --- scripts/release-on-comment.py | 59 +++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/scripts/release-on-comment.py b/scripts/release-on-comment.py index 90235fd55..5ed71f236 100644 --- a/scripts/release-on-comment.py +++ b/scripts/release-on-comment.py @@ -33,6 +33,9 @@ import sys from pathlib import Path from subprocess import check_call from subprocess import check_output +from subprocess import PIPE +from subprocess import run +from subprocess import STDOUT from textwrap import dedent from typing import Dict from typing import Optional @@ -91,6 +94,7 @@ def print_and_exit(msg) -> None: def trigger_release(payload_path: Path, token: str) -> None: + error_contents = "" # to be used to store error output in case any command fails payload, base_branch = validate_and_get_issue_comment_payload(payload_path) if base_branch is None: url = get_comment_data(payload)["html_url"] @@ -119,19 +123,42 @@ def trigger_release(payload_path: Path, token: str) -> None: release_branch = f"release-{version}" - check_call(["git", "config", "user.name", "pytest bot"]) - check_call(["git", "config", "user.email", "pytestbot@gmail.com"]) + run( + ["git", "config", "user.name", "pytest bot"], + text=True, + check=True, + capture_output=True, + ) + run( + ["git", "config", "user.email", "pytestbot@gmail.com"], + text=True, + check=True, + capture_output=True, + ) - check_call(["git", "checkout", "-b", release_branch, f"origin/{base_branch}"]) + run( + ["git", "checkout", "-b", release_branch, f"origin/{base_branch}"], + text=True, + check=True, + capture_output=True, + ) print(f"Branch {Fore.CYAN}{release_branch}{Fore.RESET} created.") - check_call( - [sys.executable, "scripts/release.py", version, "--skip-check-links"] + run( + [sys.executable, "scripts/release.py", version, "--skip-check-links"], + text=True, + check=True, + capture_output=True, ) oauth_url = f"https://{token}:x-oauth-basic@github.com/{SLUG}.git" - check_call(["git", "push", oauth_url, f"HEAD:{release_branch}", "--force"]) + run( + ["git", "push", oauth_url, f"HEAD:{release_branch}", "--force"], + text=True, + check=True, + capture_output=True, + ) print(f"Branch {Fore.CYAN}{release_branch}{Fore.RESET} pushed.") body = PR_BODY.format( @@ -151,7 +178,10 @@ def trigger_release(payload_path: Path, token: str) -> None: print(f"Notified in original comment {Fore.CYAN}{comment.url}{Fore.RESET}.") print(f"{Fore.GREEN}Success.") + except CallProcessError as e: + error_contents = e.output except Exception as e: + error_contents = str(e) link = f"https://github.com/{SLUG}/actions/runs/{os.environ['GITHUB_RUN_ID']}" issue.create_comment( dedent( @@ -168,6 +198,23 @@ def trigger_release(payload_path: Path, token: str) -> None: ) print_and_exit(f"{Fore.RED}{e}") + if error_contents: + link = f"https://github.com/{SLUG}/actions/runs/{os.environ['GITHUB_RUN_ID']}" + issue.create_comment( + dedent( + f""" + Sorry, the request to prepare release `{version}` from {base_branch} failed with: + + ``` + {error_contents} + ``` + + See: {link}. + """ + ) + ) + print_and_exit(f"{Fore.RED}{e}") + def find_next_version(base_branch: str) -> str: output = check_output(["git", "tag"], encoding="UTF-8")