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 19608: CLOUDSTACK-6282 - Added new automated testcases
Date Tue, 25 Mar 2014 13:30:52 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19608/#review38431
-----------------------------------------------------------



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70597>

    Line 9:  Move services section to config file in tools/marvin/marvin/config



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70599>

    Change name from services to test_data



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70598>

    Wrap it under try catch block and in case exception call clean up and return



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70600>

    No need to clean VM if account is getting cleaned



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70601>

    get userAPIClient, and wrap it in try catch, and in exception call clean up and then 
skip tests



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70612>

    This if elif and else can be better reorganized. We may not require elif and else, if
we verify the output previously available and then proceed with further commands.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70603>

    Keeps all the doc strings at 1 place, make sure that doc strings contains steps for test
cases, what to validate etc.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70602>

    Remove @author fields, wherever possible



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70605>

    Remove else just verify for output and fail, then we dont need else 



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70604>

    Move this assert to before else. Check for list_volumes_before as None,list type and empty
list and fail accordingly.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70634>

    self is not required here. 



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70613>

    Remove this if, and move clean step after assert. This way, clean up works if not none.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70606>

    Move this to before "if" statement and remove if statement as we already verified this
in assert



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70614>

    what if list_volumes_after is None



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70607>

    Move all this checks to a simple private function and verify for different outputs. Verify
for input dictionary of values and return fail\pass. Here, in test code use that utility and
assert once, we may not need multiple asserts 



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70608>

    what if list_volumes_by_id is None or not a list?



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70615>

    Add clear description and doc strings where ver applicable and possible. Add steps and
validation for test case.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70609>

    This if is not required as previous assert covers it.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70610>

    We can move this attach, list and detach to a simple utility function to be used by all
tests.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70611>

    Append only when the output of previous operation is not None or empty list



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70617>

    Rename the test case name properly to suit the purpose..



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70616>

    The output of it can be none or empty?



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70620>

    This test case is too big, break it in to multiple test cases if possible. 1. To verify
creation of snapshot from volume and 2. To verify creation of template from snapshot.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70619>

    Move this code to base.py 



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70618>

    Move the assertions to custom utility function.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70621>

    Move the hourly,daily ,weekly, monthly to multiple test cases, in case of hourly asserts,
daily weekly etc may not work. AS well, test case itself seems to be big, break in to multiple
validation cases.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70622>

    If is not required.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70623>

    Move this to custom utility function for validation and use one single assert based upon
pass\fail of that function.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70625>

    Here, len will break if list_snapshot_policy_after is None



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70626>

    Initialize to zero and check if list_snapshot_policy_before and assign length.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70624>

    Follow spaces instead of tabs wherever possible. 



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70629>

    This flow of steps, to verify volumes increase before and after creation is used at multiple
test cases, so may be we can write one utility function again for that and use across all
test cases. This reduces duplicate code.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70627>

    If is not required. As well, add more information to the assert



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70630>

    Again this flow can be moved to utility function, if we can keep our test case code simple
in terms of single liners then readability will be easy, as well add more description to steps.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70628>

    Remove if here.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70631>

    Fail first by verifying this logic and remove else altogether.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70633>

    Also, this flow seems to be common across test cases, move them under one private function
and use across.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70632>

    Does snapshot deletion takes time, otherwise we are moving to next step with out waiting
for it , add a sleep if required.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70637>

    Remove if.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70638>

    verify for before and output types to not None.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70639>

    Move this to base.py intermediate functions.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70636>

    Remove author flags.



test/integration/component/test_escalations.py
<https://reviews.apache.org/r/19608/#comment70635>

    what if list_volumes_before and list_volumes_after are None, then len of none will give
wrong assert


- Santhosh Edukulla


On March 25, 2014, 12:04 p.m., Vinay Varma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19608/
> -----------------------------------------------------------
> 
> (Updated March 25, 2014, 12:04 p.m.)
> 
> 
> Review request for cloudstack and Santhosh Edukulla.
> 
> 
> Bugs: CLOUDSTACK-6282
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6282
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding below new Test cases for test_escalations.py 
> 
> test_01_list_volumes_pagination 
> test_02_list_volume_byid 
> test_03_data_volume_resize 
> test_04_custom_volume 
> test_05_volume_snapshot 
> test_06_volume_snapshot_policy 
> test_07_volume_snapshots_pagination 
> test_08_volume_extract 
> test_09_volume_upload
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_escalations.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/19608/diff/
> 
> 
> Testing
> -------
> 
> @summary: Test List Volumes pagination ... ok
> @summary: Test List Volumes with Id ... ok
> @summary: Test to verify creation and resize of data volume ... ok
> @summary: Test to verify creation and resize of custom volume ... ok
> @summary: Test to verify creation of snapshot from volume and creation of template, volume
from snapshot ... ok
> @summary: Test to verify creation of Hourly, Daily, Weekly, Monthly Snapshot policies
from volume ... ok
> @summary: Test to verify pagination of snapshots for Volume ... ok
> @summary: Test to verify extract/download a Volume ... ok
> @summary: Test to verify upload volume ... ok
> 
> ----------------------------------------------------------------------
> Ran 9 tests in 299.663s
> 
> OK
> 
> 
> Thanks,
> 
> Vinay Varma
> 
>


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