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 fake behaviors to sandbox (#1815) #1842
Conversation
test/fake-test.js
Outdated
@@ -41,8 +41,8 @@ var hasFunctionNameSupport = noop.name === "noop"; | |||
|
|||
describe("fake", function () { | |||
describe("module", function () { | |||
it("should return a unary Function named 'fake'", function () { | |||
assert.equals(fake.length, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mroderick Was this important somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a little thing I like to put into place, to make sure that the api doesn't change without anyone noticing. Unless fake
suddenly accepts more than one argument, I'd like to keep it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for you PR @nivsherf!
I only have a couple of comments
lib/sinon/sandbox.js
Outdated
|
||
Object.keys(sinonFake).forEach(function (key) { | ||
var fakeBehavior = sinonFake[key]; | ||
if (sinonFake.hasOwnProperty(key) && typeof fakeBehavior === "function") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Object.keys
return only keys directly upon the object, so there's no need to use hasOwnProperty
.
test/fake-test.js
Outdated
@@ -41,8 +41,8 @@ var hasFunctionNameSupport = noop.name === "noop"; | |||
|
|||
describe("fake", function () { | |||
describe("module", function () { | |||
it("should return a unary Function named 'fake'", function () { | |||
assert.equals(fake.length, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a little thing I like to put into place, to make sure that the api doesn't change without anyone noticing. Unless fake
suddenly accepts more than one argument, I'd like to keep it
7c35ed2
to
e3025b9
Compare
Thanks for the feedback @mantoni and @mroderick! |
e3025b9
to
2a8a73c
Compare
This has been published to NPM as Thanks you ❤️ |
Purpose (TL;DR) - mandatory
Fix for issue #1815 (No obvious way to reset fakes)
Solution - optional
Add fake and fake behaviors to sandbox.
How to verify - mandatory
npm install
npm test
Checklist for author
npm run lint
passes