sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
806 stars 107 forks source link

issue #304 second fix - clearTimeout clears setInterval and clearInterval clears setTimeout #330

Closed itayperry closed 4 years ago

itayperry commented 4 years ago

Purpose

This is a follow up to pull request #324

Allowing clearTimeout to clear setInterval and clearInterval to clear setTimeout, as this is possible by HTML Living Standard. Issue #304

After starting a simple Rollup project and using this package locally through npm-link I found out that the problem wasn't solved..

Apparently, only one file is being tested and the same function [clearTimer()] exists in two different files. Therefore, I found out that only changing fake-timers-src.js was not enough to actually make the change, but only to pass the test. It seems that fake-timers.js isn't being tested.

For further info read: https://github.com/sinonjs/fake-timers/issues/304#issuecomment-629370417 and also https://github.com/sinonjs/fake-timers/issues/304#issuecomment-629640406

Oh, and one more thing: The file fake-timers.js changes entirely (as opposed to fake-timers-src.js) when you click save in VSCode, probably because of the auto-indent option. Could anyone please check it for me? I just opened it on an old sublime to not change gazillion lines :)

codecov[bot] commented 4 years ago

Codecov Report

Merging #330 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #330   +/-   ##
=======================================
  Coverage   92.77%   92.77%           
=======================================
  Files           1        1           
  Lines         554      554           
=======================================
  Hits          514      514           
  Misses         40       40           
Flag Coverage Δ
#unit 92.77% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a211987...6cda346. Read the comment docs.

benjamingr commented 4 years ago

fake-timers.js is a generated file, you shouldn't need to change it manually. Here is where it is generated: https://github.com/sinonjs/fake-timers/blob/master/package.json#L24

This doesn't happen on every pull request - it happens when the version is bumped. https://github.com/sinonjs/fake-timers/blob/master/scripts/version.sh#L17

itayperry commented 4 years ago

So in theory, if I do npm run bundle to check my changes locally, it would work? And I'll close this pull request :)

benjamingr commented 4 years ago

Right

itayperry commented 4 years ago

I learned something new, thank you :)