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

Is private(set) really desirable? #133

Closed
FranzBusch opened this issue Feb 10, 2021 · 6 comments
Closed

Is private(set) really desirable? #133

FranzBusch opened this issue Feb 10, 2021 · 6 comments

Comments

@FranzBusch
Copy link
Collaborator

We just upgraded our mockolo version and saw a bunch of build failures because we were resting the generated callCount properties during tests. This was necessary in our tests because of two reasons:

  1. To remove any unnecessary calls that happen during a test case setup
  2. If you mock a protocol with static methods, then you need to manually reset them between each test

Would it make sense to make this configurable in mockolo and for the case of the static method maybe even disable it by default?

@elsh
Copy link
Contributor

elsh commented Feb 11, 2021

We can make it configurable. How soon would you need this? Also feel free to push a PR if possible. Our ETA is within the next couple of weeks.

@FranzBusch
Copy link
Collaborator Author

@elsh not super critical for us right now. We just quickly changed the code that was making use of this.

I would like to understand the reasoning why making it private in the first place is desirable.

@marciniwanicki
Copy link
Collaborator

I think private(set) might be quite useful to ensure the internal mock state is not accidentally tempered by the tests. If the mock needs to be reused or brought to the initial state, a resetMock() (or similar) method could be an alternative to ensure all of the properties are getting into the initial state.

@FranzBusch
Copy link
Collaborator Author

A method to reset the mock sounds reasonable and even better than manually zeroing the counts.

@elsh
Copy link
Contributor

elsh commented Feb 12, 2021

Reset is a great idea, but in this case, it actually takes less bookkeeping (thus simpler) to configure setting vars directly via an input flag. See if https://github.com/uber/mockolo/releases/tag/1.3.1 resolves your scenario. If needed we can revisit a reset function in the future.

@FranzBusch
Copy link
Collaborator Author

@elsh Thanks a lot for the quick help here! Really appreciated

@elsh elsh closed this as completed Feb 17, 2021
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

No branches or pull requests

3 participants