Better exception when mock is invoked after expectations are verified. #142
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jun 3, 2016
| val call = new Call(this, arguments) | ||
| callLog += call | ||
| expectationContext.handle(call) getOrElse onUnexpected(call) | ||
| if (Option(callLog).isDefined) { |
There was a problem hiding this comment.
I would better check != null or change code to prevent null because this fancy null checking is exaggerated.
There was a problem hiding this comment.
OK, changed to != null.
I would love to see the nulls removed in favour of something more idiomatic, but not sure I want to tackle that at the moment.
|
👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In several instances during development, we have encountered situations where a NPE is thrown unexpectedly and without explanation. After some investigation it seems that it's happening when a mock is called after expectations have already been verified.
This can happen easily when using ScalaTest's PathSpec variants to test async code. The developer forgets to wait for their async call to complete, proceeds to call verifyExpectations, and only after the verification is the mock function invoked (at which point the mock's call log has been nullified by https://github.com/PagerDuty/ScalaMock/blob/master/core/src/main/scala/org/scalamock/MockFactoryBase.scala#L84).
This PR avoids the NPE by throwing a more appropriate exception if the callLog has been set to null.