cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gaurav Aradhye" <gaurav.arad...@clogeny.com>
Subject Re: Review Request 20316: CLOUDSTACK-1466: Adding automation test cases for Primary Storage Limits
Date Thu, 29 May 2014 13:11:49 GMT


> On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/lib/common.py, line 1103
> > <https://reviews.apache.org/r/20316/diff/2/?file=567838#file567838line1103>
> >
> >     Please dont add asserts to lib functions, just check the output in test code
and take it accordingly.

Assert is inside try catch block, so exception won't be raised directly from library. We are
returning FAIL in case of exception, along with the exception message.


> On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/lib/common.py, line 1122
> > <https://reviews.apache.org/r/20316/diff/2/?file=567838#file567838line1122>
> >
> >     Please remove asserts from libs.

It is inside try catch block.


> On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/lib/common.py, line 1203
> > <https://reviews.apache.org/r/20316/diff/2/?file=567838#file567838line1203>
> >
> >     This as well can be added to base.py. Maintaining too much common functions
dependent upon base.py libraries is current overhead, VirtualMachine_obj.getState(args) can
give us is expunged or stopped or started etc.

When VM is expunged, we do not get its state as "Expunged". List API throws exception in this
case as it does not find the VM id. Hence this case is not a candidate for getState function
in base.py


> On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote:
> > tools/marvin/marvin/lib/common.py, line 1185
> > <https://reviews.apache.org/r/20316/diff/2/?file=567838#file567838line1185>
> >
> >     Can we move this function inside Snapshot class only?

Moved inside snapshot class


> On May 8, 2014, 5:06 p.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ps_domain_limits.py, line 51
> > <https://reviews.apache.org/r/20316/diff/2/?file=567831#file567831line51>
> >
> >     Also, we can do a quick code walkthrough to understand the flow. It will help
to do it more thoroughly. We can do a gtm.

Sure


- Gaurav


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


On April 24, 2014, 11:01 p.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20316/
> -----------------------------------------------------------
> 
> (Updated April 24, 2014, 11:01 p.m.)
> 
> 
> Review request for cloudstack, Girish Shilamkar and Santhosh Edukulla.
> 
> 
> Bugs: CLOUDSTACK-1466
>     https://issues.apache.org/jira/browse/CLOUDSTACK-1466
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding test suits in Primary Storage Limits test cases.
> 1)Root/Domain admin limits
> 2)Domain Limits
> 3)Resize volume
> 4)Project Limits
> 5)Maximum Limits
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_ps_domain_limits.py PRE-CREATION 
>   test/integration/component/test_ps_limits.py PRE-CREATION 
>   test/integration/component/test_ps_max_limits.py PRE-CREATION 
>   test/integration/component/test_ps_project_limits.py PRE-CREATION 
>   test/integration/component/test_ps_resize_volume.py PRE-CREATION 
>   tools/marvin/marvin/codes.py 4d44c58 
>   tools/marvin/marvin/lib/base.py d753098 
>   tools/marvin/marvin/lib/common.py 8868d2d 
> 
> Diff: https://reviews.apache.org/r/20316/diff/
> 
> 
> Testing
> -------
> 
> Yes
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


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