Change in vdsm[master]: mom: add mom balloon functional tests for running vms

gvallare at redhat.com gvallare at redhat.com
Wed Jun 12 20:03:43 UTC 2013


Giuseppe Vallarelli has posted comments on this change.

Change subject: mom: add mom balloon functional tests for running vms
......................................................................


Patch Set 6: (3 inline comments)

-1 Some questions need answers.

....................................................
File tests/functional/momTests.py
Line 41: 
Line 42:     def setUp(self):
Line 43:         self.s = vdscli.connect()
Line 44: 
Line 45:     def assertOK(self, result):
Hi Mei I've been working on a first patch for functional tests networking side (http://gerrit.ovirt.org/#/c/14840/19), perhaps you may find useful the VdsProxy class that I defined in the utils module. If you look at it you will find that vdsm verbs have been abstracted to save some boilerplate, you coded assertOK exactly for the same reason, to save yourself from unpacking verb result and message.

I wonder if it make sense for us to code little utilities, slightly different for every functional test module instead of having a uniform way to implement vdsm functional tests.
Line 46:         # code == 0 means OK
Line 47:         self.assertEquals(
Line 48:             result['status']['code'], 0, str(result))
Line 49: 


Line 154:             raise SkipTest('No VM can be candidate of ballooning operation.')
Line 155: 
Line 156:         self._setPolicy(operation)
Line 157:         # Wait for the policy taking effect
Line 158:         time.sleep(35)
Mei is there a reason for having in testKSM a sleep for 10 secs and here 35 ? You can consider the slowtest decoration within testValidation module for these kind of tests.
Line 159: 
Line 160:         self._checkResult(operation, vmsOldStats)
Line 161: 
Line 162:     @testValidation.ValidateRunningAsRoot


Line 162:     @testValidation.ValidateRunningAsRoot
Line 163:     def testBalloonShrink(self):
Line 164:         self._basicBalloon("Shrink")
Line 165: 
Line 166:     @testValidation.ValidateRunningAsRoot
I'm not a big fan of this style of testing. I can understand that this behaviour is complex but that's not a good reason to not identify the 3 stages of test: Arrange (you prepare all necessary preconditions and inputs), Act (you exercise the code under test), Assert (you verify the expected result) - you did in testKSM; also every function in this module contains some assertion, do all assertions have the same value for the behaviour under test ? I wonder if the needed code to test the balloon should be somewhere else because there's enough logic to create a test for the test (not your fault).
Line 167:     def testBalloonGrow(self):


-- 
To view, visit http://gerrit.ovirt.org/13156
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I922568233dc769d83e2fdffe1c24439d13d03d7e
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck at redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list