Change in vdsm[master]: tests: new functional tests for vdsm storage
nsoffer at redhat.com
nsoffer at redhat.com
Thu Sep 4 17:31:49 UTC 2014
Nir Soffer has posted comments on this change.
Change subject: tests: new functional tests for vdsm storage
......................................................................
Patch Set 2:
(7 comments)
Some more comments.
http://gerrit.ovirt.org/#/c/32496/2/tests/functional/testlib/controlvdsm.py
File tests/functional/testlib/controlvdsm.py:
Line 2: import logging
Line 3: import vdsm.vdscli
Line 4: import socket
Line 5: import vdsm.config
Line 6: import time
Use this scheme for imports:
stdlib imports
vdsm imports
testlib imports
And keep imports sorted in each group.
Line 7:
Line 8: class ControlVDSM:
Line 9: def cleanup(self):
Line 10: self._stopService()
Line 4: import socket
Line 5: import vdsm.config
Line 6: import time
Line 7:
Line 8: class ControlVDSM:
Please rename the module to vdsm and this class to Controller:
Then we will use it as:
from . import vdsm.Controller
Line 9: def cleanup(self):
Line 10: self._stopService()
Line 11: assert not self._serviceRunning()
Line 12: self._brutallyCleanFiles()
Line 22: vdsmClient.getStorageDomainsList()
Line 23: logging.info('connection to VDSM succeeded')
Line 24: return
Line 25: except socket.error as e:
Line 26: logging.warning('could not talk to VDSM: %s' % e)
I think we just talked to vdsm, but it would not listen :-)
"Error communicating with vdsm"?
Line 27: time.sleep(1)
Line 28:
Line 29: raise Exception('could not connect to VDSM')
Line 30:
Line 25: except socket.error as e:
Line 26: logging.warning('could not talk to VDSM: %s' % e)
Line 27: time.sleep(1)
Line 28:
Line 29: raise Exception('could not connect to VDSM')
It this a test error or failure?
Line 30:
Line 31: def _stopService(self):
Line 32: self._run("sudo service vdsmd stop")
Line 33:
Line 28:
Line 29: raise Exception('could not connect to VDSM')
Line 30:
Line 31: def _stopService(self):
Line 32: self._run("sudo service vdsmd stop")
Don't use strings for running command, only lists. Don't let Python parse your command line, specify the arguments to exec.
Line 33:
Line 34: def _serviceRunning(self):
Line 35: returnCode = subprocess.call('sudo service vdsmd status', shell=True, stdout=open('/dev/null','w'), stderr=open('/dev/null','w'))
Line 36: logging.info('vdsm running: %s' % (returnCode == 0))
Line 31: def _stopService(self):
Line 32: self._run("sudo service vdsmd stop")
Line 33:
Line 34: def _serviceRunning(self):
Line 35: returnCode = subprocess.call('sudo service vdsmd status', shell=True, stdout=open('/dev/null','w'), stderr=open('/dev/null','w'))
Never use shell=True.
Line 36: logging.info('vdsm running: %s' % (returnCode == 0))
Line 37: return returnCode == 0
Line 38:
Line 39: def _restartService(self):
http://gerrit.ovirt.org/#/c/32496/2/tests/run_functional_storage_tests.sh
File tests/run_functional_storage_tests.sh:
Line 5: fi
Line 6: if [[ "$1" = "--verbose" ]]; then
Line 7: verbose="--nologcapture"
Line 8: fi
Line 9: sudo PYTHONPATH=../lib:../vdsm:functional nosetests -s $verbose functional/basicStorageTest.py
Please remove this wrapper, and do the needed setup in the test module.
We should run these tests with:
nosetests funcional/storage.py
This script does not allow running the tests with additional options to the nosetests module. This solution is bad and we should not repeat this bad idea in new tests.
--
To view, visit http://gerrit.ovirt.org/32496
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1703e7c1dc223ff707775865cd14c7dd62314caf
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yoav Kleinberger <ykleinbe at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vered Volansky <vvolansk at redhat.com>
Gerrit-Reviewer: Yoav Kleinberger <ykleinbe at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list