> On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_persistent_networks.py, line 670 > > <https://reviews.apache.org/r/17702/diff/1/?file=468548#file468548line670> > > > > This step is not visible in test code, is it like enable\disable flag > > to create network? or offeringid we are passing?
The network offering is created according to the value passed to the test case (LB through VR or LB through Netscaler) and isolated persistent network is created in the next step using that network offering. Please check line 560 (Network creation). > On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_persistent_networks.py, line 304 > > <https://reviews.apache.org/r/17702/diff/1/?file=468548#file468548line304> > > > > Not clear with break here, what to do post break. In the tests, we are > > not checking these cases of return? We are checking if the VM is expunged or not. When the vm list is empty, that means vm is expunged hence we are breaking and returning. But yes, it is not clearly understandable, I will modify the code to make it lucid. > On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_persistent_networks.py, line 690 > > <https://reviews.apache.org/r/17702/diff/1/?file=468548#file468548line690> > > > > Please empty the clean up list after every case in tear down post after > > cleanup Done > On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_persistent_networks.py, line 708 > > <https://reviews.apache.org/r/17702/diff/1/?file=468548#file468548line708> > > > > We dont check the return of function here. The function checks if router is accessible or not. We are not returning anything. If router is not accessible, assertion error will be thrown. > On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_persistent_networks.py, line 741 > > <https://reviews.apache.org/r/17702/diff/1/?file=468548#file468548line741> > > > > This step is not visible in steps though. Network is created in the test case. Adding comment to make it more readable. > On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_persistent_networks.py, line 818 > > <https://reviews.apache.org/r/17702/diff/1/?file=468548#file468548line818> > > > > Lot of places this code is repeated, may be we can use a small utility > > function to keep it simple. Done > On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_persistent_networks.py, line 848 > > <https://reviews.apache.org/r/17702/diff/1/?file=468548#file468548line848> > > > > why this code is used two times before and after router list? It is checking the router accessibility for two different networks. Same variable was used for both the lists that's why confusion. I have changed the names so that it is clear. > On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_persistent_networks.py, line 1072 > > <https://reviews.apache.org/r/17702/diff/1/?file=468548#file468548line1072> > > > > If it fails here, account wont be deleted? Dont we add to clean up in > > case if it fails and during teardown account gets cleaned up? Account is already added in the cleanup list. > On Feb. 12, 2014, 5:23 p.m., Santhosh Edukulla wrote: > > tools/marvin/marvin/integration/lib/base.py, line 125 > > <https://reviews.apache.org/r/17702/diff/1/?file=468550#file468550line125> > > > > May be we can control the behavior or true\false using one function > > only. We don't need both disable and lock i believe I created two functions just to keep it simple, without adding flags in the same function. So that it is clear what operation we are performing. We can club into same function but then the name of the function will not be that clear as exactly what operation it is performing. - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17702/#review34292 ----------------------------------------------------------- On Feb. 4, 2014, 12:08 p.m., Ashutosh Kelkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17702/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2014, 12:08 p.m.) > > > Review request for cloudstack, Girish Shilamkar and Santhosh Edukulla. > > > Bugs: CLOUDSTACK-2232 > https://issues.apache.org/jira/browse/CLOUDSTACK-2232 > > > Repository: cloudstack-git > > > Description > ------- > > Adding automation test cases for Persistent Networks feature. > > > Diffs > ----- > > test/integration/component/test_persistent_networks.py f61ccaa > tools/marvin/marvin/config/config.cfg 5849fe8 > tools/marvin/marvin/integration/lib/base.py 409530c > > Diff: https://reviews.apache.org/r/17702/diff/ > > > Testing > ------- > > > Thanks, > > Ashutosh Kelkar > >
