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

[feature] adds spy.calledImmediatelyBefore and spy.calledImmediatelyAfter #1337

Merged
merged 2 commits into from
Mar 19, 2017

Conversation

timcosta
Copy link

@timcosta timcosta commented Mar 17, 2017

Purpose (TL;DR) - mandatory

Sometimes you need to be sure that you are calling a().c() and not a().b().c(). When mocking a large library like knexjs, being able to ensure the exact order of calls is incredibly helpful.

Background (Problem in detail) - optional

When mocking a large library that chains calls, you may end up writing one large mock in the beforeEach of the test library as to not duplicate code within your tests. Unfortunately there is no way to ensure that chained calls do not sneak into calls chains currently, as the calledBefore and calledAfter implementations simply use > or <. In order to use a large mock and reduce duplicitous code in test suites when dealing with libraries that chain calls, calledImmediatelyBefore and calledImmediatelyAfter allow you to ensure that no additional modifiers end up between your chained calls.

Solution - optional

This solution works just like calledBefore and calledAfter, but uses strict equality to ensure that the calls to the spies did in fact happen sequentially.

How to verify - mandatory

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

@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage increased (+0.03%) to 95.372% when pulling 3aa9919 on timcosta:add_called_immediately_before_after into e401337 on sinonjs:master.

Copy link
Member

@mroderick mroderick left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request. I only have a couple of comments

test/spy-test.js Outdated
assert.isFunction(this.spy1.calledImmediatelyAfter);
});

it("returns true if first call to A was immediately after first to B", function () {
Copy link
Member

Choose a reason for hiding this comment

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

This is minor, but it would make the tests easier to read, if you use A in both description and implementation. Otherwise readers would have to understand that A is this.spy1

test/spy-test.js Outdated
assert.isFunction(this.spy1.calledImmediatelyBefore);
});

it("returns true if first call to A was immediately after first to B", function () {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, A is this.test1

@timcosta
Copy link
Author

Thanks for reviewing @mroderick - updated the names in my tests and the tests i stole the template from.

@coveralls
Copy link

coveralls commented Mar 19, 2017

Coverage Status

Coverage increased (+0.03%) to 95.372% when pulling a957ef3 on timcosta:add_called_immediately_before_after into e401337 on sinonjs:master.

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

4 participants