Skip to content

Commit

Permalink
Allow anomaly icons to reflect changes from tooltip.
Browse files Browse the repository at this point in the history
- Pass along anomaly data to explore sk
- Rework the toast messages to account for all situations
- Refactor updateAnomaly function entry point to support
new bug, existing bug and ignore.

Change-Id: I08b87126f77eb1382a672104185d78b0e8890d61
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/941747
Reviewed-by: Arthur Wang <[email protected]>
Commit-Queue: Tony Seaward <[email protected]>
  • Loading branch information
SeawardT authored and SkCQ committed Jan 24, 2025
1 parent a8e06b1 commit b606a76
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 39 deletions.
6 changes: 4 additions & 2 deletions perf/modules/existing-bug-dialog-sk/existing-bug-dialog-sk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,13 @@ export class ExistingBugDialogSk extends ElementSk {
// Let explore-simple-sk and chart-tooltip-sk that anomalies have changed and we need to re-render.
this.dispatchEvent(
new CustomEvent('anomaly-changed', {
bubbles: true,
composed: true,
detail: {
newBug: false,
traceNames: this._traceNames,
anomalies: this._anomalies,
bugId: this.bug_id,
},
bubbles: true,
})
);
})
Expand Down
87 changes: 56 additions & 31 deletions perf/modules/explore-simple-sk/explore-simple-sk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1306,51 +1306,76 @@ export class ExploreSimpleSk extends ElementSk {
this.triageResultToast?.hide();
return;
}
const commits: number[] = [];
if (detail.traceNames) {
// Check if event passed along traceNames for nudging.
const anomalies: AnomalyMap = {};
const anomalies: Anomaly[] = detail.anomalies;
const traceNames: string[] = detail.traceNames;
if (anomalies.length === 0 || traceNames.length === 0) {
this.triageResultToast?.hide();
return;
}
for (let i = 0; i < anomalies.length; i++) {
const commits: number[] = [];
const anomalyMap: AnomalyMap = {};
const commitMap: CommitNumberAnomalyMap = {};

// TraceName should be the same across all anomalies.
const traceName = traceNames[i] || traceNames[0];
const startRevision: number | null = anomalies[i].start_revision;
const endRevision: number | null = anomalies[i].end_revision;

// Load all commits available on the plot.
const data: (ColumnHeader | null)[] | null = this.dfRepo.value!.dataframe.header;
if (data) {
// Create array of all commits for easy lookup.
data.forEach((data) => commits.push(data!.offset));
}

// Check for start or end commit and return first found.
const anomalyCommit: number | undefined = commits.find((commit) => {
if (commit === detail.anomaly.start_revision || commit === detail.anomaly.end_revision) {
let anomalyCommit: number | undefined = 0;
anomalyCommit = commits.find((commit) => {
if (commit === startRevision || commit === endRevision) {
return commit;
}
});

if (anomalyCommit) {
commitMap[anomalyCommit] = detail.anomaly;
anomalies[detail.traceNames[0]] = commitMap;
// If anomaly was nudged, then update and re-render frame.
this.dfRepo.value?.updateAnomalies(anomalies, detail.anomaly.id);
this._stateHasChanged();
this._render();
this.disableTooltip();
return;
commitMap[anomalyCommit] = anomalies[i];
anomalyMap[traceName] = commitMap;
this.dfRepo.value?.updateAnomalies(anomalyMap, anomalies[i].id);
}
// Update pop-up with bug details from anomaly change.
const bugId = detail.bugId;
const displayIndex = detail.displayIndex;
const editAction = detail.editAction;
const toastText = document.getElementById('triage-result-text')! as HTMLSpanElement;
const toastLink = document.getElementById('triage-result-link')! as HTMLAnchorElement;
toastLink.innerText = ``;
// BugId dictates that a bug is tied to the anomaly. (New or Existing).
if (bugId) {
toastText.textContent = `Anomaly associated with: `;
const link = `https://issues.chromium.org/issues/${bugId}`;
toastLink.innerText = `${bugId}`;
toastLink.setAttribute('href', `${link}`);
toastLink.setAttribute('target', '_blank');
}
// EditAction dictates this is a state change.
if (editAction) {
toastText.textContent = `Anomaly state changed to ${editAction}`;
}
// DisplayIndex dictates this is a nudge change.
if (displayIndex) {
if (anomalyCommit) {
toastText.textContent = `Anomaly Nudge Moved (${displayIndex}) to ${anomalyCommit}`;
// Since anomaly is moving, close tooltip.
this.disableTooltip();
} else {
// If no anomaly commit was found, then the nudge failed.
toastText.textContent = `Anomaly Nudge Failed (${displayIndex}) to ${startRevision}!`;
}
}
this.triageResultToast?.show();
this._stateHasChanged();
this._render();
}
// Update pop-up with new bug details from anomaly change.
const bugId = detail.bugId;
const newBug = detail.newBug;
const toastText = document.getElementById('triage-result-text')! as HTMLSpanElement;
const toastLink = document.getElementById('triage-result-link')! as HTMLAnchorElement;
if (newBug === true) {
toastText.textContent = `New bug created: `;
} else if (commits) {
toastText.textContent = `Anomaly Nudge Failed: `;
} else {
toastText.textContent = `Anomalies associated with: `;
}
const link = `https://issues.chromium.org/issues/${bugId}`;
toastLink.setAttribute('href', `${link}`);
toastLink.setAttribute('target', '_blank');
toastLink.innerText = `${bugId}`;
this.triageResultToast?.show();
});

// Listens to the user-issue-changed event and does appropriate actions
Expand Down
6 changes: 4 additions & 2 deletions perf/modules/new-bug-dialog-sk/new-bug-dialog-sk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,11 +395,13 @@ export class NewBugDialogSk extends ElementSk {
// Let explore-simple-sk and chart-tooltip-sk that anomalies have changed and we need to re-render.
this.dispatchEvent(
new CustomEvent('anomaly-changed', {
bubbles: true,
composed: true,
detail: {
newBug: true,
traceNames: this._traceNames,
anomalies: this._anomalies,
bugId: json.bug_id,
},
bubbles: true,
})
);
})
Expand Down
3 changes: 1 addition & 2 deletions perf/modules/plot-google-chart-sk/plot-google-chart-sk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class PlotGoogleChartSk extends LitElement {
background-color: rgba(255, 255, 0, 1); /* yellow */
}
md-icon.regression {
background-color: rgba(155, 0, 255, 1); /* purple */
background-color: rgba(255, 0, 0, 1); /* red */
}
md-icon.ignored {
background-color: rgba(100, 100, 100, 0.8); /* grey */
Expand Down Expand Up @@ -591,7 +591,6 @@ export class PlotGoogleChartSk extends LitElement {

const anomaly = anomalies[offset];
let cloned: HTMLElement | null;
// TODO(b/377751454): add separate anomaly icon for "ignored"
if (anomaly.is_improvement) {
cloned = cloneSlot('improvement', key, offset);
} else if (anomaly.bug_id === 0) {
Expand Down
10 changes: 8 additions & 2 deletions perf/modules/triage-menu-sk/triage-menu-sk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export class TriageMenuSk extends ElementSk {
})
.then(jsonOrThrow)
.then((_) => {
let bug_id = null;
let bug_id: number | null = null;
if (editAction === 'RESET') {
bug_id = 0;
} else if (editAction === 'IGNORE') {
Expand All @@ -178,6 +178,11 @@ export class TriageMenuSk extends ElementSk {
new CustomEvent('anomaly-changed', {
bubbles: true,
composed: true,
detail: {
traceNames: traceNames,
editAction: editAction,
anomalies: anomalies,
},
})
);
})
Expand Down Expand Up @@ -232,7 +237,8 @@ export class TriageMenuSk extends ElementSk {
composed: true,
detail: {
traceNames: traceNames,
anomaly: entry.anomaly_data?.anomaly,
anomalies: [entry.anomaly_data?.anomaly],
displayIndex: entry.display_index,
},
})
);
Expand Down

0 comments on commit b606a76

Please sign in to comment.