cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Santhosh Edukulla" <santhosh.eduku...@citrix.com>
Subject Re: Review Request 20410: CLOUDSTACK-6282 - Added automated tests for Instances API calls in test_escalations.py
Date Wed, 16 Apr 2014 16:08:13 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message