You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Gaurav Aradhye <ga...@clogeny.com> on 2013/08/08 14:51:27 UTC
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/
-----------------------------------------------------------
(Updated Aug. 8, 2013, 12:51 p.m.)
Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
Changes
-------
Updated CPU limit resources tests as per review.
Repository: cloudstack-git
Description
-------
Adding test cases for CPU limits from Limit Resources.
Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
Diffs (updated)
-----
test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
tools/marvin/marvin/integration/lib/base.py b5d086b
tools/marvin/marvin/integration/lib/common.py 4f5acef
Diff: https://reviews.apache.org/r/13001/diff/
Testing
-------
Thanks,
Gaurav Aradhye
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Gaurav Aradhye <ga...@clogeny.com>.
> On Aug. 26, 2013, 10:09 a.m., Sanjay Tripathi wrote:
> > test/integration/component/cpu_limits/test_project_limits.py, line 275
> > <https://reviews.apache.org/r/13001/diff/3/?file=338935#file338935line275>
> >
> > use listResourceLimits API to get the resource limits of respective account/domain.
Removed the configuration as it was not being used in the test case. The project limits are tested in maximum limits test cases.
These test cases are used to test the resource count.
> On Aug. 26, 2013, 10:09 a.m., Sanjay Tripathi wrote:
> > test/integration/component/cpu_limits/test_project_limits.py, line 316
> > <https://reviews.apache.org/r/13001/diff/3/?file=338935#file338935line316>
> >
> > use listResourceLimits API to get the resource limit of respective account/domain.
Same reason.
- Gaurav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/#review25541
-----------------------------------------------------------
On Aug. 8, 2013, 12:51 p.m., Gaurav Aradhye wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13001/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2013, 12:51 p.m.)
>
>
> Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding test cases for CPU limits from Limit Resources.
> Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
>
>
> Diffs
> -----
>
> test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
> tools/marvin/marvin/integration/lib/base.py b5d086b
> tools/marvin/marvin/integration/lib/common.py 4f5acef
>
> Diff: https://reviews.apache.org/r/13001/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gaurav Aradhye
>
>
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Sanjay Tripathi <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/#review25541
-----------------------------------------------------------
test/integration/component/cpu_limits/test_project_limits.py
<https://reviews.apache.org/r/13001/#comment49993>
use listResourceLimits API to get the resource limits of respective account/domain.
test/integration/component/cpu_limits/test_project_limits.py
<https://reviews.apache.org/r/13001/#comment49992>
use listResourceLimits API to get the resource limit of respective account/domain.
tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/13001/#comment49990>
For now, we don't have support for resource type 12 and 13. So you can remove them from the comment.
Also, please remove the trailing white spaces.
tools/marvin/marvin/integration/lib/common.py
<https://reviews.apache.org/r/13001/#comment49991>
remove resource type 12 and 13 as they are not yet implemented.
- Sanjay Tripathi
On Aug. 8, 2013, 12:51 p.m., Gaurav Aradhye wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13001/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2013, 12:51 p.m.)
>
>
> Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding test cases for CPU limits from Limit Resources.
> Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
>
>
> Diffs
> -----
>
> test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
> tools/marvin/marvin/integration/lib/base.py b5d086b
> tools/marvin/marvin/integration/lib/common.py 4f5acef
>
> Diff: https://reviews.apache.org/r/13001/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gaurav Aradhye
>
>
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Prasanna Santhanam <ts...@apache.org>.
> On Aug. 27, 2013, 2:48 p.m., Sanjay Tripathi wrote:
> > Looks good to me.
Thanks for the detailed review Sanjay. I've merged this to master/4.2-forward.
Gaurav, there were many formatting errors in the patch. Please verify the merge and ensure that you've checked for formatting in your future patches.
Thanks
- Prasanna
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/#review25619
-----------------------------------------------------------
On Aug. 27, 2013, 1:32 p.m., Gaurav Aradhye wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13001/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 1:32 p.m.)
>
>
> Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding test cases for CPU limits from Limit Resources.
> Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
>
>
> Diffs
> -----
>
> test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
> tools/marvin/marvin/integration/lib/base.py 91cfebd
> tools/marvin/marvin/integration/lib/common.py 4f5acef
>
> Diff: https://reviews.apache.org/r/13001/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gaurav Aradhye
>
>
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Sanjay Tripathi <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/#review25619
-----------------------------------------------------------
Ship it!
Looks good to me.
- Sanjay Tripathi
On Aug. 27, 2013, 1:32 p.m., Gaurav Aradhye wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13001/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 1:32 p.m.)
>
>
> Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding test cases for CPU limits from Limit Resources.
> Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
>
>
> Diffs
> -----
>
> test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
> tools/marvin/marvin/integration/lib/base.py 91cfebd
> tools/marvin/marvin/integration/lib/common.py 4f5acef
>
> Diff: https://reviews.apache.org/r/13001/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gaurav Aradhye
>
>
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Gaurav Aradhye <ga...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/
-----------------------------------------------------------
(Updated Aug. 27, 2013, 1:32 p.m.)
Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
Changes
-------
Removed white-spaces.
Repository: cloudstack-git
Description
-------
Adding test cases for CPU limits from Limit Resources.
Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
Diffs (updated)
-----
test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
tools/marvin/marvin/integration/lib/base.py 91cfebd
tools/marvin/marvin/integration/lib/common.py 4f5acef
Diff: https://reviews.apache.org/r/13001/diff/
Testing
-------
Thanks,
Gaurav Aradhye
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Gaurav Aradhye <ga...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/
-----------------------------------------------------------
(Updated Aug. 27, 2013, noon)
Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
Changes
-------
Review changes.
Repository: cloudstack-git
Description
-------
Adding test cases for CPU limits from Limit Resources.
Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
Diffs (updated)
-----
test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
tools/marvin/marvin/integration/lib/base.py 91cfebd
tools/marvin/marvin/integration/lib/common.py 4f5acef
Diff: https://reviews.apache.org/r/13001/diff/
Testing
-------
Thanks,
Gaurav Aradhye
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Gaurav Aradhye <ga...@clogeny.com>.
> On Aug. 19, 2013, 11:49 a.m., Sanjay Tripathi wrote:
> > test/integration/component/cpu_limits/test_cpu_limits.py, line 526
> > <https://reviews.apache.org/r/13001/diff/3/?file=338932#file338932line526>
> >
> > As the test title says "reboot_instance" , please use reboot instance API instead of stopping and then starting an instance.
Hi Sanjay,
According to test scenarios, counts should be checked after stopping, and once again after starting the instance.
To avoid the confusion, I will change the test name.
> On Aug. 19, 2013, 11:49 a.m., Sanjay Tripathi wrote:
> > test/integration/component/cpu_limits/test_domain_limits.py, line 255
> > <https://reviews.apache.org/r/13001/diff/3/?file=338933#file338933line255>
> >
> > as the test title says "reboot_instance", please use reboot instance API.
- Gaurav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/#review25292
-----------------------------------------------------------
On Aug. 8, 2013, 12:51 p.m., Gaurav Aradhye wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13001/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2013, 12:51 p.m.)
>
>
> Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding test cases for CPU limits from Limit Resources.
> Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
>
>
> Diffs
> -----
>
> test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
> tools/marvin/marvin/integration/lib/base.py b5d086b
> tools/marvin/marvin/integration/lib/common.py 4f5acef
>
> Diff: https://reviews.apache.org/r/13001/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gaurav Aradhye
>
>
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Sanjay Tripathi <sa...@citrix.com>.
> On Aug. 19, 2013, 11:49 a.m., Sanjay Tripathi wrote:
> > test/integration/component/cpu_limits/test_domain_limits.py, line 718
> > <https://reviews.apache.org/r/13001/diff/3/?file=338933#file338933line718>
> >
> > if i am not wrong, the api_client should be api_client_cadmin_2
Can you please close the opened issues once you fix them, also remove the trailing white spaces.
- Sanjay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/#review25292
-----------------------------------------------------------
On Aug. 8, 2013, 12:51 p.m., Gaurav Aradhye wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13001/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2013, 12:51 p.m.)
>
>
> Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding test cases for CPU limits from Limit Resources.
> Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
>
>
> Diffs
> -----
>
> test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
> tools/marvin/marvin/integration/lib/base.py b5d086b
> tools/marvin/marvin/integration/lib/common.py 4f5acef
>
> Diff: https://reviews.apache.org/r/13001/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gaurav Aradhye
>
>
Re: Review Request 13001: Automation: Adding test cases for CPU limits from
Limit Resources.
Posted by Sanjay Tripathi <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13001/#review25292
-----------------------------------------------------------
test/integration/component/cpu_limits/test_cpu_limits.py
<https://reviews.apache.org/r/13001/#comment49631>
can't we have expected_resource_count which would be the exact expected value after the vm_delete operation and as we already knows about the service offering, we can easily calculate the expected value.
test/integration/component/cpu_limits/test_cpu_limits.py
<https://reviews.apache.org/r/13001/#comment49632>
As the test title says "reboot_instance" , please use reboot instance API instead of stopping and then starting an instance.
test/integration/component/cpu_limits/test_cpu_limits.py
<https://reviews.apache.org/r/13001/#comment49634>
create and use the expected_resource_count and assign the exact expected value.
test/integration/component/cpu_limits/test_cpu_limits.py
<https://reviews.apache.org/r/13001/#comment49635>
This seems like a duplicate test as there is already a test which is testing this scenario i.e. "test_02_migrate_instance"
Please remove the tests which you think are checking the same scenario more than once, as it will minimize the running time for this script.
test/integration/component/cpu_limits/test_domain_limits.py
<https://reviews.apache.org/r/13001/#comment49636>
as the test title says "reboot_instance", please use reboot instance API.
test/integration/component/cpu_limits/test_domain_limits.py
<https://reviews.apache.org/r/13001/#comment49637>
use different limits then what you have set for cadmin_1, it will help in checking more test cases.
test/integration/component/cpu_limits/test_domain_limits.py
<https://reviews.apache.org/r/13001/#comment49638>
if i am not wrong, the api_client should be api_client_cadmin_2
- Sanjay Tripathi
On Aug. 8, 2013, 12:51 p.m., Gaurav Aradhye wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13001/
> -----------------------------------------------------------
>
> (Updated Aug. 8, 2013, 12:51 p.m.)
>
>
> Review request for cloudstack, Girish Shilamkar, sailaja mada, Sanjay Tripathi, and Prasanna Santhanam.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Adding test cases for CPU limits from Limit Resources.
> Marvin changes have to be picked up from Memory Limits patch (https://reviews.apache.org/r/11626/). Those are not included in this patch.
>
>
> Diffs
> -----
>
> test/integration/component/cpu_limits/test_cpu_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_domain_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_maximum_limits.py PRE-CREATION
> test/integration/component/cpu_limits/test_project_limits.py PRE-CREATION
> tools/marvin/marvin/integration/lib/base.py b5d086b
> tools/marvin/marvin/integration/lib/common.py 4f5acef
>
> Diff: https://reviews.apache.org/r/13001/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gaurav Aradhye
>
>