----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13559/#review25613 -----------------------------------------------------------
test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50079> It would be better if we prepare services dictionary for "portable_ip_range" manually on what range it should take. Current logic just increments the range by 1 and it expects the existing physical infra to be configured with that CIDR. This approach many not suite for everyone who fires this script. This is mandating the user of the scripts to have the next cidr of his public range to be configured and available for portable ip range testing. test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50080> There is no param / code that takes VLAN info in case if it has to be set. currently it taking/assuming a native vlan environment. we need make this flexible enough to consider the VLAN info. if we take the approach of filling the service dictionary for portable_ip_range then we don't have to worry about this. test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50081> In case of have a valid reason for keeping increment_cidr (or for that matter all the other helper functions), Is it a good a idea to consider moving it to common.py because I see these definitions in all the class of this suite. test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50082> Can you move create_portable_ip_ranage this to setup()? it would be better if test just does the delete functionality. test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50083> Can you move create_portable_ip_range this to setup()? it would be better if test just does the delete functionality. test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50087> Can you add some more checks that will make sure the list is giving the right start/endip/gw/netmask values? test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50085> Can you move this (create_portable_ip_range) to setup()? test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50084> All these tests are tagged for "advanced_sg" and "sg". If there is no reason then can we add "advanced" because this what we frequently run and users use. it would be good to cover as part of advanced also. test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50086> Can you move this (create_portable_ip_range) to setup()? test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50088> Can you move service_offering, virtualmachine create operations to setupClass()? test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50089> can we have the CIDR info as well as part of services["natrule"] dictionary? test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50090> Isn't it a better idea to check the programmed NAT rule is functioning properly? test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50091> can you move service offering and virtual machine creation code to setupClass() test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50092> There are couple of tests where it says the first step as below. "# 1. Create portable ip range" but, we are not actually creating a portable ip range hence, can you remove / update the doc strings for those cases. test/integration/component/test_portable_ip.py <https://reviews.apache.org/r/13559/#comment50093> After delete, can you add a check that verifies the portable IP address is now marked as free in DB? - venkata swamy babu budumuru On Aug. 14, 2013, 11:35 a.m., Gaurav Aradhye wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13559/ > ----------------------------------------------------------- > > (Updated Aug. 14, 2013, 11:35 a.m.) > > > Review request for cloudstack, Murali Reddy, venkata swamy babu budumuru, > and Prasanna Santhanam. > > > Repository: cloudstack-git > > > Description > ------- > > Adding new test cases for feature Portable IP > > > Diffs > ----- > > test/integration/component/test_portable_ip.py PRE-CREATION > > Diff: https://reviews.apache.org/r/13559/diff/ > > > Testing > ------- > > > Thanks, > > Gaurav Aradhye > >
