----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20410/#review40552 -----------------------------------------------------------
test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73568> Just Append the values to cleanup. The list is earlier initialized. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73569> Why cant we use the earlier created apiclient? test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73570> Use @Desc and @Steps, instead of @summary. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73575> This is a self service case. We can remove the attribute provisioning. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73572> Why this case of list? We didnt verified the list is of zero size. The case mentions to see the size of vms increased by 1 post create, so size of virtualMachine.list post create should increse by 1, we verified it for None but not for empty list? test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73571> There is a function to remove multiple assert inside utils, please use that, it verifies whether it is a list, none, empty, and returns the first element. This way much code can be removed from here. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73574> We just got the first vm. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73580> why hard code hypervisor? test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73581> Verify for the empty list as well. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73578> Verify for empty list with size 0. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73582> For all big if else, may be we can just verify it and assert before only, if not continue with our tests. The if else length is big. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73576> For all asserts pertaining to list, please use the verify function added in utils. test/integration/component/test_escalations.py <https://reviews.apache.org/r/20410/#comment73577> Test Names requires better naming i believe. EX: update_vm does not signify its purpose. - Santhosh Edukulla On April 16, 2014, 8:29 a.m., Vinay Varma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20410/ > ----------------------------------------------------------- > > (Updated April 16, 2014, 8:29 a.m.) > > > Review request for cloudstack and Santhosh Edukulla. > > > Bugs: cloudstack-6282 > https://issues.apache.org/jira/browse/cloudstack-6282 > > > Repository: cloudstack-git > > > Description > ------- > > Added automated tests for Instances API calls in test_escalations.py > Modified an existing test for Volumes to handle the non-exists check > Added few utility functions in base.py which are currently not present > > > Diffs > ----- > > test/integration/component/test_escalations.py f2ae801 > tools/marvin/marvin/lib/base.py 26f920e > > Diff: https://reviews.apache.org/r/20410/diff/ > > > Testing > ------- > > Executed all the Added tests on both basic and Advance zones and attached the > results for the same > > > File Attachments > ---------------- > > Advance Zone Results > > https://reviews.apache.org/media/uploaded/files/2014/04/16/21eaf8f7-691f-4d4d-927b-0aadffe589ec__AdvZoneResults.txt > Basic Zone Results > > https://reviews.apache.org/media/uploaded/files/2014/04/16/f8754d7e-5fe3-48e6-bdfe-f822171b50cd__BasicZoneResults.txt > > > Thanks, > > Vinay Varma > >
