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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Deepak C Shetty <[email protected]> Gerrit-Reviewer: Gal Hammer <[email protected]> Gerrit-Reviewer: Mark Wu <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Royce Lv <[email protected]> Gerrit-Reviewer: ShaoHe Feng <[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
