Repository: sinonjs/fake-timers
Pull request #391 (2021-06-18 15:34:45)

Issue 390 implicit dependencies on faking interval
#### Purpose (TL;DR) - mandatory

Remove implicit dependency on `*Interval` functions in the shouldAdvanceTime feature.

#### Background (Problem in detail)  - optional
See #390


codecov[bot]: # [Codecov](https://codecov.io/gh/sinonjs/fake-timers/pull/391?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) Report
> Merging [#391](https://codecov.io/gh/sinonjs/fake-timers/pull/391?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) (dfc764e) into [master](https://codecov.io/gh/sinonjs/fake-timers/commit/54c2e8d031b09341e053dba3aa1c474b1eeee5aa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) (54c2e8d) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/sinonjs/fake-timers/pull/391/graphs/tree.svg?width=650&height=150&src=pr&token=YubuW6TySz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs)](https://codecov.io/gh/sinonjs/fake-timers/pull/391?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs)

```diff
@@           Coverage Diff           @@
##           master     #391   +/-   ##
=======================================
  Coverage   93.84%   93.84%           
=======================================
  Files           1        1           
  Lines         585      585           
=======================================
  Hits          549      549           
  Misses         36       36           
```

| Flag | Coverage Δ | |
|---|---|---|
| unit | `93.84% <100.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/sinonjs/fake-timers/pull/391?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | Coverage Δ | |
|---|---|---|
| [src/fake-timers-src.js](https://codecov.io/gh/sinonjs/fake-timers/pull/391/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs#diff-c3JjL2Zha2UtdGltZXJzLXNyYy5qcw==) | `93.84% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/sinonjs/fake-timers/pull/391?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/sinonjs/fake-timers/pull/391?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs). Last update [54c2e8d...dfc764e](https://codecov.io/gh/sinonjs/fake-timers/pull/391?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs).

fatso83: Not heard anything. Merging.

Patch:
diff --git a/src/fake-timers-src.js b/src/fake-timers-src.js
index 9229c50..58bce35 100644
--- a/src/fake-timers-src.js
+++ b/src/fake-timers-src.js
@@ -803,12 +803,6 @@ function withGlobal(_global) {
             } else {
                 if (_global[method] && _global[method].hadOwnProperty) {
                     _global[method] = clock[`_${method}`];
-                    if (
-                        method === "clearInterval" &&
-                        config.shouldAdvanceTime === true
-                    ) {
-                        _global[method](clock.attachedInterval);
-                    }
                 } else {
                     try {
                         delete _global[method];
@@ -819,6 +813,10 @@ function withGlobal(_global) {
             }
         }
 
+        if (config.shouldAdvanceTime === true) {
+            _global.clearInterval(clock.attachedInterval);
+        }
+
         // Prevent multiple executions which will completely remove these props
         clock.methods = [];
 
@@ -1602,38 +1600,37 @@ function withGlobal(_global) {
             });
         }
 
+        if (config.shouldAdvanceTime === true) {
+            const intervalTick = doIntervalTick.bind(
+                null,
+                clock,
+                config.advanceTimeDelta
+            );
+            const intervalId = _global.setInterval(
+                intervalTick,
+                config.advanceTimeDelta
+            );
+            clock.attachedInterval = intervalId;
+        }
+
         for (i = 0, l = clock.methods.length; i < l; i++) {
-            if (clock.methods[i] === "hrtime") {
+            const nameOfMethodToReplace = clock.methods[i];
+            if (nameOfMethodToReplace === "hrtime") {
                 if (
                     _global.process &&
                     typeof _global.process.hrtime === "function"
                 ) {
-                    hijackMethod(_global.process, clock.methods[i], clock);
+                    hijackMethod(_global.process, nameOfMethodToReplace, clock);
                 }
-            } else if (clock.methods[i] === "nextTick") {
+            } else if (nameOfMethodToReplace === "nextTick") {
                 if (
                     _global.process &&
                     typeof _global.process.nextTick === "function"
                 ) {
-                    hijackMethod(_global.process, clock.methods[i], clock);
+                    hijackMethod(_global.process, nameOfMethodToReplace, clock);
                 }
             } else {
-                if (
-                    clock.methods[i] === "setInterval" &&
-                    config.shouldAdvanceTime === true
-                ) {
-                    const intervalTick = doIntervalTick.bind(
-                        null,
-                        clock,
-                        config.advanceTimeDelta
-                    );
-                    const intervalId = _global[clock.methods[i]](
-                        intervalTick,
-                        config.advanceTimeDelta
-                    );
-                    clock.attachedInterval = intervalId;
-                }
-                hijackMethod(_global, clock.methods[i], clock);
+                hijackMethod(_global, nameOfMethodToReplace, clock);
             }
         }
 


Test patch:
diff --git a/test/fake-timers-test.js b/test/fake-timers-test.js
index e8c0f35..200d3d1 100644
--- a/test/fake-timers-test.js
+++ b/test/fake-timers-test.js
@@ -3955,6 +3955,24 @@ describe("FakeTimers", function () {
                 }
             }, interval);
         });
+
+        it("should not depend on having to stub setInterval or clearInterval to work", function (done) {
+            const origSetInterval = globalObject.setInterval;
+            const origClearInterval = globalObject.clearInterval;
+
+            var clock = FakeTimers.install({
+                shouldAdvanceTime: true,
+                toFake: ["setTimeout"],
+            });
+
+            assert.equals(globalObject.setInterval, origSetInterval);
+            assert.equals(globalObject.clearInterval, origClearInterval);
+
+            setTimeout(function () {
+                clock.uninstall();
+                done();
+            }, 0);
+        });
     });
 
     describe("requestAnimationFrame", function () {