search

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
794 stars 103 forks source link

fix: bind infinite loop check to clock instance #398

Open SimenB opened 2 years ago

SimenB commented 2 years ago

Purpose (TL;DR) - mandatory

Failing test from https://github.com/sinonjs/fake-timers/pull/375#issuecomment-919976426.

By not resetting, the counter is not tied to a specific clock but rather then entire withGlobal.

I'll push a second commit moving the attribute from living in the withGlobal closure to living on the clock object, which I think is more correct

SimenB commented 2 years ago

Pushed a commit that's almost there - interval fails since the timer itself doesn't have an error object - only the last one does. Not sure how to deal with it? A single interval will run forever with runAll without us ever registering an error

SimenB commented 2 years ago

@fatso83 I don't know how to deal with setInterval. Ideas?

One quick fix is of course to handle error not being present in getInfiniteLoopError and just return the error created there without trying to get a better trace

SimenB commented 2 years ago

Pushed that latest thing - not ideal, but I don't know how to solve it properly

fatso83 commented 2 years ago

This looks fine, but TBH I am just too tired to grok the details of this right now, so I cannot really say something very useful about the .error prop ...

codecov[bot] commented 2 years ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (85463d9) 94.10% compared to head (61cfb60) 94.10%. Report is 97 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #398 +/- ## ======================================= Coverage 94.10% 94.10% ======================================= Files 1 1 Lines 611 611 ======================================= Hits 575 575 Misses 36 36 ``` | [Flag](https://app.codecov.io/gh/sinonjs/fake-timers/pull/398/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | Coverage Δ | | |---|---|---| | [unit](https://app.codecov.io/gh/sinonjs/fake-timers/pull/398/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sinonjs) | `94.10% <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.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

stale[bot] commented 6 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

fatso83 commented 5 months ago

Is this going somewhere? Can I help? Not totally sure what to do between your last commend and Benji's approval.