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.

Sign up for GitHub

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

Jump to bottom

Allow anonymous mock functions to be named #1489

Merged
merged 1 commit into from
Jul 26, 2017
Merged

Allow anonymous mock functions to be named #1489

merged 1 commit into from
Jul 26, 2017

Conversation

Rowno
Copy link
Contributor

@Rowno Rowno commented Jul 15, 2017 •
edited

Purpose

Makes debugging anonymous mock functions a lot easier by allowing an optional method name to be specified. Since there's currently nothing in the sandbox.verify() error message/stacktrace that points to which mock is broken.

How to verify

  1. Check out this branch.
  2. Write a test with sinon.mock().once().verify();.
  3. Write a test with sinon.mock('functionName').once().verify();.

@coveralls
Copy link

coveralls commented Jul 15, 2017 •
edited

Coverage Status

Coverage remained the same at 95.002% when pulling 837ec0c on Rowno:named-anonymous-mocks into 259a330 on sinonjs:master.

lucasfcosta
lucasfcosta approved these changes Jul 25, 2017
View reviewed changes
Copy link
Member

@lucasfcosta lucasfcosta left a comment •
edited

Choose a reason for hiding this comment

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

LGTM, I can't see any reasons for us to not accept this. I think we should even have had this before.

I'm glad you thought about it and submitted a PR.

Well done.

@mroderick mroderick added the semver:minor changes will cause a new minor version label Jul 26, 2017
@mroderick mroderick merged commit 57bb8b3 into sinonjs:master Jul 26, 2017
@mroderick
Copy link
Member

Thank you 💯

This has become v2.4.0

@Rowno Rowno deleted the named-anonymous-mocks branch July 26, 2017 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucasfcosta lucasfcosta lucasfcosta approved these changes

Assignees
No one assigned
Labels
semver:minor changes will cause a new minor version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Rowno @coveralls @mroderick @lucasfcosta