----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10776/#review19767 -----------------------------------------------------------
test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40776> Can you include comments about the expected environment this test suite will run in? I know it is an EIP zone, but it'd be nice to include that a pre-setup NetScaler device needs to be added and the appropriate dictionary setting in `Services` be changed. test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40777> One space between methods. Document the method as a utility. Consider moving it to the utils class under integration.lib test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40778> One space between method definitions. Allow the name to be given as part of the method's name. This method will only be usable to test that the 'AssociatePublicIp' service is available. Instead if I wanted to query 'ElasticIp' then I'd have to use another method. So let's move this to utils too. test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40779> remove this comment test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40780> Let the entire scenario be part of the same docstring for the test. test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40781> remove the comment test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40782> Same issue here. include as part of the docstring. Just styling issue really :) test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40783> Consider adding self.assert_(expr, msg="On fail") after each API class so your assumption is validated before going ahead. test/integration/component/test_eip_optional_publicip.py <https://reviews.apache.org/r/10776/#comment40785> Remove trailing newlines - Prasanna Santhanam On April 25, 2013, 2:40 p.m., venkata swamy babu budumuru wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10776/ > ----------------------------------------------------------- > > (Updated April 25, 2013, 2:40 p.m.) > > > Review request for cloudstack, Prasanna Santhanam, SrikanteswaraRao Talluri, > and Girish Shilamkar. > > > Description > ------- > > CLOUDSTACK-2191: Added the following to tests to qualify "Optional Public > IP" changes happend in EIP enabled Zone > 1. Verify that the EIP zone is created with "AssociatePublicIp" > capability set to "false" > 2. Verify that VM doesn't get public ip by default after > deployVirtualMachine > 3. Eanble static NAT and verify EIP semantics > 4. disable static NAT and verify EIP semantics > > > This addresses bug CLOUDSTACK-2191. > > > Diffs > ----- > > test/integration/component/test_eip_optional_publicip.py PRE-CREATION > > Diff: https://reviews.apache.org/r/10776/diff/ > > > Testing > ------- > > Encountered few bugs while doing pilot run of the suite hence, couldn't test > it completely. > > Bugs encountered during the run are : > > CLOUDSTACK-2193 https://issues.apache.org/jira/browse/CLOUDSTACK-2193 > CLOUDSTACK-2192 https://issues.apache.org/jira/browse/CLOUDSTACK-2192 > > > Thanks, > > venkata swamy babu budumuru > >