Bug 1967984 - show the Failure Summary faster when changing the selected job by doing more network requests in parallel, r=Aryx. (#8876)

Author: fqueze

tests/ui/job-view/App_test.jsx

@@ -82,15 +82,27 @@ describe('App', () => {
       ...fullJob,
       task_id: 'secondTaskId',
     });
+    fetchMock.get(
+      getProjectUrl('/jobs/259537372/bug_suggestions/', repoName),
+      [],
+    );
 
     fetchMock.get(getProjectUrl('/jobs/259539665/', repoName), {
       ...fullJob,
       task_id: 'MirsMc8UQPeSBC3yKMSlPw',
     });
+    fetchMock.get(
+      getProjectUrl('/jobs/259539665/bug_suggestions/', repoName),
+      [],
+    );
     fetchMock.get(getProjectUrl('/jobs/259539664/', repoName), {
       ...fullJob,
       task_id: 'Fe4GqwoZQSStNUbe4EeSPQ',
     });
+    fetchMock.get(
+      getProjectUrl('/jobs/259539664/bug_suggestions/', repoName),
+      [],
+    );
     fetchMock.get(
       `begin:${getProjectUrl('/performance/data/?job_id=', repoName)}`,
       [],

tests/ui/shared/FailureSummaryTab_test.jsx

@@ -58,6 +58,7 @@ describe('FailureSummaryTab', () => {
         />
         <FailureSummaryTab
           selectedJob={selectedJob}
+          selectedJobId={selectedJob.id}
           jobLogUrls={jobLogUrls}
           logParseStatus="parsed"
           reftestUrl="boo"

ui/job-view/details/DetailsPanel.jsx

@@ -115,7 +115,6 @@ class DetailsPanel extends React.Component {
 
     this.setState(
       {
-        jobDetails: [],
         suggestions: [],
         jobDetailLoading: true,
         jobArtifactsLoading: true,
@@ -167,26 +166,96 @@ class DetailsPanel extends React.Component {
           this.selectJobController.signal,
         );
 
-        const phSeriesPromise = PerfSeriesModel.getSeriesData(
+        const performancePromise = PerfSeriesModel.getSeriesData(
           currentRepo.name,
           {
             job_id: selectedJob.id,
           },
-        );
+        ).then(async (phSeriesResult) => {
+          const performanceData = Object.values(phSeriesResult).reduce(
+            (a, b) => [...a, ...b],
+            [],
+          );
+          let perfJobDetail = [];
+
+          if (performanceData.length) {
+            const signatureIds = [
+              ...new Set(performanceData.map((perf) => perf.signature_id)),
+            ];
+            const seriesListList = await Promise.all(
+              chunk(signatureIds, 20).map((signatureIdChunk) =>
+                PerfSeriesModel.getSeriesList(currentRepo.name, {
+                  id: signatureIdChunk,
+                }),
+              ),
+            );
+            const mappedFrameworks = {};
+            frameworks.forEach((element) => {
+              mappedFrameworks[element.id] = element.name;
+            });
+
+            const seriesList = seriesListList
+              .map((item) => item.data)
+              .reduce((a, b) => [...a, ...b], []);
+
+            perfJobDetail = performanceData
+              .map((d) => ({
+                series: seriesList.find((s) => d.signature_id === s.id),
+                ...d,
+              }))
+              .map((d) => ({
+                url: `/perfherder/graphs?series=${[
+                  currentRepo.name,
+                  d.signature_id,
+                  1,
+                  d.series.frameworkId,
+                ]}&selected=${[d.signature_id, d.id]}`,
+                shouldAlert: d.series.should_alert,
+                value: d.value,
+                measurementUnit: d.series.measurementUnit,
+                lowerIsBetter: d.series.lowerIsBetter,
+                title: d.series.name,
+                suite: d.series.suite,
+                options: d.series.options.join(' '),
+                frameworkName: mappedFrameworks[d.series.frameworkId],
+                perfdocs: new Perfdocs(
+                  mappedFrameworks[d.series.frameworkId],
+                  d.series.suite,
+                  d.series.platform,
+                  d.series.name,
+                ),
+              }));
+          }
+          perfJobDetail.sort((a, b) => {
+            // Sort perfJobDetails by value of shouldAlert in a particular order:
+            // first true values, after that null values and then false.
+            if (a.shouldAlert === true) {
+              return -1;
+            }
+            if (a.shouldAlert === false) {
+              return 1;
+            }
+            if (a.shouldAlert === null && b.shouldAlert === true) {
+              return 1;
+            }
+            if (a.shouldAlert === null && b.shouldAlert === false) {
+              return -1;
+            }
+            return 0;
+          });
+          this.setState({
+            perfJobDetail,
+          });
+        });
 
         Promise.all([
           jobPromise,
           jobLogUrlPromise,
-          phSeriesPromise,
           builtFromArtifactPromise,
+          this.updateClassifications(),
         ])
           .then(
-            async ([
-              jobResult,
-              jobLogUrlResult,
-              phSeriesResult,
-              builtFromArtifactResult,
-            ]) => {
+            async ([jobResult, jobLogUrlResult, builtFromArtifactResult]) => {
               // This version of the job has more information than what we get in the main job list.  This
               // is what we'll pass to the rest of the details panel.
               // Don't update the job instance in the greater job field so as to not add the memory overhead
@@ -242,93 +311,31 @@ class DetailsPanel extends React.Component {
                 currentRepo.name,
               );
               const logViewerFullUrl = `${window.location.origin}${logViewerUrl}`;
-              const performanceData = Object.values(phSeriesResult).reduce(
-                (a, b) => [...a, ...b],
-                [],
-              );
-              let perfJobDetail = [];
-
-              if (performanceData.length) {
-                const signatureIds = [
-                  ...new Set(performanceData.map((perf) => perf.signature_id)),
-                ];
-                const seriesListList = await Promise.all(
-                  chunk(signatureIds, 20).map((signatureIdChunk) =>
-                    PerfSeriesModel.getSeriesList(currentRepo.name, {
-                      id: signatureIdChunk,
-                    }),
-                  ),
-                );
-                const mappedFrameworks = {};
-                frameworks.forEach((element) => {
-                  mappedFrameworks[element.id] = element.name;
-                });
-
-                const seriesList = seriesListList
-                  .map((item) => item.data)
-                  .reduce((a, b) => [...a, ...b], []);
-
-                perfJobDetail = performanceData
-                  .map((d) => ({
-                    series: seriesList.find((s) => d.signature_id === s.id),
-                    ...d,
-                  }))
-                  .map((d) => ({
-                    url: `/perfherder/graphs?series=${[
-                      currentRepo.name,
-                      d.signature_id,
-                      1,
-                      d.series.frameworkId,
-                    ]}&selected=${[d.signature_id, d.id]}`,
-                    shouldAlert: d.series.should_alert,
-                    value: d.value,
-                    measurementUnit: d.series.measurementUnit,
-                    lowerIsBetter: d.series.lowerIsBetter,
-                    title: d.series.name,
-                    suite: d.series.suite,
-                    options: d.series.options.join(' '),
-                    frameworkName: mappedFrameworks[d.series.frameworkId],
-                    perfdocs: new Perfdocs(
-                      mappedFrameworks[d.series.frameworkId],
-                      d.series.suite,
-                      d.series.platform,
-                      d.series.name,
-                    ),
-                  }));
+
+              const newState = {
+                selectedJobFull,
+                jobLogUrls,
+                logParseStatus,
+                logViewerUrl,
+                logViewerFullUrl,
+                jobRevision,
+              };
+
+              // Only wait for the performance data before setting
+              // jobDetailLoading to false if we will not be showing
+              // the Failure Summary panel by default.
+              if (
+                !['busted', 'testfailed', 'exception'].includes(
+                  selectedJobFull.resultStatus,
+                )
+              ) {
+                this.setState(newState);
+                await performancePromise;
+                this.setState({ jobDetailLoading: false });
+              } else {
+                newState.jobDetailLoading = false;
+                this.setState(newState);
               }
-              perfJobDetail.sort((a, b) => {
-                // Sort perfJobDetails by value of shouldAlert in a particular order:
-                // first true values, after that null values and then false.
-                if (a.shouldAlert === true) {
-                  return -1;
-                }
-                if (a.shouldAlert === false) {
-                  return 1;
-                }
-                if (a.shouldAlert === null && b.shouldAlert === true) {
-                  return 1;
-                }
-                if (a.shouldAlert === null && b.shouldAlert === false) {
-                  return -1;
-                }
-                return 0;
-              });
-
-              this.setState(
-                {
-                  selectedJobFull,
-                  jobLogUrls,
-                  logParseStatus,
-                  logViewerUrl,
-                  logViewerFullUrl,
-                  perfJobDetail,
-                  jobRevision,
-                },
-                async () => {
-                  await this.updateClassifications();
-                  this.setState({ jobDetailLoading: false });
-                },
-              );
             },
           )
           .finally(() => {
@@ -346,6 +353,7 @@ class DetailsPanel extends React.Component {
       classificationMap,
       classificationTypes,
       isPinBoardVisible,
+      selectedJob,
     } = this.props;
     const {
       selectedJobFull,
@@ -403,6 +411,7 @@ class DetailsPanel extends React.Component {
             />
             <span className="job-tabs-divider" />
             <TabsPanel
+              selectedJob={selectedJob}
               selectedJobFull={selectedJobFull}
               currentRepo={currentRepo}
               jobDetails={jobDetails}

ui/job-view/details/tabs/TabsPanel.jsx

@@ -41,26 +41,26 @@ class TabsPanel extends React.Component {
   }
 
   static getDerivedStateFromProps(props, state) {
-    const { perfJobDetail, selectedJobFull } = props;
+    const { perfJobDetail, selectedJob } = props;
 
     // This fires every time the props change.  But we only want to figure out the new default
     // tab when we get a new job.  However, the job could change, then later, the perf details fetch
     // returns.  So we need to check for a change in the size of the perfJobDetail too.
     if (
-      state.jobId !== selectedJobFull.id ||
-      state.perfJobDetailSize !== perfJobDetail.length
+      !!selectedJob &&
+      (state.jobId !== selectedJob.id ||
+        state.perfJobDetailSize !== perfJobDetail.length)
     ) {
       const tabIndex = TabsPanel.getDefaultTabIndex(
-        selectedJobFull.resultStatus,
+        selectedJob.resultStatus,
         props,
       );
-
       return {
         tabIndex,
         // Every time we select a different job we need to let the component
         // let us know if we should enable the tab
         enableTestGroupsTab: false,
-        jobId: selectedJobFull.id,
+        jobId: selectedJob.id,
         perfJobDetailSize: perfJobDetail.length,
       };
     }
@@ -135,6 +135,7 @@ class TabsPanel extends React.Component {
       classificationMap,
       logViewerFullUrl,
       clearSelectedJob,
+      selectedJob,
       selectedJobFull,
       currentRepo,
       pinJob,
@@ -224,6 +225,7 @@ class TabsPanel extends React.Component {
           <TabPanel>
             <FailureSummaryTab
               selectedJob={selectedJobFull}
+              selectedJobId={selectedJob && selectedJob.id}
               jobLogUrls={jobLogUrls}
               jobDetails={jobDetails}
               logParseStatus={logParseStatus}
@@ -295,6 +297,7 @@ TabsPanel.propTypes = {
   pinnedJobs: PropTypes.shape({}).isRequired,
   bugs: PropTypes.arrayOf(PropTypes.shape({})).isRequired,
   clearSelectedJob: PropTypes.func.isRequired,
+  selectedJob: PropTypes.shape({}).isRequired,
   selectedJobFull: PropTypes.shape({}).isRequired,
   currentRepo: PropTypes.shape({}).isRequired,
   perfJobDetail: PropTypes.arrayOf(PropTypes.shape({})),

ui/push-health/details/DetailsPanel.jsx

@@ -193,6 +193,7 @@ class DetailsPanel extends React.Component {
                 <TabPanel>
                   <FailureSummaryTab
                     selectedJob={selectedTaskFull}
+                    selectedJobId={selectedTaskFull.id}
                     jobLogUrls={selectedTaskFull.logs}
                     logParseStatus="unknown"
                     logViewerFullUrl={getLogViewerUrl(

ui/shared/tabs/failureSummary/FailureSummaryTab.jsx

@@ -41,12 +41,12 @@ class FailureSummaryTab extends React.Component {
   }
 
   componentDidUpdate(prevProps) {
-    const { selectedJob } = this.props;
+    const { selectedJobId } = this.props;
 
     if (
-      !!selectedJob &&
+      !!selectedJobId &&
       !!prevProps.selectedJob &&
-      selectedJob.id !== prevProps.selectedJob.id
+      selectedJobId !== prevProps.selectedJobId
     ) {
       this.loadBugSuggestions();
     }
@@ -129,13 +129,13 @@ class FailureSummaryTab extends React.Component {
   };
 
   loadBugSuggestions = () => {
-    const { selectedJob } = this.props;
+    const { selectedJobId } = this.props;
 
-    if (!selectedJob) {
+    if (!selectedJobId) {
       return;
     }
     this.setState({ bugSuggestionsLoading: true });
-    BugSuggestionsModel.get(selectedJob.id).then(async (suggestions) => {
+    BugSuggestionsModel.get(selectedJobId).then(async (suggestions) => {
       suggestions.forEach((suggestion) => {
         suggestion.bugs.too_many_open_recent =
           suggestion.bugs.open_recent.length > thBugSuggestionLimit;
@@ -351,6 +351,7 @@ class FailureSummaryTab extends React.Component {
 
 FailureSummaryTab.propTypes = {
   selectedJob: PropTypes.shape({}).isRequired,
+  selectedJobId: PropTypes.number.isRequired,
   jobLogUrls: PropTypes.arrayOf({
     id: PropTypes.number.isRequired,
     job_id: PropTypes.number.isRequired,