Drop usage of overly broad mock.Mock() in tests. #3063
Merged
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
dhermes
added
api: clouderrorreporting
googlebot
added
the
cla: yes
dhermes
commented
Feb 23, 2017
| """ | ||
| channel = make_secure_channel( | ||
| client._connection.credentials, | ||
| client._credentials, |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Contributor
|
Should mocks like |
Contributor
Author
|
@daspecster All mocks should state their spec. Maybe we should even extend this to |
Contributor
|
Apologies if you don't think unit tests are up to snuff, thank you for improving. I think a system test will inspire the most confidence in the functionality. When I first contributed the PR the API wasn't full fleshed out, but now that it is I can write one. |
This actually required a much more comprehensive set of unit test changes than expected. Also incorporated a change from googleapis#3057 (which slipped through due to overly broad mocks).
dhermes
force-pushed
the
make-mocks-less-broad
branch
from
f4c9830 to
c8e8a49
Compare
Contributor
Author
|
Travis seems to be having issues, merging with CircleCI already green.
|
Contributor
Author
|
Ahhh sorry I thought @daspecster had LGTM-ed! Eeek. Worth reverting or someone just take a look ex-post-merge-o? |
Contributor
|
It's fine. :-) |
richkadel
pushed a commit
to richkadel/google-cloud-python
that referenced
this pull request
May 6, 2017
Drop usage of overly broad mock.Mock() in tests.
parthea
pushed a commit
that referenced
this pull request
Oct 21, 2023
parthea
pushed a commit
that referenced
this pull request
Oct 21, 2023
Drop usage of overly broad mock.Mock() in tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
@lukesneeringer @tseaver any ideas on an easy "lint"-like check that will scream if people create a
Mock()without aspec?SIDEBAR: The error reporting unit tests are in a very bad state. This change opened my eyes to that fact, but I fear there is more hiding under the surface.