----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21628/#review43594 -----------------------------------------------------------
test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77881> Rename services to test data or some thing better. Its not an issue, but services seems to be little misnomer. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77882> Step1 repeated twice. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77860> We dont need this check, its verified at line 320 earlier. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77861> Move this clean up to above assert. Otherwise it will miss from cleanup. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77862> These 3 statements seems to be widely used. Can we move them to a common function? test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77867> May be get the count as it is for before vpc list, need not be zero? Especially if multiple tests run in parallel, there could be a possibility we may get the count varying. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77863> Can test name be changed based upon description, currently its very generic? test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77864> These line needs to be moved up. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77865> Similarly, move these verifications to a commonly used function. That way its easy to maintain. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77866> Why vpc_count_before is zero? test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77868> Move this to above line after vpc creation. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77870> Do we wanted to move this assert before cleaning up of network? test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77871> Lot of code is repetitive, may be more can be modularized. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77872> Test name related to vpc listing ? currently it just mentions normal network creation\deletion. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77873> Tags were commented. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77874> Move this line before cleanup. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77875> Why cant we move this line to after 814 itself? and then we dont require if else. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77876> Not clear with this check. Why cant we just append to cleanup? test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77877> Can this be moved after create? test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77878> Move this before cleanup. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77880> Why this check is required? Its take care during validateList itself. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77879> Move this line before cleanup. test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77883> Do we require some time to check whether vpc started or failed? test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77884> For delete we require some time to wait? test/integration/component/test_escalations_networks.py <https://reviews.apache.org/r/21628/#comment77885> Do we not require clean up for this creation? - Santhosh Edukulla On May 19, 2014, 11:36 a.m., Monis Majeed wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21628/ > ----------------------------------------------------------- > > (Updated May 19, 2014, 11:36 a.m.) > > > Review request for cloudstack and Santhosh Edukulla. > > > Bugs: CLOUDSTACK-6282 > https://issues.apache.org/jira/browse/CLOUDSTACK-6282 > > > Repository: cloudstack-git > > > Description > ------- > > Automated network API Tests and uploaded patch > > > Diffs > ----- > > test/integration/component/test_escalations_networks.py PRE-CREATION > tools/marvin/marvin/config/test_data.py 5577ae1 > tools/marvin/marvin/lib/base.py 0a6405d > > Diff: https://reviews.apache.org/r/21628/diff/ > > > Testing > ------- > > Executed all the Network tests and uploaded the results file > > > File Attachments > ---------------- > > results.txt > > https://reviews.apache.org/media/uploaded/files/2014/05/19/0574d454-3aae-4cf4-9c45-af8fc678b487__results.txt > > > Thanks, > > Monis Majeed > >
