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

Move printf to proxy #2186

Merged
merged 2 commits into from
Jan 6, 2020
Merged

Move printf to proxy #2186

merged 2 commits into from
Jan 6, 2020

Conversation

mroderick
Copy link
Member

This PR is a solution for the failing tests in sinonjs/referee-sinon#64

Purpose

Make fake as useful in referee as spy and stub are.

Background

When I originally created fake, I didn't realise that we needed a printf method on them, and omitted it.

Solution

  • Move printf to proxy, making it available on fake as well as spy and stub
  • Validate via tests that fake, spy and stub all delegate their printf method to proxy

How to verify - mandatory

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

@mroderick
Move .printf to proxy
a50ff4f 
This means that .printf is also available on fakes, which makes fakes
more useful when used with referee.

See sinonjs/referee-sinon#64
@mroderick
Use test-only proxy instance
dd506cb 
Instead of using spy, use a test only proxy instance for tests, showing
that proxy can be used outside of spies.
@mroderick mroderick requested a review from mantoni December 31, 2019 12:38
mantoni
mantoni approved these changes Jan 6, 2020
View reviewed changes
Copy link
Member

@mantoni mantoni left a comment

Choose a reason for hiding this comment

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

Very nice!

@fatso83 fatso83 merged commit eea3551 into master Jan 6, 2020
@fatso83 fatso83 deleted the move-printf-to-proxy branch January 6, 2020 10:55
@mroderick mroderick added the semver:patch changes will cause a new patch version label Jan 6, 2020
@mroderick
Copy link
Member Author

This has been published as [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fatso83 fatso83 fatso83 approved these changes

@mantoni mantoni mantoni approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@mroderick @mantoni @fatso83