> On Feb. 12, 2014, 10:09 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_cpu_limits.py, line 580 > > <https://reviews.apache.org/r/18016/diff/1/?file=483529#file483529line580> > > > > Do we need to skip the test or fail it?
It's not a failure since the setup is not equipped with required number of suitable hosts. So I think we should skip instead of failing. And after each build we can look at the skipped tests to know why they were skipped. > On Feb. 12, 2014, 10:09 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_cpu_project_limits.py, line 294 > > <https://reviews.apache.org/r/18016/diff/1/?file=483530#file483530line294> > > > > Keep the function find_suitable_host, and use it test modules. Use the > > new api mentioned i.e., listForMigration there in that function and > > abstract that to test cases. This way, test modules dont change often. Yes I thought so, but changed because anyway all modules needed change because the fail/skip step was not handled anywhere. I will abstract out the function as you said, and will also add the fail/skip step in each test module according to result obtained from the function because we don't want to just raise exception from common library, that won't ensure graceful cleanup. Your thoughts? > On Feb. 12, 2014, 10:09 p.m., Santhosh Edukulla wrote: > > test/integration/component/test_memory_limits.py, line 254 > > <https://reviews.apache.org/r/18016/diff/1/?file=483531#file483531line254> > > > > Why modify all test suites? The better thing may be to use and abstract > > the listForMigration inside the common library and so with this no tests > > get changed? As I said, all modules will need one line additional step, but for future test cases function abstraction will be beneficial. - Gaurav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18016/#review34291 ----------------------------------------------------------- On Feb. 12, 2014, 9:39 p.m., Gaurav Aradhye wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18016/ > ----------------------------------------------------------- > > (Updated Feb. 12, 2014, 9:39 p.m.) > > > Review request for cloudstack, Girish Shilamkar and Santhosh Edukulla. > > > Bugs: CLOUDSTACK-5626 > https://issues.apache.org/jira/browse/CLOUDSTACK-5626 > > > Repository: cloudstack-git > > > Description > ------- > > Removing different ways of obtaining suitable hosts. Used built in API > "findHostsForMigration" and skipped the test if suitable host is not found > instead of throwing assertion error. > Removed unnecessary common function. > > > Diffs > ----- > > test/integration/component/test_cpu_domain_limits.py c427e4f > test/integration/component/test_cpu_limits.py bdf2869 > test/integration/component/test_cpu_project_limits.py a8a1b3c > test/integration/component/test_memory_limits.py 7921e4b > test/integration/component/test_mm_domain_limits.py 68660c1 > test/integration/component/test_mm_project_limits.py c314011 > test/integration/component/test_vpc_vm_life_cycle.py 01373ac > tools/marvin/marvin/integration/lib/common.py e202391 > > Diff: https://reviews.apache.org/r/18016/diff/ > > > Testing > ------- > > Yes. > > > Thanks, > > Gaurav Aradhye > >
