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

Correctly reset Sinon fake timers #60

Merged
merged 2 commits into from
Apr 21, 2022
Merged

Conversation

cjbarth
Copy link
Collaborator

@cjbarth cjbarth commented Apr 6, 2022

During some testing of the tests, it was found that some tests randomly failed. Diagnosis revealed it was because we were overwriting the fakeClock variable at times and therefore were not correctly restoreing the built-in timers, causing problems for following tests. This will fix that.

The pattern followed is to allow a describe blocks beforeEach and afterEach to set up and tear down fake timers if, and only if, each child it block needs the same fake timer. In every other case a try...finally block was used in the it block to build a local fake timer and tear it down, no matter what failures the test might experience.

@cjbarth cjbarth added the bug Something isn't working label Apr 6, 2022
@cjbarth
Copy link
Collaborator Author

cjbarth commented Apr 14, 2022

This problem will be easier to find once this is released.

@cjbarth
Copy link
Collaborator Author

cjbarth commented Apr 21, 2022

The entire method of this PR was changed because there is now an update to the Sinon library that will catch if you're using the fake timers incorrectly. Thus, the changes were paired back to be only what was needed. The code is still a little complex, but that can be saved for a proper test refactor.

@cjbarth cjbarth merged commit 7e5e385 into node-saml:master Apr 21, 2022
@cjbarth cjbarth deleted the fix-fake-timers branch April 21, 2022 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant