cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Prasanna Santhanam" <...@apache.org>
Subject Re: Review Request: Automation: CPU limits tests from Resource Limits Tests
Date Mon, 29 Apr 2013 08:30:11 GMT

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



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40950>

    Also - please separate the suite into the following files. In this patch we can do:
    
    test_resource_limits (regular CPU tests/Max CPU tests)
    test_project_limits (resource limit tests for projects)
    test_domain_subdomain_limits (resource limit tests related to domain-admin, sub-domains
etc)
    
    Having these in separate modules also helps the test runner drive the suites in parallel.



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40928>

    Move this dictionary to utils? and rename to utils.get_resource_type()? 



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40929>

    `delete` is not handled?



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40930>

    There should be an API named findHostsForMigration. This was recently added to admin roles
so they can identify the hosts suitable for migration.



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40931>

    The service_offering however has only 2vCPUs in the dictionary. Can we just test for multicore?
Any 2/3/4 multicore setting should work?
    test_deploy_vm_with_multiplecores



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40935>

    Should these utility methods be included in every test class? Can we not include them
in a single class and import as necessary? 



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40943>

    method name doesn't reflect intent. you are doing a checkResourceCount but doing an updateCount
within? Was this intended?



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40938>

    Can you add the "simulator" tag? This will allow us to run it against the simulator quickly.
VM operations take longer on hardware.



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40936>

    service_offering has only 2 cores



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40939>

    There's some confusion I have here:
    
    1. you are using the apiClient of the admin role and the limits apply on the account (domadmin,
chlilddomadmin) 
    2. use the api client of the account you created instead to establish the limits? createUserApiClient
from marvin's library can be used here.
    



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40937>

    can we name either camel_case or camelCase. Let's not mix both?



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40940>

    Can you also put in a docstring @configuration with the value the configuration setting
is expected to take in this test? 



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40941>

    Can this be used to udpdate resoruce counts of 1-8?
    If not put in an assert that rtype in [9, 10, 11, 12, 13, 14] only



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40942>

    Not sure the comment reflects the test being done. Seems to be the same as previous tests?



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40944>

    How are these tests different from the ones in the previous class?
    TestCPULimitsUpdateResources .test_02_deploy_multiple_vm_with_5_cpus vs TestDomainCPULimits.test_02_deploy_multiple_vm_with_5_cpus?



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40945>

    Can we have createDefaultTestAccounts([domadmin, admin, childadmin]) , updateLimits(accountname,
domainid) and updateCounts(accountname, domainid) as methods instead of the ones used in all
the classes here? 



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40946>

    This can be broken down further. There are too many tests within this single test.
    
    1. two domains sharing a single pool of cores
    2. a heirarchy of root-parent-child sharing the same pool of cores
    3. In each - deploying the VM, deletion of account affecting the resources
    



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40948>

    setupProjectAccounts



test/integration/component/test_cpu_limits.py
<https://reviews.apache.org/r/10828/#comment40949>

    Assign is not a regular vm lifecycle operation so please separate this test from the rest.
Also - should the test be named test_(role=account/domain/project)_counts_vmlifecycle? It's
the effect of vmlifecycle being tested on the resource counts. The name you've given is a
bit generic.
    
    


- Prasanna Santhanam


On April 29, 2013, 2:29 a.m., Chirag Jog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10828/
> -----------------------------------------------------------
> 
> (Updated April 29, 2013, 2:29 a.m.)
> 
> 
> Review request for cloudstack, Prasanna Santhanam and Girish Shilamkar.
> 
> 
> Description
> -------
> 
> Here is the detailed test plan: https://issues.apache.org/jira/secure/attachment/12571551/LimitResourcesTestPlan1.xlsx
> 
> 
> This addresses bug https://issues.apache.org/jira/browse/CLOUDSTACK-1466.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_cpu_limits.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/10828/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chirag Jog
> 
>


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