Change in vdsm[master]: The return of super vdsm tests

danken at redhat.com danken at redhat.com
Wed Nov 21 19:39:59 UTC 2012


Dan Kenigsberg has posted comments on this change.

Change subject: The return of super vdsm tests
......................................................................


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

(4 inline comments)

....................................................
File tests/superVdsmTests.py
Line 17:     base = os.path.dirname(testDir)
Line 18:     vdsmPath = os.path.join(base, 'vdsm')
Line 19:     cliPath = os.path.join(base, 'vdsm_cli')
Line 20: 
Line 21:     path = "PYTHONPATH=$PYTHONPATH:%s:%s:%s" % (base, vdsmPath, cliPath)
this would work by half chance.. if I have an old copy of vdscli.py lying vdsmPath it would be imported instead of the correct one.

not so pretty, but I can make it slide for tests.

But wouldn't it be simpler to add an optional arg to supervdsm._start(), say extraPythonPath, and provide it with values here?

Thus you could avoid re-writing _start here.

P.S. it would be great if you can make supervdsm test run as a normal user.
Line 22: 
Line 23:     superVdsmCmd = [path,
Line 24:                     constants.EXT_PYTHON, supervdsm.SUPERVDSM,
Line 25:                     self._authkey, str(os.getpid()),


Line 30: 
Line 31: class TestSuperVdsm(TestCaseBase):
Line 32: 
Line 33:     def setUp(self):
Line 34:         #supervdsm.SuperVdsmProxy._start = monkeyStart
why is this commented out? you are not starting a new supervdsm for each test method
Line 35:         testValidation.checkSudo(['python', supervdsm.SUPERVDSM])
Line 36:         self._proxy = supervdsm.getProxy()
Line 37: 
Line 38:         # temporary values to run temporary svdsm


Line 31: class TestSuperVdsm(TestCaseBase):
Line 32: 
Line 33:     def setUp(self):
Line 34:         #supervdsm.SuperVdsmProxy._start = monkeyStart
Line 35:         testValidation.checkSudo(['python', supervdsm.SUPERVDSM])
nitpick: you now need to check the permission of setting PYTHONPATH in the commandline
Line 36:         self._proxy = supervdsm.getProxy()
Line 37: 
Line 38:         # temporary values to run temporary svdsm
Line 39:         self.pidfd, pidfile = tempfile.mkstemp()


....................................................
File vdsm/supervdsm.py
Line 34: _g_singletonSupervdsmInstance = None
Line 35: _g_singletonSupervdsmInstance_lock = threading.Lock()
Line 36: 
Line 37: 
Line 38: def foo():
forgotten debug code?
Line 39:     print 'a'
Line 40: 
Line 41: 
Line 42: def __supervdsmServerPath(serverFile):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52e935662633601ce85150171c37fbf7c6196c15
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list