-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add more unit tests #164
Add more unit tests #164
Conversation
Signed-off-by: cfreksen <casper@freksen.dk>
Thank you for this @cfreksen! We'll get this reviewed as soon as we can. |
mock_get.assert_called_with(url='the-url', params={'a': 'b'}, | ||
headers={'c': 'd'}) | ||
|
||
self.assertEqual(result, '{"x": "y"}') |
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.
Should this also assert the status code?
|
||
for error_status in [400, 500, None]: | ||
mock_post.return_value.status_code = error_status | ||
mock_post.return_value.text = '{"error": "SomeError"}' |
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.
Our errors typically come back with both an error
property and an error_description
one. I don't know OTOH but I'm guessing the exception
that's returned will include the description as an error message. Might be good to check for that as well.
@@ -17,11 +17,11 @@ def test_get(self, mock_rc): | |||
) | |||
|
|||
@mock.patch('auth0.v3.management.jobs.RestClient') | |||
def get_failed_job(self, mock_rc): | |||
def test_get_failed_job(self, mock_rc): |
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.
👍
self.assertEqual(context.exception.error_code, 'SomeError') | ||
|
||
@mock.patch('requests.post') | ||
def test_file_post(self, mock_post): |
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.
Are we doing this anywhere in the SDK?
self.assertEqual(response, {'a': 'b'}) | ||
|
||
@mock.patch('requests.post') | ||
def test_file_post_errors(self, mock_post): |
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.
Are we doing this anywhere in the SDK?
@cfreksen any extra guidance you need to complete this PR? Don't hesitate to ask any questions. |
@cfreksen - We'd like to merge this but need a few changes. We'll leave this open for a bit longer and then we'll have to close it. Thank you! |
This adds more unit tests (and fixes a broken one) so that almost all code is covered.