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

Update sinon to avoid timeouts #30

Merged
merged 1 commit into from Jun 27, 2018

Conversation

sebakerckhof
Copy link
Contributor

@sebakerckhof sebakerckhof commented May 31, 2018

This PR updates sinon. It's kind of hard to explain why it's important, but I'll give it a try:

My first test was timing out when calling the stub function of this library. I found out that this was happening when I had a lot of test files. My investigation revealed that

This can be seen in the attached performance profile ( between 1100ms-1250ms)

sourcemaps - CPU-20180531T160228.zip

Sinon 5 no longer creates this stack trace in the wrapMethod function:
https://github.com/sinonjs/sinon/blob/master/lib/sinon/util/core/wrap-method.js#L110

Oh and interestingly enough. The slow piece of code (SourceMapConsumer) that resulted in this timeout is exactly what is being replaced by WASM, which you started working on meteor/meteor#9568 (comment) . So if we can upgrade to source-map v0.7, we can probably also upgrade meteor's source-map-support package and avoid such issues in the first place.

@sebakerckhof
Copy link
Contributor Author

ping @hwillson

Copy link
Owner

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Sorry for the delay @sebakerckhof - thanks!

@hwillson hwillson merged commit 4afadfb into hwillson:master Jun 27, 2018
@sebakerckhof
Copy link
Contributor Author

Actually, I missed one deprecation, gonna do a quick follow-up PR.
And in the meantime sinon 6 came out (without breaking changes), but I'll give it a quick test anyway.

@hwillson
Copy link
Owner

Awesome @sebakerckhof - thanks!

@sebakerckhof
Copy link
Contributor Author

@hwillson . I opened a new one: #31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants