> On Dec. 11, 2013, 6:04 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_advancedsg_networks.py, line 376 > > <https://reviews.apache.org/r/16178/diff/1/?file=396533#file396533line376> > > > > Just verify the order of deletion. nwofferings or domain?
If we have domain specific networks, then domain removal does not remove the network in it (unlike in case of account), instead it throws exception while removing the domain, hence in cleanup, first networks are removed and then domain. And for network offering cleanup, order does not matter as long as network is removed before removing the offering. > On Dec. 11, 2013, 6:04 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_advancedsg_networks.py, line 512 > > <https://reviews.apache.org/r/16178/diff/1/?file=396533#file396533line512> > > > > Please follow proper spacing and lining In RB it looks distorted because it cannot occupy the full horizontal width, I checked the spacing in py file, it is ok. > On Dec. 11, 2013, 6:04 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_advancedsg_networks.py, line 823 > > <https://reviews.apache.org/r/16178/diff/1/?file=396533#file396533line823> > > > > clean up with new way of list assertion Sure will do that. > On Dec. 11, 2013, 6:04 p.m., Santhosh Edukulla wrote: > > tools/marvin/marvin/integration/lib/common.py, line 778 > > <https://reviews.apache.org/r/16178/diff/1/?file=396537#file396537line778> > > > > Whats the significance of this loop? > > We are not taking any action based upon some matched\unmatched > > condition here. > > We are either continuing or breaking but no action as what to do. > > Little unclear here. Here we are storing the random vlan value in shared_ntwk_vlan, and if the value is present in already used vlans list, then we are going for another value (continue), and when we get such value which is not used by any of the existing network, we break the loop. I will change the code so that it is easier to understand. > On Dec. 11, 2013, 6:04 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_advancedsg_networks.py, line 340 > > <https://reviews.apache.org/r/16178/diff/1/?file=396533#file396533line340> > > > > Are we ok to raise for one deletion failure? we have other entities > > below for deletion. I think we should just print the cleanup failures thorough debug, and if one of the cleanup has failed, then let's fail the test case after all the cleanups are done. What do you think? - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16178/#review30203 ----------------------------------------------------------- On Dec. 11, 2013, 8:30 a.m., Ashutosh Kelkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16178/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2013, 8:30 a.m.) > > > Review request for cloudstack, Girish Shilamkar, Santhosh Edukulla, and > SrikanteswaraRao Talluri. > > > Bugs: CLOUDSTACK-2237 > https://issues.apache.org/jira/browse/CLOUDSTACK-2237 > > > Repository: cloudstack-git > > > Description > ------- > > Adding Automation tests for feature "Security Group Isolation in advanced > zone". > > @Santhosh: Please check the change in configGenerator file. Made changes to > take relative path. > > > Diffs > ----- > > test/integration/component/test_advancedsg_networks.py 4834351 > tools/marvin/marvin/config/config.cfg PRE-CREATION > tools/marvin/marvin/configGenerator.py 6d5b70d > tools/marvin/marvin/integration/lib/base.py 86f962a > tools/marvin/marvin/integration/lib/common.py 096b073 > > Diff: https://reviews.apache.org/r/16178/diff/ > > > Testing > ------- > > Tested locally on Advanced zone setup with security group enabled. > > > Thanks, > > Ashutosh Kelkar > >
