-
Notifications
You must be signed in to change notification settings - Fork 55
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 minion_manager.rpc.client.py/tasks.py
unit tests
#321
Add minion_manager.rpc.client.py/tasks.py
unit tests
#321
Conversation
5ad95ce
to
22807f0
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.
First review pass done! Please look out for messages (log, event, progress updates, whatever) we do not want to include in tests, and remove all of them please.
exception message for proper formatting. Signed-off-by: Mihaela Balutoiu <mbalutoiu@cloudbasesolutions.com>
22807f0
to
092aac2
Compare
module Signed-off-by: Mihaela Balutoiu <mbalutoiu@cloudbasesolutions.com>
092aac2
to
4f3b9f5
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.
I updated the code, please take a look now. Thank you!
4f3b9f5
to
024a2a0
Compare
Mostly lgtm, you only have a little missed case in |
module Signed-off-by: Mihaela Balutoiu <mbalutoiu@cloudbasesolutions.com>
024a2a0
to
edb08b4
Compare
mock_get_minion_machine.return_value = self.minion_machine | ||
mock_execute.side_effect = CoriolisTestException |
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.
I don't get what you're testing here. You mock the tested method, make it raise and then check that the raise has occurred?
For this case, you need to:
- set
mock_get_minion_machine.return_value = None
; - remove the
mock_execute
entirely; - keep
_fail_on_error
onTrue
; - Check that the call raises
InvalidMinionMachineState
.
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.
Nevermind this, this case was already covered. This case is also good. Thank you!
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. Thank you!
This PR adds test cases for the following module:
minion_manager.rpc.tasks.py
minion_manager.rpc.client.py