Change in vdsm[master]: Supervdsm tests: Fail tests When supervdsm does not start as...

danken at redhat.com danken at redhat.com
Sun Apr 14 09:55:41 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: Supervdsm tests: Fail tests When supervdsm does not start as expected
......................................................................


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

(3 inline comments)

thanks for quickly addressing this issue.

....................................................
File tests/superVdsmTests.py
Line 30:                     self._authkey, str(os.getpid()),
Line 31:                     self.pidfile, self.timestamp, self.address,
Line 32:                     str(os.getuid())]
Line 33:     p = misc.execCmd(superVdsmCmd, sync=False, sudo=True)
Line 34:     p.wait(3)
Any reason 3 seconds is better than 2 here?
Line 35:     if p.returncode:
Line 36:         raise ("supervdsm is corrupted, executing supervdsmCmd failed")
Line 37: 
Line 38: 


Line 32:                     str(os.getuid())]
Line 33:     p = misc.execCmd(superVdsmCmd, sync=False, sudo=True)
Line 34:     p.wait(3)
Line 35:     if p.returncode:
Line 36:         raise ("supervdsm is corrupted, executing supervdsmCmd failed")
Raising a string is deprecated. Please use an Exception object.

The string is a bit two dramatic - I'd prefer a plain "executing supervdsmCmd failed", with as much as you have from p.stderr.
Line 37: 
Line 38: 
Line 39: class TestSuperVdsm(TestCaseBase):
Line 40:     def setUp(self):


Line 57:     def pingStart(self):
Line 58:         try:
Line 59:             return self._proxy.ping()  # this call initiate svdsm
Line 60:         except:
Line 61:             # starting svdsm raises exception
If you want to convert all exceptions to AssertionError, just raise the relevant AssertionError here.
Line 62:             return False
Line 63: 
Line 64:     @MonkeyPatch(supervdsm.SuperVdsmProxy, '_start', monkeyStart)
Line 65:     def testIsSuperUp(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I33a5564a8e67d70087fef83ae651e8729bdb59a5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list