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

Re-expose lib/sinon/behaviour as a top-level export in Sinon 2 #993

Closed
HowlingEverett opened this issue Feb 25, 2016 · 11 comments · Fixed by #1302
Closed

Re-expose lib/sinon/behaviour as a top-level export in Sinon 2 #993

HowlingEverett opened this issue Feb 25, 2016 · 11 comments · Fixed by #1302
Milestone

Comments

@HowlingEverett
Copy link

Without an official method for extending Sinon's stub API, a few modules—particularly sinon-as-promised—extend the available stub behaviours by patching methods onto sinon.stub and sinon.behavior directly.

The Sinon 2 beta version doesn't expose behavior as a top-level export anymore (rather it imports it as an internal dependency for lib/sinon/stub), which breaks these modules. While monkey-patching sinon isn't the greatest way to extend the library, it does work, and work nicely.

I attempted to solve this issue at the monkey-patch level using optional imports, but that breaks ES6/CommonJS bundlers.

Since Sinon 2 is required for Webpack/Babel module support, it'd be nice if sinon/lib/behavior was re-exposed as a top-level export to maintain compatibility with these modules.

@fatso83
Copy link
Contributor

fatso83 commented Feb 25, 2016

I haven't thought long and hard about this, but what if we exposed a way of injecting the behavior? Instead of overwriting it.

@HowlingEverett
Copy link
Author

An injector would be nice. I suppose the most backwards-compatible way would be to pass behaviors to inject on the sinon.stub call itself?

Something like...

sinon.stub(myObject, "method", {
  behaviors: {
    resolves: require("sinon-as-promised").resolves,
    rejects: require("sinon-as-promised").rejects
    // Custom example, dispatches async Redux actions
    dispatchesAsync: function(store, action) {
      this.returns(action(store.dispatch, store.getState)));
    }
  }
});

@fatso83
Copy link
Contributor

fatso83 commented Feb 26, 2016

That does seem a bit verbose from a client perspective ... Maybe we can come up with something friendlier. I think it is solvable, but might not be the first thing to prioritize in this transitioning phase to version 2.

@mroderick
Copy link
Member

Here's a wacky idea, why don't we incorporate sinon-as-promised into Sinon.JS?

  1. Promises are now first class citizens in JavaScript
  2. It's a small addition, with great benefit
  3. It's flexible and allows for using different polyfills/abstractions

If we do decide to go that route, then the default polyfill should probably be something like https://github.com/timjansen/PinkySwear.js, as that doesn't pollute the global scope with any Promise constructor. The last thing we want to do, is to change the user's environment

@fatso83
Copy link
Contributor

fatso83 commented Mar 13, 2016

@mroderick I'm for it, since it supports a very common usecase. PinkySwear is nice, but then I contributed to it, so I am biased :bowtie: Not sure if it solves @HowlingEverett's issue, but even if it did, should we have a way of customizing behavior?

@HowlingEverett
Copy link
Author

Adding built-in Promise support solves it for me, so I'd be happy either way. It really comes down to 'other than Promises, is there any other gap in Sinon that would be really useful for some, but not so common they should be in core'.

For example, of the top of my head, a 'rendersJSX' function on stub would be nice for handling the React edge cases where the shallow rendering test library won't work for tests. I wouldn't want it in the core Sinon.js repo since it's framework-specific. Similarly 'emitsEvent' on stub and emitted on spy would be super-useful for testing/stubbing Node edge cases where modules extend EventEmitter, as would pipesTo for testing NodeJS streams.

Honestly, all of these things (including Promises) are possible to work around in a hacky way using returns and yields, but being able to extend the spy and stub modules would make for a nice little stubbing ecosystem that could obviate the need for some of the bigger/nastier mocking libraries like Proxyquire.

@fatso83
Copy link
Contributor

fatso83 commented Mar 13, 2016

As long as we found some nice API to do this that won't be too bothersome or difficult to use. I honestly don't understand how the suggestion above works or is meant to illustrate, but then I haven't used sinon-as-promised ... Seems a bit awkward to have to specify all that jazz for every stub, compared to how sinon-as-promised works today. Since we can configure sandboxes, maybe custom stubbing behaviours could be a sandbox-only feature? That way it would not affect the sinon instance in other tests.

@HowlingEverett
Copy link
Author

I'm not a huge fan of that syntax either, I was just trying to think of a way to add an injector without breaking the current public API. I think I'd prefer something closer to how should.js allows extensions: Should.Assertion.add(customAssertFunction).

Example of someone using that API to extend should with sinon assertions here: https://github.com/shouldjs/sinon/blob/master/should-sinon.js

Honestly, that's much closer to my original request - re-expose Behaviour. sinon.behaviour.add could be used by Sinon plugins to extend behaviour that stub could use, and allow them to do it once or in an imported module.

@fatso83
Copy link
Contributor

fatso83 commented Jul 11, 2016

Just rediscovered this and wanted to note that I think we have two good ideas here

  1. Merging in the functionality of Sinon as Promised (all seems pro).
  2. Making it possible to add behaviors (which I now understand means adding to the set of methods that are callable on spies and stubs). I just don't think exposing the whole behaviour prototype directly is such a good idea for the sole purpose of making it modifiable, although I like the sinon.behaviour.add(name, func) API.

So I think we can revisit these ideas once Sinon 2.0 is out the door. Maybe as 2.1 :-)

@jonnyreeves jonnyreeves added this to the 2.1 milestone Jul 11, 2016
@launchcg-ztonia
Copy link

Whoa whoa, behavior is going away? I needed that for handling stuff like the AWS sdk, where the invocation (for some reason) returns another method, which returns a promise.

@fatso83
Copy link
Contributor

fatso83 commented Feb 26, 2017

You can now add behaviours in Sinon using the official API. See #1302

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 a pull request may close this issue.

5 participants