Skip to content

Commit

Permalink
1459 ba pr message tweak (#468)
Browse files Browse the repository at this point in the history
  • Loading branch information
adrian-codecov committed May 27, 2024
1 parent e18b4ff commit 8390733
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 25 deletions.
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
https://github.com/codecov/shared/archive/ae6e12f3cf43188a6d6fa0156ccb78252d3405a8.tar.gz#egg=shared
https://github.com/codecov/shared/archive/4b51e8af9a42660c7c2f84831534349016ef5113.tar.gz#egg=shared
https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem
https://github.com/codecov/test-results-parser/archive/5515e960d5d38881036e9127f86320efca649f13.tar.gz#egg=test-results-parser
boto3>=1.34
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ sentry-sdk==1.40.0
# via
# -r requirements.in
# shared
shared @ https://github.com/codecov/shared/archive/ae6e12f3cf43188a6d6fa0156ccb78252d3405a8.tar.gz
shared @ https://github.com/codecov/shared/archive/4b51e8af9a42660c7c2f84831534349016ef5113.tar.gz
# via -r requirements.in
six==1.16.0
# via
Expand Down
66 changes: 43 additions & 23 deletions services/bundle_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
)
from services.storage import get_storage_client
from services.urls import get_bundle_analysis_pull_url
from services.yaml import read_yaml_field

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -364,36 +365,59 @@ async def notify(self) -> bool:
return False

pullid = pull.database_pull.pullid
message = self._build_message(pull)
bundle_comparison = ComparisonLoader(pull).get_comparison()
skip_comment_without_size_changes = (
bundle_comparison.total_size_delta == 0
and read_yaml_field(
self.current_yaml, ("comment", "require_bundle_changes"), False
)
)

try:
comment_id = pull.database_pull.bundle_analysis_commentid
if comment_id:
await self.repository_service.edit_comment(pullid, comment_id, message)
else:
res = await self.repository_service.post_comment(pullid, message)
pull.database_pull.bundle_analysis_commentid = res["id"]
return True
except TorngitClientError:
log.error(
"Error creating/updapting PR comment",
if skip_comment_without_size_changes:
# Skips the comment and returns successful notification
log.info(
"Skipping bundle PR comment without size change",
extra=dict(
commitid=self.commit.commitid,
report_key=self.commit_report.external_id,
pullid=pullid,
),
)
return False
return True
else:
message = self._build_message(pull=pull, comparison=bundle_comparison)
try:
comment_id = pull.database_pull.bundle_analysis_commentid
if comment_id:
await self.repository_service.edit_comment(
pullid, comment_id, message
)
else:
res = await self.repository_service.post_comment(pullid, message)
pull.database_pull.bundle_analysis_commentid = res["id"]
return True
except TorngitClientError:
log.error(
"Error creating/updating PR comment",
extra=dict(
commitid=self.commit.commitid,
report_key=self.commit_report.external_id,
pullid=pullid,
),
)
return False

def _build_message(self, pull: EnrichedPull) -> str:
comparison = ComparisonLoader(pull).get_comparison()
def _build_message(
self, pull: EnrichedPull, comparison: BundleAnalysisComparison
) -> str:
bundle_changes = comparison.bundle_changes()

bundle_rows, total_size_delta = self._create_bundle_rows(
bundle_rows = self._create_bundle_rows(
bundle_changes=bundle_changes, comparison=comparison
)
return self._write_lines(
bundle_rows=bundle_rows, total_size_delta=total_size_delta, pull=pull
bundle_rows=bundle_rows,
total_size_delta=comparison.total_size_delta,
pull=pull,
)

def _create_bundle_rows(
Expand All @@ -402,13 +426,9 @@ def _create_bundle_rows(
comparison: BundleAnalysisComparison,
) -> Tuple[List[BundleRows], int]:
bundle_rows = []
total_size_delta = 0

# Calculate bundle change data in one loop since bundle_changes is a generator
for bundle_change in bundle_changes:
# TODO: make this a ComparisonReport property
total_size_delta += bundle_change.size_delta

# Define row table data
bundle_name = bundle_change.bundle_name
if bundle_change.change_type == BundleChange.ChangeType.REMOVED:
Expand All @@ -432,7 +452,7 @@ def _create_bundle_rows(
)
)

return (bundle_rows, total_size_delta)
return bundle_rows

def _write_lines(
self, bundle_rows: List[BundleRows], total_size_delta: int, pull: EnrichedPull
Expand Down
79 changes: 79 additions & 0 deletions services/tests/test_bundle_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,85 @@ async def test_bundle_analysis_notify_size_unchanged(
)


@pytest.mark.asyncio
async def test_bundle_analysis_no_notification_size_unchanged(
dbsession, mocker, mock_storage, mock_repo_provider
):
base_commit = CommitFactory()
dbsession.add(base_commit)
base_commit_report = CommitReport(
commit=base_commit, report_type=ReportType.BUNDLE_ANALYSIS.value
)
dbsession.add(base_commit_report)

head_commit = CommitFactory(repository=base_commit.repository)
dbsession.add(head_commit)
head_commit_report = CommitReport(
commit=head_commit, report_type=ReportType.BUNDLE_ANALYSIS.value
)
dbsession.add(head_commit_report)

pull = PullFactory(
repository=base_commit.repository,
head=head_commit.commitid,
base=base_commit.commitid,
compared_to=base_commit.commitid,
)
dbsession.add(pull)
dbsession.commit()

notifier = Notifier(
head_commit, UserYaml.from_dict({"comment": {"require_bundle_changes": True}})
)

repo_key = ArchiveService.get_archive_hash(base_commit.repository)
mock_storage.write_file(
get_bucket_name(),
f"v1/repos/{repo_key}/{base_commit_report.external_id}/bundle_report.sqlite",
"test-content",
)
mock_storage.write_file(
get_bucket_name(),
f"v1/repos/{repo_key}/{head_commit_report.external_id}/bundle_report.sqlite",
"test-content",
)

mocker.patch(
"services.bundle_analysis.get_appropriate_storage_service",
return_value=mock_storage,
)
mocker.patch("shared.bundle_analysis.report.BundleAnalysisReport._setup")
mocker.patch(
"services.bundle_analysis.get_repo_provider_service",
return_value=mock_repo_provider,
)

bundle_changes = mocker.patch(
"shared.bundle_analysis.comparison.BundleAnalysisComparison.bundle_changes"
)
bundle_changes.return_value = [
BundleChange("test-bundle", BundleChange.ChangeType.CHANGED, size_delta=0),
]

bundle_report = mocker.patch(
"shared.bundle_analysis.report.BundleAnalysisReport.bundle_report"
)
bundle_report.return_value = MockBundleReport()

fetch_pr = mocker.patch(
"services.bundle_analysis.fetch_and_update_pull_request_information_from_commit"
)
fetch_pr.return_value = EnrichedPull(
database_pull=pull,
provider_pull={},
)

success = await notifier.notify()
assert success == True
mock_repo_provider.post_comment.assert_not_called()
mock_repo_provider.edit_comment.assert_not_called()


@pytest.mark.asyncio
async def test_bundle_analysis_notify_missing_commit_report(
dbsession, mocker, mock_storage, mock_repo_provider
Expand Down

0 comments on commit 8390733

Please sign in to comment.