build(deps-dev): bump sinon from 17.0.1 to 19.0.2 #6302
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,14 +109,14 @@ describe('ClassificationQueue', function () { | |
| }) | ||
|
|
||
| describe('keeps classifications in localStorage if backend fails', function () { | ||
| before(function () { | ||
| sinon.stub(global, 'setTimeout').callsFake(() => 100) | ||
| sinon.stub(global, 'clearTimeout') | ||
|
There was a problem hiding this comment. @eatyourgreens the use of There was a problem hiding this comment. From the release notes for 19, they’ve upgraded Fake Timers to the latest version. Do you know what the classification queue tests are using the fake clock for? There was a problem hiding this comment. The docs for fake timers are here but, as noted, it’s just a thin wrapper for the Fake Timers package. These docs do explain There was a problem hiding this comment. With your change, the tests run using the local clock ie. the clock continues to tick during the tests. With fake timers, I think the test clock doesn’t tick unless you tell it to tick. There was a problem hiding this comment. This test now looks like it’s evergreen to me, because timeouts can’t be set by the mocks. If there were bugs in the timeout code, I think the mocks might hide them. There was a problem hiding this comment. Similarly, this should mock By turning off timeouts, you've turned off retries in the classification queue. That's potentially a problem because the tests check whether or not a retry has been scheduled for a given response. |
||
| }) | ||
|
|
||
| after(function () { | ||
| global.setTimeout.restore() | ||
| global.clearTimeout.restore() | ||
| }) | ||
|
|
||
| beforeEach(function () { | ||
|
|
@@ -166,14 +166,14 @@ describe('ClassificationQueue', function () { | |
| }) | ||
|
|
||
| describe('with invalid classifications', function () { | ||
| before(function () { | ||
| sinon.stub(global, 'setTimeout').callsFake(() => 100) | ||
| sinon.stub(global, 'clearTimeout') | ||
| }) | ||
|
|
||
| after(function () { | ||
| global.setTimeout.restore() | ||
| global.clearTimeout.restore() | ||
| }) | ||
|
|
||
| beforeEach(function () { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've changed the return type ofMy mistake,setTimeouthere.setTimeoutreturns an integer, not a timeout.This should really mock
setTimeoutwith a function that creates a mock timeout object and returns its ID. ThenclearTimeoutshould destroy the mock timeout for a given ID.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fake Timers is useful here because it will take care of that mocking for you.
https://github.com/sinonjs/fake-timers#api-reference