Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(deps-dev): bump sinon from 17.0.1 to 19.0.2 #6302

Merged
merged 4 commits into from
Dec 11, 2024

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Sep 13, 2024

Bumps sinon from 17.0.1 to 19.0.2.

Changelog

Sourced from sinon's changelog.

19.0.2

  • 4eb4c4bc Use fix 13.0.2 version of fake-timers to get Date to pass instanceof checks (Carl-Erik Kopseng)
  • a5b03db3 Add links to code that is affected by the breaking changes (Carl-Erik Kopseng)

Released by Carl-Erik Kopseng on 2024-09-13.

19.0.1

  • 037ec2d2 Update migration docs (Carl-Erik Kopseng)

Released by Carl-Erik Kopseng on 2024-09-13.

19.0.0

  • 3534ab4f Bump samsam and nise to latest versions (#2617) (Carl-Erik Kopseng)

    Ensures consistency and less breakage when there are "circular" dependencies.

  • 912c568d upgrade fake timers and others (#2612) (Carl-Erik Kopseng)
    • Upgrade dependencies (includes breaking API in Fake Timers)
    • fake-timers: no longer creating dates using the original Date class, but a subclass (proxy)
  • 9715798e Use newer @mochify/* packages (#2609) (Carl-Erik Kopseng)

    Co-authored-by: Maximilian Antoni mail@maxantoni.de

Released by Carl-Erik Kopseng on 2024-09-13.

18.0.1

Basically a patch release to update a transitive dependency in Nise.

  • 03e33ec6 Pin fake-timers@11.2.2 to avoid breaking change (Carl-Erik Kopseng)
  • 5a7924ad Add createStubInstance header in stubs.md (Daniel Kaplan)
  • ad6804cd Bump elliptic from 6.5.5 to 6.5.7 (#2608) (dependabot[bot])
  • 033287bd Bump path-to-regexp and nise (#2611) (dependabot[bot])

    Bumps path-to-regexp to 8.1.0 and updates ancestor dependency nise. These dependencies need to be updated together.

    Updates path-to-regexp from 6.2.2 to 8.1.0

... (truncated)

Commits

Dependabot compatibility score

You can trigger a rebase of this PR by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Note
Automatic rebases have been disabled on this pull request as it has been open for over 30 days.

@dependabot dependabot bot added dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code labels Sep 13, 2024
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/sinon-19.0.2 branch 7 times, most recently from 01f7464 to 9b77329 Compare September 19, 2024 14:58
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/sinon-19.0.2 branch 2 times, most recently from 9ef6db9 to eb959e0 Compare September 25, 2024 20:41
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/sinon-19.0.2 branch from eb959e0 to 15367c5 Compare October 3, 2024 22:01
@goplayoutside3
Copy link
Contributor

@dependabot rebase

Bumps [sinon](https://github.com/sinonjs/sinon) from 17.0.1 to 19.0.2.
- [Release notes](https://github.com/sinonjs/sinon/releases)
- [Changelog](https://github.com/sinonjs/sinon/blob/main/docs/changelog.md)
- [Commits](sinonjs/sinon@v17.0.1...v19.0.2)

---
updated-dependencies:
- dependency-name: sinon
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/npm_and_yarn/sinon-19.0.2 branch from 15367c5 to 8ecda5d Compare December 10, 2024 22:32
@goplayoutside3
Copy link
Contributor

This is failing because of the sinon.useFakeTimers({ global }) config in 10 unit tests in ClassificationQueue.

Comment on lines 115 to 114
clock = sinon.useFakeTimers({ global })
sinon.stub(global, 'setTimeout').callsFake(() => 100)
sinon.stub(global, 'clearTimeout')
Copy link
Contributor

@goplayoutside3 goplayoutside3 Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eatyourgreens the use of sinon.useFakeTimers() was added in #3019. I'm not totally sure of the difference between .setFakeTimers() vs .stub(global) here, but reverting to .stub() allows the tests to pass. Any thoughts on this sinon method?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the release notes for 19, they’ve upgraded Fake Timers to the latest version.

sinonjs/sinon#2612

Do you know what the classification queue tests are using the fake clock for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 config.global.

https://sinonjs.org/releases/latest/fake-timers/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

it('should not set a timer to retry failed classifications', async function () {
expect(classificationQueue.flushTimeout).to.be.null()
await classificationQueue.add(classificationData)
expect(classificationQueue.flushTimeout).to.be.null()
})

before(function () {
clock = sinon.useFakeTimers({ global })
sinon.stub(global, 'setTimeout').callsFake(() => 100)
Copy link
Contributor

@eatyourgreens eatyourgreens Dec 11, 2024

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 of setTimeout here. My mistake, setTimeout returns an integer, not a timeout.

This should really mock setTimeout with a function that creates a mock timeout object and returns its ID. Then clearTimeout should destroy the mock timeout for a given ID.

Copy link
Contributor

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

before(function () {
clock = sinon.useFakeTimers({ global })
sinon.stub(global, 'setTimeout').callsFake(() => 100)
sinon.stub(global, 'clearTimeout')
Copy link
Contributor

@eatyourgreens eatyourgreens Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this should mock clearTimeout with a function that takes a timeout and clears it.

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.

@eatyourgreens
Copy link
Contributor

eatyourgreens commented Dec 11, 2024

This is failing because of the sinon.useFakeTimers({ global }) config in 10 unit tests in ClassificationQueue.

Have you tried sinon.useFakeTimers()? According to the Fake Timers docs, createClock should automatically detect the global object in Node.

https://github.com/sinonjs/fake-timers?tab=readme-ov-file#faking-the-native-timers

@goplayoutside3
Copy link
Contributor

Using sinon.useFakeTimers() throws the same test errors as the original sinon.useFakeTimers({ global }).

@eatyourgreens
Copy link
Contributor

After a bit of searching, I think it's this issue with nock: nock/nock#698

Nock uses Node's internal timers, but fake timers turns those off.

@eatyourgreens
Copy link
Contributor

Fixed in #6544.

@goplayoutside3
Copy link
Contributor

the tests check whether or not a retry has been scheduled for a given response.

This is good to know about the ClassificationQueue tests.

Thank you for the help!

eatyourgreens and others added 2 commits December 11, 2024 09:17
Configure the fake timers for `ClassificationQueue` tests so that they only mock `setTimeout` and `clearTimeout`.
@eatyourgreens
Copy link
Contributor

Thank you for the help!

You're welcome. This made me realise that I've been lazy about mockiing in the past, and globally mocked timer functions that might be used in third-party libraries.

@goplayoutside3 goplayoutside3 merged commit 57c52de into master Dec 11, 2024
9 checks passed
@goplayoutside3 goplayoutside3 deleted the dependabot/npm_and_yarn/sinon-19.0.2 branch December 11, 2024 15:24
@github-actions github-actions bot added the approved This PR is approved for merging label Dec 11, 2024
@coveralls
Copy link

coveralls commented Dec 11, 2024

Coverage Status

coverage: 77.4% (+0.03%) from 77.369%
when pulling 2d3e150 on dependabot/npm_and_yarn/sinon-19.0.2
into 29dac35 on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved for merging dependencies Pull requests that update a dependency file javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants