Merge pull request #86 from craigatk/status-error

(fix) Ensure refresh on test run status page doesn't run indefinitely
craigatk · May 8, 2020 · 278ffe5 · 278ffe5
2 parents 11a5f96 + e91ce44
commit 278ffe5
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 1 deletion.
diff --git a/ui/src/TestResults/TestResultsProcessingCheck.tsx b/ui/src/TestResults/TestResultsProcessingCheck.tsx
@@ -14,6 +14,7 @@ interface TestResultsProcessingCheckProps extends RouteComponentProps {
   publicId: string;
   processingSucceeded: () => void;
   refreshInterval: number;
+  autoRefreshTimeout: number;
 }
 
 const useStyles = makeStyles((theme) => ({
@@ -46,8 +47,10 @@ const TestResultsProcessingCheck = ({
   publicId,
   processingSucceeded,
   refreshInterval,
+  autoRefreshTimeout,
 }: TestResultsProcessingCheckProps) => {
   const classes = useStyles({});
+  let totalWaitTime = 0;
 
   const [resultsProcessing, setResultsProcessing] = React.useState<
     TestResultsProcessing
@@ -62,8 +65,12 @@ const TestResultsProcessingCheck = ({
         setResultsProcessing(response.data);
         setLoadingState(LoadingState.Success);
 
-        if (resultsAreStillProcessing(response.data)) {
+        if (
+          resultsAreStillProcessing(response.data) &&
+          totalWaitTime < autoRefreshTimeout
+        ) {
           setTimeout(loadTestResultsProcessing, refreshInterval);
+          totalWaitTime += refreshInterval;
         }
       })
       .catch(() => setLoadingState(LoadingState.Error));

diff --git a/ui/src/TestResults/__tests__/TestResultsProcessingCheck.spec.tsx b/ui/src/TestResults/__tests__/TestResultsProcessingCheck.spec.tsx
@@ -44,6 +44,7 @@ describe("TestResultsProcessingCheck", () => {
         publicId={publicId}
         processingSucceeded={succeededFunc}
         refreshInterval={5000}
+        autoRefreshTimeout={60000}
       />
     );
 
@@ -52,6 +53,32 @@ describe("TestResultsProcessingCheck", () => {
     expect(succeededFunc).not.toHaveBeenCalled();
   });
 
+  it("should refresh status when still processing up to max timeout", async (done) => {
+    const succeededFunc = jest.fn();
+
+    mockProcessingStatus(TestResultsProcessingStatus.PROCESSING);
+
+    const { findByTestId } = render(
+      <TestResultsProcessingCheck
+        publicId={publicId}
+        processingSucceeded={succeededFunc}
+        refreshInterval={100}
+        autoRefreshTimeout={400}
+      />
+    );
+
+    await findByTestId("results-still-processing");
+
+    await waitFor(() =>
+      expect(mockAxios.history.get.length).toBeGreaterThan(1)
+    );
+
+    setTimeout(() => {
+      expect(mockAxios.history.get.length).toBeLessThanOrEqual(5);
+      done();
+    }, 1000);
+  });
+
   it("should display failure message when results processing failed", async () => {
     const succeededFunc = jest.fn();
 
@@ -62,6 +89,7 @@ describe("TestResultsProcessingCheck", () => {
         publicId={publicId}
         processingSucceeded={succeededFunc}
         refreshInterval={5000}
+        autoRefreshTimeout={60000}
       />
     );
 
@@ -80,6 +108,7 @@ describe("TestResultsProcessingCheck", () => {
         publicId={publicId}
         processingSucceeded={succeededFunc}
         refreshInterval={5000}
+        autoRefreshTimeout={60000}
       />
     );
 
@@ -98,6 +127,7 @@ describe("TestResultsProcessingCheck", () => {
         publicId={publicId}
         processingSucceeded={succeededFunc}
         refreshInterval={5000}
+        autoRefreshTimeout={60000}
       />
     );
 

diff --git a/ui/src/TestRun/TestRunDataWrapper.tsx b/ui/src/TestRun/TestRunDataWrapper.tsx
@@ -56,6 +56,7 @@ const TestRunDataWrapper = ({ publicId }: TestRunDataWrapperProps) => {
           publicId={publicId}
           processingSucceeded={loadTestRunSummary}
           refreshInterval={3000}
+          autoRefreshTimeout={300000}
         />
       }
     />