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/09/24 15:28:53 UTC

Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

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

Review request for cloudstack, Harikrishna Patnala, venkata swamy babu  budumuru, and Prasanna Santhanam.


Repository: cloudstack-git


Description
-------

Adding Automation test cases for feature - Non Contiguous VLAN ranges
CLOUDSTACk 2238.


Diffs
-----

  test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 

Diff: https://reviews.apache.org/r/14319/diff/


Testing
-------

Tested locally.


Thanks,

Gaurav Aradhye


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Gaurav Aradhye <ga...@clogeny.com>.

> On Oct. 29, 2013, 11:38 p.m., SrikanteswaraRao Talluri wrote:
> >

Committed by Talluri

Master - 8a6694b396500897a3d5ce3f292864f2c5c2530f
4.2 - c6ce74fe61ae463a964215ab892c6cb399909801


- Gaurav


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


On Oct. 24, 2013, 7:37 p.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2013, 7:37 p.m.)
> 
> 
> Review request for cloudstack, bharat kumar and sanjeev n.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested again after review changes:
> 
> Log:
> 
> test_01_add_non_contiguous_ranges (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_02_add_existing_vlan_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_03_extend_contiguous_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_04_remove_unused_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_05_remove_used_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> 
> ----------------------------------------------------------------------
> Ran 5 tests in 345.586s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by SrikanteswaraRao Talluri <sr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/#review27707
-----------------------------------------------------------

Ship it!


- SrikanteswaraRao Talluri


On Oct. 24, 2013, 2:07 p.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2013, 2:07 p.m.)
> 
> 
> Review request for cloudstack, bharat kumar and sanjeev n.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested again after review changes:
> 
> Log:
> 
> test_01_add_non_contiguous_ranges (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_02_add_existing_vlan_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_03_extend_contiguous_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_04_remove_unused_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_05_remove_used_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> 
> ----------------------------------------------------------------------
> Ran 5 tests in 345.586s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Gaurav Aradhye <ga...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/
-----------------------------------------------------------

(Updated Oct. 24, 2013, 7:37 p.m.)


Review request for cloudstack, bharat kumar and sanjeev n.


Changes
-------

Review Changes.


Repository: cloudstack-git


Description
-------

Adding Automation test cases for feature - Non Contiguous VLAN ranges
CLOUDSTACk 2238.


Diffs (updated)
-----

  test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 

Diff: https://reviews.apache.org/r/14319/diff/


Testing (updated)
-------

Tested again after review changes:

Log:

test_01_add_non_contiguous_ranges (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_02_add_existing_vlan_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_03_extend_contiguous_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_04_remove_unused_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_05_remove_used_range (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok

----------------------------------------------------------------------
Ran 5 tests in 345.586s

OK


Thanks,

Gaurav Aradhye


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/#review27377
-----------------------------------------------------------



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53233>

    test_add_non_contiguous_ranges



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53234>

    test_add_existing_vlan_range



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53235>

    test_extend_contiguous_range



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53236>

    test_remove_unused_range



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53237>

    test_remove_used_range


I've given names I think are appropriate for the tests. Let me know if they make sense to you. In such cases the @docstring should include a more complete gist of what the test is doing. 

I'm aware of the account limit. At least we hit that limitation in a test. AFAIK, we don't have tests for these boundary limits for db columns. I remember it failed with an embarrassing looking stacktrace. However, I don't recall if the API failed gracefully or not.


- Prasanna Santhanam


On Oct. 23, 2013, 9:01 a.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 9:01 a.m.)
> 
> 
> Review request for cloudstack, bharat kumar and sanjeev n.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally on XenServer Setup
> 
> Log:
> 
> test_01 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_02 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_03 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_04 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_05 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> 
> ----------------------------------------------------------------------
> Ran 5 tests in 22.387s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Girish Shilamkar <gi...@clogeny.com>.

On Oct. 23, 2013, 9:08 a.m., Gaurav Aradhye wrote:
> > The naming of the tests should be improved. test_01/02/03 doesn't tell much about the test. Also is the ordering strictly required? Can I reorder the tests and still expect the tests to pass? If not, we should remove the 01 , 02 , 03 in the tests.
> 
> Gaurav Aradhye wrote:
>     Yes the tests can be reordered and run successfully. I will change the test names. The only reason to keep such test names was to avoid long test names which tell the same thing which is written in the comments (Steps and validation steps), 01,02,03 does not stand for any ordering.
> 
> Prasanna Santhanam wrote:
>     A meaningful name still makes sense for the tests as the Marvin TestRunner shows the first line in your docstring and if that's not available the name of the test itself which will look like test_01. The report is easier to understand with names. That's my only complaint.

The problem with earlier test naming was that neither the name could indicate purpose of the test, given its complexity nor name was concise enough to be called out. With new names it is at least easy to refer tests "non_contiguous_vlan test_03" instead of some 
test_name_too_long_to_use_but_to_short_to_describe_test. And the tests will have the description of what it does.

Secondly, we use testname to construct account name and the db cannot handle account name more that 100 chars. I have seen at least one such defect.

The numbering does not indicate sequential order for execution.


- Girish


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


On Oct. 23, 2013, 9:01 a.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 9:01 a.m.)
> 
> 
> Review request for cloudstack, bharat kumar and sanjeev n.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally on XenServer Setup
> 
> Log:
> 
> test_01 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_02 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_03 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_04 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_05 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> 
> ----------------------------------------------------------------------
> Ran 5 tests in 22.387s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Gaurav Aradhye <ga...@clogeny.com>.

On Oct. 23, 2013, 2:38 p.m., Gaurav Aradhye wrote:
> > The naming of the tests should be improved. test_01/02/03 doesn't tell much about the test. Also is the ordering strictly required? Can I reorder the tests and still expect the tests to pass? If not, we should remove the 01 , 02 , 03 in the tests.

Yes the tests can be reordered and run successfully. I will change the test names. The only reason to keep such test names was to avoid long test names which tell the same thing which is written in the comments (Steps and validation steps), 01,02,03 does not stand for any ordering.


- Gaurav


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


On Oct. 23, 2013, 2:31 p.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 2:31 p.m.)
> 
> 
> Review request for cloudstack, bharat kumar and sanjeev n.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally on XenServer Setup
> 
> Log:
> 
> test_01 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_02 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_03 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_04 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_05 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> 
> ----------------------------------------------------------------------
> Ran 5 tests in 22.387s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Prasanna Santhanam <ts...@apache.org>.

