-
-
Notifications
You must be signed in to change notification settings - Fork 771
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
Expose assert
on sandbox instances
#1075
Comments
Maybe you can use |
@mantoni according to http://sinonjs.org/docs/#assert-expose, that means that the actual |
Well, then you can add the following here:
Or is there more to it? |
Indeed! Per https://github.com/wealthfront/sinon-sandbox/issues/7#issuecomment-228912571 however, I think it would be better for the upstream to do it - since the sandbox already exposes Is there a reason that this semver-minor change wouldn't be a good thing? |
Fine with me, was just curious. We're going to review the API surface for the 3.0 release and I wouldn't want to introduce anything that doesn't fit with that. Maybe @mroderick or @fatso83 have any feelings? |
I just reviewed #1000 (offtopic: we should try and get this ball rolling again) and I could not really see much preventing us from doing this. It is a very minor change AFAIK, and seemingly only aligns the sandbox api with that of the So I am pro. |
Submitted #1076 |
Yay, thanks everyone! Looking forward to the release. |
Per https://github.com/wealthfront/sinon-sandbox/issues/7:
Would you be willing to accept a PR that attaches
sinon.assert
to sandbox instances?The text was updated successfully, but these errors were encountered: