Github user gauravaradhye commented on the pull request:
https://github.com/apache/cloudstack/pull/268#issuecomment-104535446
Posting review here as I am not able to put inline review comments.
Comments in the format (line number space comment)
133 space between self,vm
147 There should not be space between parameter=value
133 Function name should resemble an order given to function, like doThis,
doThat. change name to createVmSnapshotForDataIntegrityCheck or something
suitable
133 Are we not actually checking integrity or only creating Vm snapshot?
164 space between parameters. You can do this automatically using autopep8
tool. pip install autopep8. autopep8 -i -a -a fileName.py. And then check if
any pep8 issues are not fixed automatically by pep8 fileName.py and fix them
manually.
2480 If test restarts management server by default, then move it to maint
folder. Or you can use key "restartManagementServerThroughTestCase"
912 use self.skipTest("") instead of raise unittest.SkipTest, change all
such instances
1235 Check if destinationHost is returned as None, in that migrate should
not be performed, change everywhere applicable
1278 Compare string after converting to lowercase only. Also don't use
string directly, make use of constant strings from codes.py file, change
everywhere applicable
Also, post the marvin run logs showing test cases passed successfully.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