On Oct. 23, 2013, 9:08 a.m., Gaurav Aradhye wrote:
> > The naming of the tests should be improved. test_01/02/03 doesn't tell much about the test. Also is the ordering strictly required? Can I reorder the tests and still expect the tests to pass? If not, we should remove the 01 , 02 , 03 in the tests.
> 
> Gaurav Aradhye wrote:
>     Yes the tests can be reordered and run successfully. I will change the test names. The only reason to keep such test names was to avoid long test names which tell the same thing which is written in the comments (Steps and validation steps), 01,02,03 does not stand for any ordering.

A meaningful name still makes sense for the tests as the Marvin TestRunner shows the first line in your docstring and if that's not available the name of the test itself which will look like test_01. The report is easier to understand with names. That's my only complaint.


- Prasanna


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


On Oct. 23, 2013, 9:01 a.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 9:01 a.m.)
> 
> 
> Review request for cloudstack, bharat kumar and sanjeev n.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally on XenServer Setup
> 
> Log:
> 
> test_01 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_02 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_03 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_04 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_05 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> 
> ----------------------------------------------------------------------
> Ran 5 tests in 22.387s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/#review27372
-----------------------------------------------------------



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53228>

    why a new Services() instance when you can use the attribute from the class? self.services["vlan"]?



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53229>

    can we give this a better name?


The naming of the tests should be improved. test_01/02/03 doesn't tell much about the test. Also is the ordering strictly required? Can I reorder the tests and still expect the tests to pass? If not, we should remove the 01 , 02 , 03 in the tests.

- Prasanna Santhanam


