Skip to content

Issue 1598 (Feature Request): sandbox.createStubInstance - #1602

Merged
mroderick merged 1 commit into
sinonjs:masterfrom
dpmott:issue-1598-implement-sandbox.createStubInstance
Nov 2, 2017
Merged

Issue 1598 (Feature Request): sandbox.createStubInstance#1602
mroderick merged 1 commit into
sinonjs:masterfrom
dpmott:issue-1598-implement-sandbox.createStubInstance

Conversation

@dpmott

@dpmott dpmott commented Oct 30, 2017

Copy link
Copy Markdown
Contributor

Purpose (TL;DR) - mandatory

Fix issue #1598 by implementing sandbox.createStubInstance(), tests, and documentation.

Background (Problem in detail) - optional

The addition of sandbox.createStubInstance() increases orthogonality with the Sinon stub interface.

Solution - optional

Implemented sandbox.createStubInstance() as a convenience wrapper around sandbox.stub(), for the case of stubbing an entire object.

How to verify - mandatory

  1. Check out this branch
  2. npm install
  3. npm test

Checklist for author

  • npm run lint passes
  • References to standard library functions are cached.
  • Verify that documentation is sufficient for new functionality
  • Verify that unit test coverage is sufficient for new functionality.

Comment thread package.json
"test": "run-s test-node test-headless test-webworker",
"check-dependencies": "dependency-check package.json --unused --no-dev",
"build": "./build.js",
"build": "node ./build.js",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change was necessary to run npm build or npm test locally on a Windows box. If it's inappropriate, then I'll happily revert this change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's fine. At some point we agreed that it was not a pre-requisite that the entire build process runs on Windows, as none of the maintainers were using it, but we are certainly not opposed to improving the situation :-)

Comment thread test/collection-test.js
assert.isFunction(collection.mock);
});

describe(".createStubInstance", function () {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These tests have largely been copied from stub-test.js.

@dpmott

dpmott commented Oct 30, 2017

Copy link
Copy Markdown
Contributor Author

Do I need to add any additional tests in sadnbox-test.js?

@fatso83 fatso83 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mroderick
mroderick merged commit 201a652 into sinonjs:master Nov 2, 2017
@mroderick mroderick added the semver:minor changes will cause a new minor version label Nov 2, 2017
@mroderick

Copy link
Copy Markdown
Member

Thank you 🚀

This has become sinon@4.1.0

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.

3 participants