cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ashutosh Kelkar" <ashut...@clogeny.com>
Subject Re: Review Request 19039: CLOUDSTACK-2266: Adding automation tests for IP reservation feature
Date Thu, 20 Mar 2014 17:30:34 GMT


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 258
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line258>
> >
> >     Move this to a utility function i believe. Also, do we have a class in base.py
for Router to verify state. If not update or use that.

We are just listing the routers here and matching the state. There is no API for matching
router state as such. I have moved this function to utility.


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 296
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line296>
> >
> >     The validation comment at step2 mentions about to verify that the guest vm ip
is not changed. I believe we are just listing the vms, should that satisfy to verify ip as
well, i mean is listing vms is equal to verify that ip is not changed?

IPs have been matched in step below it to verify it is not changed.


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 318
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line318>
> >
> >     Step1 is not validated it seems?

It is verified in the function called on line 246


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 328
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line328>
> >
> >     is 10.1.1.9 outside of 10.1.1.0/29?

10.1.1.0/29 translates to 10.1.1.0 to 10.1.1.7


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 1021
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line1021>
> >
> >     Change this to one single assert?

The utility function you have suggested checks if the element is present in list. But here
we have to check particular attributes of object, I think it won't be possible to combine
two alerts with using utility function.


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 265
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line265>
> >
> >     We have a utility function under utils.py "verifyElementInList", this can help
remove two asserts in to one i believe, please check.

The utility function you have suggested checks if the element is present in list. But here
we have to check particular attributes of object, I think it won't be possible to combine
two alerts with using utility function.


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 483
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line483>
> >
> >     Will it be possible to check the update state of router rather sleep? It will
help other test cases as well.

I have used "router.check.interval" instead of sleeping for any random time, and we are using
verifyRouterState function to check the router state is changed or not. This is a validation
part of test case.


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 1271
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line1271>
> >
> >     This function seems to be used in many classes, so if possible move it as a
util\library function for better usage.

This is used only in this class and very specific to the test cases in this suite.


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 1277
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line1277>
> >
> >     Will this be cleaned automatically?

It will be cleaned up as part of account cleanup.


> On March 18, 2014, 6:57 a.m., Santhosh Edukulla wrote:
> > test/integration/component/test_ip_reservation.py, line 1317
> > <https://reviews.apache.org/r/19039/diff/3/?file=519653#file519653line1317>
> >
> >     Is this ok to hardcode these values? instead put it in one place, this way,
the values can be altered at one place and effect takes place at all rather changing all test
cases in future.

Removed hard-coded value, now subnet is taken randomly.


- Ashutosh


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


On March 14, 2014, 4:44 p.m., Ashutosh Kelkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19039/
> -----------------------------------------------------------
> 
> (Updated March 14, 2014, 4:44 p.m.)
> 
> 
> Review request for cloudstack, Santhosh Edukulla and SrikanteswaraRao Talluri.
> 
> 
> Bugs: CLOUDSTACK-2266
>     https://issues.apache.org/jira/browse/CLOUDSTACK-2266
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Adding first set of automation tests for IP reservation feature.
> 
> 
> Diffs
> -----
> 
>   test/integration/component/test_ip_reservation.py 224212f 
>   tools/marvin/marvin/config/config.cfg 356a291 
> 
> Diff: https://reviews.apache.org/r/19039/diff/
> 
> 
> Testing
> -------
> 
> Yes. Log below.
> 
> test_RVR_network (test_ip_reservation.TestIpReservation) ... SKIP: Skip - WIP
> test_ip_reservation_in_multiple_networks_same_account (test_ip_reservation.TestIpReservation)
... ok
> test_nat_rules_nat rule (test_ip_reservation.TestIpReservation) ... ok
> test_nat_rules_static nat rule (test_ip_reservation.TestIpReservation) ... ok
> test_update_cidr_multiple_vms_not_all_inclusive (test_ip_reservation.TestIpReservation)
... ok
> test_update_cidr_single_vm_not_inclusive (test_ip_reservation.TestIpReservation) ...
ok
> test_user_defined_cidr (test_ip_reservation.TestIpReservation) ... ok
> test_vm_create_after_reservation_LB-NS (test_ip_reservation.TestIpReservation) ... SKIP:
Skipping - this test required netscaler configured in the network
> test_vm_create_after_reservation_LB-VR (test_ip_reservation.TestIpReservation) ... ok
> test_vm_create_outside_cidr_after_reservation_LB-NS (test_ip_reservation.TestIpReservation)
... SKIP: Skipping - this test required netscaler configured in the network
> test_vm_create_outside_cidr_after_reservation_LB-VR (test_ip_reservation.TestIpReservation)
... ok
> ----------------------------------------------------------------------
> Ran 11 tests in 3753.613s
> 
> OK
> 
> Two tests which require Netscaler configured are skipped due to netscaler was not available
on setup.
> 
> 
> Thanks,
> 
> Ashutosh Kelkar
> 
>


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