On Oct. 23, 2013, 9:01 a.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 9:01 a.m.)
> 
> 
> Review request for cloudstack, bharat kumar and sanjeev n.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally on XenServer Setup
> 
> Log:
> 
> test_01 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_02 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_03 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_04 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_05 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> 
> ----------------------------------------------------------------------
> Ran 5 tests in 22.387s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by sanjeev n <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/#review27374
-----------------------------------------------------------



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53230>

    This test would fail if the existing vlan rang is say 4090-4095. In this case code execution will go to elif block and the ranges will be like as follows:
    4093-4092,4091-4090,4093-4090 which are not valid vlan ranges(higher to lower number). Please change the logic in elif block.



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53231>

    Instead of simply depending on updatePhysicalNetworkResponse parameter for fail/passing the test case also list the updated physical netowrk, read the vlan info and validate it. This applies to all the tests.



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment53232>

    No validation to check step3 mentioned in the dotted strings in the test. Please add code to validate step3.


- sanjeev n


On Oct. 23, 2013, 9:01 a.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 9:01 a.m.)
> 
> 
> Review request for cloudstack, bharat kumar and sanjeev n.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally on XenServer Setup
> 
> Log:
> 
> test_01 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_02 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_03 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_04 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> test_05 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
> 
> ----------------------------------------------------------------------
> Ran 5 tests in 22.387s
> 
> OK
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Prasanna Santhanam <ts...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/
-----------------------------------------------------------

(Updated Oct. 23, 2013, 9:01 a.m.)


Review request for cloudstack, bharat kumar and sanjeev n.


Changes
-------

assigning to corresponding dev and QA


Repository: cloudstack-git


Description
-------

Adding Automation test cases for feature - Non Contiguous VLAN ranges
CLOUDSTACk 2238.


Diffs
-----

  test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 

Diff: https://reviews.apache.org/r/14319/diff/


Testing
-------

Tested locally on XenServer Setup

Log:

test_01 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_02 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_03 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_04 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_05 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok

----------------------------------------------------------------------
Ran 5 tests in 22.387s

OK


Thanks,

Gaurav Aradhye


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Gaurav Aradhye <ga...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/
-----------------------------------------------------------

(Updated Oct. 15, 2013, 7:25 p.m.)


Review request for cloudstack, Harikrishna Patnala, venkata swamy babu  budumuru, and Prasanna Santhanam.


Repository: cloudstack-git


Description
-------

Adding Automation test cases for feature - Non Contiguous VLAN ranges
CLOUDSTACk 2238.


Diffs
-----

  test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 

Diff: https://reviews.apache.org/r/14319/diff/


Testing (updated)
-------

Tested locally on XenServer Setup

Log:

test_01 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_02 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_03 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_04 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok
test_05 (test_non_contiguous_vlan.TestNonContiguousVLANRanges) ... ok

----------------------------------------------------------------------
Ran 5 tests in 22.387s

OK


Thanks,

Gaurav Aradhye


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Gaurav Aradhye <ga...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/
-----------------------------------------------------------

(Updated Oct. 15, 2013, 7:23 p.m.)


Review request for cloudstack, Harikrishna Patnala, venkata swamy babu  budumuru, and Prasanna Santhanam.


Changes
-------

Review Changes


Repository: cloudstack-git


Description
-------

Adding Automation test cases for feature - Non Contiguous VLAN ranges
CLOUDSTACk 2238.


Diffs (updated)
-----

  test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 

Diff: https://reviews.apache.org/r/14319/diff/


Testing
-------

Tested locally.


Thanks,

Gaurav Aradhye


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Gaurav Aradhye <ga...@clogeny.com>.

> On Oct. 15, 2013, 4:08 p.m., venkata swamy babu  budumuru wrote:
> > test/integration/component/test_non_contiguous_vlan.py, line 117
> > <https://reviews.apache.org/r/14319/diff/3/?file=359634#file359634line117>
> >
> >     1. Have something like "self.phy_network" instead of slf.network. This is little confusing.
> >     
> >     2. This script is fetch phy network info here as well as in setNonContiguousVlanIds. Can you remove the redundancy here?
> 
> Gaurav Aradhye wrote:
>     Sure, will do.

Removed the redundancy


