Bug 1867010 - Integrate replicates into the Graphs View. (#7887)

This patch adds a button to enable using replicates in the graphs view. The button is made available all the time as the cases for where replicates are available are not limited to a single branch anymore after PR #7886 lands. It also changes the API to return replicates for the specific case that the graphs view querys for (all_data).
mozilla · Dec 12, 2023 · fbb9eb4 · fbb9eb4
1 parent 0512e42
commit fbb9eb4
Show file tree

Hide file tree

Showing 6 changed files with 164 additions and 6 deletions.
diff --git a/tests/ui/perfherder/graphs-view/graphs_view_test.jsx b/tests/ui/perfherder/graphs-view/graphs_view_test.jsx
@@ -74,6 +74,7 @@ const mockShowModal = jest
 const graphsViewControls = (
   data = testData,
   hasNoData = true,
+  replicates = false,
   handleUpdateStateParams,
 ) => {
   const updateStateParams = () => {};
@@ -104,6 +105,7 @@ const graphsViewControls = (
       }}
       user={{ isStaff: true }}
       updateData={() => {}}
+      replicates={replicates}
     />,
   );
 };
@@ -214,6 +216,26 @@ test('Using select query param displays tooltip for correct datapoint', async ()
   expect(platform).toHaveTextContent(testData[0].platform);
 });
 
