> 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 > >
