-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feature/test coverage #42
Conversation
Codecov ReportBase: 94.81% // Head: 95.51% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #42 +/- ##
===========================================
+ Coverage 94.81% 95.51% +0.69%
===========================================
Files 32 34 +2
Lines 1409 1472 +63
===========================================
+ Hits 1336 1406 +70
+ Misses 73 66 -7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All LGTM! Left one question
|
||
|
||
@pytest.mark.vcr() | ||
def test_exception(client): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would we want to have a test actually against Welkin or does it not matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_exception
does actually hit the Welkin API at https://api.live.welkincloud.io/{{ tenant }}/admin/users/me
, which is a real endpoint that IIRC isn't allowed for API clients.
I'm pointing test_exception_no_json
because I don't know of a Welkin endpoint that returns a blank or text-based response (it has happened, I just don't recall when), and httpbin.org is a tried-and-true test domain. This also illustrates how the underlying client can be used without the defined object models. IMO tests should also illustrate usage.
Increase test coverage