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

Add context to wrapped function #1906

Merged
merged 4 commits into from Sep 18, 2018

Conversation

fatso83
Copy link
Contributor

@fatso83 fatso83 commented Sep 17, 2018

Summary

How to verify

  1. Check out this branch
  2. npm install
  3. npm test - see the test added in the first commit pass
  • npm run lint passes
  • References to standard library functions are cached.

@fatso83 fatso83 changed the title - Fix #1850 by keeping this context - Added a regression test to verify it. Add context to wrapped function Sep 17, 2018
@coveralls
Copy link

coveralls commented Sep 17, 2018

Pull Request Test Coverage Report for Build 2680

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 95.356%

Totals Coverage Status
Change from base Build 2673: 0.0%
Covered Lines: 2043
Relevant Lines: 2097

💛 - Coveralls

@fatso83
Copy link
Contributor Author

fatso83 commented Sep 17, 2018

For some reason crashed in the webworker test on Node 10, but no other versions ... Weird, since it doesn't actually run on Node, but in chromium.

@fatso83
Copy link
Contributor Author

fatso83 commented Sep 17, 2018

@fearphage This fails on the unstable Chrome version. Works fine on stable Google Chrome 65.0.3325.162 (Linux). Any idea why we are not running on the stable branch? I remember you setting it up, but I don't remember any details. I suspect this being the culprit:

npm run test-webworker -- --chrome $(which google-chrome-unstable) --allow-chrome-as-root

@mantoni
Copy link
Member

mantoni commented Sep 18, 2018

@fatso83 Stable chrome install doesn't succeed in docker containers. Not sure if / when this will be supported in the regular version.

@fatso83
Copy link
Contributor Author

fatso83 commented Sep 18, 2018

Made it work by installing both stable and unstable. Needed both - unstable for headless and stable for WebWorker ...

@fatso83 fatso83 merged commit 24c35c0 into sinonjs:master Sep 18, 2018
@fatso83 fatso83 deleted the 1850-keep-context-in-fake branch September 18, 2018 07:06
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

3 participants