Dima Kuznetsov has posted comments on this change.
Change subject: vdsm-tool: Change upgrade mechanism ......................................................................
Patch Set 9:
(4 comments)
http://gerrit.ovirt.org/#/c/27193/9/tests/toolTests.py File tests/toolTests.py:
Line 370: def assertNotSealed(self, name): Line 371: self.assertFalse(self._checkSealExists(name)) Line 372: Line 373: def testRunOnce(self): Line 374: lst = [0]
According to the principle of least surprise, I think you should have the n
Ok, will change. Line 375: ret = upgrade.apply_upgrade(self.UpgraduratorTM(lst), 'test') Line 376: self.assertEquals(ret, 0) Line 377: self.assertEquals(lst[0], 1) Line 378: self.assertSealed('test')
Line 380: def testErrorInUpgrade(self): Line 381: bad = self.BadUpgraduratorTM() Line 382: ret = upgrade.apply_upgrade(bad, 'foobar') Line 383: self.assertEquals(ret, 1) Line 384: self.assertNotSealed('bad')
'bad' will never be sealed because the upgrade name is 'foobar'.
Upgrade name defined as BadUpgraduratorTM.name which is 'bad'.
The 1st argument passed is not used for name to avoid coupling with @exposed command name (in case you want to expose a single command under several names) Line 385: Line 386: def testRunMany(self): Line 387: lst = [0] Line 388: for _ in xrange(5):
Line 388: xrange
xrange is not defined in Python 3. Are you guys using a library like six to
Will change to range()
Line 404: upgrade.apply_upgrade(self.UpgraduratorTM(lst), 'test') Line 405: self.assertEquals(lst[0], 2) Line 406: self.assertSealed('test') Line 407: Line 408: def testNoUpgrade(self):
I think you can safely remove this test...
Ok Line 409: self.assertNotSealed('test') Line 410: Line 411: def testUpgradeArgs(self): Line 412: u = self.UpgraduratorTM([0])