From 7e9f3bf171f2bf9162b318ef0113ce0ef463cbc8 Mon Sep 17 00:00:00 2001 From: matt-codecov <137832199+matt-codecov@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:28:53 -0800 Subject: [PATCH 01/16] fix: remove team plan gate from patch coverage base fix (#266) --- tasks/notify.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tasks/notify.py b/tasks/notify.py index 8ccfbf7b5..49934ee71 100644 --- a/tasks/notify.py +++ b/tasks/notify.py @@ -377,15 +377,8 @@ async def submit_third_party_notifications( # doesn't exist in our database, the next-oldest commit that does. That # is unnecessary/incorrect for patch coverage, for which we want to # compare against the original PR base. - # - # For now, fix this for the patch-coverage-focused team plan and avoid - # maybe perturbing anything for project coverage. For other plans, set - # `patch_coverage_base_commitid` to the adjusted base commitid. - # Follow-up: https://github.com/codecov/engineering-team/issues/887 - plan = commit.repository.owner.plan pull = enriched_pull.database_pull if enriched_pull else None - - if pull and plan in (BillingPlan.team_monthly, BillingPlan.team_yearly): + if pull: patch_coverage_base_commitid = pull.base elif base_commit is not None: patch_coverage_base_commitid = base_commit.commitid From 81be95728b733e7aa1c076e0f3825877c536f13e Mon Sep 17 00:00:00 2001 From: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Date: Tue, 13 Feb 2024 08:51:17 -0500 Subject: [PATCH 02/16] build: upload test results (#245) * build: upload test results * build: remove print workflow Signed-off-by: joseph-sentry --- .github/workflows/ci.yml | 2 +- Makefile | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1642e1b41..6cf48716c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,7 +51,7 @@ jobs: test: name: Test needs: [build] - uses: codecov/gha-workflows/.github/workflows/run-tests.yml@v1.2.12 + uses: codecov/gha-workflows/.github/workflows/run-tests.yml@v1.2.15 secrets: inherit with: repo: ${{ vars.CODECOV_IMAGE_V2 || 'codecov/self-hosted-worker' }} diff --git a/Makefile b/Makefile index 4ed5b137f..4b25f51c4 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,7 @@ export WORKER_DOCKER_VERSION=${VERSION} export CODECOV_TOKEN=${CODECOV_UPLOAD_TOKEN} # Codecov CLI version to use -CODECOV_CLI_VERSION := 0.4.1 +CODECOV_CLI_VERSION := 0.4.6 build: $(MAKE) build.requirements @@ -48,13 +48,13 @@ lint: make lint.run test: - python -m pytest --cov=./ + python -m pytest --cov=./ --junitxml=junit.xml test.unit: - python -m pytest --cov=./ -m "not integration" --cov-report=xml:unit.coverage.xml + python -m pytest --cov=./ -m "not integration" --cov-report=xml:unit.coverage.xml --junitxml=unit.junit.xml test.integration: - python -m pytest --cov=./ -m "integration" --cov-report=xml:integration.coverage.xml + python -m pytest --cov=./ -m "integration" --cov-report=xml:integration.coverage.xml --junitxml=integration.junit.xml update-requirements: @@ -220,12 +220,16 @@ test_env.run_integration: test_env.upload: docker-compose exec worker make test_env.container_upload CODECOV_UPLOAD_TOKEN=${CODECOV_UPLOAD_TOKEN} CODECOV_URL=${CODECOV_URL} + docker-compose exec worker make test_env.container_upload_test_results CODECOV_UPLOAD_TOKEN=${CODECOV_UPLOAD_TOKEN} CODECOV_URL=${CODECOV_URL} test_env.container_upload: codecovcli -u ${CODECOV_URL} do-upload --flag latest-uploader-overall codecovcli -u ${CODECOV_URL} do-upload --flag unit --file unit.coverage.xml codecovcli -u ${CODECOV_URL} do-upload --flag integration --file integration.coverage.xml +test_env.container_upload_test_results: + codecovcli -v -u ${CODECOV_URL} do-upload --report-type test_results || true + test_env.static_analysis: docker-compose exec worker make test_env.container_static_analysis CODECOV_STATIC_TOKEN=${CODECOV_STATIC_TOKEN} From 02d48d73084824e49534ef9105e3a13cbc276ee2 Mon Sep 17 00:00:00 2001 From: daniel-codecov <159859649+daniel-codecov@users.noreply.github.com> Date: Thu, 15 Feb 2024 10:05:50 -0500 Subject: [PATCH 03/16] chore: add commit id to logs (#267) * add commit id to logs * lint --- services/report/raw_upload_processor.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/services/report/raw_upload_processor.py b/services/report/raw_upload_processor.py index b015bfae5..823f9b1ec 100644 --- a/services/report/raw_upload_processor.py +++ b/services/report/raw_upload_processor.py @@ -336,6 +336,8 @@ def _adjust_sessions( for f in flags_under_carryforward_rules if f not in to_partially_overwrite_flags ] + if upload is not None: + commit_id = upload.report.commit_id if upload is None and to_partially_overwrite_flags: log.warning("Upload is None, but there are partial_overwrite_flags present") if ( @@ -358,23 +360,36 @@ def _adjust_sessions( session_ids_to_partially_delete.append(sess_id) actually_fully_deleted_sessions = set() if session_ids_to_fully_delete: + extra = dict( + deleted_sessions=session_ids_to_fully_delete, + ) + if upload is not None: + extra["commit_id"] = commit_id log.info( "Deleted multiple sessions due to carriedforward overwrite", - extra=dict(deleted_sessions=session_ids_to_fully_delete), + extra=extra, ) original_report.delete_multiple_sessions(session_ids_to_fully_delete) actually_fully_deleted_sessions.update(session_ids_to_fully_delete) if session_ids_to_partially_delete: + extra = dict( + deleted_sessions=session_ids_to_partially_delete, + ) + if upload is not None: + extra["commit_id"] = commit_id log.info( "Partially deleting sessions due to label carryforward overwrite", - extra=dict(deleted_sessions=session_ids_to_partially_delete), + extra=extra, ) all_labels = get_all_report_labels(to_merge_report) original_report.delete_labels(session_ids_to_partially_delete, all_labels) for s in session_ids_to_partially_delete: labels_now = get_labels_per_session(original_report, s) if not labels_now: - log.info("Session has now no new labels, deleting whole session") + log.info( + "Session has now no new labels, deleting whole session", + extra=dict(commit_id=commit_id) if upload is not None else dict(), + ) actually_fully_deleted_sessions.add(s) original_report.delete_session(s) return SessionAdjustmentResult( From 5aa7d03280fec198f12811557d324dc954ed1db1 Mon Sep 17 00:00:00 2001 From: Trent Schmidt Date: Thu, 15 Feb 2024 10:46:12 -0500 Subject: [PATCH 04/16] Update bots.py (#268) Removing warn that is part of normal app flow. --- services/bots.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/services/bots.py b/services/bots.py index 823f1b0cd..8952824d1 100644 --- a/services/bots.py +++ b/services/bots.py @@ -56,14 +56,6 @@ def get_owner_installation_id( if owner.integration_id and deprecated_using_integration: return owner.integration_id # DEPRECATED FLOW - end - log.warning( - "(owner has no ghapp installation AND no integration_id) OR not using integration", - extra=dict( - repoid=(repository.repoid if repository else "no_repo"), - ownerid=owner.ownerid, - using_integration=deprecated_using_integration, - ), - ) return None From d3fb7f8758871fc90071b8ec936bdd8fcba1b140 Mon Sep 17 00:00:00 2001 From: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:05:02 -0500 Subject: [PATCH 05/16] Add metrics for test results ingestion using Sentry (#253) * deps: bump sentry_sdk version * feat(test_results): add metrics using sentry_sdk * test: small fix to tests * test: fix tests * test: fix tests Signed-off-by: joseph-sentry --- requirements.in | 2 +- requirements.txt | 2 +- tasks/test_results_finisher.py | 27 +++- tasks/test_results_processor.py | 111 +++++++++------ .../tests/unit/test_test_results_finisher.py | 81 ++++++++++- .../unit/test_test_results_processor_task.py | 127 +++++++++++++++++- 6 files changed, 292 insertions(+), 58 deletions(-) diff --git a/requirements.in b/requirements.in index 93ca36f93..9d00a5636 100644 --- a/requirements.in +++ b/requirements.in @@ -29,7 +29,7 @@ PyYAML redis>=4.4.4 requests>=2.31.0 respx -sentry-sdk +sentry-sdk>=1.40.0 SQLAlchemy-Utils SQLAlchemy statsd diff --git a/requirements.txt b/requirements.txt index 4a34827af..6e09699f5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -342,7 +342,7 @@ s3transfer==0.3.4 # via boto3 screen==1.0.1 # via excel-base -sentry-sdk==1.19.1 +sentry-sdk==1.40.0 # via -r requirements.in shared @ https://github.com/codecov/shared/archive/55c9af294031d3027fed9b49a83153cacf437bdb.tar.gz # via -r requirements.in diff --git a/tasks/test_results_finisher.py b/tasks/test_results_finisher.py index 6ad6be487..e1e5f8c60 100644 --- a/tasks/test_results_finisher.py +++ b/tasks/test_results_finisher.py @@ -1,6 +1,7 @@ import logging from typing import Any, Dict +from sentry_sdk import metrics from shared.yaml import UserYaml from test_results_parser import Outcome @@ -95,13 +96,22 @@ async def process_async_within_lock( if self.check_if_no_success(previous_result): # every processor errored, nothing to notify on + metrics.incr( + "test_results.finisher", + tags={"status": "failure", "reason": "no_success"}, + ) return {"notify_attempted": False, "notify_succeeded": False} - test_instances = latest_test_instances_for_a_given_commit( - db_session, commit.id_ - ) + with metrics.timing("test_results.finisher.fetch_latest_test_instances"): + test_instances = latest_test_instances_for_a_given_commit( + db_session, commit.id_ + ) if self.check_if_no_failures(test_instances): + metrics.incr( + "test_results.finisher", + tags={"status": "success", "reason": "no_failures"}, + ) self.app.tasks[notify_task_name].apply_async( args=None, kwargs=dict( @@ -110,8 +120,11 @@ async def process_async_within_lock( ) return {"notify_attempted": False, "notify_succeeded": False} + metrics.incr("test_results.finisher", tags={"status": "failures_exist"}) + notifier = TestResultsNotifier(commit, commit_yaml, test_instances) - success = await notifier.notify() + with metrics.timing("test_results.finisher.notification"): + success = await notifier.notify() log.info( "Finished test results notify", @@ -120,9 +133,15 @@ async def process_async_within_lock( commit=commitid, commit_yaml=commit_yaml, parent_task=self.request.parent_id, + success=success, ), ) + # using a var as a tag here will be fine as it's a boolean + metrics.incr( + "test_results.finisher", + tags={"status": success, "reason": "notified"}, + ) return {"notify_attempted": True, "notify_succeeded": success} def check_if_no_success(self, previous_result): diff --git a/tasks/test_results_processor.py b/tasks/test_results_processor.py index 1a3a1d3a5..020bf759d 100644 --- a/tasks/test_results_processor.py +++ b/tasks/test_results_processor.py @@ -5,6 +5,7 @@ from io import BytesIO from typing import List +from sentry_sdk import metrics from shared.celery_config import test_results_processor_task_name from shared.config import get_config from shared.yaml import UserYaml @@ -65,19 +66,22 @@ async def run_async( upload_list = [] # process each report session's test information - for args in arguments_list: - upload_obj: Upload = ( - db_session.query(Upload).filter_by(id_=args.get("upload_pk")).first() - ) + with metrics.timing("test_results.processor"): + for args in arguments_list: + upload_obj: Upload = ( + db_session.query(Upload) + .filter_by(id_=args.get("upload_pk")) + .first() + ) - res = self.process_individual_upload( - db_session, repoid, commitid, upload_obj - ) + res = self.process_individual_upload( + db_session, repoid, commitid, upload_obj + ) - # concat existing and new test information - testrun_dict_list.append(res) + # concat existing and new test information + testrun_dict_list.append(res) - upload_list.append(upload_obj) + upload_list.append(upload_obj) if self.should_delete_archive(commit_yaml): repository = ( @@ -95,9 +99,10 @@ def process_individual_upload( self, db_session, repoid, commitid, upload_obj: Upload ): upload_id = upload_obj.id - parsed_testruns: List[Testrun] = self.process_individual_arg( - upload_obj, upload_obj.report.commit.repository - ) + with metrics.timing("test_results.processor.process_individual_arg"): + parsed_testruns: List[Testrun] = self.process_individual_arg( + upload_obj, upload_obj.report.commit.repository + ) if not parsed_testruns: log.error( "No test result files were successfully parsed for this upload", @@ -112,37 +117,37 @@ def process_individual_upload( } flags_hash = generate_flags_hash(upload_obj.flag_names) upload_id = upload_obj.id + with metrics.timing("test_results.processor.write_to_db"): + for testrun in parsed_testruns: + name = testrun.name + testsuite = testrun.testsuite + outcome = str(testrun.outcome) + duration_seconds = testrun.duration + failure_message = testrun.failure_message + test_id = generate_test_id(repoid, testsuite, name, flags_hash) + insert_on_conflict_do_nothing = ( + insert(Test.__table__) + .values( + id=test_id, + repoid=repoid, + name=name, + testsuite=testsuite, + flags_hash=flags_hash, + ) + .on_conflict_do_nothing() + ) + db_session.execute(insert_on_conflict_do_nothing) + db_session.flush() - for testrun in parsed_testruns: - name = testrun.name - testsuite = testrun.testsuite - outcome = str(testrun.outcome) - duration_seconds = testrun.duration - failure_message = testrun.failure_message - test_id = generate_test_id(repoid, testsuite, name, flags_hash) - insert_on_conflict_do_nothing = ( - insert(Test.__table__) - .values( - id=test_id, - repoid=repoid, - name=name, - testsuite=testsuite, - flags_hash=flags_hash, + ti = TestInstance( + test_id=test_id, + upload_id=upload_id, + duration_seconds=duration_seconds, + outcome=outcome, + failure_message=failure_message, ) - .on_conflict_do_nothing() - ) - db_session.execute(insert_on_conflict_do_nothing) - db_session.flush() - - ti = TestInstance( - test_id=test_id, - upload_id=upload_id, - duration_seconds=duration_seconds, - outcome=outcome, - failure_message=failure_message, - ) - db_session.add(ti) - db_session.flush() + db_session.add(ti) + db_session.flush() return { "successful": True, @@ -180,9 +185,15 @@ def parse_single_file( self, file_bytes, ): + try: - parser, parsing_function = self.match_report(file_bytes) + with metrics.timing("test_results.processor.parser_matching"): + parser, parsing_function = self.match_report(file_bytes) except ParserNotSupportedError as e: + metrics.incr( + "test_results.processor.parsing", + tags={"status": "failure", "reason": "match_report_failure"}, + ) raise ParserFailureError( err_msg="File did not match any parser format", file_content=file_bytes.read().decode()[:300], @@ -190,14 +201,26 @@ def parse_single_file( try: file_content = file_bytes.read() - res = parsing_function(file_content) + with metrics.timing("test_results.processor.file_parsing"): + res = parsing_function(file_content) except ParserError as e: + # aware of cardinality issues with using a variable here in the reason field but + # parser is defined by us and limited to the amount of different parsers we will + # write, so I don't expect this to be a problem for us + metrics.incr( + "test_results.processor.parsing", + tags={"status": "failure", "reason": f"failed_to_parse_{parser}"}, + ) raise ParserFailureError( err_msg="Error parsing file", file_content=file_content.decode()[:300], parser=parser, parser_err_msg=str(e), ) from e + metrics.incr( + "test_results.processor.parsing", + tags={"status": "success", "parser": parser}, + ) return res diff --git a/tasks/tests/unit/test_test_results_finisher.py b/tasks/tests/unit/test_test_results_finisher.py index 8f69ad9cf..f0b87f557 100644 --- a/tasks/tests/unit/test_test_results_finisher.py +++ b/tasks/tests/unit/test_test_results_finisher.py @@ -2,7 +2,7 @@ from pathlib import Path import pytest -from mock import AsyncMock +from mock import AsyncMock, call from shared.torngit.exceptions import TorngitClientError from test_results_parser import Outcome @@ -73,6 +73,10 @@ async def test_upload_finisher_task_call( provider_pull={}, ), ) + mock_metrics = mocker.patch( + "tasks.test_results_finisher.metrics", + mocker.MagicMock(), + ) mocker.patch.object(TestResultsFinisherTask, "hard_time_limit_task", 0) @@ -200,6 +204,11 @@ async def test_upload_finisher_task_call_multi_env_fail( tasks={"app.tasks.notify.Notify": mocker.MagicMock()}, ) + mock_metrics = mocker.patch( + "tasks.test_results_finisher.metrics", + mocker.MagicMock(), + ) + commit = CommitFactory.create( message="hello world", commitid="cd76b0821854a780b60012aed85af0a8263004ad", @@ -357,6 +366,22 @@ async def test_upload_finisher_task_call_multi_env_fail( assert expected_result == result + mock_metrics.incr.assert_has_calls( + [ + call("test_results.finisher", tags={"status": "failures_exist"}), + call( + "test_results.finisher", + tags={"status": True, "reason": "notified"}, + ), + ] + ) + calls = [ + call("test_results.finisher.fetch_latest_test_instances"), + call("test_results.finisher.notification"), + ] + for c in calls: + assert c in mock_metrics.timing.mock_calls + @pytest.mark.asyncio @pytest.mark.integration async def test_upload_finisher_task_call_no_failures( @@ -424,6 +449,10 @@ async def test_upload_finisher_task_call_no_failures( "services.test_results.get_repo_provider_service", return_value=m, ) + mock_metrics = mocker.patch( + "tasks.test_results_finisher.metrics", + mocker.MagicMock(), + ) repoid = upload1.report.commit.repoid upload2.report.commit.repoid = repoid @@ -515,6 +544,20 @@ async def test_upload_finisher_task_call_no_failures( assert expected_result == result + mock_metrics.incr.assert_has_calls( + [ + call( + "test_results.finisher", + tags={"status": "success", "reason": "no_failures"}, + ), + ] + ) + calls = [ + call("test_results.finisher.fetch_latest_test_instances"), + ] + for c in calls: + assert c in mock_metrics.timing.mock_calls + @pytest.mark.asyncio @pytest.mark.integration async def test_upload_finisher_task_call_no_success( @@ -583,6 +626,11 @@ async def test_upload_finisher_task_call_no_success( return_value=m, ) + mock_metrics = mocker.patch( + "tasks.test_results_finisher.metrics", + mocker.MagicMock(), + ) + repoid = upload1.report.commit.repoid upload2.report.commit.repoid = repoid dbsession.flush() @@ -665,6 +713,16 @@ async def test_upload_finisher_task_call_no_success( assert expected_result == result + mock_metrics.incr.assert_has_calls( + [ + call( + "test_results.finisher", + tags={"status": "failure", "reason": "no_success"}, + ), + ] + ) + assert mock_metrics.timing.mock_calls == [] + @pytest.mark.asyncio @pytest.mark.integration async def test_upload_finisher_task_call_existing_comment( @@ -892,6 +950,11 @@ async def test_upload_finisher_task_call_comment_fails( return_value=m, ) + mock_metrics = mocker.patch( + "tasks.test_results_finisher.metrics", + mocker.MagicMock(), + ) + repoid = upload1.report.commit.repoid upload2.report.commit.repoid = repoid dbsession.flush() @@ -973,3 +1036,19 @@ async def test_upload_finisher_task_call_comment_fails( expected_result = {"notify_attempted": True, "notify_succeeded": False} assert expected_result == result + + mock_metrics.incr.assert_has_calls( + [ + call("test_results.finisher", tags={"status": "failures_exist"}), + call( + "test_results.finisher", + tags={"status": False, "reason": "notified"}, + ), + ] + ) + calls = [ + call("test_results.finisher.fetch_latest_test_instances"), + call("test_results.finisher.notification"), + ] + for c in calls: + assert c in mock_metrics.timing.mock_calls diff --git a/tasks/tests/unit/test_test_results_processor_task.py b/tasks/tests/unit/test_test_results_processor_task.py index f95ade2db..6886dafd6 100644 --- a/tasks/tests/unit/test_test_results_processor_task.py +++ b/tasks/tests/unit/test_test_results_processor_task.py @@ -1,6 +1,7 @@ from pathlib import Path import pytest +from mock import call from shared.storage.exceptions import FileNotInStorageError from test_results_parser import Outcome @@ -8,7 +9,11 @@ from database.models.reports import Test, TestInstance from database.tests.factories import CommitFactory, UploadFactory from services.test_results import generate_test_id -from tasks.test_results_processor import ParserFailureError, TestResultsProcessorTask +from tasks.test_results_processor import ( + ParserError, + ParserNotSupportedError, + TestResultsProcessorTask, +) here = Path(__file__) @@ -35,6 +40,10 @@ async def test_upload_processor_task_call( dbsession.flush() redis_queue = [{"url": url, "upload_pk": upload.id_}] mocker.patch.object(TestResultsProcessorTask, "app", celery_app) + mock_metrics = mocker.patch( + "tasks.test_results_processor.metrics", + mocker.MagicMock(), + ) commit = CommitFactory.create( message="hello world", @@ -81,6 +90,23 @@ async def test_upload_processor_task_call( assert expected_result == result assert commit.message == "hello world" + mock_metrics.incr.assert_has_calls( + [ + call( + "test_results.processor.parsing", + tags={"status": "success", "parser": "junit_xml"}, + ) + ] + ) + calls = [ + call("test_results.processor"), + call("test_results.processor.process_individual_arg"), + call("test_results.processor.file_parsing"), + call("test_results.processor.write_to_db"), + ] + for c in calls: + assert c in mock_metrics.timing.mock_calls + @pytest.mark.asyncio @pytest.mark.integration async def test_upload_processor_task_call_pytest_reportlog( @@ -216,7 +242,7 @@ async def test_upload_processor_task_call_vitest( @pytest.mark.asyncio @pytest.mark.integration - async def test_test_result_processor_task_error_parsing( + async def test_test_result_processor_task_error_report_matching( self, caplog, mocker, @@ -238,10 +264,12 @@ async def test_test_result_processor_task_error_parsing( mocker.patch.object(TestResultsProcessorTask, "app", celery_app) mocker.patch.object( TestResultsProcessorTask, - "parse_single_file", - side_effect=ParserFailureError( - err_msg="Test error message", file_content="" - ), + "match_report", + side_effect=ParserNotSupportedError(), + ) + mock_metrics = mocker.patch( + "tasks.test_results_processor.metrics", + mocker.MagicMock(), ) commit = CommitFactory.create( @@ -267,7 +295,92 @@ async def test_test_result_processor_task_error_parsing( arguments_list=redis_queue, ) print(caplog.text) - assert "Test error message" in caplog.text + assert "File did not match any parser format" in caplog.text + mock_metrics.incr.assert_has_calls( + [ + call( + "test_results.processor.parsing", + tags={"status": "failure", "reason": "match_report_failure"}, + ) + ] + ) + calls = [ + call("test_results.processor"), + call("test_results.processor.process_individual_arg"), + ] + for c in calls: + assert c in mock_metrics.timing.mock_calls + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_test_result_processor_task_error_parsing_file( + self, + caplog, + mocker, + mock_configuration, + dbsession, + codecov_vcr, + mock_storage, + mock_redis, + celery_app, + ): + url = "v4/raw/2019-05-22/C3C4715CA57C910D11D5EB899FC86A7E/4c4e4654ac25037ae869caeb3619d485970b6304/a84d445c-9c1e-434f-8275-f18f1f320f81.txt" + with open(here.parent.parent / "samples" / "sample_vitest.txt") as f: + content = f.read() + mock_storage.write_file("archive", url, content) + upload = UploadFactory.create(storage_path=url) + dbsession.add(upload) + dbsession.flush() + redis_queue = [{"url": url, "upload_pk": upload.id_}] + mocker.patch.object(TestResultsProcessorTask, "app", celery_app) + mocker.patch.object( + TestResultsProcessorTask, + "match_report", + return_value=("test_parser", mocker.MagicMock(side_effect=ParserError)), + ) + mock_metrics = mocker.patch( + "tasks.test_results_processor.metrics", + mocker.MagicMock(), + ) + + commit = CommitFactory.create( + message="hello world", + commitid="cd76b0821854a780b60012aed85af0a8263004ad", + repository__owner__unencrypted_oauth_token="test7lk5ndmtqzxlx06rip65nac9c7epqopclnoy", + repository__owner__username="joseph-sentry", + repository__owner__service="github", + repository__name="codecov-demo", + ) + + dbsession.add(commit) + dbsession.flush() + current_report_row = CommitReport(commit_id=commit.id_) + dbsession.add(current_report_row) + dbsession.flush() + + result = await TestResultsProcessorTask().run_async( + dbsession, + repoid=commit.repoid, + commitid=commit.commitid, + commit_yaml={"codecov": {"max_report_age": False}}, + arguments_list=redis_queue, + ) + print(caplog.text) + assert "Error parsing file" in caplog.text + mock_metrics.incr.assert_has_calls( + [ + call( + "test_results.processor.parsing", + tags={"status": "failure", "reason": "failed_to_parse_test_parser"}, + ) + ] + ) + calls = [ + call("test_results.processor"), + call("test_results.processor.process_individual_arg"), + ] + for c in calls: + assert c in mock_metrics.timing.mock_calls @pytest.mark.asyncio @pytest.mark.integration From 72d008fbe8e532ec8326426a52a31a0abd4a027c Mon Sep 17 00:00:00 2001 From: matt-codecov <137832199+matt-codecov@users.noreply.github.com> Date: Thu, 15 Feb 2024 10:40:55 -0800 Subject: [PATCH 06/16] fix: add python-redis-lock dep for django migrate command override (#270) --- requirements.in | 1 + requirements.txt | 16 ++++++---------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/requirements.in b/requirements.in index 9d00a5636..b66de0071 100644 --- a/requirements.in +++ b/requirements.in @@ -25,6 +25,7 @@ pytest-freezegun pytest python-dateutil python-json-logger +python-redis-lock PyYAML redis>=4.4.4 requests>=2.31.0 diff --git a/requirements.txt b/requirements.txt index 6e09699f5..9947035a6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,8 @@ # -# This file is autogenerated by pip-compile with Python 3.9 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # -# pip-compile +# pip-compile requirements.in # amqp==5.1.0 # via kombu @@ -102,8 +102,6 @@ ecdsa==0.18.0 # via tlslite-ng excel-base==1.0.4 # via django-excel-response2 -exceptiongroup==1.2.0 - # via pytest factory-boy==3.2.0 # via -r requirements.in faker==8.8.2 @@ -308,6 +306,8 @@ python-dateutil==2.8.1 # timeconvert python-json-logger==0.1.11 # via -r requirements.in +python-redis-lock==4.0.0 + # via -r requirements.in pytz==2022.1 # via # celery @@ -321,6 +321,7 @@ pyyaml==6.0.1 redis==4.5.4 # via # -r requirements.in + # python-redis-lock # shared requests==2.31.0 # via @@ -333,9 +334,7 @@ requests==2.31.0 respx==0.20.2 # via -r requirements.in rfc3986[idna2008]==1.4.0 - # via - # httpx - # rfc3986 + # via httpx rsa==4.7.2 # via google-auth s3transfer==0.3.4 @@ -388,15 +387,12 @@ timestring==1.6.4 # via -r requirements.in tlslite-ng==0.8.0b1 # via shared -tomli==2.0.1 - # via pytest tqdm==4.66.1 # via openai typing==3.7.4.3 # via shared typing-extensions==4.6.3 # via - # asgiref # openai # opentelemetry-sdk # pydantic From fa3b1a3a97f8dc78300152f2fe6ac957c8ba9ce7 Mon Sep 17 00:00:00 2001 From: jason-ford-codecov <107567073+jason-ford-codecov@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:15:07 -0600 Subject: [PATCH 07/16] removed ats (#271) --- .github/workflows/ci.yml | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6cf48716c..a80000e53 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,17 +37,6 @@ jobs: uses: codecov/gha-workflows/.github/workflows/codecov-startup.yml@v1.2.12 secrets: inherit - ats: - name: ATS - needs: [build] - if: ${{ !github.event.pull_request.head.repo.fork && github.repository_owner == 'codecov' }} - uses: codecov/gha-workflows/.github/workflows/run-ats.yml@v1.2.12 - secrets: inherit - with: - repo: ${{ vars.CODECOV_IMAGE_V2 || 'codecov/self-hosted-worker' }} - codecov_cli_upload_args: '--plugin pycoverage --plugin compress-pycoverage --flag onlysomelabels' - app_container_name: worker - test: name: Test needs: [build] From d648ba71e6be02867fce7261842662ba310eab80 Mon Sep 17 00:00:00 2001 From: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Date: Fri, 16 Feb 2024 10:16:49 -0500 Subject: [PATCH 08/16] Fix PR comment (#269) * feat: add patch coverage to comment * feat: change pr comment copy --- .../notifiers/mixins/message/sections.py | 31 ++++--- .../tests/integration/test_comment.py | 8 +- .../notifiers/tests/unit/test_checks.py | 12 +-- .../notifiers/tests/unit/test_comment.py | 80 +++++++++---------- tasks/tests/integration/test_notify_task.py | 2 +- 5 files changed, 66 insertions(+), 67 deletions(-) diff --git a/services/notification/notifiers/mixins/message/sections.py b/services/notification/notifiers/mixins/message/sections.py index 0faa50414..9606d4c15 100644 --- a/services/notification/notifiers/mixins/message/sections.py +++ b/services/notification/notifiers/mixins/message/sections.py @@ -107,19 +107,19 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non yaml = self.current_yaml base_report = comparison.project_coverage_base.report head_report = comparison.head.report - pull = comparison.pull pull_dict = comparison.enriched_pull.provider_pull repo_service = comparison.repository_service.service diff_totals = head_report.apply_diff(diff) if diff_totals: misses_and_partials = diff_totals.misses + diff_totals.partials + patch_coverage = diff_totals.coverage else: misses_and_partials = None - + patch_coverage = None if misses_and_partials: yield ( - f"Attention: `{misses_and_partials} lines` in your changes are missing coverage. Please review." + f"Attention: Patch coverage is `{patch_coverage}%` with `{misses_and_partials} lines` in your changes are missing coverage. Please review." ) else: yield "All modified and coverable lines are covered by tests :white_check_mark:" @@ -130,13 +130,10 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non if base_report and head_report: yield ( - "> Comparison is base [(`{commitid_base}`)]({links[base]}?el=desc) {base_cov}% compared to head [(`{commitid_head}`)]({links[pull]}?src=pr&el=desc) {head_cov}%.".format( - pull=pull.pullid, - base=pull_dict["base"]["branch"], + "> Project coverage is {head_cov}%. Comparing base [(`{commitid_base}`)]({links[base]}?el=desc) to head [(`{commitid_head}`)]({links[pull]}?src=pr&el=desc).".format( commitid_head=comparison.head.commit.commitid[:7], commitid_base=comparison.project_coverage_base.commit.commitid[:7], links=links, - base_cov=round_number(yaml, Decimal(base_report.totals.coverage)), head_cov=round_number(yaml, Decimal(head_report.totals.coverage)), ) ) @@ -148,9 +145,9 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non commit=pull_dict["base" if not base_report else "head"]["commitid"][ :7 ], - request_type="merge request" - if repo_service == "gitlab" - else "pull request", + request_type=( + "merge request" if repo_service == "gitlab" else "pull request" + ), ) ) @@ -634,9 +631,9 @@ async def do_write_section(self, comparison, diff, changes, links, behind_by=Non "name": name, "before": get_totals_from_file_in_reports(base_flags, name), "after": flag.totals, - "diff": flag.apply_diff(diff) - if walk(diff, ("files",)) - else None, + "diff": ( + flag.apply_diff(diff) if walk(diff, ("files",)) else None + ), "carriedforward": flag.carriedforward, "carriedforward_from": flag.carriedforward_from, } @@ -754,9 +751,11 @@ async def _get_table_data_for_components( component_data.append( { "name": component.get_display_name(), - "before": filtered_comparison.project_coverage_base.report.totals - if filtered_comparison.project_coverage_base.report is not None - else None, + "before": ( + filtered_comparison.project_coverage_base.report.totals + if filtered_comparison.project_coverage_base.report is not None + else None + ), "after": filtered_comparison.head.report.totals, "diff": filtered_comparison.head.report.apply_diff( diff, _save=False diff --git a/services/notification/notifiers/tests/integration/test_comment.py b/services/notification/notifiers/tests/integration/test_comment.py index 994b200f7..bd3fc69d6 100644 --- a/services/notification/notifiers/tests/integration/test_comment.py +++ b/services/notification/notifiers/tests/integration/test_comment.py @@ -356,7 +356,7 @@ async def test_notify(self, sample_comparison, codecov_vcr, mock_configuration): message = [ "## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - "> Comparison is base [(`5b174c2`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) 50.00% compared to head [(`5601846`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc) 60.00%.", + "> Project coverage is 60.00%. Comparing base [(`5b174c2`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) to head [(`5601846`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc).", "> Report is 2 commits behind head on main.", "", ":exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.", @@ -502,7 +502,7 @@ async def test_notify_gitlab( message = [ "## [Codecov](https://app.codecov.io/gl/joseph-sentry/example-python/pull/1?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - "> Comparison is base [(`0fc784a`)](https://app.codecov.io/gl/joseph-sentry/example-python/commit/0fc784af11c401449e56b24a174bae7b9af86c98?el=desc) 50.00% compared to head [(`0b6a213`)](https://app.codecov.io/gl/joseph-sentry/example-python/pull/1?src=pr&el=desc) 60.00%.", + "> Project coverage is 60.00%. Comparing base [(`0fc784a`)](https://app.codecov.io/gl/joseph-sentry/example-python/commit/0fc784af11c401449e56b24a174bae7b9af86c98?el=desc) to head [(`0b6a213`)](https://app.codecov.io/gl/joseph-sentry/example-python/pull/1?src=pr&el=desc).", "", "[![Impacted file tree graph](https://app.codecov.io/gl/joseph-sentry/example-python/pull/1/graphs/tree.svg?width=650&height=150&src=pr&token=abcdefghij)](https://app.codecov.io/gl/joseph-sentry/example-python/pull/1?src=pr&el=tree)", "", @@ -568,7 +568,7 @@ async def test_notify_new_layout( message = [ "## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - "> Comparison is base [(`5b174c2`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) 50.00% compared to head [(`5601846`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc) 60.00%.", + "> Project coverage is 60.00%. Comparing base [(`5b174c2`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) to head [(`5601846`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc).", "> Report is 2 commits behind head on main.", "", ":exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.", @@ -644,7 +644,7 @@ async def test_notify_with_components( message = [ "## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - "> Comparison is base [(`5b174c2`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) 50.00% compared to head [(`5601846`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc) 60.00%.", + "> Project coverage is 60.00%. Comparing base [(`5b174c2`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) to head [(`5601846`)](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc).", "> Report is 2 commits behind head on main.", "", ":exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.", diff --git a/services/notification/notifiers/tests/unit/test_checks.py b/services/notification/notifiers/tests/unit/test_checks.py index d823e476c..f7fd76d6f 100644 --- a/services/notification/notifiers/tests/unit/test_checks.py +++ b/services/notification/notifiers/tests/unit/test_checks.py @@ -1509,8 +1509,8 @@ async def test_build_default_payload( "text": "\n".join( [ f"## [Codecov](test.example.br/gh/test_build_default_payload/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{base_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload/{repo.name}/commit/{base_commit.commitid}?el=desc) 50.00% compared to head [(`{head_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=desc) 60.00%." + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{base_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload/{repo.name}/commit/{base_commit.commitid}?el=desc) to head [(`{head_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=desc)." f"", f"", f"| [Files](test.example.br/gh/test_build_default_payload/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=tree) | Coverage Δ | Complexity Δ | |", @@ -1552,8 +1552,8 @@ async def test_build_default_payload_with_flags( "text": "\n".join( [ f"## [Codecov](test.example.br/gh/test_build_default_payload_with_flags/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{base_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload_with_flags/{repo.name}/commit/{base_commit.commitid}?el=desc) 50.00% compared to head [(`{head_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload_with_flags/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=desc) 60.00%." + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{base_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload_with_flags/{repo.name}/commit/{base_commit.commitid}?el=desc) to head [(`{head_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload_with_flags/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=desc)." f"", f"", f"| [Files](test.example.br/gh/test_build_default_payload_with_flags/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=tree) | Coverage Δ | Complexity Δ | |", @@ -1596,8 +1596,8 @@ async def test_build_default_payload_with_flags_and_footer( "text": "\n".join( [ f"## [Codecov](test.example.br/gh/{test_name}/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{base_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload_with_flags_and_footer/{repo.name}/commit/{base_commit.commitid}?el=desc) 50.00% compared to head [(`{head_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload_with_flags_and_footer/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{base_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload_with_flags_and_footer/{repo.name}/commit/{base_commit.commitid}?el=desc) to head [(`{head_commit.commitid[:7]}`)](test.example.br/gh/test_build_default_payload_with_flags_and_footer/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=desc).", f"", f"| [Files](test.example.br/gh/{test_name}/{repo.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=tree) | Coverage Δ | Complexity Δ | |", f"|---|---|---|---|", diff --git a/services/notification/notifiers/tests/unit/test_comment.py b/services/notification/notifiers/tests/unit/test_comment.py index 9e9e3720f..b95a5ec05 100644 --- a/services/notification/notifiers/tests/unit/test_comment.py +++ b/services/notification/notifiers/tests/unit/test_comment.py @@ -742,7 +742,7 @@ async def test_create_message_files_section( expected_result = [ f"## [Codecov](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{sample_comparison.head.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"| [Files](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree) | Coverage Δ | Complexity Δ | |", f"|---|---|---|---|", @@ -899,7 +899,7 @@ async def test_create_message_files_section_with_critical_files( expected_result = [ f"## [Codecov](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{sample_comparison.head.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", "", "Changes have been made to critical files, which contain lines commonly executed in production. [Learn more](https://docs.codecov.com/docs/impact-analysis)", "", @@ -935,8 +935,8 @@ async def test_build_message( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{sample_comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -1010,7 +1010,7 @@ async def test_build_message_flags_empty_coverage( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 100.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 100.00%.", + f"> Project coverage is 100.00%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"| [Flag](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/flags?src=pr&el=flags) | Coverage Δ | |", "|---|---|---|", @@ -1360,8 +1360,8 @@ async def test_build_message_hide_complexity( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -1434,7 +1434,7 @@ async def test_build_message_no_base_report( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", f"> :exclamation: No coverage uploaded for pull request base (`master@{comparison.project_coverage_base.commit.commitid[:7]}`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", @@ -1505,7 +1505,7 @@ async def test_build_message_no_base_commit( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", f"> :exclamation: No coverage uploaded for pull request base (`master@cdf9aa4`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", @@ -1578,8 +1578,8 @@ async def test_build_message_no_change( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 60.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -1654,7 +1654,7 @@ async def test_build_message_negative_change( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 60.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 50.00%.", + f"> Project coverage is 50.00%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -1751,7 +1751,7 @@ async def test_build_message_negative_change_tricky_rounding( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 88.58% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 88.54%.", + f"> Project coverage is 88.54%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"```diff", f"@@ Coverage Diff @@", @@ -1820,7 +1820,7 @@ async def test_build_message_negative_change_tricky_rounding_newheader( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", f"All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 88.58% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 88.54%.", + f"> Project coverage is 88.54%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", ] li = 0 @@ -1850,8 +1850,8 @@ async def test_build_message_show_carriedforward_flags_no_cf_coverage( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -1927,7 +1927,7 @@ async def test_build_message_with_without_flags( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 65.38% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 65.38%.", + f"> Project coverage is 65.38%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -1987,7 +1987,7 @@ async def test_build_message_with_without_flags( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 65.38% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 65.38%.", + f"> Project coverage is 65.38%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -2063,7 +2063,7 @@ async def test_build_message_show_carriedforward_flags_has_cf_coverage( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 65.38% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 65.38%.", + f"> Project coverage is 65.38%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -2139,7 +2139,7 @@ async def test_build_message_hide_carriedforward_flags_has_cf_coverage( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 65.38% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 65.38%.", + f"> Project coverage is 65.38%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -2239,8 +2239,8 @@ async def test_build_message_no_flags( result = await notifier.build_message(sample_comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -3294,8 +3294,8 @@ async def test_message_hide_details_github( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"
Additional details and impacted files\n", f"", @@ -3329,8 +3329,8 @@ async def test_message_announcements_only( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", "", ":mega: message", "", @@ -3366,8 +3366,8 @@ async def test_message_hide_details_bitbucket( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - "Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + "Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", @@ -3980,7 +3980,7 @@ async def test_new_header_section_writer(self, mocker, sample_comparison): print(res) assert res == [ "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](urlurl?el=desc) 0% compared to head [(`{sample_comparison.head.commit.commitid[:7]}`)](urlurl?src=pr&el=desc) 0%.", + f"> Project coverage is 0%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](urlurl?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](urlurl?src=pr&el=desc).", ] @pytest.mark.asyncio @@ -4010,7 +4010,7 @@ async def test_new_header_section_writer_with_behind_by( print(res) assert res == [ "All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](urlurl?el=desc) 0% compared to head [(`{sample_comparison.head.commit.commitid[:7]}`)](urlurl?src=pr&el=desc) 0%.", + f"> Project coverage is 0%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](urlurl?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](urlurl?src=pr&el=desc).", "> Report is 3 commits behind head on master.", ] @@ -4322,7 +4322,7 @@ async def test_create_message_files_section_with_critical_files_new_layout( expected_result = [ f"## [Codecov](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", f"All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + f"> Project coverage is 60.00%. Comparing base [(`{sample_comparison.project_coverage_base.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/commit/{sample_comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{sample_comparison.head.commit.commitid[:7]}`)](https://app.codecov.io/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", "", "Changes have been made to critical files, which contain lines commonly executed in production. [Learn more](https://docs.codecov.com/docs/impact-analysis)", "", @@ -4365,7 +4365,7 @@ async def test_build_message_no_base_commit_new_layout( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - f"Attention: `1 lines` in your changes are missing coverage. Please review.", + f"Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", f"> :exclamation: No coverage uploaded for pull request base (`master@cdf9aa4`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", @@ -4431,7 +4431,7 @@ async def test_build_message_no_base_report_new_layout( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - f"Attention: `1 lines` in your changes are missing coverage. Please review.", + f"Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", f"> :exclamation: No coverage uploaded for pull request base (`master@{comparison.project_coverage_base.commit.commitid[:7]}`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).", f"", f"
Additional details and impacted files\n", @@ -4501,7 +4501,7 @@ async def test_build_message_no_project_coverage( pull_url = f"test.example.br/gh/{repository.slug}/pull/{pull.pullid}" expected_result = [ f"## [Codecov]({pull_url}?src=pr&el=h1) Report", - f"Attention: `1 lines` in your changes are missing coverage. Please review.", + f"Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", f"", f"| [Files]({pull_url}?src=pr&el=tree) | Patch % | Lines |", f"|---|---|---|", @@ -4542,7 +4542,7 @@ async def test_build_message_no_project_coverage_condensed_yaml_configs( pull_url = f"test.example.br/gh/{repository.slug}/pull/{pull.pullid}" expected_result = [ f"## [Codecov]({pull_url}?src=pr&el=h1) Report", - f"Attention: `1 lines` in your changes are missing coverage. Please review.", + f"Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", f"", f"| [Files]({pull_url}?src=pr&el=tree) | Patch % | Lines |", f"|---|---|---|", @@ -4582,8 +4582,8 @@ async def test_build_message_head_and_pull_head_differ_new_layout( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - f"Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + f"Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"> :exclamation: Current head {comparison.head.commit.commitid[:7]} differs from pull request most recent head {comparison.enriched_pull.provider_pull['head']['commitid'][:7]}. Consider uploading reports for the commit {comparison.enriched_pull.provider_pull['head']['commitid'][:7]} to get more accurate results", f"", @@ -4657,8 +4657,8 @@ async def test_build_message_head_and_pull_head_differ_with_components( result = await notifier.build_message(comparison) expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", - f"Attention: `1 lines` in your changes are missing coverage. Please review.", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 50.00% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 60.00%.", + f"Attention: Patch coverage is `66.66667%` with `1 lines` in your changes are missing coverage. Please review.", + f"> Project coverage is 60.00%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"> :exclamation: Current head {comparison.head.commit.commitid[:7]} differs from pull request most recent head {comparison.enriched_pull.provider_pull['head']['commitid'][:7]}. Consider uploading reports for the commit {comparison.enriched_pull.provider_pull['head']['commitid'][:7]} to get more accurate results", f"", @@ -4729,7 +4729,7 @@ async def test_build_message_no_patch_or_proj_change( expected_result = [ f"## [Codecov](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=h1) Report", f"All modified and coverable lines are covered by tests :white_check_mark:", - f"> Comparison is base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) 65.38% compared to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc) 65.38%.", + f"> Project coverage is 65.38%. Comparing base [(`{comparison.project_coverage_base.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/commit/{comparison.project_coverage_base.commit.commitid}?el=desc) to head [(`{comparison.head.commit.commitid[:7]}`)](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=desc).", f"", f"[![Impacted file tree graph](test.example.br/gh/{repository.slug}/pull/{pull.pullid}/graphs/tree.svg?width=650&height=150&src=pr&token={repository.image_token})](test.example.br/gh/{repository.slug}/pull/{pull.pullid}?src=pr&el=tree)", f"", diff --git a/tasks/tests/integration/test_notify_task.py b/tasks/tests/integration/test_notify_task.py index 3917a622b..2b34ba8fa 100644 --- a/tasks/tests/integration/test_notify_task.py +++ b/tasks/tests/integration/test_notify_task.py @@ -1158,7 +1158,7 @@ async def test_simple_call_status_and_notifiers( "message": [ "## [Codecov](https://myexamplewebsite.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=h1) Report", "All modified and coverable lines are covered by tests :white_check_mark:", - "> Comparison is base [(`5b174c2`)](https://myexamplewebsite.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) 85.00% compared to head [(`5601846`)](https://myexamplewebsite.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc) 85.00%.", + f"> Project coverage is 85.00%. Comparing base [(`5b174c2`)](https://myexamplewebsite.io/gh/joseph-sentry/codecov-demo/commit/5b174c2b40d501a70c479e91025d5109b1ad5c1b?el=desc) to head [(`5601846`)](https://myexamplewebsite.io/gh/joseph-sentry/codecov-demo/pull/9?src=pr&el=desc).", "> Report is 2 commits behind head on main.", "", ":exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.", From e59dff51bdc5772c450e3af856a2e296da2ccb98 Mon Sep 17 00:00:00 2001 From: Giovanni M Guidini <99758426+giovanni-guidini@users.noreply.github.com> Date: Tue, 20 Feb 2024 16:40:32 +0100 Subject: [PATCH 09/16] update ghapp model (#274) Bring the GithubAppInstallation model to parity with api's Depends on codecov/codecov-api#407 --- database/models/core.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/database/models/core.py b/database/models/core.py index a46c6d2a0..c76959762 100644 --- a/database/models/core.py +++ b/database/models/core.py @@ -186,6 +186,9 @@ class GithubAppInstallation(CodecovBaseModel, MixinBaseClass): repository_service_ids = Column( postgresql.ARRAY(types.Text), server_default=FetchedValue() ) + # Data required to get a token from gh + app_id = Column(types.Text, server_default=FetchedValue()) + pem_path = Column(types.Text, server_default=FetchedValue()) ownerid = Column("owner_id", types.Integer, ForeignKey("owners.ownerid")) owner = relationship( From b3937af1c89b3db3e8ced7220de215456ddef6bd Mon Sep 17 00:00:00 2001 From: matt-codecov <137832199+matt-codecov@users.noreply.github.com> Date: Tue, 20 Feb 2024 09:40:27 -0800 Subject: [PATCH 10/16] chore: update jinja dep (#275) --- requirements.in | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.in b/requirements.in index b66de0071..79ca94763 100644 --- a/requirements.in +++ b/requirements.in @@ -8,7 +8,7 @@ coverage factory-boy google-cloud-storage>=2.10.0 httpx -jinja2 +jinja2>=3.1.3 analytics-python==1.3.0b1 lxml>=4.9.1 mock diff --git a/requirements.txt b/requirements.txt index 9947035a6..533c6d8f9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -181,7 +181,7 @@ iniconfig==1.1.1 # via pytest isoweek==1.3.3 # via timeconvert -jinja2==3.1.2 +jinja2==3.1.3 # via -r requirements.in jmespath==0.10.0 # via From 17ea2269432a75c257ccd7f6598ff49b09e6dc90 Mon Sep 17 00:00:00 2001 From: Giovanni M Guidini <99758426+giovanni-guidini@users.noreply.github.com> Date: Wed, 21 Feb 2024 15:41:45 +0100 Subject: [PATCH 11/16] revamp sync_repos task (#259) Adds a new flow for sync_repos task in which the caller already knows what repos we should check. This happens in INSTALLATION webhooks by GitHub, that include a list of affected repos. The new flow makes use of github graphQL api via torngit to get info directly on the repos affected. --------- Co-authored-by: Nora Shapiro <159961121+nora-codecov@users.noreply.github.com> --- requirements.txt | 4 +- tasks/sync_repos.py | 98 +++++++++++++-- tasks/tests/unit/test_sync_repos_task.py | 154 +++++++++++++++++++++++ 3 files changed, 248 insertions(+), 8 deletions(-) diff --git a/requirements.txt b/requirements.txt index 533c6d8f9..206f438a9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -334,7 +334,9 @@ requests==2.31.0 respx==0.20.2 # via -r requirements.in rfc3986[idna2008]==1.4.0 - # via httpx + # via + # httpx + # rfc3986 rsa==4.7.2 # via google-auth s3transfer==0.3.4 diff --git a/tasks/sync_repos.py b/tasks/sync_repos.py index aa7240b73..0c4ff4c57 100644 --- a/tasks/sync_repos.py +++ b/tasks/sync_repos.py @@ -1,6 +1,6 @@ import logging from datetime import datetime -from typing import List +from typing import List, Optional, Tuple from celery.exceptions import SoftTimeLimitExceeded from redis.exceptions import LockError @@ -39,6 +39,14 @@ class SyncReposTask(BaseCodecovTask, name=sync_repos_task_name): 5. Fire off a task to sync every repository's available languages with its provider after finishing the sync. + + # About the `using_integration` argument + `using_integration` is specific to GitHub users. When `using_integration==True` then this refresh + task came from receiving some INSTALLATION event from GitHub indicating that the app installation + for the user suffered some change. + + In this case we use the installation token to list repos from github, as opposed to the owner's token + (there's possibly a difference in what repos the owner can see and what repos the app installation can see) """ ignore_result = False @@ -46,12 +54,19 @@ class SyncReposTask(BaseCodecovTask, name=sync_repos_task_name): async def run_async( self, db_session, + # `previous_results`` is added by celery if the task is chained. + # It contains the results of tasks that came before this one in the chain previous_results=None, *, ownerid, username=None, using_integration=False, manual_trigger=False, + # `repository_service_ids` is optionally passed to the task + # when using_integration=True so we know what are the repos affected. + # Speeds up getting info from the git provider, but not required + # objects are (service_id, node_id) + repository_service_ids: Optional[List[Tuple[str, str]]] = None, **kwargs, ): log.info( @@ -81,7 +96,11 @@ async def run_async( if using_integration: with metrics.timer(f"{metrics_scope}.sync_repos_using_integration"): synced_repoids = await self.sync_repos_using_integration( - db_session, git, owner, username + db_session, + git, + owner, + username, + repository_service_ids=repository_service_ids, ) else: with metrics.timer(f"{metrics_scope}.sync_repos"): @@ -96,14 +115,73 @@ async def run_async( except LockError: log.warning("Unable to sync repos because another task is already doing it") - async def sync_repos_using_integration(self, db_session, git, owner, username): + async def sync_repos_affected_repos_known( + self, + db_session, + git, + owner: Owner, + repository_service_ids: Optional[List[Tuple[str, str]]], + ): + repoids_added = [] + # Casting to str in case celery interprets the service ID as a integer for some reason + # As that has caused issues with testing locally + service_ids = set(str(x[0]) for x in repository_service_ids) + # Check what repos we already have in the DB + existing_repos = set( + map( + lambda row_result: row_result[0], + db_session.query(Repository.service_id) + .filter(Repository.service_id.in_(service_ids)) + .all(), + ) + ) + missing_repo_service_ids = service_ids.difference(existing_repos) + + # Get info from provider on the repos we don't have + repos_to_search = [ + x[1] for x in repository_service_ids if x[0] in missing_repo_service_ids + ] + async for repo_data in git.get_repos_from_nodeids_generator( + repos_to_search, owner.username + ): + # Insert those repos + new_repo = Repository( + service_id=repo_data["service_id"], + name=repo_data["name"], + language=repo_data["language"], + private=repo_data["private"], + branch=repo_data["branch"], + using_integration=True, + ) + if repo_data["owner"]["is_expected_owner"]: + new_repo.ownerid = owner.ownerid + else: + upserted_owner_id = self.upsert_owner( + db_session, + git.service, + repo_data["owner"]["service_id"], + repo_data["owner"]["username"], + ) + new_repo.ownerid = upserted_owner_id + db_session.add(new_repo) + db_session.flush() + repoids_added.append(new_repo.repoid) + return repoids_added + + async def sync_repos_using_integration( + self, + db_session, + git, + owner, + username, + repository_service_ids: Optional[List[Tuple[str, str]]] = None, + ): ownerid = owner.ownerid log.info( "Syncing repos using integration", extra=dict(ownerid=ownerid, username=username), ) - total_missing_repos = [] repoids = [] # We're testing processing repos a page at a time and this helper # function avoids duplicating the code in the old and new paths @@ -151,11 +229,17 @@ def process_repos(repos): db_session.add(new_repo) db_session.flush() repoids.append(new_repo.repoid) - total_missing_repos.extend(missing_repos) # Here comes the actual function received_repos = False - if LIST_REPOS_GENERATOR_BY_OWNER_SLUG.check_value( + if repository_service_ids: + # This flow is different from the ones below because the API already informed us the repos affected + # So we can update those values directly + repoids_added = await self.sync_repos_affected_repos_known( + db_session, git, owner, repository_service_ids + ) + repoids = repoids_added + elif LIST_REPOS_GENERATOR_BY_OWNER_SLUG.check_value( owner_slug(owner), default=False ): with metrics.timer( @@ -182,7 +266,7 @@ def process_repos(repos): log.info( "Repo sync using integration done", - extra=dict(repoids=total_missing_repos), + extra=dict(repoids_created=repoids, repoids_created_count=len(repoids)), ) return repoids diff --git a/tasks/tests/unit/test_sync_repos_task.py b/tasks/tests/unit/test_sync_repos_task.py index 66f1112a0..07d16d692 100644 --- a/tasks/tests/unit/test_sync_repos_task.py +++ b/tasks/tests/unit/test_sync_repos_task.py @@ -1,5 +1,6 @@ from datetime import datetime from pathlib import Path +from unittest.mock import MagicMock, call import httpx import pytest @@ -997,3 +998,156 @@ def repo_obj(service_id, name, language, private, branch, using_integration): mocked_app.tasks[sync_repo_languages_task_name].apply_async.assert_any_call( kwargs={"repoid": new_repo_list[0].repoid, "manual_trigger": False} ) + + @pytest.mark.asyncio + async def test_sync_repos_usgin_integration_affected_repos_known( + self, + mocker, + dbsession, + mock_owner_provider, + mock_redis, + ): + + user = OwnerFactory.create( + organizations=[], + service="github", + username="1nf1n1t3l00p", + unencrypted_oauth_token="sometesttoken", + permission=[], + service_id="45343385", + ) + dbsession.add(user) + + mocked_app = mocker.patch.object( + SyncReposTask, + "app", + tasks={ + sync_repo_languages_task_name: mocker.MagicMock(), + }, + ) + repository_service_ids = [ + ("460565350", "R_kgDOG3OrZg"), + ("665728948", "R_kgDOJ643tA"), + ("553624697", "R_kgDOIP-keQ"), + ("631985885", "R_kgDOJatW3Q"), # preseeded + ("623359086", "R_kgDOJSe0bg"), # preseeded + ] + service_ids = [x[0] for x in repository_service_ids] + service_ids_to_add = service_ids[:3] + + def repo_obj(service_id, name, language, private, branch, using_integration): + return { + "owner": { + "service_id": "test-owner-service-id", + "username": "test-owner-username", + }, + "repo": { + "service_id": service_id, + "name": name, + "language": language, + "private": private, + "branch": branch, + }, + "_using_integration": using_integration, + } + + preseeded_repos = [ + repo_obj("631985885", "example-python", "python", False, "main", True), + repo_obj("623359086", "sentry", "python", False, "main", True), + ] + + for repo in preseeded_repos: + new_repo = RepositoryFactory.create( + private=repo["repo"]["private"], + name=repo["repo"]["name"], + using_integration=repo["_using_integration"], + service_id=repo["repo"]["service_id"], + owner=user, + ) + dbsession.add(new_repo) + dbsession.flush() + + # These are the repos we're supposed to query from the service provider + async def side_effect(*args, **kwargs): + results = [ + { + "branch": "main", + "language": "python", + "name": "codecov-cli", + "owner": { + "is_expected_owner": False, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjgyMjYyMDU=", + "service_id": "8226205", + "username": "codecov", + }, + "service_id": "460565350", + "private": False, + }, + { + "branch": "main", + "language": "python", + "name": "worker", + "owner": { + "is_expected_owner": False, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjgyMjYyMDU=", + "service_id": "8226205", + "username": "codecov", + }, + "service_id": "665728948", + "private": False, + }, + { + "branch": "main", + "language": "python", + "name": "components-demo", + "owner": { + "is_expected_owner": True, + "node_id": "U_kgDOBfIxWg", + "username": "giovanni-guidini", + }, + "service_id": "553624697", + "private": False, + }, + ] + for r in results: + yield r + + mock_owner_provider.get_repos_from_nodeids_generator.side_effect = side_effect + mock_owner_provider.service = "github" + + await SyncReposTask().run_async( + dbsession, + ownerid=user.ownerid, + using_integration=True, + repository_service_ids=repository_service_ids, + ) + dbsession.commit() + + mock_owner_provider.get_repos_from_nodeids_generator.assert_called_with( + ["R_kgDOG3OrZg", "R_kgDOJ643tA", "R_kgDOIP-keQ"], user.username + ) + + repos = ( + dbsession.query(Repository) + .filter(Repository.service_id.in_(service_ids)) + .all() + ) + repos_added = list( + filter(lambda repo: repo.service_id in service_ids_to_add, repos) + ) + assert len(repos) == 5 + + mocked_app.tasks[sync_repo_languages_task_name].apply_async.calls( + [ + call(kwargs={"repoid": repo.repoid, "manual_trigger": False}) + for repo in repos_added + ] + ) + + upserted_owner = ( + dbsession.query(Owner) + .filter(Owner.service == "github", Owner.service_id == "8226205") + .first() + ) + assert upserted_owner is not None + assert upserted_owner.username == "codecov" From f0c512d48558ef6d9fb8b7fea1dec1f9e1ea431c Mon Sep 17 00:00:00 2001 From: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:42:55 -0500 Subject: [PATCH 12/16] fix: try junit parser if file extension is xml (#273) * fix: try junit parser if file extension is xml * fix: add some typing Signed-off-by: joseph-sentry --- tasks/test_results_processor.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tasks/test_results_processor.py b/tasks/test_results_processor.py index 020bf759d..0100ddbf8 100644 --- a/tasks/test_results_processor.py +++ b/tasks/test_results_processor.py @@ -161,11 +161,12 @@ def process_individual_arg(self, upload: Upload, repository) -> List[Testrun]: testrun_list = [] - for file in data["test_results_files"]: - file = file["data"] + for file_dict in data["test_results_files"]: + filename = file_dict["filename"] + file = file_dict["data"] file_bytes = BytesIO(zlib.decompress(base64.b64decode(file))) try: - testrun_list += self.parse_single_file(file_bytes) + testrun_list += self.parse_single_file(filename, file_bytes) except ParserFailureError as exc: log.error( exc.err_msg, @@ -183,12 +184,13 @@ def process_individual_arg(self, upload: Upload, repository) -> List[Testrun]: def parse_single_file( self, - file_bytes, + filename: str, + file_bytes: BytesIO, ): try: with metrics.timing("test_results.processor.parser_matching"): - parser, parsing_function = self.match_report(file_bytes) + parser, parsing_function = self.match_report(filename, file_bytes) except ParserNotSupportedError as e: metrics.incr( "test_results.processor.parsing", @@ -224,7 +226,7 @@ def parse_single_file( return res - def match_report(self, file_bytes): + def match_report(self, filename: str, file_bytes: BytesIO): first_line = file_bytes.readline() second_line = file_bytes.readline() file_bytes.seek(0) @@ -233,7 +235,7 @@ def match_report(self, file_bytes): first_two_lines = first_line + second_line parser = "no parser" - if first_two_lines.startswith(b" Date: Thu, 22 Feb 2024 09:40:43 -0400 Subject: [PATCH 13/16] fix: Change how kilobytes are displayed to international standard (#278) Adjust how kilobytes are displayed in the PR comment from KB to kB. --- services/bundle_analysis.py | 2 +- services/tests/test_bundle_analysis.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/services/bundle_analysis.py b/services/bundle_analysis.py index 564b4e5a5..8ab7adb4b 100644 --- a/services/bundle_analysis.py +++ b/services/bundle_analysis.py @@ -414,7 +414,7 @@ def _bytes_readable(self, bytes: int) -> str: kilobytes = bytes / 1000 if kilobytes < 1000: kilobytes = round(kilobytes, 2) - return f"{kilobytes}KB" + return f"{kilobytes}kB" megabytes = kilobytes / 1000 if megabytes < 1000: diff --git a/services/tests/test_bundle_analysis.py b/services/tests/test_bundle_analysis.py index 2621f3308..ea9ee1813 100644 --- a/services/tests/test_bundle_analysis.py +++ b/services/tests/test_bundle_analysis.py @@ -11,21 +11,21 @@ expected_message_increase = """## Bundle Report -Changes will increase total bundle size by 14.57KB :arrow_up: +Changes will increase total bundle size by 14.57kB :arrow_up: | Bundle name | Size | Change | | ----------- | ---- | ------ | -| added-bundle | 123.46KB | 12.35KB :arrow_up: | -| changed-bundle | 123.46KB | 3.46KB :arrow_up: | -| removed-bundle | (removed) | 1.23KB :arrow_down: |""" +| added-bundle | 123.46kB | 12.35kB :arrow_up: | +| changed-bundle | 123.46kB | 3.46kB :arrow_up: | +| removed-bundle | (removed) | 1.23kB :arrow_down: |""" expected_message_decrease = """## Bundle Report -Changes will decrease total bundle size by 3.46KB :arrow_down: +Changes will decrease total bundle size by 3.46kB :arrow_down: | Bundle name | Size | Change | | ----------- | ---- | ------ | -| test-bundle | 123.46KB | 3.46KB :arrow_down: |""" +| test-bundle | 123.46kB | 3.46kB :arrow_down: |""" expected_message_unchanged = """## Bundle Report @@ -40,8 +40,8 @@ def total_size(self): def test_bytes_readable(): notifier = Notifier(None, UserYaml.from_dict({})) assert notifier._bytes_readable(999) == "999 bytes" - assert notifier._bytes_readable(1234) == "1.23KB" - assert notifier._bytes_readable(123456) == "123.46KB" + assert notifier._bytes_readable(1234) == "1.23kB" + assert notifier._bytes_readable(123456) == "123.46kB" assert notifier._bytes_readable(1234567) == "1.23MB" assert notifier._bytes_readable(1234567890) == "1.23GB" From a35a2da6078f79ff63c4fc05971f49a6edc41ecb Mon Sep 17 00:00:00 2001 From: Giovanni M Guidini <99758426+giovanni-guidini@users.noreply.github.com> Date: Thu, 22 Feb 2024 18:38:04 +0100 Subject: [PATCH 14/16] chore: Change wording in statuses messages (#280) context: codecov/engineering-team#1131 Changes 'unexpected changes' to 'indirect changes' in statuses sent by the notify task. --- services/notification/notifiers/mixins/status.py | 8 +++----- .../notifiers/tests/unit/test_checks.py | 8 ++++---- .../notifiers/tests/unit/test_status.py | 8 ++++---- tasks/tests/integration/test_notify_task.py | 14 +++++++------- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/services/notification/notifiers/mixins/status.py b/services/notification/notifiers/mixins/status.py index ff0dfca62..a56bf30ca 100644 --- a/services/notification/notifiers/mixins/status.py +++ b/services/notification/notifiers/mixins/status.py @@ -87,9 +87,7 @@ async def get_changes_status(self, comparison) -> Tuple[str, str]: lpc = len(changes) eng = "files have" if lpc > 1 else "file has" description = ( - "{0} {1} unexpected coverage changes not visible in diff".format( - lpc, eng - ) + "{0} {1} indirect coverage changes not visible in diff".format(lpc, eng) ) state = ( "success" @@ -98,7 +96,7 @@ async def get_changes_status(self, comparison) -> Tuple[str, str]: ) return (state, description) - description = "No unexpected coverage changes found" + description = "No indirect coverage changes found" return ("success", description) @@ -260,7 +258,7 @@ async def _apply_fully_covered_patch_behavior( if coverage == 100.0: return ( "success", - ", passed because patch was fully covered by tests with no unexpected coverage changes", + ", passed because patch was fully covered by tests, and no indirect coverage changes", ) return None diff --git a/services/notification/notifiers/tests/unit/test_checks.py b/services/notification/notifiers/tests/unit/test_checks.py index f7fd76d6f..9fa1a0a31 100644 --- a/services/notification/notifiers/tests/unit/test_checks.py +++ b/services/notification/notifiers/tests/unit/test_checks.py @@ -1231,8 +1231,8 @@ async def test_build_payload( expected_result = { "state": "success", "output": { - "title": "No unexpected coverage changes found", - "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=h1)\n\nNo unexpected coverage changes found", + "title": "No indirect coverage changes found", + "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload/{sample_comparison.head.commit.repository.name}/pull/{sample_comparison.pull.pullid}?src=pr&el=h1)\n\nNo indirect coverage changes found", }, } result = await notifier.build_payload(sample_comparison) @@ -1287,8 +1287,8 @@ async def test_build_payload_with_multiple_changes( expected_result = { "state": "failure", "output": { - "title": "3 files have unexpected coverage changes not visible in diff", - "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_with_multiple_changes/{comparison_with_multiple_changes.head.commit.repository.name}/pull/{comparison_with_multiple_changes.pull.pullid}?src=pr&el=h1)\n\n3 files have unexpected coverage changes not visible in diff", + "title": "3 files have indirect coverage changes not visible in diff", + "summary": f"[View this Pull Request on Codecov](test.example.br/gh/test_build_payload_with_multiple_changes/{comparison_with_multiple_changes.head.commit.repository.name}/pull/{comparison_with_multiple_changes.pull.pullid}?src=pr&el=h1)\n\n3 files have indirect coverage changes not visible in diff", }, } result = await notifier.build_payload(comparison_with_multiple_changes) diff --git a/services/notification/notifiers/tests/unit/test_status.py b/services/notification/notifiers/tests/unit/test_status.py index 29653491b..19fa621e9 100644 --- a/services/notification/notifiers/tests/unit/test_status.py +++ b/services/notification/notifiers/tests/unit/test_status.py @@ -1656,7 +1656,7 @@ async def test_notify_fully_covered_patch_behavior_success( current_yaml=UserYaml({}), ) expected_result = { - "message": "28.57% (target 70.00%), passed because patch was fully covered by tests with no unexpected coverage changes", + "message": "28.57% (target 70.00%), passed because patch was fully covered by tests, and no indirect coverage changes", "state": "success", } result = await notifier.build_payload(comparison_100_percent_patch) @@ -2091,7 +2091,7 @@ async def test_build_payload( current_yaml=UserYaml({}), ) expected_result = { - "message": "No unexpected coverage changes found", + "message": "No indirect coverage changes found", "state": "success", } result = await notifier.build_payload(sample_comparison) @@ -2137,7 +2137,7 @@ async def test_build_payload_with_multiple_changes( current_yaml=UserYaml({}), ) expected_result = { - "message": "3 files have unexpected coverage changes not visible in diff", + "message": "3 files have indirect coverage changes not visible in diff", "state": "failure", } result = await notifier.build_payload(comparison_with_multiple_changes) @@ -2183,7 +2183,7 @@ async def test_notify_path_filter( ) base_commit = sample_comparison.project_coverage_base.commit expected_result = { - "message": "No unexpected coverage changes found", + "message": "No indirect coverage changes found", "state": "success", "url": f"test.example.br/gh/{sample_comparison.head.commit.repository.slug}/pull/{sample_comparison.pull.pullid}", } diff --git a/tasks/tests/integration/test_notify_task.py b/tasks/tests/integration/test_notify_task.py index 2b34ba8fa..d47d06bd1 100644 --- a/tasks/tests/integration/test_notify_task.py +++ b/tasks/tests/integration/test_notify_task.py @@ -436,7 +436,7 @@ async def test_simple_call_only_status_notifiers_no_pull_request( "data_sent": { "title": "codecov/changes", "state": "failure", - "message": "1 file has unexpected coverage changes not visible in diff", + "message": "1 file has indirect coverage changes not visible in diff", }, "data_received": {"id": 9333281703}, }, @@ -664,7 +664,7 @@ async def test_simple_call_only_status_notifiers_with_pull_request( "data_sent": { "title": "codecov/changes", "state": "success", - "message": "No unexpected coverage changes found", + "message": "No indirect coverage changes found", }, "data_received": {"id": 9333363801}, }, @@ -1039,15 +1039,15 @@ async def test_simple_call_status_and_notifiers( "notifier": "status-changes", "title": "default", "result": { - "notification_attempted": False, - "notification_successful": None, - "explanation": "already_done", + "notification_attempted": True, + "notification_successful": True, + "explanation": None, "data_sent": { "title": "codecov/changes", "state": "success", - "message": "No unexpected coverage changes found", + "message": "No indirect coverage changes found", }, - "data_received": None, + "data_received": {"id": 24846000025}, }, }, { From 9d47abe08b1ee28738fd20b18389d9104faea4b9 Mon Sep 17 00:00:00 2001 From: Giovanni M Guidini <99758426+giovanni-guidini@users.noreply.github.com> Date: Thu, 22 Feb 2024 19:55:48 +0100 Subject: [PATCH 15/16] chore: Change default RCB to adjust_base (#276) These changes set the default Removed Code Behavior (RCB) to `adjust_base` (instead of `fully_covered_patch`). The change in the test is because the status actually passes via adjust_base. I decided to preserve the outcome of the test, and changed the rcb configured for the test. context: codecov/engineering-team#1130 --- services/notification/notifiers/mixins/status.py | 2 +- services/notification/notifiers/tests/unit/test_checks.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/notification/notifiers/mixins/status.py b/services/notification/notifiers/mixins/status.py index a56bf30ca..486a5ab92 100644 --- a/services/notification/notifiers/mixins/status.py +++ b/services/notification/notifiers/mixins/status.py @@ -101,7 +101,7 @@ async def get_changes_status(self, comparison) -> Tuple[str, str]: class StatusProjectMixin(object): - DEFAULT_REMOVED_CODE_BEHAVIOR = "fully_covered_patch" + DEFAULT_REMOVED_CODE_BEHAVIOR = "adjust_base" async def _apply_removals_only_behavior( self, comparison: Union[ComparisonProxy, FilteredComparison] diff --git a/services/notification/notifiers/tests/unit/test_checks.py b/services/notification/notifiers/tests/unit/test_checks.py index 9fa1a0a31..ae80f32d4 100644 --- a/services/notification/notifiers/tests/unit/test_checks.py +++ b/services/notification/notifiers/tests/unit/test_checks.py @@ -1653,7 +1653,7 @@ async def test_build_default_payload_negative_change_comment_off( notifier = ProjectChecksNotifier( repository=sample_comparison_negative_change.head.commit.repository, title="default", - notifier_yaml_settings={}, + notifier_yaml_settings={"removed_code_behavior": "removals_only"}, notifier_site_settings=True, current_yaml={"comment": False}, ) From ab83780e405d51d4c073d8cc7524ddb7280dfb65 Mon Sep 17 00:00:00 2001 From: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Date: Thu, 22 Feb 2024 16:11:20 -0500 Subject: [PATCH 16/16] fix: put test name in code block in test result pr comment (#281) Signed-off-by: joseph-sentry --- services/test_results.py | 2 +- tasks/tests/unit/test_test_results_finisher.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/services/test_results.py b/services/test_results.py index c059ce32f..bcb7943d8 100644 --- a/services/test_results.py +++ b/services/test_results.py @@ -213,7 +213,7 @@ def build_message(self, url, test_instances): message += details failure_table = [ - "| {0} | 
{1}
|".format( + "| 
{0}
| 
{1}
|".format( ( "
".join( self.insert_breaks( diff --git a/tasks/tests/unit/test_test_results_finisher.py b/tasks/tests/unit/test_test_results_finisher.py index f0b87f557..37155b977 100644 --- a/tasks/tests/unit/test_test_results_finisher.py +++ b/tasks/tests/unit/test_test_results_finisher.py @@ -181,7 +181,7 @@ async def test_upload_finisher_task_call( expected_result = {"notify_attempted": True, "notify_succeeded": True} m.post_comment.assert_called_with( pull.pullid, - f"## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/{pull.pullid}) Report\n\n**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 2 tests with **`2 failed`**, 0 passed and 0 skipped.\n
View the full list of failed tests\n\n| **File path** | **Failure message** |\n| :-- | :-- |\n| test_testsuite::test_name[(b)] | 
not that bad
|\n| test_testsuite::test_name[(a)] | 
okay i guess
|", + f"## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/{pull.pullid}) Report\n\n**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 2 tests with **`2 failed`**, 0 passed and 0 skipped.\n
View the full list of failed tests\n\n| **File path** | **Failure message** |\n| :-- | :-- |\n| 
test_testsuite::test_name[(b)]
| 
not that bad
|\n| 
test_testsuite::test_name[(a)]
| 
okay i guess
|", ) assert expected_result == result @@ -361,7 +361,7 @@ async def test_upload_finisher_task_call_multi_env_fail( expected_result = {"notify_attempted": True, "notify_succeeded": True} m.post_comment.assert_called_with( pull.pullid, - f"## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/{pull.pullid}) Report\n\n**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 3 tests with **`3 failed`**, 0 passed and 0 skipped.\n
View the full list of failed tests\n\n| **File path** | **Failure message** |\n| :-- | :-- |\n| test_testsuite::test_name[(a),(b)]
test_testsuite::test_name_2[(a)] | 
not that bad
|", + f"## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/{pull.pullid}) Report\n\n**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 3 tests with **`3 failed`**, 0 passed and 0 skipped.\n
View the full list of failed tests\n\n| **File path** | **Failure message** |\n| :-- | :-- |\n| 
test_testsuite::test_name[(a),(b)]
test_testsuite::test_name_2[(a)]
| 
not that bad
|", ) assert expected_result == result @@ -876,7 +876,7 @@ async def test_upload_finisher_task_call_existing_comment( m.edit_comment.assert_called_with( pull.pullid, 1, - "## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/1) Report\n\n**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 2 tests with **`2 failed`**, 0 passed and 0 skipped.\n
View the full list of failed tests\n\n| **File path** | **Failure message** |\n| :-- | :-- |\n| test_testsuite::test_name[(b)] | 
not that bad
|\n| test_testsuite::test_name[(a)] | 
okay i guess
|", + "## [Codecov](https://app.codecov.io/gh/joseph-sentry/codecov-demo/pull/1) Report\n\n**Test Failures Detected**: Due to failing tests, we cannot provide coverage reports at this time.\n\n### :x: Failed Test Results: \nCompleted 2 tests with **`2 failed`**, 0 passed and 0 skipped.\n
View the full list of failed tests\n\n| **File path** | **Failure message** |\n| :-- | :-- |\n| 
test_testsuite::test_name[(b)]
| 
not that bad
|\n| 
test_testsuite::test_name[(a)]
| 
okay i guess
|", ) assert expected_result == result