From 93d1366a34678750b2cd45a24a571635fe1ae35b Mon Sep 17 00:00:00 2001 From: Michelle Tran Date: Thu, 3 Oct 2024 21:19:06 -0400 Subject: [PATCH] Add ability to update existing GitHub Actions comment --- .github/workflows/ci.yml | 3 +- codecov_cli/commands/process_test_results.py | 155 ++++++++++---- codecov_cli/helpers/request.py | 7 + requirements.txt | 4 +- setup.py | 2 +- tests/commands/test_process_test_results.py | 200 ++++++++++++++++--- 6 files changed, 304 insertions(+), 67 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3119712a..2753a5d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -161,7 +161,8 @@ jobs: pattern: "*junit.xml" path: "test_results" merge-multiple: true + - name: Dogfooding codecov-cli if: ${{ !cancelled() && github.ref && contains(github.ref, 'pull') }} run: | - codecovcli process-test-results --provider-token ${{ secrets.GITHUB_TOKEN }} --dir test_results + codecovcli process-test-results --dir test_results --github-token ${{ secrets.GITHUB_TOKEN }} diff --git a/codecov_cli/commands/process_test_results.py b/codecov_cli/commands/process_test_results.py index f887c5b9..f9950b3a 100644 --- a/codecov_cli/commands/process_test_results.py +++ b/codecov_cli/commands/process_test_results.py @@ -1,8 +1,9 @@ +import json import logging import os import pathlib from dataclasses import dataclass -from typing import List +from typing import Any, Dict, List, Optional import click from test_results_parser import ( @@ -16,13 +17,17 @@ from codecov_cli.helpers.args import get_cli_args from codecov_cli.helpers.request import ( log_warnings_and_errors_if_any, + send_get_request, send_post_request, ) from codecov_cli.services.upload.file_finder import select_file_finder -from codecov_cli.types import CommandContext +from codecov_cli.types import CommandContext, RequestResult, UploadCollectionResultFile logger = logging.getLogger("codecovcli") +# Search marker so that we can find the comment when looking for previously created comments +CODECOV_SEARCH_MARKER = "" + _process_test_results_options = [ click.option( @@ -61,8 +66,8 @@ default=False, ), click.option( - "--provider-token", - help="Token used to make calls to Repo provider API", + "--github-token", + help="If specified, output the message to the specified GitHub PR.", type=str, default=None, ), @@ -92,65 +97,147 @@ def process_test_results( files=None, exclude_folders=None, disable_search=None, - provider_token=None, + github_token=None, ): - if provider_token is None: + file_finder = select_file_finder( + dir, exclude_folders, files, disable_search, report_type="test_results" + ) + + upload_collection_results: List[ + UploadCollectionResultFile + ] = file_finder.find_files() + if len(upload_collection_results) == 0: raise click.ClickException( - "Provider token was not provided. Make sure to pass --provider-token option with the contents of the GITHUB_TOKEN secret, so we can make a comment." + "No JUnit XML files were found. Make sure to specify them using the --file option." ) + payload: TestResultsNotificationPayload = generate_message_payload( + upload_collection_results + ) + + message: str = f"{build_message(payload)} {CODECOV_SEARCH_MARKER}" + + args: Dict[str, str] = get_cli_args(ctx) + + maybe_write_to_github_action(message, github_token, args) + + click.echo(message) + + +def maybe_write_to_github_action( + message: str, github_token: str, args: Dict[str, str] +) -> None: + if github_token is None: + # If no token is passed, then we will assume users are not running in a GitHub Action + return + + maybe_write_to_github_summary(message) + maybe_write_to_github_comment(message, github_token, args) + + +def maybe_write_to_github_summary(message: str) -> None: summary_file_path = os.getenv("GITHUB_STEP_SUMMARY") if summary_file_path is None: raise click.ClickException( - "Error getting step summary file path from environment. Can't find GITHUB_STEP_SUMMARY environment variable." + "Error getting step summary file path from environment. " + "Can't find GITHUB_STEP_SUMMARY environment variable." ) + # write to step summary file + with open(summary_file_path, "w") as f: + f.write(message) + + +def maybe_write_to_github_comment( + message: str, github_token: str, args: Dict[str, str] +) -> None: slug = os.getenv("GITHUB_REPOSITORY") if slug is None: raise click.ClickException( - "Error getting repo slug from environment. Can't find GITHUB_REPOSITORY environment variable." + "Error getting repo slug from environment. " + "Can't find GITHUB_REPOSITORY environment variable." ) ref = os.getenv("GITHUB_REF") if ref is None or "pull" not in ref: raise click.ClickException( - "Error getting PR number from environment. Can't find GITHUB_REF environment variable." + "Error getting PR number from environment. " + "Can't find GITHUB_REF environment variable." ) + # GITHUB_REF is documented here: https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables + pr_number = ref.split("/")[2] - file_finder = select_file_finder( - dir, exclude_folders, files, disable_search, report_type="test_results" + existing_comment = find_existing_github_comment(github_token, slug, pr_number) + comment_id = None + if existing_comment is not None: + comment_id = existing_comment.get("id") + + create_or_update_github_comment( + github_token, slug, pr_number, message, comment_id, args ) - upload_collection_results = file_finder.find_files() - if len(upload_collection_results) == 0: - raise click.ClickException( - "No JUnit XML files were found. Make sure to specify them using the --file option." - ) - payload = generate_message_payload(upload_collection_results) +def find_existing_github_comment( + github_token: str, repo_slug: str, pr_number: int +) -> Optional[Dict[str, Any]]: + url = f"https://api.github.com/repos/{repo_slug}/issues/{pr_number}/comments" + + headers = { + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {github_token}", + "X-GitHub-Api-Version": "2022-11-28", + } + page = 1 + + results = get_github_response_or_error(url, headers, page) + while results != []: + for comment in results: + comment_user = comment.get("user") + if ( + CODECOV_SEARCH_MARKER in comment.get("body", "") + and comment_user + and comment_user.get("login", "") == "github-actions[bot]" + ): + return comment - message = build_message(payload) + page += 1 + results = get_github_response_or_error(url, headers, page) - # write to step summary file - with open(summary_file_path, "w") as f: - f.write(message) + # No matches, return None + return None - # GITHUB_REF is documented here: https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables - pr_number = ref.split("/")[2] - args = get_cli_args(ctx) - create_github_comment(provider_token, slug, pr_number, message, args) +def get_github_response_or_error( + url: str, headers: Dict[str, str], page: int +) -> Dict[str, Any]: + request_results: RequestResult = send_get_request( + url, headers, params={"page": page} + ) + if request_results.status_code != 200: + raise click.ClickException("Cannot find existing GitHub comment for PR.") + results = json.loads(request_results.text) + return results -def create_github_comment(token, repo_slug, pr_number, message, args): - url = f"https://api.github.com/repos/{repo_slug}/issues/{pr_number}/comments" +def create_or_update_github_comment( + token: str, + repo_slug: str, + pr_number: str, + message: str, + comment_id: Optional[str], + args: Dict[str, Any], +) -> None: + if comment_id is not None: + url = f"https://api.github.com/repos/{repo_slug}/issues/comments/{comment_id}" + else: + url = f"https://api.github.com/repos/{repo_slug}/issues/{pr_number}/comments" headers = { "Accept": "application/vnd.github+json", "Authorization": f"Bearer {token}", "X-GitHub-Api-Version": "2022-11-28", } - logger.info("Posting github comment") + logger.info(f"Posting GitHub comment {comment_id}") log_warnings_and_errors_if_any( send_post_request( @@ -165,15 +252,15 @@ def create_github_comment(token, repo_slug, pr_number, message, args): ) -def generate_message_payload(upload_collection_results): +def generate_message_payload( + upload_collection_results: List[UploadCollectionResultFile], +) -> TestResultsNotificationPayload: payload = TestResultsNotificationPayload(failures=[]) for result in upload_collection_results: - testruns = [] try: - logger.info(f"Parsing {result.get_filename()}") - testruns = parse_junit_xml(result.get_content()) - for testrun in testruns: + parsed_info = parse_junit_xml(result.get_content()) + for testrun in parsed_info.testruns: if ( testrun.outcome == Outcome.Failure or testrun.outcome == Outcome.Error diff --git a/codecov_cli/helpers/request.py b/codecov_cli/helpers/request.py index dbafc9a3..6c153bd5 100644 --- a/codecov_cli/helpers/request.py +++ b/codecov_cli/helpers/request.py @@ -95,6 +95,13 @@ def send_post_request( return request_result(post(url=url, data=data, headers=headers, params=params)) +@retry_request +def send_get_request( + url: str, headers: dict = None, params: dict = None +) -> RequestResult: + return request_result(get(url=url, headers=headers, params=params)) + + def get_token_header_or_fail(token: str) -> dict: if token is None: raise click.ClickException( diff --git a/requirements.txt b/requirements.txt index d40822af..34d5de72 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.11 +# This file is autogenerated by pip-compile with Python 3.12 # by the following command: # # pip-compile setup.py @@ -43,7 +43,7 @@ sniffio==1.3.0 # anyio # httpcore # httpx -test-results-parser==0.1.0 +test-results-parser @ https://github.com/codecov/test-results-parser/archive/f35880f760ca05eb87bcc6893869c5f95ea9abd7.tar.gz # via codecov-cli (setup.py) tree-sitter==0.20.2 # via codecov-cli (setup.py) diff --git a/setup.py b/setup.py index 2102345e..4280b8ca 100644 --- a/setup.py +++ b/setup.py @@ -24,7 +24,7 @@ "pyyaml==6.*", "responses==0.21.*", "tree-sitter==0.20.*", - "test-results-parser==0.1.*", + "test-results-parser @ https://github.com/codecov/test-results-parser/archive/f35880f760ca05eb87bcc6893869c5f95ea9abd7.tar.gz#egg=test-results-parser", "regex", ], entry_points={ diff --git a/tests/commands/test_process_test_results.py b/tests/commands/test_process_test_results.py index 1baf73b2..6a713386 100644 --- a/tests/commands/test_process_test_results.py +++ b/tests/commands/test_process_test_results.py @@ -1,9 +1,8 @@ -import logging +import json import os from click.testing import CliRunner -from codecov_cli import __version__ from codecov_cli.main import cli from codecov_cli.types import RequestResult @@ -34,8 +33,6 @@ def test_process_test_results( cli, [ "process-test-results", - "--provider-token", - "whatever", "--file", "samples/junit.xml", "--disable-search", @@ -44,31 +41,178 @@ def test_process_test_results( ) assert result.exit_code == 0 + # Ensure that there's an output + assert result.output + + +def test_process_test_results_create_github_message( + mocker, + tmpdir, +): + + tmp_file = tmpdir.mkdir("folder").join("summary.txt") - mocked_post.assert_called_with( - url="https://api.github.com/repos/fake/repo/issues/pull/comments", - data={ - "body": "### :x: Failed Test Results: \nCompleted 4 tests with **`1 failed`**, 3 passed and 0 skipped.\n
View the full list of failed tests\n\n| **Test Description** | **Failure message** |\n| :-- | :-- |\n| 
Testsuite:
api.temp.calculator.test_calculator::test_divide

Test name:
pytest
| 
def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError
|", - "cli_args": { - "auto_load_params_from": None, - "codecov_yml_path": None, - "enterprise_url": None, - "verbose": False, - "version": f"cli-{__version__}", - "command": "process-test-results", - "provider_token": "whatever", - "disable_search": True, - "dir": os.getcwd(), - "exclude_folders": (), - }, + mocker.patch.dict( + os.environ, + { + "GITHUB_REPOSITORY": "fake/repo", + "GITHUB_REF": "pull/fake/123", + "GITHUB_STEP_SUMMARY": tmp_file.dirname + tmp_file.basename, }, - headers={ - "Accept": "application/vnd.github+json", - "Authorization": "Bearer whatever", - "X-GitHub-Api-Version": "2022-11-28", + ) + + mocker.patch( + "codecov_cli.commands.process_test_results.send_get_request", + return_value=RequestResult(status_code=200, error=None, warnings=[], text="[]"), + ) + + mocked_post = mocker.patch( + "codecov_cli.commands.process_test_results.send_post_request", + return_value=RequestResult( + status_code=200, error=None, warnings=[], text="yay it worked" + ), + ) + runner = CliRunner() + result = runner.invoke( + cli, + [ + "process-test-results", + "--github-token", + "fake-token", + "--file", + "samples/junit.xml", + "--disable-search", + ], + obj={}, + ) + + assert result.exit_code == 0 + assert ( + mocked_post.call_args.kwargs["url"] + == "https://api.github.com/repos/fake/repo/issues/123/comments" + ) + + +def test_process_test_results_update_github_message( + mocker, + tmpdir, +): + + tmp_file = tmpdir.mkdir("folder").join("summary.txt") + + mocker.patch.dict( + os.environ, + { + "GITHUB_REPOSITORY": "fake/repo", + "GITHUB_REF": "pull/fake/123", + "GITHUB_STEP_SUMMARY": tmp_file.dirname + tmp_file.basename, }, ) + github_fake_comments1 = [ + {"id": 54321, "user": {"login": "fake"}, "body": "some text"}, + ] + github_fake_comments2 = [ + { + "id": 12345, + "user": {"login": "github-actions[bot]"}, + "body": " and some other fake body", + }, + ] + + mocker.patch( + "codecov_cli.commands.process_test_results.send_get_request", + side_effect=[ + RequestResult( + status_code=200, + error=None, + warnings=[], + text=json.dumps(github_fake_comments1), + ), + RequestResult( + status_code=200, + error=None, + warnings=[], + text=json.dumps(github_fake_comments2), + ), + ], + ) + + mocked_post = mocker.patch( + "codecov_cli.commands.process_test_results.send_post_request", + return_value=RequestResult( + status_code=200, error=None, warnings=[], text="yay it worked" + ), + ) + runner = CliRunner() + result = runner.invoke( + cli, + [ + "process-test-results", + "--github-token", + "fake-token", + "--file", + "samples/junit.xml", + "--disable-search", + ], + obj={}, + ) + + assert result.exit_code == 0 + assert ( + mocked_post.call_args.kwargs["url"] + == "https://api.github.com/repos/fake/repo/issues/comments/12345" + ) + + +def test_process_test_results_errors_getting_comments( + mocker, + tmpdir, +): + + tmp_file = tmpdir.mkdir("folder").join("summary.txt") + + mocker.patch.dict( + os.environ, + { + "GITHUB_REPOSITORY": "fake/repo", + "GITHUB_REF": "pull/fake/123", + "GITHUB_STEP_SUMMARY": tmp_file.dirname + tmp_file.basename, + }, + ) + + mocker.patch( + "codecov_cli.commands.process_test_results.send_get_request", + return_value=RequestResult( + status_code=400, + error=None, + warnings=[], + text="", + ), + ) + + mocked_post = mocker.patch( + "codecov_cli.commands.process_test_results.send_post_request", + return_value=RequestResult( + status_code=200, error=None, warnings=[], text="yay it worked" + ), + ) + runner = CliRunner() + result = runner.invoke( + cli, + [ + "process-test-results", + "--github-token", + "fake-token", + "--file", + "samples/junit.xml", + "--disable-search", + ], + obj={}, + ) + + assert result.exit_code == 1 + def test_process_test_results_non_existent_file(mocker, tmpdir): tmp_file = tmpdir.mkdir("folder").join("summary.txt") @@ -92,8 +236,6 @@ def test_process_test_results_non_existent_file(mocker, tmpdir): cli, [ "process-test-results", - "--provider-token", - "whatever", "--file", "samples/fake.xml", "--disable-search", @@ -133,7 +275,7 @@ def test_process_test_results_missing_repo(mocker, tmpdir): cli, [ "process-test-results", - "--provider-token", + "--github-token", "whatever", "--file", "samples/junit.xml", @@ -175,7 +317,7 @@ def test_process_test_results_missing_ref(mocker, tmpdir): cli, [ "process-test-results", - "--provider-token", + "--github-token", "whatever", "--file", "samples/junit.xml", @@ -216,7 +358,7 @@ def test_process_test_results_missing_step_summary(mocker, tmpdir): cli, [ "process-test-results", - "--provider-token", + "--github-token", "whatever", "--file", "samples/junit.xml",