Add traceTimeout to state and stopPolling action

**Why?**
This was causing an issue in tests because the poll
was never properly disposed.

https://gitlab.com/gitlab-org/gitlab/issues/55254

app/assets/javascripts/jobs/components/job_app.vue

@@ -176,7 +176,9 @@ export default {
   mounted() {
     this.updateSidebar();
   },
-  destroyed() {
+  beforeDestroy() {
+    this.stopPollingTrace();
+    this.stopPolling();
     window.removeEventListener('resize', this.onResize);
     window.removeEventListener('scroll', this.updateScroll);
   },
@@ -192,6 +194,8 @@ export default {
       'fetchTrace',
       'scrollBottom',
       'scrollTop',
+      'stopPollingTrace',
+      'stopPolling',
       'toggleScrollButtons',
       'toggleScrollAnimation',
     ]),

app/assets/javascripts/jobs/store/actions.js

@@ -147,7 +147,6 @@ export const toggleScrollisInBottom = ({ commit }, toggle) => {
 
 export const requestTrace = ({ commit }) => commit(types.REQUEST_TRACE);
 
-let traceTimeout;
 export const fetchTrace = ({ dispatch, state }) =>
   axios
     .get(`${state.traceEndpoint}/trace.json`, {
@@ -157,24 +156,32 @@ export const fetchTrace = ({ dispatch, state }) =>
       dispatch('toggleScrollisInBottom', isScrolledToBottom());
       dispatch('receiveTraceSuccess', data);
 
-      if (!data.complete) {
-        traceTimeout = setTimeout(() => {
-          dispatch('fetchTrace');
-        }, 4000);
-      } else {
+      if (data.complete) {
         dispatch('stopPollingTrace');
+      } else if (!state.traceTimeout) {
+        dispatch('startPollingTrace');
       }
     })
     .catch(() => dispatch('receiveTraceError'));
 
-export const stopPollingTrace = ({ commit }) => {
+export const startPollingTrace = ({ dispatch, commit }) => {
+  const traceTimeout = setTimeout(() => {
+    commit(types.SET_TRACE_TIMEOUT, 0);
+    dispatch('fetchTrace');
+  }, 4000);
+
+  commit(types.SET_TRACE_TIMEOUT, traceTimeout);
+};
+
+export const stopPollingTrace = ({ state, commit }) => {
+  clearTimeout(state.traceTimeout);
+  commit(types.SET_TRACE_TIMEOUT, 0);
   commit(types.STOP_POLLING_TRACE);
-  clearTimeout(traceTimeout);
 };
+
 export const receiveTraceSuccess = ({ commit }, log) => commit(types.RECEIVE_TRACE_SUCCESS, log);
-export const receiveTraceError = ({ commit }) => {
-  commit(types.RECEIVE_TRACE_ERROR);
-  clearTimeout(traceTimeout);
+export const receiveTraceError = ({ dispatch }) => {
+  dispatch('stopPollingTrace');
   flash(__('An error occurred while fetching the job log.'));
 };
 /**

app/assets/javascripts/jobs/store/mutation_types.js

@@ -20,6 +20,7 @@ export const RECEIVE_JOB_SUCCESS = 'RECEIVE_JOB_SUCCESS';
 export const RECEIVE_JOB_ERROR = 'RECEIVE_JOB_ERROR';
 
 export const REQUEST_TRACE = 'REQUEST_TRACE';
+export const SET_TRACE_TIMEOUT = 'SET_TRACE_TIMEOUT';
 export const STOP_POLLING_TRACE = 'STOP_POLLING_TRACE';
 export const RECEIVE_TRACE_SUCCESS = 'RECEIVE_TRACE_SUCCESS';
 export const RECEIVE_TRACE_ERROR = 'RECEIVE_TRACE_ERROR';

app/assets/javascripts/jobs/store/mutations.js

@@ -53,17 +53,14 @@ export default {
     state.isTraceComplete = log.complete || state.isTraceComplete;
   },
 
-  /**
-   * Will remove loading animation
-   */
-  [types.STOP_POLLING_TRACE](state) {
-    state.isTraceComplete = true;
+  [types.SET_TRACE_TIMEOUT](state, id) {
+    state.traceTimeout = id;
   },
 
   /**
    * Will remove loading animation
    */
-  [types.RECEIVE_TRACE_ERROR](state) {
+  [types.STOP_POLLING_TRACE](state) {
     state.isTraceComplete = true;
   },
 

app/assets/javascripts/jobs/store/state.js

@@ -22,6 +22,7 @@ export default () => ({
   isTraceComplete: false,
   traceSize: 0,
   isTraceSizeVisible: false,
+  traceTimeout: 0,
 
   // used as a query parameter to fetch the trace
   traceState: null,

spec/frontend/jobs/store/mutations_spec.js

@@ -157,17 +157,21 @@ describe('Jobs Store Mutations', () => {
     });
   });
 
-  describe('STOP_POLLING_TRACE', () => {
-    it('sets isTraceComplete to true', () => {
-      mutations[types.STOP_POLLING_TRACE](stateCopy);
+  describe('SET_TRACE_TIMEOUT', () => {
+    it('sets the traceTimeout id', () => {
+      const id = 7;
 
-      expect(stateCopy.isTraceComplete).toEqual(true);
+      expect(stateCopy.traceTimeout).not.toEqual(id);
+
+      mutations[types.SET_TRACE_TIMEOUT](stateCopy, id);
+
+      expect(stateCopy.traceTimeout).toEqual(id);
     });
   });
 
-  describe('RECEIVE_TRACE_ERROR', () => {
-    it('resets trace state and sets error to true', () => {
-      mutations[types.RECEIVE_TRACE_ERROR](stateCopy);
+  describe('STOP_POLLING_TRACE', () => {
+    it('sets isTraceComplete to true', () => {
+      mutations[types.STOP_POLLING_TRACE](stateCopy);
 
       expect(stateCopy.isTraceComplete).toEqual(true);
     });

spec/javascripts/jobs/components/job_app_spec.js

@@ -46,9 +46,10 @@ describe('Job App ', () => {
   });
 
   afterEach(() => {
-    resetStore(store);
     vm.$destroy();
     mock.restore();
+
+    resetStore(store);
   });
 
   describe('while loading', () => {
@@ -384,7 +385,6 @@ describe('Job App ', () => {
           })
           .then(done)
           .catch(done.fail);
-        done();
       });
 
       it('displays remaining time for a delayed job', done => {

spec/javascripts/jobs/store/actions_spec.js

@@ -15,6 +15,7 @@ import {
   scrollBottom,
   requestTrace,
   fetchTrace,
+  startPollingTrace,
   stopPollingTrace,
   receiveTraceSuccess,
   receiveTraceError,
@@ -241,6 +242,50 @@ describe('Job State actions', () => {
           done,
         );
       });
+
+      describe('when job is incomplete', () => {
+        let tracePayload;
+
+        beforeEach(() => {
+          tracePayload = {
+            html: 'I, [2018-08-17T22:57:45.707325 #1841]  INFO -- :',
+            complete: false,
+          };
+
+          mock.onGet(`${TEST_HOST}/endpoint/trace.json`).replyOnce(200, tracePayload);
+        });
+
+        it('dispatches startPollingTrace', done => {
+          testAction(
+            fetchTrace,
+            null,
+            mockedState,
+            [],
+            [
+              { type: 'toggleScrollisInBottom', payload: true },
+              { type: 'receiveTraceSuccess', payload: tracePayload },
+              { type: 'startPollingTrace' },
+            ],
+            done,
+          );
+        });
+
+        it('does not dispatch startPollingTrace when timeout is non-empty', done => {
+          mockedState.traceTimeout = 1;
+
+          testAction(
+            fetchTrace,
+            null,
+            mockedState,
+            [],
+            [
+              { type: 'toggleScrollisInBottom', payload: true },
+              { type: 'receiveTraceSuccess', payload: tracePayload },
+            ],
+            done,
+          );
+        });
+      });
     });
 
     describe('error', () => {
@@ -265,16 +310,69 @@ describe('Job State actions', () => {
     });
   });
 
+  describe('startPollingTrace', () => {
+    let dispatch;
+    let commit;
+
+    beforeEach(() => {
+      jasmine.clock().install();
+
+      dispatch = jasmine.createSpy();
+      commit = jasmine.createSpy();
+
+      startPollingTrace({ dispatch, commit });
+    });
+
+    afterEach(() => {
+      jasmine.clock().uninstall();
+    });
+
+    it('should save the timeout id but not call fetchTrace', () => {
+      expect(commit).toHaveBeenCalledWith(types.SET_TRACE_TIMEOUT, 1);
+      expect(dispatch).not.toHaveBeenCalledWith('fetchTrace');
+    });
+
+    describe('after timeout has passed', () => {
+      beforeEach(() => {
+        jasmine.clock().tick(4000);
+      });
+
+      it('should clear the timeout id and fetchTrace', () => {
+        expect(commit).toHaveBeenCalledWith(types.SET_TRACE_TIMEOUT, 0);
+        expect(dispatch).toHaveBeenCalledWith('fetchTrace');
+      });
+    });
+  });
+
   describe('stopPollingTrace', () => {
+    let origTimeout;
+
+    beforeEach(() => {
+      // Can't use spyOn(window, 'clearTimeout') because this caused unrelated specs to timeout
+      // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23838#note_280277727
+      origTimeout = window.clearTimeout;
+      window.clearTimeout = jasmine.createSpy();
+    });
+
+    afterEach(() => {
+      window.clearTimeout = origTimeout;
+    });
+
     it('should commit STOP_POLLING_TRACE mutation ', done => {
+      const traceTimeout = 7;
+
       testAction(
         stopPollingTrace,
         null,
-        mockedState,
-        [{ type: types.STOP_POLLING_TRACE }],
+        { ...mockedState, traceTimeout },
+        [{ type: types.SET_TRACE_TIMEOUT, payload: 0 }, { type: types.STOP_POLLING_TRACE }],
         [],
-        done,
-      );
+      )
+        .then(() => {
+          expect(window.clearTimeout).toHaveBeenCalledWith(traceTimeout);
+        })
+        .then(done)
+        .catch(done.fail);
     });
   });
 
@@ -292,15 +390,8 @@ describe('Job State actions', () => {
   });
 
   describe('receiveTraceError', () => {
-    it('should commit RECEIVE_TRACE_ERROR mutation ', done => {
-      testAction(
-        receiveTraceError,
-        null,
-        mockedState,
-        [{ type: types.RECEIVE_TRACE_ERROR }],
-        [],
-        done,
-      );
+    it('should commit stop polling trace', done => {
+      testAction(receiveTraceError, null, mockedState, [], [{ type: 'stopPollingTrace' }], done);
     });
   });