> On Oct. 15, 2013, 4:08 p.m., venkata swamy babu  budumuru wrote:
> > test/integration/component/test_non_contiguous_vlan.py, line 311
> > <https://reviews.apache.org/r/14319/diff/3/?file=359634#file359634line311>
> >
> >     I dont see this instructions and the steps mentioned in the doc string matches. Can you please double check this test?
> 
> Gaurav Aradhye wrote:
>     The test verifies that if the vlan id in a range is in use, then that vlan range can't be removed.
>     The test updates the vlan with a range, deploys an instance so that it automatically creates a network which uses a vlan id from the current range.
>     Then it tries to update vlan with different range (This is same as removing the current range). It is checked that this operation should fail.
>     
>     Please let me know which step is unclear so that I can update it with appropriate comment.
>

Provided the comments so that the steps are clear.
Also, added one condition where the vlan id from the existing range is in use. In that case, removing this range should throw error and test case purpose succeeds at this step only, no need to go on adding new range and use vlan id from the new range.


- Gaurav


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


On Oct. 15, 2013, 7:23 p.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2013, 7:23 p.m.)
> 
> 
> Review request for cloudstack, Harikrishna Patnala, venkata swamy babu  budumuru, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Gaurav Aradhye <ga...@clogeny.com>.

> On Oct. 15, 2013, 4:08 p.m., venkata swamy babu  budumuru wrote:
> > test/integration/component/test_non_contiguous_vlan.py, line 311
> > <https://reviews.apache.org/r/14319/diff/3/?file=359634#file359634line311>
> >
> >     I dont see this instructions and the steps mentioned in the doc string matches. Can you please double check this test?

The test verifies that if the vlan id in a range is in use, then that vlan range can't be removed.
The test updates the vlan with a range, deploys an instance so that it automatically creates a network which uses a vlan id from the current range.
Then it tries to update vlan with different range (This is same as removing the current range). It is checked that this operation should fail.

Please let me know which step is unclear so that I can update it with appropriate comment.


> On Oct. 15, 2013, 4:08 p.m., venkata swamy babu  budumuru wrote:
> > test/integration/component/test_non_contiguous_vlan.py, line 117
> > <https://reviews.apache.org/r/14319/diff/3/?file=359634#file359634line117>
> >
> >     1. Have something like "self.phy_network" instead of slf.network. This is little confusing.
> >     
> >     2. This script is fetch phy network info here as well as in setNonContiguousVlanIds. Can you remove the redundancy here?

Sure, will do.


- Gaurav


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


On Sept. 30, 2013, 1:51 p.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2013, 1:51 p.m.)
> 
> 
> Review request for cloudstack, Harikrishna Patnala, venkata swamy babu  budumuru, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by venkata swamy babu budumuru <ve...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/#review27016
-----------------------------------------------------------



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment52585>

    1. Have something like "self.phy_network" instead of slf.network. This is little confusing.
    
    2. This script is fetch phy network info here as well as in setNonContiguousVlanIds. Can you remove the redundancy here?



test/integration/component/test_non_contiguous_vlan.py
<https://reviews.apache.org/r/14319/#comment52586>

    I dont see this instructions and the steps mentioned in the doc string matches. Can you please double check this test?


- venkata swamy babu  budumuru


On Sept. 30, 2013, 8:21 a.m., Gaurav Aradhye wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14319/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2013, 8:21 a.m.)
> 
> 
> Review request for cloudstack, Harikrishna Patnala, venkata swamy babu  budumuru, and Prasanna Santhanam.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding Automation test cases for feature - Non Contiguous VLAN ranges
> CLOUDSTACk 2238.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/14319/diff/
> 
> 
> Testing
> -------
> 
> Tested locally.
> 
> 
> Thanks,
> 
> Gaurav Aradhye
> 
>


Re: Review Request 14319: CLOUDSTACK 2238: Automation - Non Contiguous VLAN Ranges

Posted by Gaurav Aradhye <ga...@clogeny.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14319/
-----------------------------------------------------------

(Updated Sept. 30, 2013, 1:51 p.m.)


Review request for cloudstack, Harikrishna Patnala, venkata swamy babu  budumuru, and Prasanna Santhanam.


Changes
-------

Updated test case names.


Repository: cloudstack-git


Description
-------

Adding Automation test cases for feature - Non Contiguous VLAN ranges
CLOUDSTACk 2238.


Diffs (updated)
-----

  test/integration/component/test_non_contiguous_vlan.py PRE-CREATION 

Diff: https://reviews.apache.org/r/14319/diff/


Testing
-------

Tested locally.


Thanks,

Gaurav Aradhye