-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add unit tests for coriolisclient.v1.*
modules
#87
Conversation
1483fc7
to
6983cc9
Compare
result = self.licence.list(mock.sentinel.appliance_id) | ||
|
||
self.assertEqual( | ||
mock_resource_class.return_value, |
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.
Please fix this duplicate line bug. Because you set the expected value twice, the actual result got passed as message
to assertEqual
.
period, | ||
expected_result, | ||
raises, | ||
has_logs |
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.
Since it's not an edgeg-case to log warnings in this method, we don't really care if logs are outputted. You can remove it and therefore simplify this test.
("1234567890123456789", None, True, True), | ||
("abc", None, True, True), | ||
("", None, True, True), | ||
("10s", mock.ANY, False, True), |
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.
Put a timestamp result here please. You could achieve this by mocking datetime.datetime.utcnow
, that way you have a predefined time, and can anticipate the resulting period timestamp.
mock_get | ||
): | ||
logs_path = os.path.dirname(os.path.realpath(__file__)) | ||
logs_path = os.path.join(logs_path, 'data') |
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.
Please setup a tempfile instead of pushing test results to repository :) https://docs.python.org/3/library/tempfile.html
…n period is string
d281a2b
to
5a7078a
Compare
5a7078a
to
b41f349
Compare
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.
LGTM. Will merge after the previous PR is also merged (this one has __init__.py
missing in tests/v1.
This PR adds unit tests for coriolisclient.v1.licensing...logging modules.