Merge branch '38394-smarter-interval' into 'master'

don't re-run smart interval callback if there is already one in progress

Closes #38394

See merge request gitlab-org/gitlab-ce!15032

2 files changed, 139 insertions, 122 deletions

diff --git a/spec/javascripts/smart_interval_spec.js b/spec/javascripts/smart_interval_spec.js
index 7833bf3fb04..1c87fcec245 100644
--- a/spec/javascripts/smart_interval_spec.js
+++ b/spec/javascripts/smart_interval_spec.js
@@ -1,6 +1,6 @@
-import '~/smart_interval';
+import SmartInterval from '~/smart_interval';
 
-(() => {
+describe('SmartInterval', function () {
   const DEFAULT_MAX_INTERVAL = 100;
   const DEFAULT_STARTING_INTERVAL = 5;
   const DEFAULT_SHORT_TIMEOUT = 75;
@@ -9,7 +9,7 @@ import '~/smart_interval';
 
   function createDefaultSmartInterval(config) {
     const defaultParams = {
-      callback: () => {},
+      callback: () => Promise.resolve(),
       startingInterval: DEFAULT_STARTING_INTERVAL,
       maxInterval: DEFAULT_MAX_INTERVAL,
       incrementByFactorOf: DEFAULT_INCREMENT_FACTOR,
@@ -22,158 +22,171 @@ import '~/smart_interval';
       _.extend(defaultParams, config);
     }
 
-    return new gl.SmartInterval(defaultParams);
+    return new SmartInterval(defaultParams);
   }
 
-  describe('SmartInterval', function () {
-    describe('Increment Interval', function () {
-      beforeEach(function () {
-        this.smartInterval = createDefaultSmartInterval();
-      });
+  describe('Increment Interval', function () {
+    beforeEach(function () {
+      this.smartInterval = createDefaultSmartInterval();
+    });
 
-      it('should increment the interval delay', function (done) {
-        const interval = this.smartInterval;
-        setTimeout(() => {
-          const intervalConfig = this.smartInterval.cfg;
-          const iterationCount = 4;
-          const maxIntervalAfterIterations = intervalConfig.startingInterval *
-            (intervalConfig.incrementByFactorOf ** (iterationCount - 1)); // 40
-          const currentInterval = interval.getCurrentInterval();
-
-          // Provide some flexibility for performance of testing environment
-          expect(currentInterval).toBeGreaterThan(intervalConfig.startingInterval);
-          expect(currentInterval <= maxIntervalAfterIterations).toBeTruthy();
-
-          done();
-        }, DEFAULT_SHORT_TIMEOUT); // 4 iterations, increment by 2x = (5 + 10 + 20 + 40)
-      });
+    it('should increment the interval delay', function (done) {
+      const interval = this.smartInterval;
+      setTimeout(() => {
+        const intervalConfig = this.smartInterval.cfg;
+        const iterationCount = 4;
+        const maxIntervalAfterIterations = intervalConfig.startingInterval *
+          (intervalConfig.incrementByFactorOf ** (iterationCount - 1)); // 40
+        const currentInterval = interval.getCurrentInterval();
+
+        // Provide some flexibility for performance of testing environment
+        expect(currentInterval).toBeGreaterThan(intervalConfig.startingInterval);
+        expect(currentInterval <= maxIntervalAfterIterations).toBeTruthy();
+
+        done();
+      }, DEFAULT_SHORT_TIMEOUT); // 4 iterations, increment by 2x = (5 + 10 + 20 + 40)
+    });
 
-      it('should not increment past maxInterval', function (done) {
-        const interval = this.smartInterval;
+    it('should not increment past maxInterval', function (done) {
+      const interval = this.smartInterval;
 
-        setTimeout(() => {
-          const currentInterval = interval.getCurrentInterval();
-          expect(currentInterval).toBe(interval.cfg.maxInterval);
+      setTimeout(() => {
+        const currentInterval = interval.getCurrentInterval();
+        expect(currentInterval).toBe(interval.cfg.maxInterval);
 
-          done();
-        }, DEFAULT_LONG_TIMEOUT);
-      });
+        done();
+      }, DEFAULT_LONG_TIMEOUT);
     });
 
-    describe('Public methods', function () {
-      beforeEach(function () {
-        this.smartInterval = createDefaultSmartInterval();
+    it('does not increment while waiting for callback', function () {
+      jasmine.clock().install();
+
+      const smartInterval = createDefaultSmartInterval({
+        callback: () => new Promise($.noop),
       });
 
-      it('should cancel an interval', function (done) {
-        const interval = this.smartInterval;
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
+
+      const oneInterval = smartInterval.cfg.startingInterval * DEFAULT_INCREMENT_FACTOR;
+      expect(smartInterval.getCurrentInterval()).toEqual(oneInterval);
 
-        setTimeout(() => {
-          interval.cancel();
+      jasmine.clock().uninstall();
+    });
+  });
 
-          const intervalId = interval.state.intervalId;
-          const currentInterval = interval.getCurrentInterval();
-          const intervalLowerLimit = interval.cfg.startingInterval;
+  describe('Public methods', function () {
+    beforeEach(function () {
+      this.smartInterval = createDefaultSmartInterval();
+    });
 
-          expect(intervalId).toBeUndefined();
-          expect(currentInterval).toBe(intervalLowerLimit);
+    it('should cancel an interval', function (done) {
+      const interval = this.smartInterval;
 
-          done();
-        }, DEFAULT_SHORT_TIMEOUT);
-      });
+      setTimeout(() => {
+        interval.cancel();
 
-      it('should resume an interval', function (done) {
-        const interval = this.smartInterval;
+        const intervalId = interval.state.intervalId;
+        const currentInterval = interval.getCurrentInterval();
+        const intervalLowerLimit = interval.cfg.startingInterval;
 
-        setTimeout(() => {
-          interval.cancel();
+        expect(intervalId).toBeUndefined();
+        expect(currentInterval).toBe(intervalLowerLimit);
 
-          interval.resume();
+        done();
+      }, DEFAULT_SHORT_TIMEOUT);
+    });
 
-          const intervalId = interval.state.intervalId;
+    it('should resume an interval', function (done) {
+      const interval = this.smartInterval;
 
-          expect(intervalId).toBeTruthy();
+      setTimeout(() => {
+        interval.cancel();
 
-          done();
-        }, DEFAULT_SHORT_TIMEOUT);
-      });
+        interval.resume();
+
+        const intervalId = interval.state.intervalId;
+
+        expect(intervalId).toBeTruthy();
+
+        done();
+      }, DEFAULT_SHORT_TIMEOUT);
     });
+  });
 
-    describe('DOM Events', function () {
-      beforeEach(function () {
-        // This ensures DOM and DOM events are initialized for these specs.
-        setFixtures('<div></div>');
+  describe('DOM Events', function () {
+    beforeEach(function () {
+      // This ensures DOM and DOM events are initialized for these specs.
+      setFixtures('<div></div>');
 
-        this.smartInterval = createDefaultSmartInterval();
-      });
+      this.smartInterval = createDefaultSmartInterval();
+    });
 
-      it('should pause when page is not visible', function (done) {
-        const interval = this.smartInterval;
+    it('should pause when page is not visible', function (done) {
+      const interval = this.smartInterval;
 
-        setTimeout(() => {
-          expect(interval.state.intervalId).toBeTruthy();
+      setTimeout(() => {
+        expect(interval.state.intervalId).toBeTruthy();
 
-          // simulates triggering of visibilitychange event
-          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
+        // simulates triggering of visibilitychange event
+        interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
 
-          expect(interval.state.intervalId).toBeUndefined();
-          done();
-        }, DEFAULT_SHORT_TIMEOUT);
-      });
+        expect(interval.state.intervalId).toBeUndefined();
+        done();
+      }, DEFAULT_SHORT_TIMEOUT);
+    });
 
-      it('should change to the hidden interval when page is not visible', function (done) {
-        const HIDDEN_INTERVAL = 1500;
-        const interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL });
+    it('should change to the hidden interval when page is not visible', function (done) {
+      const HIDDEN_INTERVAL = 1500;
+      const interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL });
 
-        setTimeout(() => {
-          expect(interval.state.intervalId).toBeTruthy();
-          expect(interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
-            interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL).toBeTruthy();
+      setTimeout(() => {
+        expect(interval.state.intervalId).toBeTruthy();
+        expect(interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
+          interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL).toBeTruthy();
 
-          // simulates triggering of visibilitychange event
-          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
+        // simulates triggering of visibilitychange event
+        interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
 
-          expect(interval.state.intervalId).toBeTruthy();
-          expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
-          done();
-        }, DEFAULT_SHORT_TIMEOUT);
-      });
+        expect(interval.state.intervalId).toBeTruthy();
+        expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
+        done();
+      }, DEFAULT_SHORT_TIMEOUT);
+    });
 
-      it('should resume when page is becomes visible at the previous interval', function (done) {
-        const interval = this.smartInterval;
+    it('should resume when page is becomes visible at the previous interval', function (done) {
+      const interval = this.smartInterval;
 
-        setTimeout(() => {
-          expect(interval.state.intervalId).toBeTruthy();
+      setTimeout(() => {
+        expect(interval.state.intervalId).toBeTruthy();
 
-          // simulates triggering of visibilitychange event
-          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
+        // simulates triggering of visibilitychange event
+        interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
 
-          expect(interval.state.intervalId).toBeUndefined();
+        expect(interval.state.intervalId).toBeUndefined();
 
-          // simulates triggering of visibilitychange event
-          interval.handleVisibilityChange({ target: { visibilityState: 'visible' } });
+        // simulates triggering of visibilitychange event
+        interval.handleVisibilityChange({ target: { visibilityState: 'visible' } });
 
-          expect(interval.state.intervalId).toBeTruthy();
+        expect(interval.state.intervalId).toBeTruthy();
 
-          done();
-        }, DEFAULT_SHORT_TIMEOUT);
-      });
+        done();
+      }, DEFAULT_SHORT_TIMEOUT);
+    });
 
-      it('should cancel on page unload', function (done) {
-        const interval = this.smartInterval;
+    it('should cancel on page unload', function (done) {
+      const interval = this.smartInterval;
 
-        setTimeout(() => {
-          $(document).triggerHandler('beforeunload');
-          expect(interval.state.intervalId).toBeUndefined();
-          expect(interval.getCurrentInterval()).toBe(interval.cfg.startingInterval);
-          done();
-        }, DEFAULT_SHORT_TIMEOUT);
-      });
+      setTimeout(() => {
+        $(document).triggerHandler('beforeunload');
+        expect(interval.state.intervalId).toBeUndefined();
+        expect(interval.getCurrentInterval()).toBe(interval.cfg.startingInterval);
+        done();
+      }, DEFAULT_SHORT_TIMEOUT);
+    });
 
-      it('should execute callback before first interval', function () {
-        const interval = createDefaultSmartInterval({ immediateExecution: true });
-        expect(interval.cfg.immediateExecution).toBeFalsy();
-      });
+    it('should execute callback before first interval', function () {
+      const interval = createDefaultSmartInterval({ immediateExecution: true });
+      expect(interval.cfg.immediateExecution).toBeFalsy();
     });
   });
-})(window.gl || (window.gl = {}));
+});
diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
index e4324e91502..ba3721e7c84 100644
--- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
+++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
@@ -121,24 +121,28 @@ describe('mrWidgetOptions', () => {
 
     describe('initPolling', () => {
       it('should call SmartInterval', () => {
-        spyOn(gl, 'SmartInterval').and.returnValue({
-          resume() {},
-          stopTimer() {},
-        });
+        spyOn(vm, 'checkStatus').and.returnValue(Promise.resolve());
+        jasmine.clock().install();
         vm.initPolling();
 
+        expect(vm.checkStatus).not.toHaveBeenCalled();
+
+        jasmine.clock().tick(10000);
+
         expect(vm.pollingInterval).toBeDefined();
-        expect(gl.SmartInterval).toHaveBeenCalled();
+        expect(vm.checkStatus).toHaveBeenCalled();
+
+        jasmine.clock().uninstall();
       });
     });
 
     describe('initDeploymentsPolling', () => {
       it('should call SmartInterval', () => {
-        spyOn(gl, 'SmartInterval');
+        spyOn(vm, 'fetchDeployments').and.returnValue(Promise.resolve());
         vm.initDeploymentsPolling();
 
         expect(vm.deploymentsInterval).toBeDefined();
-        expect(gl.SmartInterval).toHaveBeenCalled();
+        expect(vm.fetchDeployments).toHaveBeenCalled();
       });
     });