Some places we are explicitly verifying for exception, EX: with assert for exception, with assert for not exception from CS, at some places we are verifying for return value say failed. If we return failed, then cases above are not clear. So, made it uniform for raising exception. Even earlier, the command raises the exception caught for command failure case.
I believe all the test cases will still work. Because of returning failed, base.py classes are directly calling on operation on the returned response, assuming response has received. So, the message dumped when failure happens is little misleading, it says string object has no attribute, but actual exception was some thing different. During phase2 changes, we can make it more deterministic, by returning failure to test code, they can verify and assert, and we as well provide error information to them. Santhosh ________________________________________ From: Gaurav Aradhye [gaurav.arad...@clogeny.com] Sent: Monday, May 05, 2014 3:04 AM To: dev@cloudstack.apache.org; Santhosh Edukulla Cc: Sanjeev Neelarapu Subject: Re: Review Request 21070: Fixed few exception cases. Hi Santhosh, I would like to mention one concern here. If we are raising exception through "marvinRequest" function in cloudstackConnection.py, instead of returning FAILED, then this will need many test cases to be changed too, where we are explicitly asserting the output of particular marvin command as FAILED or not as FAILED. Do you think this can be avoided? Many test cases failed last time we changed raising exception to returning FAILED. And those fixes will stop working in this case. May be we can solve the problem by extracting the exception message and raising it as a string e.g raise Exception(string), and this will be logged to the failure logs, and ultimately we return FAILED. Regards, Gaurav On Mon, May 5, 2014 at 12:19 PM, Santhosh Edukulla <santhosh.eduku...@citrix.com<mailto:santhosh.eduku...@citrix.com>> wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21070/ ----------------------------------------------------------- Review request for cloudstack and sanjeev n. Repository: cloudstack-git Description ------- Fixed few exception cases. When some request fails, it throws up the stack, but the message str..does not contains a given attribute is little misleading. So, raising exceptions for all cases. Added testsuite name for userprovided\default log folder path Fixed pep8 changes. Diffs ----- tools/marvin/marvin/cloudstackConnection.py caa8609 tools/marvin/marvin/cloudstackTestClient.py d3a6b94 tools/marvin/marvin/configGenerator.py 4f03fd0 tools/marvin/marvin/marvinInit.py de580ce tools/marvin/marvin/marvinLog.py 65687aa tools/marvin/marvin/marvinPlugin.py c817cd6 Diff: https://reviews.apache.org/r/21070/diff/ Testing ------- Thanks, Santhosh Edukulla
