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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Doron Fediuck <[email protected]> Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]> Gerrit-Reviewer: Mark Wu <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Mei Liu <[email protected]> Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
