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 support for mocking AWS SDK's promises #24

Merged
merged 9 commits into from
Jun 10, 2016
Merged

Conversation

jcready
Copy link
Contributor

@jcready jcready commented May 5, 2016

As of March 31, 2016 the AWS SDK now supports Promises. This means that you can now do things like this:

var s3 = new AWS.S3({apiVersion: '2006-03-01', region: 'us-west-2'});
var params = {
  Bucket: 'bucket',
  Key: 'example2.txt',
  Body: 'Uploaded text using the promise-based method!'
};
var putObjectPromise = s3.putObject(params).promise();
putObjectPromise.then(function(data) {
  console.log('Success');
}).catch(function(err) {
  console.log(err);
});

Attempting to use aws-sdk-mock to mock methods that now no longer require a callback function causes issues, most notably: TypeError: callback is not a function. This PR attempts to resolve this issue by checking to see if the mocked method was passed a callback function. If it was passed a callback function then everything should behave exactly as it did before. But if it wasn't then it returns an object containing a single method promise which, when invoked, will execute the user's mocked response and the promise will branch based on what the callback returns. It will utilize the native/global promise if available, but it will also pick up on calls to AWS.config.setPromisesDependency so that the mocks can create new promises with the provided promise constructor.

As of March 31, 2016 [the AWS SDK now supports Promises](https://blogs.aws.amazon.com/javascript/post/Tx3BZ2DC4XARUGG/Support-for-Promises-in-the-SDK). This means that you can now do things like this:

```js
var s3 = new AWS.S3({apiVersion: '2006-03-01', region: 'us-west-2'});
var params = {
  Bucket: 'bucket',
  Key: 'example2.txt',
  Body: 'Uploaded text using the promise-based method!'
};
var putObjectPromise = s3.putObject(params).promise();
putObjectPromise.then(function(data) {
  console.log('Success');
}).catch(function(err) {
  console.log(err);
});
```

Attempting to use aws-sdk-mock to mock methods that now no longer require a callback function causes issues, most notably: `TypeError: callback is not a function`. This PR attempts to resolve this issue by checking to see if the mocked method was passed a callback function. If it was passed a callback function then everything should behave exactly as it did before. But if it wasn't then it returns an object containing a single method `promise` which, when invoked, will execute the user's mocked response and the promise will branch based on what the callback returns. It will utilize the native/global promise if available, but it will also pick up on calls to `AWS.config.setPromisesDependency` so that the mocks can create new promises with the provided promise constructor.
@nelsonic
Copy link
Member

nelsonic commented May 5, 2016

@jcready thanks for the PR! would you mind adding some tests & docs (in the readme) for this addition?

@codecov-io
Copy link

codecov-io commented May 5, 2016

Current coverage is 100%

Merging #24 into master will not change coverage

@@           master   #24   diff @@
===================================
  Files           1     1          
  Lines          58    73    +15   
  Methods         0     0          
  Messages        0     0          
  Branches        0     0          
===================================
+ Hits           58    73    +15   
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last updated by 43ee653...ba1739f

@jcready
Copy link
Contributor Author

jcready commented May 5, 2016

@nelsonic to properly test this I would either need to depend on a native Promise implementation (which Node 0.10.36 doesn't have AFAIK) or I would have to include a polyfill for it as a devDependency. What's your preference?

@nelsonic
Copy link
Member

nelsonic commented May 5, 2016

Good question. I think we should probably migrate to using 4.3.2 as the baseline node.js version for this module...
@nikhilaravi thoughts?

@jcready
Copy link
Contributor Author

jcready commented May 9, 2016

I've added tests for environments that support Promises natively. I'm explicitly telling Istanbul to ignore one line because it would require that you test against an older version of the aws-sdk, which that line is specifically trying to handle.

@nikhilaravi
Copy link
Member

@jcready @nelsonic I think upgrading to node 4.3.2 should be fine! There's no reason not to now Lambda supports it.

@nikhilaravi
Copy link
Member

@nelsonic @jruts has this PR had a final review?

@jruts
Copy link
Member

jruts commented Jun 7, 2016

@nikhilaravi looks fine to me

@nelsonic
Copy link
Member

nelsonic commented Jun 8, 2016

@nikhilaravi yeah, looks good.
@jcready would you mind increasing the version number in package.json to 1.3.0
or ... #32 ?

@jcready
Copy link
Contributor Author

jcready commented Jun 8, 2016

@nelsonic done.

@nikhilaravi
Copy link
Member

@jcready ah looks like there are some conflicts - would you be able to resolve them?

@nelsonic
Copy link
Member

nelsonic commented Jun 9, 2016

@jcready thanks for updating the version, please let us know when merge conflicts are resolved so we can merge and npm publish! 👍

# Conflicts:
#	package.json
@jcready
Copy link
Contributor Author

jcready commented Jun 10, 2016

@nikhilaravi @nelsonic I've resolve the merge conflict (literally just the package.json version).

@nelsonic
Copy link
Member

@jcready let's ship it! :shipit:

@nelsonic nelsonic merged commit fde482f into dwyl:master Jun 10, 2016
@nelsonic
Copy link
Member

@jcready your new version is on NPM: aws-sdk-mock@1.3.0 🚀
thanks again for the contribution! welcome to @dwyl! 👍

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

5 participants