Change in vdsm[master]: Initial commit for vdsm-tool testing infrastructure

asegurap at redhat.com asegurap at redhat.com
Mon Mar 17 09:06:02 UTC 2014


Antoni Segura Puimedon has posted comments on this change.

Change subject: Initial commit for vdsm-tool testing infrastructure
......................................................................


Patch Set 6:

(3 comments)

very minor things. Otherwise it looks good for a first version. I'd like to see a follow up patch that does not write to the .conf files so that errors will not carry over.

http://gerrit.ovirt.org/#/c/25263/6/lib/vdsm/tool/libvirt_configure.sh.in
File lib/vdsm/tool/libvirt_configure.sh.in:

Line 27: ENABLE_LIBVIRT_SANLOCK="@ENABLE_LIBVIRT_SANLOCK@"
Line 28: ENABLE_LIBVIRT_SELINUX="@ENABLE_LIBVIRT_SELINUX@"
Line 29: 
Line 30: QEMU_DUMP_PATH="/var/log/core"
Line 31: # prefer env vars if exist. otherwise take defaults (mostley for testings)
s/mostley/mostly/
s/testings/testing/
Line 32: if [ "${VDSM_CONF_FILE}" = '' ]; then
Line 33:     VDSM_CONF_FILE="@CONFDIR@/vdsm.conf"
Line 34: fi
Line 35: if [ "${GETCONFITEM}" = '' ]; then


http://gerrit.ovirt.org/#/c/25263/6/lib/vdsm/tool/tests.py
File lib/vdsm/tool/tests.py:

Line 46: class configuratorTests(TestCase):
Line 47:     def setUp(self):
Line 48:         pass
Line 49: 
Line 50:     def setSslTrueConfig(self):
I'd rather have _setSslTrueConfig
Same for the one under. I know that nose will not call them, but this way it is easier to see that they are not used out of the class.
Line 51:         with open(test_conf_files['VDSM_CONF_FILE'], 'w') as f:
Line 52:             f.write(sample_config['withssl'])
Line 53: 
Line 54:     def setSslFalseConfig(self):


Line 54:     def setSslFalseConfig(self):
Line 55:         with open(test_conf_files['VDSM_CONF_FILE'], 'w') as f:
Line 56:             f.write(sample_config['withnossl'])
Line 57: 
Line 58:     def testInValidateiSSLConfig(self):
what is the 'i' between Validate and SSL?
Line 59:         self.setSslTrueConfig()
Line 60:         lconf = configurator.LibvirtModuleConfigure(test_conf_files)
Line 61:         self.assertTrue(lconf.validate())
Line 62: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae0fec9b2057c5ba932f38ed5d34d654486043ef
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list