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

Added usingPromise method to stub. #1364

Merged
merged 4 commits into from
Apr 10, 2017

Conversation

blacksun1
Copy link
Contributor

@blacksun1 blacksun1 commented Mar 24, 2017

In progress

Things to still be done:

  • Get agreement on implementation
  • Add .usingPromise to higher level objects such as sandbox
  • Add documentation

Purpose

The usingPromise method allows the setting of the promise library
that will be used by the resolve and reject method. If it is not set then it will use the default promise implementation by default.

This will eventually fix #1354

Background

To allow the setting of a custom promise library when creating a stub that resolves or rejects. Sometimes you need to get back a custom promise library to have access to it's extended methods and so on, such as .tap, .done etc.

Solution

Add a .usingPromise method that sets the promise library to use.

Example using sinon.stub():

var assert = require("assert");
var bluebird = require("bluebird");
var sinon = require("sinon");

var myObject = {};
var myStub = sinon.stub()
    .usingPromise(bluebird.Promise)
    .resolves(myObject);

myStub()
    // Tap should now be available!!!
    .tap(function(actual) {
        assert.strictEqual(actual, myObject);
    })
    .catch(function(err) {
        console.err("Error", err);
    });

Example using sinon.sandbox():

var assert = require("assert");
var bluebird = require("bluebird");
var sinon = require("sinon");

var myObject = {
  myMethod: function() { return; }
};

var sandbox = sinon.sandbox.create()

sandbox.stub(myObject);

myObject.myMethod
    .usingPromise(bluebird.Promise)
    .resolves("baz");

myObject.myMethod()
    // Tap should now be available!!!
    .tap(function(actual) {
        assert.strictEqual(actual, "baz");
    });

How to verify

  1. Check out this branch (see github instructions below)
  2. npm install
  3. npm test

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 95.375% when pulling 4601eb2 on blacksun1:add-custom-promise into f701abe on sinonjs:master.

@mantoni
Copy link
Member

mantoni commented Mar 27, 2017

Nice work. I'm not so sure about the name setPromise, because you're not setting a promise instance but rather the type of promise to use. Alternatives that I can think of are usesPromise or usingPromise or returnsPromise. I'm not using promises myself, so I'm not the best to make a decision here.

@blacksun1
Copy link
Contributor Author

I would prefer usePromise or usingPromise. usesPromise sounds like something in the past.

@fatso83
Copy link
Contributor

fatso83 commented Mar 27, 2017

I'd go for usingPromise (or something like that - no hard feelings on this matter).

@coveralls
Copy link

coveralls commented Mar 28, 2017

Coverage Status

Coverage increased (+0.01%) to 95.382% when pulling cccc574 on blacksun1:add-custom-promise into f701abe on sinonjs:master.

@blacksun1 blacksun1 changed the title Added setPromise method to stub. Added usingPromise method to stub. Mar 28, 2017
@blacksun1
Copy link
Contributor Author

@fatso83, @mantoni

@blacksun1
Copy link
Contributor Author

If I have agreement then I will continue with the rest of the implementation this week when I have some "free time" - meaning I steal some time from somewhere else. ;)

@mantoni
Copy link
Member

mantoni commented Mar 28, 2017

@blacksun1 Sure, usingPromise sounds good. We all steal our time elsewhere 🍻

@coveralls
Copy link

coveralls commented Mar 28, 2017

Coverage Status

Coverage increased (+0.02%) to 95.388% when pulling fde9c1c on blacksun1:add-custom-promise into f701abe on sinonjs:master.

Copy link
Member

@mantoni mantoni left a comment

Choose a reason for hiding this comment

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

LGTM

@coveralls
Copy link

coveralls commented Mar 28, 2017

Coverage Status

Coverage increased (+0.01%) to 95.388% when pulling 8e0e191 on blacksun1:add-custom-promise into c5bc9ab on sinonjs:master.

@blacksun1
Copy link
Contributor Author

Any idea where I should put the documentation for the feature?

@mantoni
Copy link
Member

mantoni commented Mar 29, 2017

Please put documentation into the docs folder (https://github.com/sinonjs/sinon/tree/master/docs). The README and the CONTRIBUTING files in there explain how to work with it.

@coveralls
Copy link

coveralls commented Apr 5, 2017

Coverage Status

Coverage increased (+0.01%) to 95.388% when pulling 249f818 on blacksun1:add-custom-promise into c5bc9ab on sinonjs:master.

@blacksun1
Copy link
Contributor Author

Docs added. @mantoni @fatso83

blacksun1 and others added 3 commits April 6, 2017 03:24
The `usingPromise` method allows the setting of the promise library
that will be used by the resolve and reject method. If it is not set
then it will use the default promise implementation by default.

Example:

```js
var assert = require("assert");
var bluebird = require("bluebird");
var sinon = require("sinon");

var myObject = {};
var myStub = sinon.stub()
    .usingPromise(bluebird.Promise)
    .resolves(myObject);

myStub()
    // Tap should now be available!!!
    .tap(function(actual) {
        assert.strictEqual(actual, myObject);
    })
    .catch(function(err) {
        console.err("Error", err);
    })
```

Documentation still to come.
The `usingPromise` method allows the setting of the promise library
that will be used by the resolve and reject method. If it is not set
then it will use the default promise implementation by default.

Example:

```js
var assert = require("assert");
var bluebird = require("bluebird");
var sinon = require("sinon");

var myObject = {
  myMethod: function() { return; }
};

var sandbox = sinon.sandbox.create()

sandbox.stub(myObject);

myObject.myMethod
    .usingPromise(bluebird.Promise)
    .resolves("baz");

myObject.myMethod()
    // Tap should now be available!!!
    .tap(function(actual) {
        assert.strictEqual(actual, "baz");
    });
```

Documentation still to come.
@coveralls
Copy link

coveralls commented Apr 5, 2017

Coverage Status

Coverage increased (+0.01%) to 95.388% when pulling 25bfde0 on blacksun1:add-custom-promise into c5bc9ab on sinonjs:master.

var stub = createStub.create();

assert(stub.usingPromise);
assert.isTrue(typeof stub.usingPromise === "function");
Copy link
Member

Choose a reason for hiding this comment

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

assert.isFunction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@jrnail23
Copy link

jrnail23 commented Apr 6, 2017

I just wanted to say thanks for doing this, @blacksun1. This should fill the gap left by obsolescing sinon-as-promised which left us without a way to plug bluebird in, and thus preventing us from upgrading to sinon v2.x 😄

@coveralls
Copy link

coveralls commented Apr 7, 2017

Coverage Status

Coverage increased (+0.01%) to 95.388% when pulling 7ac4f60 on blacksun1:add-custom-promise into c5bc9ab on sinonjs:master.

@fatso83
Copy link
Contributor

fatso83 commented Apr 10, 2017

Never mind my issue comment - found out I could review and merge this using even an old-skool cell phone :-) Thanks!

@fatso83 fatso83 merged commit 3d4bc16 into sinonjs:master Apr 10, 2017
@mroderick mroderick added the semver:minor changes will cause a new minor version label May 2, 2017
@mroderick
Copy link
Member

sinon@2.2.0 is now on npm

@blacksun1 blacksun1 deleted the add-custom-promise branch September 5, 2022 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor changes will cause a new minor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: Add ability to override global promise library
7 participants