+test('Using select query param displays tooltip for correct datapoint with replicates', async () => {
+  const { getByTestId, getByText } = graphsViewControls(graphData, false, true);
+
+  const graphContainer = await waitFor(() => getByTestId('graphContainer'));
+
+  expect(graphContainer).toBeInTheDocument();
+
+  const graphTooltip = await waitFor(() => getByTestId('graphTooltip'));
+  const expectedRevision = '3afb892abb74c6d281f3e66431408cbb2e16b8c4';
+  const revision = await waitFor(() =>
+    getByText(expectedRevision.slice(0, 13)),
+  );
+  const repoName = await waitFor(() => getByTestId('repoName'));
+  const platform = await waitFor(() => getByTestId('platform'));
+  expect(graphTooltip).toBeInTheDocument();
+  expect(revision).toBeInTheDocument();
+  expect(repoName).toHaveTextContent(testData[0].repository_name);
+  expect(platform).toHaveTextContent(testData[0].platform);
+});
+
 test('InputFilter from TestDataModal can filter by application name', async () => {
   const {
     getByText,
@@ -377,6 +399,7 @@ describe('Mocked API calls', () => {
     const { getByText } = graphsViewControls(
       graphData,
       false,
+      false,
       updateStateParams,
     );
 
@@ -396,6 +419,7 @@ describe('Mocked API calls', () => {
     const { getByText } = graphsViewControls(
       graphData,
       false,
+      false,
       updateStateParams,
     );
 
@@ -410,5 +434,45 @@ describe('Mocked API calls', () => {
     expect(updateStateParams).toHaveBeenCalledTimes(1);
   });
 
+  test("'Use replicates' button can be turned on", async () => {
+    const updateStateParams = jest.fn();
+    const { getByText } = graphsViewControls(
+      graphData,
+      false,
+      false,
+      updateStateParams,
+    );
+
+    const useReplicatesButton = await waitFor(() =>
+      getByText('Use replicates'),
+    );
+
+    expect(useReplicatesButton.classList).not.toContain('active');
+
+    fireEvent.click(useReplicatesButton);
+
+    expect(updateStateParams).toHaveBeenCalledTimes(1);
+  });
+
+  test("'Use replicates' button can be turned off", async () => {
+    const updateStateParams = jest.fn();
+    const { getByText } = graphsViewControls(
+      graphData,
+      false,
+      true,
+      updateStateParams,
+    );
+
+    const useReplicatesButton = await waitFor(() =>
+      getByText('Use replicates'),
+    );
+
+    expect(useReplicatesButton.classList).toContain('active');
+
+    fireEvent.click(useReplicatesButton);
+
+    expect(updateStateParams).toHaveBeenCalledTimes(1);
+  });
+
   // Add here high level GraphsView tests...
 });
diff --git a/treeherder/webapp/api/performance_data.py b/treeherder/webapp/api/performance_data.py
@@ -703,9 +703,54 @@ def list(self, request):
 
         if signature and all_data:
             for item in self.queryset:
-                item['data'] = data.values(
-                    'value', 'job_id', 'id', 'push_id', 'push_timestamp', 'push__revision'
-                ).order_by('push_timestamp', 'push_id', 'job_id')
+                if replicates:
+                    item['data'] = list()
+                    for (
+                        value,
+                        job_id,
+                        datum_id,
+                        push_id,
+                        push_timestamp,
+                        push_revision,
+                        replicate_value,
+                    ) in data.values_list(
+                        'value',
+                        'job_id',
+                        'id',
+                        'push_id',
+                        'push_timestamp',
+                        'push__revision',
+                        'performancedatumreplicate__value',
+                    ).order_by(
+                        'push_timestamp', 'push_id', 'job_id'
+                    ):
+                        if replicate_value is not None:
+                            item['data'].append(
+                                {
+                                    "value": replicate_value,
+                                    "job_id": job_id,
+                                    "id": datum_id,
+                                    "push_id": push_id,
+                                    "push_timestamp": push_timestamp,
+                                    "push__revision": push_revision,
+                                }
+                            )
+                        elif value is not None:
+                            item['data'].append(
+                                {
+                                    "value": value,
+                                    "job_id": job_id,
+                                    "id": datum_id,
+                                    "push_id": push_id,
+                                    "push_timestamp": push_timestamp,
+                                    "push__revision": push_revision,
+                                }
+                            )
+                else:
+                    item['data'] = data.values(
+                        'value', 'job_id', 'id', 'push_id', 'push_timestamp', 'push__revision'
+                    ).order_by('push_timestamp', 'push_id', 'job_id')
+
                 item['option_name'] = option_collection_map[item['option_collection_id']]
                 item['repository_name'] = repository_name
 

diff --git a/ui/perfherder/graphs/GraphsView.jsx b/ui/perfherder/graphs/GraphsView.jsx
@@ -52,6 +52,7 @@ class GraphsView extends React.Component {
       showModal: false,
       showTable: false,
       visibilityChanged: false,
+      replicates: false,
     };
   }
 
@@ -62,6 +63,11 @@ class GraphsView extends React.Component {
   componentDidUpdate(prevProps) {
     const { location } = this.props;
     const { testData, loading } = this.state;
+    const { replicates } = queryString.parse(this.props.location.search);
+    const { replicates: prevReplicates } = queryString.parse(
+      prevProps.location.search,
+    );
+
     if (
       location.search === '' &&
       testData.length !== 0 &&
@@ -73,6 +79,12 @@ class GraphsView extends React.Component {
         testData: [],
       });
     }
+
+    if (prevReplicates !== undefined) {
+      if (replicates !== prevReplicates) {
+        window.location.reload(false);
+      }
+    }
   }
 
   getDefaultTimeRange = () => {
@@ -94,13 +106,20 @@ class GraphsView extends React.Component {
       highlightCommonAlerts,
       highlightChangelogData,
       highlightedRevisions,
+      replicates,
     } = queryString.parse(this.props.location.search);
 
     const updates = {};
 
     if (series) {
+      // TODO: Move series/test data fetch to after the params are parsed, and
+      // the component is updated. Here, it's using default settings even if
+      // the parameters are different.
       const _series = typeof series === 'string' ? [series] : series;
-      const seriesParams = this.parseSeriesParam(_series);
+      const seriesParams = this.parseSeriesParam(
+        _series,
+        Boolean(parseInt(replicates, 10)),
+      );
       this.getTestData(seriesParams, true);
     }
 
@@ -120,6 +139,10 @@ class GraphsView extends React.Component {
       );
     }
 
+    if (replicates) {
+      updates.replicates = Boolean(parseInt(replicates, 10));
+    }
+
     if (highlightedRevisions) {
       updates.highlightedRevisions =
         typeof highlightedRevisions === 'string'
@@ -150,6 +173,7 @@ class GraphsView extends React.Component {
       repository_name: repositoryName,
       signature_id: signatureId,
       framework_id: frameworkId,
+      replicates,
     } = series;
     const { timeRange } = this.state;
 
@@ -159,6 +183,7 @@ class GraphsView extends React.Component {
       framework: frameworkId,
       interval: timeRange.value,
       all_data: true,
+      replicates,
     };
   };
 
@@ -202,7 +227,7 @@ class GraphsView extends React.Component {
   };
 
   createGraphObject = async (seriesData) => {
-    const { colors, symbols, timeRange } = this.state;
+    const { colors, symbols, timeRange, replicates } = this.state;
     const alertSummaries = await Promise.all(
       seriesData.map((series) =>
         this.getAlertSummaries(series.signature_id, series.repository_id),
@@ -222,6 +247,7 @@ class GraphsView extends React.Component {
       newColors,
       newSymbols,
       commonAlerts,
+      replicates,
     );
 
     this.setState({ colors: newColors, symbols: newSymbols });
@@ -287,7 +313,7 @@ class GraphsView extends React.Component {
     this.setState({ testData: newTestData });
   };
 
-  parseSeriesParam = (series) =>
+  parseSeriesParam = (series, replicates) =>
     series.map((encodedSeries) => {
       const partialSeriesArray = encodedSeries.split(',');
       const partialSeriesObject = {
@@ -301,6 +327,7 @@ class GraphsView extends React.Component {
         // for visibility of test legend cards but isn't actually being used
         // to control visibility so it should be removed at some point
         framework_id: parseInt(partialSeriesArray[3], 10),
+        replicates,
       };
 
       return partialSeriesObject;
@@ -330,6 +357,7 @@ class GraphsView extends React.Component {
       highlightChangelogData,
       highlightedRevisions,
       timeRange,
+      replicates,
     } = this.state;
 
     const newSeries = testData.map(
@@ -342,6 +370,7 @@ class GraphsView extends React.Component {
       highlightCommonAlerts: +highlightCommonAlerts,
       highlightChangelogData: +highlightChangelogData,
       timerange: timeRange.value,
+      replicates: +replicates,
       zoom,
     };
 
@@ -388,6 +417,7 @@ class GraphsView extends React.Component {
       showModal,
       showTable,
       visibilityChanged,
+      replicates,
     } = this.state;
 
     const { projects, frameworks, user } = this.props;
@@ -473,6 +503,7 @@ class GraphsView extends React.Component {
                 updateData={this.updateData}
                 toggle={() => this.setState({ showModal: !showModal })}
                 toggleTableView={() => this.setState({ showTable: !showTable })}
+                replicates={replicates}
                 updateTimeRange={(newTimeRange) =>
                   this.setState(
                     {

diff --git a/ui/perfherder/graphs/GraphsViewControls.jsx b/ui/perfherder/graphs/GraphsViewControls.jsx
@@ -78,6 +78,7 @@ export default class GraphsViewControls extends React.Component {
       hasNoData,
       toggle,
       toggleTableView,
+      replicates,
       showModal,
       showTable,
       testData,
@@ -200,6 +201,19 @@ export default class GraphsViewControls extends React.Component {
                   >
                     Highlight common alerts
                   </Button>
+                  <Button
+                    className="ml-3"
+                    color="darker-info"
+                    outline
+                    onClick={() =>
+                      updateStateParams({
+                        replicates: !replicates,
+                      })
+                    }
+                    active={replicates}
+                  >
+                    Use replicates
+                  </Button>
                 </Col>
               )}
             </Row>

diff --git a/ui/perfherder/graphs/TestDataModal.jsx b/ui/perfherder/graphs/TestDataModal.jsx
@@ -383,12 +383,14 @@ export default class TestDataModal extends React.Component {
       getTestData,
       timeRange: parentTimeRange,
       updateTestsAndTimeRange,
+      replicates,
     } = this.props;
 
     const displayedTestParams = selectedTests.map((series) => ({
       repository_name: series.projectName,
       signature_id: parseInt(series.id, 10),
       framework_id: parseInt(series.frameworkId, 10),
+      replicates,
     }));
 
     this.setState({

diff --git a/ui/perfherder/perf-helpers/helpers.js b/ui/perfherder/perf-helpers/helpers.js
@@ -811,6 +811,7 @@ export const createGraphData = (
   colors,
   symbols,
   commonAlerts,
+  replicates,
 ) =>
   seriesData.map((series) => {
     const color = colors.pop();
@@ -858,6 +859,7 @@ export const createGraphData = (
       lowerIsBetter: series.lower_is_better,
       resultSetData: series.data.map((dataPoint) => dataPoint.push_id),
       parentSignature: series.parent_signature,
+      replicates,
     };
   });