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])


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Assaf Muller <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Dima Kuznetsov <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to