Change in vdsm[master]: add simple VM creation functional test

danken at redhat.com danken at redhat.com
Thu Aug 30 20:35:13 UTC 2012


Dan Kenigsberg has posted comments on this change.

Change subject: add simple VM creation functional test
......................................................................


Patch Set 6: I would prefer that you didn't submit this

(3 inline comments)

thanks for this rebase. would you review (and not only verify) my http://gerrit.ovirt.org/#/c/5788/3 ?

....................................................
File tests/functional/xmlrpcTests.py
Line 78:         # code == 0 means OK
Line 79:         self.assertEquals(vdsResult['status']['code'], 0)
Line 80: 
Line 81:     def testStartEmptyVM(self):
Line 82:         VMID = '66666666-ffff-4444-bbbb-333333333333'
I've liked this whitespace. why kill it?
Line 83:         r = self.s.create({'memSize': '100', 'display': 'vnc', 'vmId': VMID,
Line 84:                            'vmName': 'foo'})
Line 85:         self.assertVdsOK(r)
Line 86:         try:


Line 91:             self.assertVdsOK(r)
Line 92: 
Line 93:     # This test relies a initramfs in /boot, so it is marked as "brokentest"
Line 94:     # until we add support for those distros with initrd in /boot.
Line 95:     @brokentest
I know that I suggested brokentest, but it would hide real Fedora failures, too. Better raise SkipTest if initramfsPath is not found.
Line 96:     def testStartSmallVM(self):
Line 97:         VMID = '77777777-ffff-3333-bbbb-222222222222'
Line 98:         kernelVer = os.uname()[2]
Line 99:         kernelPath = "/boot/vmlinuz-" + kernelVer


Line 121:             self.retryAssert(assertVMAndGuestUp, 65, 1)
Line 122:         except Exception:
Line 123:             # If the guest OS gets stuck when boot, or other exception raised,
Line 124:             # destroy the VM anyway to release the resources.
Line 125:             self.s.destroy(VMID)
I think it would be nicer to put

 r = self.s.destroy(VMID)

in a "finally" block, and

 self.assertVdsOK(r)

in an external block. Then, you would not need to write self.s.destroy(VMID) twice, and the comments won't be really needed.
Line 126:             raise
Line 127:         else:
Line 128:             # guest OS boot OK, assert we destroy the machine properly


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb0d86ce20a547ef809d5407fe12d6ade474c4d2
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Deepak C Shetty <deepakcs at linux.vnet.ibm.com>
Gerrit-Reviewer: Gal Hammer <ghammer at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef 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