diff --git a/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py b/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py new file mode 100644 index 00000000000000..db4266446e9206 --- /dev/null +++ b/src/sentry/preprod/vcs/pr_comments/snapshot_templates.py @@ -0,0 +1,128 @@ +from __future__ import annotations + +from django.utils.translation import gettext_lazy as _ + +from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval +from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics +from sentry.preprod.url_utils import get_preprod_artifact_comparison_url, get_preprod_artifact_url + +_HEADER = "## Sentry Snapshot Testing" +PROCESSING_STATUS = "⏳ Processing" +COMPARISON_TABLE_HEADER = ( + "| Name | Added | Removed | Modified | Renamed | Unchanged | Status |\n" + "| :--- | :---: | :---: | :---: | :---: | :---: | :---: |\n" +) + + +def format_snapshot_pr_comment( + artifacts: list[PreprodArtifact], + snapshot_metrics_map: dict[int, PreprodSnapshotMetrics], + comparisons_map: dict[int, PreprodSnapshotComparison], + base_artifact_map: dict[int, PreprodArtifact], + changes_map: dict[int, bool], + approvals_map: dict[int, PreprodComparisonApproval] | None = None, +) -> str: + """Format a PR comment for snapshot comparisons.""" + if not artifacts: + raise ValueError("Cannot format PR comment for empty artifact list") + + table_rows = [] + + for artifact in artifacts: + name_cell = _name_cell(artifact, snapshot_metrics_map, base_artifact_map) + metrics = snapshot_metrics_map.get(artifact.id) + + if not metrics: + table_rows.append(f"| {name_cell} | - | - | - | - | - | {PROCESSING_STATUS} |") + continue + + comparison = comparisons_map.get(metrics.id) + has_base = artifact.id in base_artifact_map + + if not comparison and not has_base: + # No base to compare against — show snapshot count only + table_rows.append( + f"| {name_cell} | - | - | - | - | - | ✅ {metrics.image_count} uploaded |" + ) + continue + + if not comparison: + table_rows.append(f"| {name_cell} | - | - | - | - | - | {PROCESSING_STATUS} |") + continue + + if comparison.state in ( + PreprodSnapshotComparison.State.PENDING, + PreprodSnapshotComparison.State.PROCESSING, + ): + table_rows.append(f"| {name_cell} | - | - | - | - | - | {PROCESSING_STATUS} |") + elif comparison.state == PreprodSnapshotComparison.State.FAILED: + table_rows.append(f"| {name_cell} | - | - | - | - | - | ❌ Comparison failed |") + else: + base_artifact = base_artifact_map.get(artifact.id) + artifact_url = ( + get_preprod_artifact_comparison_url( + artifact, base_artifact, comparison_type="snapshots" + ) + if base_artifact + else get_preprod_artifact_url(artifact, view_type="snapshots") + ) + + has_changes = changes_map.get(artifact.id, False) + is_approved = approvals_map is not None and artifact.id in approvals_map + if has_changes and is_approved: + status = "✅ Approved" + elif has_changes: + status = "⏳ Needs approval" + else: + status = "✅ Unchanged" + + table_rows.append( + f"| {name_cell}" + f" | {_section_cell(comparison.images_added, 'added', artifact_url)}" + f" | {_section_cell(comparison.images_removed, 'removed', artifact_url)}" + f" | {_section_cell(comparison.images_changed, 'changed', artifact_url)}" + f" | {_section_cell(comparison.images_renamed, 'renamed', artifact_url)}" + f" | {_section_cell(comparison.images_unchanged, 'unchanged', artifact_url)}" + f" | {status} |" + ) + + return f"{_HEADER}\n\n{COMPARISON_TABLE_HEADER}" + "\n".join(table_rows) + + +def _name_cell( + artifact: PreprodArtifact, + snapshot_metrics_map: dict[int, PreprodSnapshotMetrics], + base_artifact_map: dict[int, PreprodArtifact], +) -> str: + app_display, app_id = _app_display_info(artifact) + metrics = snapshot_metrics_map.get(artifact.id) + base_artifact = base_artifact_map.get(artifact.id) + + if base_artifact and metrics: + artifact_url = get_preprod_artifact_comparison_url( + artifact, base_artifact, comparison_type="snapshots" + ) + else: + artifact_url = get_preprod_artifact_url(artifact, view_type="snapshots") + + return _format_name_cell(app_display, app_id, artifact_url) + + +def _app_display_info(artifact: PreprodArtifact) -> tuple[str, str]: + mobile_app_info = getattr(artifact, "mobile_app_info", None) + app_name = mobile_app_info.app_name if mobile_app_info else None + app_display = app_name or artifact.app_id or str(_("Unknown App")) + app_id = artifact.app_id or "" + return app_display, app_id + + +def _format_name_cell(app_display: str, app_id: str, url: str) -> str: + if app_id: + return f"[{app_display}]({url})
`{app_id}`" + return f"[{app_display}]({url})" + + +def _section_cell(count: int, section: str, artifact_url: str) -> str: + if count > 0: + return f"[{count}]({artifact_url}?section={section})" + return str(count) diff --git a/src/sentry/preprod/vcs/status_checks/snapshots/templates.py b/src/sentry/preprod/vcs/status_checks/snapshots/templates.py index 37a655c4d7cc5f..148def254f090f 100644 --- a/src/sentry/preprod/vcs/status_checks/snapshots/templates.py +++ b/src/sentry/preprod/vcs/status_checks/snapshots/templates.py @@ -7,9 +7,16 @@ from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics from sentry.preprod.url_utils import get_preprod_artifact_comparison_url, get_preprod_artifact_url +from sentry.preprod.vcs.pr_comments.snapshot_templates import ( + COMPARISON_TABLE_HEADER, + PROCESSING_STATUS, + _app_display_info, + _format_name_cell, + _name_cell, + _section_cell, +) _SNAPSHOT_TITLE_BASE = _("Snapshot Testing") -_PROCESSING_STATUS = "⏳ Processing" def format_snapshot_status_check_messages( @@ -193,25 +200,16 @@ def _format_solo_snapshot_summary( table_rows = [] for artifact in artifacts: - mobile_app_info = getattr(artifact, "mobile_app_info", None) - app_name = mobile_app_info.app_name if mobile_app_info else None - app_display = app_name or artifact.app_id or str(_("Unknown App")) - app_id = artifact.app_id or "" - + app_display, app_id = _app_display_info(artifact) artifact_url = get_preprod_artifact_url(artifact, view_type="snapshots") - - name_cell = ( - f"[{app_display}]({artifact_url})
`{app_id}`" - if app_id - else f"[{app_display}]({artifact_url})" - ) + name = _format_name_cell(app_display, app_id, artifact_url) metrics = snapshot_metrics_map.get(artifact.id) if not metrics: - table_rows.append(f"| {name_cell} | - | {_PROCESSING_STATUS} |") + table_rows.append(f"| {name} | - | {PROCESSING_STATUS} |") continue - table_rows.append(f"| {name_cell} | {metrics.image_count} | ✅ Uploaded |") + table_rows.append(f"| {name} | {metrics.image_count} | ✅ Uploaded |") table_header = "| Name | Snapshots | Status |\n| :--- | :---: | :---: |\n" @@ -229,47 +227,33 @@ def _format_snapshot_summary( table_rows = [] for artifact in artifacts: - mobile_app_info = getattr(artifact, "mobile_app_info", None) - app_name = mobile_app_info.app_name if mobile_app_info else None - app_display = app_name or artifact.app_id or str(_("Unknown App")) - app_id = artifact.app_id or "" + name = _name_cell(artifact, snapshot_metrics_map, base_artifact_map) metrics = snapshot_metrics_map.get(artifact.id) - base_artifact = base_artifact_map.get(artifact.id) - - if base_artifact and metrics: - artifact_url = get_preprod_artifact_comparison_url( - artifact, base_artifact, comparison_type="snapshots" - ) - else: - artifact_url = get_preprod_artifact_url(artifact, view_type="snapshots") - - name_cell = ( - f"[{app_display}]({artifact_url})
`{app_id}`" - if app_id - else f"[{app_display}]({artifact_url})" - ) - if not metrics: - table_rows.append(f"| {name_cell} | - | - | - | - | - | {_PROCESSING_STATUS} |") + table_rows.append(f"| {name} | - | - | - | - | - | {PROCESSING_STATUS} |") continue comparison = comparisons_map.get(metrics.id) if not comparison: - table_rows.append(f"| {name_cell} | - | - | - | - | - | {_PROCESSING_STATUS} |") + table_rows.append(f"| {name} | - | - | - | - | - | {PROCESSING_STATUS} |") continue if comparison.state in ( PreprodSnapshotComparison.State.PENDING, PreprodSnapshotComparison.State.PROCESSING, ): - table_rows.append(f"| {name_cell} | - | - | - | - | - | {_PROCESSING_STATUS} |") + table_rows.append(f"| {name} | - | - | - | - | - | {PROCESSING_STATUS} |") else: - added = comparison.images_added - removed = comparison.images_removed - modified = comparison.images_changed - renamed = comparison.images_renamed - unchanged = comparison.images_unchanged + base_artifact = base_artifact_map.get(artifact.id) + artifact_url = ( + get_preprod_artifact_comparison_url( + artifact, base_artifact, comparison_type="snapshots" + ) + if base_artifact + else get_preprod_artifact_url(artifact, view_type="snapshots") + ) + has_changes = changes_map.get(artifact.id, False) is_approved = approvals_map is not None and artifact.id in approvals_map if has_changes and is_approved: @@ -279,25 +263,14 @@ def _format_snapshot_summary( else: status = "✅ Unchanged" - def _section_cell(count: int, section: str) -> str: - if count > 0: - section_url = f"{artifact_url}?section={section}" - return f"[{count}]({section_url})" - return str(count) - table_rows.append( - f"| {name_cell}" - f" | {_section_cell(added, 'added')}" - f" | {_section_cell(removed, 'removed')}" - f" | {_section_cell(modified, 'changed')}" - f" | {_section_cell(renamed, 'renamed')}" - f" | {_section_cell(unchanged, 'unchanged')}" + f"| {name}" + f" | {_section_cell(comparison.images_added, 'added', artifact_url)}" + f" | {_section_cell(comparison.images_removed, 'removed', artifact_url)}" + f" | {_section_cell(comparison.images_changed, 'changed', artifact_url)}" + f" | {_section_cell(comparison.images_renamed, 'renamed', artifact_url)}" + f" | {_section_cell(comparison.images_unchanged, 'unchanged', artifact_url)}" f" | {status} |" ) - table_header = ( - "| Name | Added | Removed | Modified | Renamed | Unchanged | Status |\n" - "| :--- | :---: | :---: | :---: | :---: | :---: | :---: |\n" - ) - - return table_header + "\n".join(table_rows) + return COMPARISON_TABLE_HEADER + "\n".join(table_rows) diff --git a/tests/sentry/preprod/vcs/pr_comments/test_snapshot_templates.py b/tests/sentry/preprod/vcs/pr_comments/test_snapshot_templates.py new file mode 100644 index 00000000000000..86977281ae2df9 --- /dev/null +++ b/tests/sentry/preprod/vcs/pr_comments/test_snapshot_templates.py @@ -0,0 +1,389 @@ +from __future__ import annotations + +import pytest + +from sentry.preprod.models import PreprodArtifact, PreprodComparisonApproval +from sentry.preprod.snapshots.models import PreprodSnapshotComparison, PreprodSnapshotMetrics +from sentry.preprod.vcs.pr_comments.snapshot_templates import ( + format_snapshot_pr_comment, +) +from sentry.testutils.cases import TestCase +from sentry.testutils.silo import cell_silo_test + + +class SnapshotPrCommentTestBase(TestCase): + def setUp(self) -> None: + super().setUp() + self.organization = self.create_organization(owner=self.user) + self.team = self.create_team(organization=self.organization) + self.project = self.create_project( + teams=[self.team], organization=self.organization, name="test_project" + ) + + def _create_artifact_with_metrics( + self, + app_id: str = "com.example.app", + app_name: str | None = None, + build_version: str = "1.0.0", + build_number: int = 1, + image_count: int = 10, + ) -> tuple[PreprodArtifact, PreprodSnapshotMetrics]: + artifact = self.create_preprod_artifact( + project=self.project, + state=PreprodArtifact.ArtifactState.PROCESSED, + app_id=app_id, + app_name=app_name, + build_version=build_version, + build_number=build_number, + ) + metrics = PreprodSnapshotMetrics.objects.create( + preprod_artifact=artifact, + image_count=image_count, + ) + return artifact, metrics + + def _create_comparison( + self, + head_metrics: PreprodSnapshotMetrics, + base_metrics: PreprodSnapshotMetrics, + state: int = PreprodSnapshotComparison.State.SUCCESS, + images_changed: int = 0, + images_added: int = 0, + images_removed: int = 0, + images_renamed: int = 0, + images_unchanged: int = 10, + ) -> PreprodSnapshotComparison: + return PreprodSnapshotComparison.objects.create( + head_snapshot_metrics=head_metrics, + base_snapshot_metrics=base_metrics, + state=state, + images_changed=images_changed, + images_added=images_added, + images_removed=images_removed, + images_renamed=images_renamed, + images_unchanged=images_unchanged, + ) + + def _create_approval(self, artifact: PreprodArtifact) -> PreprodComparisonApproval: + return PreprodComparisonApproval.objects.create( + preprod_artifact=artifact, + preprod_feature_type=PreprodComparisonApproval.FeatureType.SNAPSHOTS, + approval_status=PreprodComparisonApproval.ApprovalStatus.APPROVED, + ) + + +@cell_silo_test +class FormatSnapshotPrCommentEmptyTest(SnapshotPrCommentTestBase): + def test_empty_artifacts_raises(self) -> None: + with pytest.raises(ValueError, match="Cannot format PR comment for empty artifact list"): + format_snapshot_pr_comment([], {}, {}, {}, {}) + + +@cell_silo_test +class FormatSnapshotPrCommentProcessingTest(SnapshotPrCommentTestBase): + def test_no_metrics_shows_processing(self) -> None: + artifact = self.create_preprod_artifact( + project=self.project, + app_id="com.example.app", + ) + + result = format_snapshot_pr_comment([artifact], {}, {}, {}, {}) + + assert "## Sentry Snapshot Testing" in result + assert "Processing" in result + assert "com.example.app" in result + + def test_no_comparison_with_base_shows_processing(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics() + base_artifact, _ = self._create_artifact_with_metrics(app_id="com.example.base") + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {}, + {head_artifact.id: base_artifact}, + {}, + ) + + assert "Processing" in result + + def test_pending_comparison_shows_processing(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics(app_id="com.example.head") + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison( + head_metrics, base_metrics, state=PreprodSnapshotComparison.State.PENDING + ) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {head_artifact.id: base_artifact}, + {}, + ) + + assert "Processing" in result + + def test_processing_comparison_shows_processing(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics(app_id="com.example.head") + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison( + head_metrics, base_metrics, state=PreprodSnapshotComparison.State.PROCESSING + ) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {head_artifact.id: base_artifact}, + {}, + ) + + assert "Processing" in result + + +@cell_silo_test +class FormatSnapshotPrCommentFailedTest(SnapshotPrCommentTestBase): + def test_failed_comparison_shows_failure(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics(app_id="com.example.head") + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison( + head_metrics, base_metrics, state=PreprodSnapshotComparison.State.FAILED + ) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {head_artifact.id: base_artifact}, + {}, + ) + + assert "Comparison failed" in result + assert "com.example.head" in result + + +@cell_silo_test +class FormatSnapshotPrCommentSuccessTest(SnapshotPrCommentTestBase): + def test_no_changes_shows_unchanged(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics( + app_id="com.example.app", app_name="My App" + ) + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison( + head_metrics, + base_metrics, + images_changed=0, + images_added=0, + images_removed=0, + images_unchanged=10, + ) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {head_artifact.id: base_artifact}, + {}, + ) + + assert "## Sentry Snapshot Testing" in result + assert "Unchanged" in result + assert "My App" in result + assert "`com.example.app`" in result + # Zero counts are plain text, non-zero unchanged is linked + assert "| 0 | 0 | 0 | 0 |" in result + assert "?section=unchanged" in result + + def test_changes_show_needs_approval(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics() + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison( + head_metrics, + base_metrics, + images_changed=3, + images_added=2, + images_removed=1, + images_renamed=1, + images_unchanged=5, + ) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {head_artifact.id: base_artifact}, + {head_artifact.id: True}, + ) + + assert "Needs approval" in result + assert "?section=added" in result + assert "?section=removed" in result + assert "?section=changed" in result + assert "?section=renamed" in result + + def test_approved_shows_approved_status(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics() + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison( + head_metrics, + base_metrics, + images_changed=3, + images_unchanged=7, + ) + approval = self._create_approval(head_artifact) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {head_artifact.id: base_artifact}, + {head_artifact.id: True}, + approvals_map={head_artifact.id: approval}, + ) + + assert "Approved" in result + assert "Needs approval" not in result + + def test_multiple_artifacts(self) -> None: + artifacts = [] + snapshot_metrics_map: dict[int, PreprodSnapshotMetrics] = {} + comparisons_map: dict[int, PreprodSnapshotComparison] = {} + base_artifact_map: dict[int, PreprodArtifact] = {} + + for i in range(3): + head_artifact, head_metrics = self._create_artifact_with_metrics( + app_id=f"com.example.app{i}", build_number=i + 1 + ) + base_artifact, base_metrics = self._create_artifact_with_metrics( + app_id=f"com.example.base{i}", build_number=i + 10 + ) + comparison = self._create_comparison(head_metrics, base_metrics) + + artifacts.append(head_artifact) + snapshot_metrics_map[head_artifact.id] = head_metrics + comparisons_map[head_metrics.id] = comparison + base_artifact_map[head_artifact.id] = base_artifact + + result = format_snapshot_pr_comment( + artifacts, + snapshot_metrics_map, + comparisons_map, + base_artifact_map, + {}, + ) + + for i in range(3): + assert f"com.example.app{i}" in result + + def test_section_links_include_artifact_url(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics() + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison( + head_metrics, + base_metrics, + images_changed=1, + images_unchanged=9, + ) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {head_artifact.id: base_artifact}, + {head_artifact.id: True}, + ) + + assert f"/preprod/snapshots/{head_artifact.id}" in result + assert "?section=changed" in result + + def test_zero_counts_are_not_linked(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics() + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison( + head_metrics, + base_metrics, + images_changed=0, + images_added=0, + images_removed=0, + images_unchanged=0, + ) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {head_artifact.id: base_artifact}, + {}, + ) + + assert "?section=" not in result + + def test_table_header_present(self) -> None: + head_artifact, head_metrics = self._create_artifact_with_metrics() + base_artifact, base_metrics = self._create_artifact_with_metrics(app_id="com.example.base") + + comparison = self._create_comparison(head_metrics, base_metrics) + + result = format_snapshot_pr_comment( + [head_artifact], + {head_artifact.id: head_metrics}, + {head_metrics.id: comparison}, + {}, + {}, + ) + + assert "| Name | Added | Removed | Modified | Renamed | Unchanged | Status |" in result + + +@cell_silo_test +class FormatSnapshotPrCommentNoBaseTest(SnapshotPrCommentTestBase): + def test_no_base_shows_uploaded_with_count(self) -> None: + artifact, metrics = self._create_artifact_with_metrics(app_name="My App", image_count=15) + + result = format_snapshot_pr_comment([artifact], {artifact.id: metrics}, {}, {}, {}) + + assert "## Sentry Snapshot Testing" in result + assert "15 uploaded" in result + assert "My App" in result + assert "`com.example.app`" in result + + def test_no_base_uses_same_table_header(self) -> None: + artifact, metrics = self._create_artifact_with_metrics() + + result = format_snapshot_pr_comment([artifact], {artifact.id: metrics}, {}, {}, {}) + + assert "| Name | Added | Removed | Modified | Renamed | Unchanged | Status |" in result + + def test_no_base_multiple_artifacts(self) -> None: + artifacts = [] + metrics_map: dict[int, PreprodSnapshotMetrics] = {} + + for i in range(2): + artifact, metrics = self._create_artifact_with_metrics( + app_id=f"com.example.app{i}", build_number=i + 1, image_count=5 + i + ) + artifacts.append(artifact) + metrics_map[artifact.id] = metrics + + result = format_snapshot_pr_comment(artifacts, metrics_map, {}, {}, {}) + + assert "com.example.app0" in result + assert "com.example.app1" in result + assert "5 uploaded" in result + assert "6 uploaded" in result + + def test_no_base_app_id_shown(self) -> None: + artifact, metrics = self._create_artifact_with_metrics(app_id="com.example.myapp") + + result = format_snapshot_pr_comment([artifact], {artifact.id: metrics}, {}, {}, {}) + + assert "`com.example.myapp`" in result