Commit bd39b6bb authored by Thomas Thrainer's avatar Thomas Thrainer

Enable pylint and PEP8 for test support code

The test support code is mainly written from scratch, so in order to
ensure to keep the code quality high, enable pylint and PEP8 checks
for it.

Due to some specialities of the test code, a dedicate pylintrc-test file
is created which configures pylint for test code.

Those differences include:
 - lowercase functions are allowed if the first word is test, assert or
   main
 - lowercase methods are allowed if the first word(s) is test, assert,
   runTests, setUp or tearDown
 - R0201 (method could be a function) is disabled
Signed-off-by: default avatarThomas Thrainer <thomasth@google.com>
Reviewed-by: default avatarMichele Tartara <mtartara@google.com>
parent 3efa7659
...@@ -991,6 +991,7 @@ EXTRA_DIST = \ ...@@ -991,6 +991,7 @@ EXTRA_DIST = \
UPGRADE \ UPGRADE \
epydoc.conf.in \ epydoc.conf.in \
pylintrc \ pylintrc \
pylintrc-test \
autotools/build-bash-completion \ autotools/build-bash-completion \
autotools/build-rpc \ autotools/build-rpc \
autotools/check-header \ autotools/check-header \
...@@ -1451,7 +1452,8 @@ pep8_python_code = \ ...@@ -1451,7 +1452,8 @@ pep8_python_code = \
$(CHECK_HEADER) \ $(CHECK_HEADER) \
$(DOCPP) \ $(DOCPP) \
$(PYTHON_BOOTSTRAP) \ $(PYTHON_BOOTSTRAP) \
qa qa \
$(python_test_support)
test/py/daemon-util_unittest.bash: daemons/daemon-util test/py/daemon-util_unittest.bash: daemons/daemon-util
...@@ -1931,7 +1933,7 @@ PEP8_IGNORE = E111,E121,E125,E127,E261,E501 ...@@ -1931,7 +1933,7 @@ PEP8_IGNORE = E111,E121,E125,E127,E261,E501
# For excluding pep8 expects filenames only, not whole paths # For excluding pep8 expects filenames only, not whole paths
PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip $(notdir $(BUILT_PYTHON_SOURCES)))) PEP8_EXCLUDE = $(subst $(space),$(comma),$(strip $(notdir $(BUILT_PYTHON_SOURCES))))
LINT_TARGETS = pylint pylint-qa LINT_TARGETS = pylint pylint-qa pylint-test
if HAS_PEP8 if HAS_PEP8
LINT_TARGETS += pep8 LINT_TARGETS += pep8
endif endif
...@@ -1953,6 +1955,12 @@ pylint-qa: $(GENERATED_FILES) ...@@ -1953,6 +1955,12 @@ pylint-qa: $(GENERATED_FILES)
cd $(top_srcdir)/qa && \ cd $(top_srcdir)/qa && \
PYTHONPATH=$(abs_top_srcdir) $(PYLINT) $(LINT_OPTS) \ PYTHONPATH=$(abs_top_srcdir) $(PYLINT) $(LINT_OPTS) \
--rcfile ../pylintrc $(patsubst qa/%.py,%,$(qa_scripts)) --rcfile ../pylintrc $(patsubst qa/%.py,%,$(qa_scripts))
# FIXME: lint all test code, not just the newly added test support
pylint-test: $(GENERATED_FILES)
@test -n "$(PYLINT)" || { echo 'pylint' not found during configure; exit 1; }
cd $(top_srcdir) && \
PYTHONPATH=.:./test/py $(PYLINT) $(LINT_OPTS) \
--rcfile=pylintrc-test $(python_test_support)
.PHONY: pep8 .PHONY: pep8
pep8: $(GENERATED_FILES) pep8: $(GENERATED_FILES)
......
# Configuration file for pylint (http://www.logilab.org/project/pylint). See # Configuration file for pylint (http://www.logilab.org/project/pylint). See
# http://www.logilab.org/card/pylintfeatures for more detailed variable # http://www.logilab.org/card/pylintfeatures for more detailed variable
# descriptions. # descriptions.
#
# NOTE: Keep this file in sync (as much as possible) with pylintrc-test!
[MASTER] [MASTER]
profile = no profile = no
......
# pylint configuration file tailored to test code.
#
# NOTE: Keep in sync as much as possible with the standard pylintrc file.
# Only a few settings had to be adapted for the test code.
[MASTER]
profile = no
ignore =
persistent = no
cache-size = 50000
load-plugins =
[REPORTS]
output-format = colorized
include-ids = yes
files-output = no
reports = no
evaluation = 10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10)
comment = yes
[BASIC]
required-attributes =
# disabling docstring checks since we have way too many without (complex
# inheritance hierarchies)
#no-docstring-rgx = __.*__
no-docstring-rgx = .*
module-rgx = (([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
# added lower-case names
const-rgx = ((_{0,2}[A-Za-z][A-Za-z0-9_]*)|(__.*__))$
class-rgx = _?[A-Z][a-zA-Z0-9]+$
# added lower-case names
function-rgx = (_?((([A-Z]+[a-z0-9]+)|test|assert)([A-Z]+[a-z0-9]*)*)|main|([a-z_][a-z0-9_]*))$
# add lower-case names, since derived classes must obey method names
method-rgx = (_{0,2}(([A-Z]+[a-z0-9]+)|test|assert)([A-Z]+[a-z0-9]*)*|__.*__|runTests|setUp|tearDown|([a-z_][a-z0-9_]*))$
attr-rgx = [a-z_][a-z0-9_]{1,30}$
argument-rgx = [a-z_][a-z0-9_]*$
variable-rgx = (_?([a-z_][a-z0-9_]*)|(_?[A-Z0-9_]+))$
inlinevar-rgx = [A-Za-z_][A-Za-z0-9_]*$
good-names = i,j,k,_
bad-names = foo,bar,baz,toto,tutu,tata
bad-functions = xrange
[TYPECHECK]
ignore-mixin-members = yes
zope = no
acquired-members =
[VARIABLES]
init-import = no
dummy-variables-rgx = _
additional-builtins =
[CLASSES]
ignore-iface-methods =
defining-attr-methods = __init__,__new__,setUp
[DESIGN]
max-args = 15
max-locals = 50
max-returns = 10
max-branchs = 80
max-statements = 200
max-parents = 7
max-attributes = 20
# zero as struct-like (PODS) classes don't export any methods
min-public-methods = 0
max-public-methods = 50
[IMPORTS]
deprecated-modules = regsub,string,TERMIOS,Bastion,rexec
import-graph =
ext-import-graph =
int-import-graph =
[FORMAT]
max-line-length = 80
max-module-lines = 4500
indent-string = " "
[MISCELLANEOUS]
notes = FIXME,XXX,TODO
[SIMILARITIES]
min-similarity-lines = 4
ignore-comments = yes
ignore-docstrings = yes
[MESSAGES CONTROL]
# Enable only checker(s) with the given id(s). This option conflicts with the
# disable-checker option
#enable-checker=
# Enable all checker(s) except those with the given id(s). This option
# conflicts with the enable-checker option
#disable-checker=
disable-checker=similarities
# Enable all messages in the listed categories (IRCWEF).
#enable-msg-cat=
# Disable all messages in the listed categories (IRCWEF).
disable-msg-cat=
# Enable the message(s) with the given id(s).
#enable-msg=
# Disable the message(s) with the given id(s).
disable-msg=W0511,R0922,W0201
# The new pylint 0.21+ style (plus the similarities checker, which is no longer
# a separate opiton, but a generic disable control)
disable=W0511,R0922,W0201,R0922,R0801,I0011,R0201
...@@ -18,4 +18,4 @@ ...@@ -18,4 +18,4 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
# 02110-1301, USA. # 02110-1301, USA.
"""This module contains all python test code""" """This module contains all python test code"""
\ No newline at end of file
...@@ -18,4 +18,4 @@ ...@@ -18,4 +18,4 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
# 02110-1301, USA. # 02110-1301, USA.
"""This module contains all cmdlib unit tests""" """This module contains all cmdlib unit tests"""
\ No newline at end of file
...@@ -19,9 +19,7 @@ ...@@ -19,9 +19,7 @@
# 02110-1301, USA. # 02110-1301, USA.
"""Tests for LUTest* """Tests for LUTest*"""
"""
from ganeti import constants from ganeti import constants
......
...@@ -23,12 +23,13 @@ ...@@ -23,12 +23,13 @@
""" """
from cmdlib_testcase import CmdlibTestCase from cmdlib.testsupport.cmdlib_testcase import CmdlibTestCase
from config_mock import ConfigMock from cmdlib.testsupport.config_mock import ConfigMock
from iallocator_mock import CreateIAllocatorMock from cmdlib.testsupport.iallocator_mock import CreateIAllocatorMock
from lock_manager_mock import LockManagerMock from cmdlib.testsupport.lock_manager_mock import LockManagerMock
from processor_mock import ProcessorMock from cmdlib.testsupport.processor_mock import ProcessorMock
from rpc_runner_mock import CreateRpcRunnerMock, RpcResultsBuilder from cmdlib.testsupport.rpc_runner_mock import CreateRpcRunnerMock, \
RpcResultsBuilder
__all__ = ["CmdlibTestCase", __all__ = ["CmdlibTestCase",
"ConfigMock", "ConfigMock",
...@@ -37,4 +38,4 @@ __all__ = ["CmdlibTestCase", ...@@ -37,4 +38,4 @@ __all__ = ["CmdlibTestCase",
"LockManagerMock", "LockManagerMock",
"ProcessorMock", "ProcessorMock",
"RpcResultsBuilder", "RpcResultsBuilder",
] ]
\ No newline at end of file
...@@ -18,11 +18,15 @@ ...@@ -18,11 +18,15 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
# 02110-1301, USA. # 02110-1301, USA.
from config_mock import *
from iallocator_mock import * """Main module of the cmdlib test framework"""
from lock_manager_mock import *
from processor_mock import *
from rpc_runner_mock import * from cmdlib.testsupport.config_mock import ConfigMock
from cmdlib.testsupport.iallocator_mock import CreateIAllocatorMock
from cmdlib.testsupport.lock_manager_mock import LockManagerMock
from cmdlib.testsupport.processor_mock import ProcessorMock
from cmdlib.testsupport.rpc_runner_mock import CreateRpcRunnerMock
import testutils import testutils
...@@ -34,6 +38,7 @@ class GanetiContextMock(object): ...@@ -34,6 +38,7 @@ class GanetiContextMock(object):
self.rpc = rpc self.rpc = rpc
# pylint: disable=R0904
class CmdlibTestCase(testutils.GanetiTestCase): class CmdlibTestCase(testutils.GanetiTestCase):
"""Base class for cmdlib tests. """Base class for cmdlib tests.
...@@ -83,3 +88,4 @@ class CmdlibTestCase(testutils.GanetiTestCase): ...@@ -83,3 +88,4 @@ class CmdlibTestCase(testutils.GanetiTestCase):
""" """
self.mcpu.assertLogContainsRegex(expected_regex) self.mcpu.assertLogContainsRegex(expected_regex)
...@@ -18,7 +18,11 @@ ...@@ -18,7 +18,11 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
# 02110-1301, USA. # 02110-1301, USA.
import uuid
"""Support for mocking the cluster configuration"""
import uuid as uuid_module
from ganeti import config from ganeti import config
from ganeti import constants from ganeti import constants
...@@ -31,6 +35,7 @@ def _StubGetEntResolver(): ...@@ -31,6 +35,7 @@ def _StubGetEntResolver():
return mocks.FakeGetentResolver() return mocks.FakeGetentResolver()
# pylint: disable=R0904
class ConfigMock(config.ConfigWriter): class ConfigMock(config.ConfigWriter):
"""A mocked cluster configuration with added methods for easy customization. """A mocked cluster configuration with added methods for easy customization.
...@@ -45,7 +50,7 @@ class ConfigMock(config.ConfigWriter): ...@@ -45,7 +50,7 @@ class ConfigMock(config.ConfigWriter):
_getents=_StubGetEntResolver()) _getents=_StubGetEntResolver())
def _GetUuid(self): def _GetUuid(self):
return str(uuid.uuid4()) return str(uuid_module.uuid4())
def AddNewNodeGroup(self, def AddNewNodeGroup(self,
uuid=None, uuid=None,
...@@ -56,7 +61,7 @@ class ConfigMock(config.ConfigWriter): ...@@ -56,7 +61,7 @@ class ConfigMock(config.ConfigWriter):
hv_state_static=None, hv_state_static=None,
disk_state_static=None, disk_state_static=None,
alloc_policy=None, alloc_policy=None,
networks=[]): networks=None):
"""Add a new L{objects.NodeGroup} to the cluster configuration """Add a new L{objects.NodeGroup} to the cluster configuration
See L{objects.NodeGroup} for parameter documentation. See L{objects.NodeGroup} for parameter documentation.
...@@ -72,6 +77,8 @@ class ConfigMock(config.ConfigWriter): ...@@ -72,6 +77,8 @@ class ConfigMock(config.ConfigWriter):
uuid = self._GetUuid() uuid = self._GetUuid()
if name is None: if name is None:
name = "mock_group_%d" % group_id name = "mock_group_%d" % group_id
if networks is None:
networks = []
group = objects.NodeGroup(uuid=uuid, group = objects.NodeGroup(uuid=uuid,
name=name, name=name,
...@@ -87,6 +94,7 @@ class ConfigMock(config.ConfigWriter): ...@@ -87,6 +94,7 @@ class ConfigMock(config.ConfigWriter):
self.AddNodeGroup(group, None) self.AddNodeGroup(group, None)
return group return group
# pylint: disable=R0913
def AddNewNode(self, def AddNewNode(self,
uuid=None, uuid=None,
name=None, name=None,
...@@ -154,12 +162,12 @@ class ConfigMock(config.ConfigWriter): ...@@ -154,12 +162,12 @@ class ConfigMock(config.ConfigWriter):
primary_node=None, primary_node=None,
os="mocked_os", os="mocked_os",
hypervisor=constants.HT_FAKE, hypervisor=constants.HT_FAKE,
hvparams={}, hvparams=None,
beparams={}, beparams=None,
osparams={}, osparams=None,
admin_state=constants.ADMINST_DOWN, admin_state=constants.ADMINST_DOWN,
nics=[], nics=None,
disks=[], disks=None,
disk_template=constants.DT_DISKLESS, disk_template=constants.DT_DISKLESS,
disks_active=False, disks_active=False,
network_port=None): network_port=None):
...@@ -182,6 +190,16 @@ class ConfigMock(config.ConfigWriter): ...@@ -182,6 +190,16 @@ class ConfigMock(config.ConfigWriter):
primary_node = self._master_node.uuid primary_node = self._master_node.uuid
if isinstance(primary_node, objects.Node): if isinstance(primary_node, objects.Node):
primary_node = self._master_node.uuid primary_node = self._master_node.uuid
if hvparams is None:
hvparams = {}
if beparams is None:
beparams = {}
if osparams is None:
osparams = {}
if nics is None:
nics = []
if disks is None:
disks = []
inst = objects.Instance(uuid=uuid, inst = objects.Instance(uuid=uuid,
name=name, name=name,
...@@ -241,4 +259,4 @@ class ConfigMock(config.ConfigWriter): ...@@ -241,4 +259,4 @@ class ConfigMock(config.ConfigWriter):
pass pass
def _GetRpc(self, address_list): def _GetRpc(self, address_list):
raise NotImplementedError raise AssertionError("This should not be used during tests!")
...@@ -18,6 +18,10 @@ ...@@ -18,6 +18,10 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
# 02110-1301, USA. # 02110-1301, USA.
"""Support for mocking the IAllocator interface"""
import mock import mock
from ganeti.masterd import iallocator from ganeti.masterd import iallocator
......
...@@ -18,6 +18,10 @@ ...@@ -18,6 +18,10 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
# 02110-1301, USA. # 02110-1301, USA.
"""Support for mocking the lock manager"""
from ganeti import locking from ganeti import locking
......
...@@ -18,6 +18,10 @@ ...@@ -18,6 +18,10 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
# 02110-1301, USA. # 02110-1301, USA.
"""Support for mocking the opcode processor"""
import re import re
from ganeti import constants from ganeti import constants
...@@ -42,6 +46,9 @@ class LogRecordingCallback(mcpu.OpExecCbBase): ...@@ -42,6 +46,9 @@ class LogRecordingCallback(mcpu.OpExecCbBase):
self.processor.log_entries.append((log_type, log_msg)) self.processor.log_entries.append((log_type, log_msg))
def SubmitManyJobs(self, jobs):
return mcpu.OpExecCbBase.SubmitManyJobs(self, jobs)
class ProcessorMock(mcpu.Processor): class ProcessorMock(mcpu.Processor):
"""Mocked opcode processor for tests. """Mocked opcode processor for tests.
...@@ -89,8 +96,8 @@ class ProcessorMock(mcpu.Processor): ...@@ -89,8 +96,8 @@ class ProcessorMock(mcpu.Processor):
"""Return a string with all log entries separated by a newline. """Return a string with all log entries separated by a newline.
""" """
return "\n".join("%s: %s" % (type, msg) return "\n".join("%s: %s" % (log_type, msg)
for type, msg in self.GetLogEntries()) for log_type, msg in self.GetLogEntries())
def GetLogMessagesString(self): def GetLogMessagesString(self):
"""Return a string with all log messages separated by a newline. """Return a string with all log messages separated by a newline.
...@@ -107,8 +114,8 @@ class ProcessorMock(mcpu.Processor): ...@@ -107,8 +114,8 @@ class ProcessorMock(mcpu.Processor):
@param expected_msg: the expected message @param expected_msg: the expected message
""" """
for type, msg in self.log_entries: for log_type, msg in self.log_entries:
if type == expected_type and msg == expected_msg: if log_type == expected_type and msg == expected_msg:
return return
raise AssertionError( raise AssertionError(
...@@ -144,4 +151,4 @@ class ProcessorMock(mcpu.Processor): ...@@ -144,4 +151,4 @@ class ProcessorMock(mcpu.Processor):
raise AssertionError( raise AssertionError(
"Could not find '%s' in LU log messages. Log is:\n%s" % "Could not find '%s' in LU log messages. Log is:\n%s" %
(expected_regex, self.GetLogMessagesString()) (expected_regex, self.GetLogMessagesString())
) )
\ No newline at end of file
...@@ -18,6 +18,10 @@ ...@@ -18,6 +18,10 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
# 02110-1301, USA. # 02110-1301, USA.
"""Support for mocking the RPC runner"""
import mock import mock
from ganeti import objects from ganeti import objects
...@@ -91,7 +95,7 @@ class RpcResultsBuilder(object): ...@@ -91,7 +95,7 @@ class RpcResultsBuilder(object):
else: else:
return node.uuid return node.uuid
def CreateSuccessfulNodeResult(self, node, data={}): def CreateSuccessfulNodeResult(self, node, data=None):
"""@see L{RpcResultsBuilder} """@see L{RpcResultsBuilder}
@param node: @see L{RpcResultsBuilder}. @param node: @see L{RpcResultsBuilder}.
...@@ -99,6 +103,8 @@ class RpcResultsBuilder(object): ...@@ -99,6 +103,8 @@ class RpcResultsBuilder(object):
@param data: the data as returned by the RPC @param data: the data as returned by the RPC
@rtype: L{rpc.RpcResult} @rtype: L{rpc.RpcResult}
""" """
if data is None:
data = {}
return rpc.RpcResult(data=(True, data), node=self._GetNodeId(node)) return rpc.RpcResult(data=(True, data), node=self._GetNodeId(node))
def CreateFailedNodeResult(self, node): def CreateFailedNodeResult(self, node):
...@@ -127,7 +133,7 @@ class RpcResultsBuilder(object): ...@@ -127,7 +133,7 @@ class RpcResultsBuilder(object):
""" """
return rpc.RpcResult(data=(False, error_msg), node=self._GetNodeId(node)) return rpc.RpcResult(data=(False, error_msg), node=self._GetNodeId(node))
def AddSuccessfulNode(self, node, data={}): def AddSuccessfulNode(self, node, data=None):
"""@see L{CreateSuccessfulNode}""" """@see L{CreateSuccessfulNode}"""
self._results.append(self.CreateSuccessfulNodeResult(node, data)) self._results.append(self.CreateSuccessfulNodeResult(node, data))
return self return self
...@@ -150,4 +156,4 @@ class RpcResultsBuilder(object): ...@@ -150,4 +156,4 @@ class RpcResultsBuilder(object):
@rtype: dict @rtype: dict
""" """
return dict((result.node, result) for result in self._results) return dict((result.node, result) for result in self._results)
\ No newline at end of file
...@@ -24,7 +24,6 @@ ...@@ -24,7 +24,6 @@
import os import os
from ganeti import utils
from ganeti import netutils from ganeti import netutils
...@@ -37,7 +36,7 @@ FAKE_CLUSTER_KEY = ("AAAAB3NzaC1yc2EAAAABIwAAAQEAsuGLw70et3eApJ/ZEJkAVZogIrm" ...@@ -37,7 +36,7 @@ FAKE_CLUSTER_KEY = ("AAAAB3NzaC1yc2EAAAABIwAAAQEAsuGLw70et3eApJ/ZEJkAVZogIrm"
"vYdB2nQds7/+Bf40C/OpbvnAxna1kVtgFHAo18cQ==") "vYdB2nQds7/+Bf40C/OpbvnAxna1kVtgFHAo18cQ==")
class FakeConfig: class FakeConfig(object):
"""Fake configuration object""" """Fake configuration object"""
def IsCluster(self): def IsCluster(self):
...@@ -61,7 +60,7 @@ class FakeConfig: ...@@ -61,7 +60,7 @@ class FakeConfig:
def GetMasterNodeName(self): def GetMasterNodeName(self):
return netutils.Hostname.GetSysName() return netutils.Hostname.GetSysName()
def GetDefaultIAllocator(Self): def GetDefaultIAllocator(self):
return "testallocator" return "testallocator"
def GetNodeName(self, node_uuid): def GetNodeName(self, node_uuid):
...@@ -74,7 +73,7 @@ class FakeConfig: ...@@ -74,7 +73,7 @@ class FakeConfig:
return map(self.GetNodeName, node_uuids) return map(self.GetNodeName, node_uuids)
class FakeProc: class FakeProc(object):
"""Fake processor object""" """Fake processor object"""
def Log(self, msg, *args, **kwargs): def Log(self, msg, *args, **kwargs):
...@@ -90,14 +89,14 @@ class FakeProc: ...@@ -90,14 +89,14 @@ class FakeProc:
pass pass
class FakeGLM: class FakeGLM(object):
"""Fake global lock manager object""" """Fake global lock manager object"""
def list_owned(self, level):