diff --git a/tests/perf/auto_perf_sheriffing/test_backfill_record.py b/tests/perf/auto_perf_sheriffing/test_backfill_record.py new file mode 100644 index 00000000000..c4349802330 --- /dev/null +++ b/tests/perf/auto_perf_sheriffing/test_backfill_record.py @@ -0,0 +1,69 @@ +import json + +import pytest + +from treeherder.perf.models import BackfillRecord, BackfillReport +from treeherder.webapp.api.performance_serializers import BackfillRecordSerializer + + +@pytest.fixture +def backfill_record_with_logs(test_perf_alert): + report = BackfillReport.objects.create(summary=test_perf_alert.summary) + record = BackfillRecord.objects.create(alert=test_perf_alert, report=report) + record.backfill_logs = json.dumps( + [ + { + "iteration": 0, + "status": "initial", + "detected_push_id": 111, + "detected_push_revision": "aaaa1111bbbb", + }, + {"iteration": 0, "status": "backfill_requested"}, # no push → must be skipped + { + "iteration": 1, + "status": "right", + "detected_push_id": 222, + "detected_push_revision": "cccc2222dddd", + }, + ] + ) + record.save() + return record + + +@pytest.mark.django_db +def test_get_latest_detected_push_returns_most_recent(backfill_record_with_logs): + assert backfill_record_with_logs.get_latest_detected_push() == { + "detected_push_id": 222, + "detected_push_revision": "cccc2222dddd", + } + + +@pytest.mark.django_db +def test_serializer_exposes_detected_push(backfill_record_with_logs): + data = BackfillRecordSerializer(backfill_record_with_logs).data + assert data["detected_push_id"] == 222 + assert data["detected_push_revision"] == "cccc2222dddd" + + +@pytest.mark.django_db +def test_detected_push_null_when_no_logs(test_perf_alert): + report = BackfillReport.objects.create(summary=test_perf_alert.summary) + record = BackfillRecord.objects.create( + alert=test_perf_alert, report=report + ) # backfill_logs='[]' + data = BackfillRecordSerializer(record).data + assert data["detected_push_id"] is None + assert data["detected_push_revision"] is None + + +@pytest.mark.django_db +def test_detected_push_falls_back_to_scalar(test_perf_alert): + report = BackfillReport.objects.create(summary=test_perf_alert.summary) + record = BackfillRecord.objects.create( + alert=test_perf_alert, report=report, last_detected_push_id=999 + ) + assert record.get_latest_detected_push() == { + "detected_push_id": 999, + "detected_push_revision": None, + } diff --git a/tests/ui/perfherder/alerts-view/alerts_table_row_test.jsx b/tests/ui/perfherder/alerts-view/alerts_table_row_test.jsx index 9b8c95563a2..8dc3d4a65d5 100644 --- a/tests/ui/perfherder/alerts-view/alerts_table_row_test.jsx +++ b/tests/ui/perfherder/alerts-view/alerts_table_row_test.jsx @@ -3,6 +3,7 @@ import { render, cleanup, waitFor, fireEvent } from '@testing-library/react'; import AlertTableRow from '../../../../ui/perfherder/alerts/AlertTableRow'; import testAlertSummaries from '../../mock/alert_summaries'; import { thPlatformMap } from '../../../../ui/helpers/constants'; +import { alertBackfillResultStatusMap } from '../../../../ui/perfherder/perf-helpers/constants'; const testUser = { username: 'mozilla-ldap/test_user@mozilla.com', @@ -445,3 +446,60 @@ describe('graph link highlight', () => { expect(setLastClickedGraphAlertId).toHaveBeenCalledWith(testAlert.id); }); }); + +describe('detected push revision', () => { + const alertWithBackfill = (backfillOverrides = {}) => ({ + ...testAlert, + backfill_record: { + status: alertBackfillResultStatusMap.successful, + total_backfills_successful: 2, + ...backfillOverrides, + }, + }); + + test('renders the suggested culprit revision, styled italic/muted/small', async () => { + const revision = 'a1b2c3d4e5f6'; + const { getByText } = alertTableRowTest({ + alert: alertWithBackfill({ detected_push_revision: revision }), + tags: false, + }); + + const revisionEl = await waitFor(() => + getByText(`Suggested culprit: ${revision}`), + ); + expect(revisionEl).toBeInTheDocument(); + expect(revisionEl).toHaveClass('fst-italic'); + expect(revisionEl).toHaveClass('text-muted'); + expect(revisionEl).toHaveClass('small'); + }); + + test('truncates the suggested culprit revision to 12 characters', async () => { + const revision = 'abcdef0123456789abcdef0123456789abcdef01'; // 40-char sha + const { getByText, queryByText } = alertTableRowTest({ + alert: alertWithBackfill({ detected_push_revision: revision }), + tags: false, + }); + + const revisionEl = await waitFor(() => + getByText(`Suggested culprit: ${revision.slice(0, 12)}`), + ); + expect(revisionEl).toBeInTheDocument(); + // the full, untruncated revision is not shown + expect(queryByText(`Suggested culprit: ${revision}`)).toBeNull(); + }); + + test('does not render the revision span when detected_push_revision is absent', async () => { + const alert = alertWithBackfill(); // backfill record present, but no detected push + const { container, getByTestId } = alertTableRowTest({ + alert, + tags: false, + }); + + // the Sherlock icon still renders (backfill_record is present)... + await waitFor(() => getByTestId(`alert ${alert.id} sherlock icon`)); + // ...but the styled detected-push revision span does not + expect( + container.querySelector('.fst-italic.text-muted.small'), + ).toBeNull(); + }); +}); diff --git a/tests/ui/perfherder/alerts-view/alerts_test.jsx b/tests/ui/perfherder/alerts-view/alerts_test.jsx index 15095a78495..c81dec3d2fa 100644 --- a/tests/ui/perfherder/alerts-view/alerts_test.jsx +++ b/tests/ui/perfherder/alerts-view/alerts_test.jsx @@ -291,7 +291,7 @@ test('selecting all alerts and marking them as acknowledged updates all alerts', expect(alertCheckbox1).toHaveProperty('checked', true); expect(alertCheckbox2).toHaveProperty('checked', true); }); - let acknowledgeButton = await waitFor(() => getByText('Acknowledge')); + const acknowledgeButton = await waitFor(() => getByText('Acknowledge')); fireEvent.click(acknowledgeButton); @@ -391,7 +391,7 @@ test('selecting the alert summary checkbox then deselecting one alert only updat expect(alertCheckbox4).toHaveProperty('checked', true); }); - let acknowledgeButton = await waitFor(() => getByText('Acknowledge')); + const acknowledgeButton = await waitFor(() => getByText('Acknowledge')); fireEvent.click(acknowledgeButton); // only the selected alert has been updated diff --git a/treeherder/perf/models.py b/treeherder/perf/models.py index f12a03bb62a..e71eff9670a 100644 --- a/treeherder/perf/models.py +++ b/treeherder/perf/models.py @@ -1196,6 +1196,26 @@ def get_backfill_log(self, iteration: int) -> dict: return entry return None + def get_latest_detected_push(self) -> dict | None: + """ + Parse backfill_logs and return the most recent iteration's detected culprit push + as {"detected_push_id": int|None, "detected_push_revision": str|None}, or None + if nothing was ever detected. + """ + for entry in reversed(self.get_backfill_logs()): + if entry.get("detected_push_id") is not None: + return { + "detected_push_id": entry.get("detected_push_id"), + "detected_push_revision": entry.get("detected_push_revision"), + } + # Fallback for legacy records or logs that only hold the scalar. + if self.last_detected_push_id is not None: + return { + "detected_push_id": self.last_detected_push_id, + "detected_push_revision": None, + } + return None + def save(self, *args, **kwargs): # refresh parent's latest update time super().save(*args, **kwargs) diff --git a/treeherder/webapp/api/performance_serializers.py b/treeherder/webapp/api/performance_serializers.py index 974200dd635..34ffa5069ef 100644 --- a/treeherder/webapp/api/performance_serializers.py +++ b/treeherder/webapp/api/performance_serializers.py @@ -90,6 +90,16 @@ class BackfillRecordSerializer(serializers.Serializer): total_backfills_failed = serializers.IntegerField() total_backfills_successful = serializers.IntegerField() total_backfills_in_progress = serializers.IntegerField() + detected_push_id = serializers.SerializerMethodField() + detected_push_revision = serializers.SerializerMethodField() + + def get_detected_push_id(self, obj): + detected = obj.get_latest_detected_push() + return detected["detected_push_id"] if detected else None + + def get_detected_push_revision(self, obj): + detected = obj.get_latest_detected_push() + return detected["detected_push_revision"] if detected else None class Meta: model = BackfillRecord @@ -101,6 +111,8 @@ class Meta: "total_backfills_failed", "total_backfills_successful", "total_backfills_in_progress", + "detected_push_id", + "detected_push_revision", ) diff --git a/ui/perfherder/alerts/AlertTableRow.jsx b/ui/perfherder/alerts/AlertTableRow.jsx index c6acfcec8db..7ecebf33319 100644 --- a/ui/perfherder/alerts/AlertTableRow.jsx +++ b/ui/perfherder/alerts/AlertTableRow.jsx @@ -362,7 +362,13 @@ export default class AlertTableRow extends React.Component { } render() { - const { user = null, alert, alertSummary, lastClickedGraphAlertId, setLastClickedGraphAlertId } = this.props; + const { + user = null, + alert, + alertSummary, + lastClickedGraphAlertId, + setLastClickedGraphAlertId, + } = this.props; const { starred, checkboxSelected, icons } = this.state; const { repository, framework, revision } = alertSummary; @@ -384,7 +390,8 @@ export default class AlertTableRow extends React.Component { ? `Classified by ${alert.classifier_email}` : 'Classified automatically'; const bookmarkClass = starred ? 'visible' : ''; - const graphActive = lastClickedGraphAlertId !== null && lastClickedGraphAlertId === alert.id; + const graphActive = + lastClickedGraphAlertId !== null && lastClickedGraphAlertId === alert.id; const noiseProfile = alert.noise_profile || 'N\\A'; const noiseProfileTooltip = alert.noise_profile ? noiseProfiles[alert.noise_profile.replace('/', '')] @@ -398,6 +405,7 @@ export default class AlertTableRow extends React.Component { alert.side_by_side_available; const backfillStatusInfo = this.getBackfillStatusInfo(alert); + const detectedPushRevision = alert.backfill_record?.detected_push_revision; let sherlockTooltip = backfillStatusInfo?.message; if (backfillStatusInfo?.displayTasksCount) { sherlockTooltip = ( @@ -484,19 +492,26 @@ export default class AlertTableRow extends React.Component { this.getTitleText(alert, alertStatus) )} {backfillStatusInfo && ( - - - } - tooltipText={sherlockTooltip} - /> - + <> + + + } + tooltipText={sherlockTooltip} + /> + + {detectedPushRevision && ( + + Suggested culprit: {detectedPushRevision.slice(0, 12)} + + )} + )}