From bd39b6bb19d66d378d5b5c18a63bc274e5f967ff Mon Sep 17 00:00:00 2001 From: Thomas Thrainer Date: Mon, 29 Jul 2013 11:38:15 +0200 Subject: [PATCH] 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: Thomas Thrainer Reviewed-by: Michele Tartara --- Makefile.am | 12 +- pylintrc | 2 + pylintrc-test | 113 ++++++++++++++++++ test/py/__init__.py | 2 +- test/py/cmdlib/__init__.py | 2 +- test/py/cmdlib/test_unittest.py | 4 +- test/py/cmdlib/testsupport/__init__.py | 15 +-- test/py/cmdlib/testsupport/cmdlib_testcase.py | 16 ++- test/py/cmdlib/testsupport/config_mock.py | 36 ++++-- test/py/cmdlib/testsupport/iallocator_mock.py | 4 + .../cmdlib/testsupport/lock_manager_mock.py | 4 + test/py/cmdlib/testsupport/processor_mock.py | 17 ++- test/py/cmdlib/testsupport/rpc_runner_mock.py | 12 +- test/py/mocks.py | 13 +- test/py/testutils.py | 5 +- 15 files changed, 212 insertions(+), 45 deletions(-) create mode 100644 pylintrc-test diff --git a/Makefile.am b/Makefile.am index 7cabd8fd6..ce24accc6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -991,6 +991,7 @@ EXTRA_DIST = \ UPGRADE \ epydoc.conf.in \ pylintrc \ + pylintrc-test \ autotools/build-bash-completion \ autotools/build-rpc \ autotools/check-header \ @@ -1451,7 +1452,8 @@ pep8_python_code = \ $(CHECK_HEADER) \ $(DOCPP) \ $(PYTHON_BOOTSTRAP) \ - qa + qa \ + $(python_test_support) test/py/daemon-util_unittest.bash: daemons/daemon-util @@ -1931,7 +1933,7 @@ PEP8_IGNORE = E111,E121,E125,E127,E261,E501 # For excluding pep8 expects filenames only, not whole paths 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 LINT_TARGETS += pep8 endif @@ -1953,6 +1955,12 @@ pylint-qa: $(GENERATED_FILES) cd $(top_srcdir)/qa && \ PYTHONPATH=$(abs_top_srcdir) $(PYLINT) $(LINT_OPTS) \ --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 pep8: $(GENERATED_FILES) diff --git a/pylintrc b/pylintrc index b47abcd77..c6359a86c 100644 --- a/pylintrc +++ b/pylintrc @@ -1,6 +1,8 @@ # Configuration file for pylint (http://www.logilab.org/project/pylint). See # http://www.logilab.org/card/pylintfeatures for more detailed variable # descriptions. +# +# NOTE: Keep this file in sync (as much as possible) with pylintrc-test! [MASTER] profile = no diff --git a/pylintrc-test b/pylintrc-test new file mode 100644 index 000000000..d61735a87 --- /dev/null +++ b/pylintrc-test @@ -0,0 +1,113 @@ +# 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 diff --git a/test/py/__init__.py b/test/py/__init__.py index 13e3dca01..5e1233974 100644 --- a/test/py/__init__.py +++ b/test/py/__init__.py @@ -18,4 +18,4 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. -"""This module contains all python test code""" \ No newline at end of file +"""This module contains all python test code""" diff --git a/test/py/cmdlib/__init__.py b/test/py/cmdlib/__init__.py index 2c9e5d9a5..5d00d83aa 100644 --- a/test/py/cmdlib/__init__.py +++ b/test/py/cmdlib/__init__.py @@ -18,4 +18,4 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. -"""This module contains all cmdlib unit tests""" \ No newline at end of file +"""This module contains all cmdlib unit tests""" diff --git a/test/py/cmdlib/test_unittest.py b/test/py/cmdlib/test_unittest.py index c391eed82..3658e5ef2 100644 --- a/test/py/cmdlib/test_unittest.py +++ b/test/py/cmdlib/test_unittest.py @@ -19,9 +19,7 @@ # 02110-1301, USA. -"""Tests for LUTest* - -""" +"""Tests for LUTest*""" from ganeti import constants diff --git a/test/py/cmdlib/testsupport/__init__.py b/test/py/cmdlib/testsupport/__init__.py index 69fc33df7..ae64c97ac 100644 --- a/test/py/cmdlib/testsupport/__init__.py +++ b/test/py/cmdlib/testsupport/__init__.py @@ -23,12 +23,13 @@ """ -from cmdlib_testcase import CmdlibTestCase -from config_mock import ConfigMock -from iallocator_mock import CreateIAllocatorMock -from lock_manager_mock import LockManagerMock -from processor_mock import ProcessorMock -from rpc_runner_mock import CreateRpcRunnerMock, RpcResultsBuilder +from cmdlib.testsupport.cmdlib_testcase import CmdlibTestCase +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, \ + RpcResultsBuilder __all__ = ["CmdlibTestCase", "ConfigMock", @@ -37,4 +38,4 @@ __all__ = ["CmdlibTestCase", "LockManagerMock", "ProcessorMock", "RpcResultsBuilder", - ] \ No newline at end of file + ] diff --git a/test/py/cmdlib/testsupport/cmdlib_testcase.py b/test/py/cmdlib/testsupport/cmdlib_testcase.py index 3b3485341..37151e698 100644 --- a/test/py/cmdlib/testsupport/cmdlib_testcase.py +++ b/test/py/cmdlib/testsupport/cmdlib_testcase.py @@ -18,11 +18,15 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. -from config_mock import * -from iallocator_mock import * -from lock_manager_mock import * -from processor_mock import * -from rpc_runner_mock import * + +"""Main module of the cmdlib test framework""" + + +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 @@ -34,6 +38,7 @@ class GanetiContextMock(object): self.rpc = rpc +# pylint: disable=R0904 class CmdlibTestCase(testutils.GanetiTestCase): """Base class for cmdlib tests. @@ -83,3 +88,4 @@ class CmdlibTestCase(testutils.GanetiTestCase): """ self.mcpu.assertLogContainsRegex(expected_regex) + diff --git a/test/py/cmdlib/testsupport/config_mock.py b/test/py/cmdlib/testsupport/config_mock.py index a91d84b82..2fea5a942 100644 --- a/test/py/cmdlib/testsupport/config_mock.py +++ b/test/py/cmdlib/testsupport/config_mock.py @@ -18,7 +18,11 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. -import uuid + +"""Support for mocking the cluster configuration""" + + +import uuid as uuid_module from ganeti import config from ganeti import constants @@ -31,6 +35,7 @@ def _StubGetEntResolver(): return mocks.FakeGetentResolver() +# pylint: disable=R0904 class ConfigMock(config.ConfigWriter): """A mocked cluster configuration with added methods for easy customization. @@ -45,7 +50,7 @@ class ConfigMock(config.ConfigWriter): _getents=_StubGetEntResolver()) def _GetUuid(self): - return str(uuid.uuid4()) + return str(uuid_module.uuid4()) def AddNewNodeGroup(self, uuid=None, @@ -56,7 +61,7 @@ class ConfigMock(config.ConfigWriter): hv_state_static=None, disk_state_static=None, alloc_policy=None, - networks=[]): + networks=None): """Add a new L{objects.NodeGroup} to the cluster configuration See L{objects.NodeGroup} for parameter documentation. @@ -72,6 +77,8 @@ class ConfigMock(config.ConfigWriter): uuid = self._GetUuid() if name is None: name = "mock_group_%d" % group_id + if networks is None: + networks = [] group = objects.NodeGroup(uuid=uuid, name=name, @@ -87,6 +94,7 @@ class ConfigMock(config.ConfigWriter): self.AddNodeGroup(group, None) return group + # pylint: disable=R0913 def AddNewNode(self, uuid=None, name=None, @@ -154,12 +162,12 @@ class ConfigMock(config.ConfigWriter): primary_node=None, os="mocked_os", hypervisor=constants.HT_FAKE, - hvparams={}, - beparams={}, - osparams={}, + hvparams=None, + beparams=None, + osparams=None, admin_state=constants.ADMINST_DOWN, - nics=[], - disks=[], + nics=None, + disks=None, disk_template=constants.DT_DISKLESS, disks_active=False, network_port=None): @@ -182,6 +190,16 @@ class ConfigMock(config.ConfigWriter): primary_node = self._master_node.uuid if isinstance(primary_node, objects.Node): 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, name=name, @@ -241,4 +259,4 @@ class ConfigMock(config.ConfigWriter): pass def _GetRpc(self, address_list): - raise NotImplementedError + raise AssertionError("This should not be used during tests!") diff --git a/test/py/cmdlib/testsupport/iallocator_mock.py b/test/py/cmdlib/testsupport/iallocator_mock.py index e706c1da4..1ff3cdada 100644 --- a/test/py/cmdlib/testsupport/iallocator_mock.py +++ b/test/py/cmdlib/testsupport/iallocator_mock.py @@ -18,6 +18,10 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. + +"""Support for mocking the IAllocator interface""" + + import mock from ganeti.masterd import iallocator diff --git a/test/py/cmdlib/testsupport/lock_manager_mock.py b/test/py/cmdlib/testsupport/lock_manager_mock.py index 19c65b568..5c5f8f652 100644 --- a/test/py/cmdlib/testsupport/lock_manager_mock.py +++ b/test/py/cmdlib/testsupport/lock_manager_mock.py @@ -18,6 +18,10 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. + +"""Support for mocking the lock manager""" + + from ganeti import locking diff --git a/test/py/cmdlib/testsupport/processor_mock.py b/test/py/cmdlib/testsupport/processor_mock.py index 0eb707398..9c950612c 100644 --- a/test/py/cmdlib/testsupport/processor_mock.py +++ b/test/py/cmdlib/testsupport/processor_mock.py @@ -18,6 +18,10 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. + +"""Support for mocking the opcode processor""" + + import re from ganeti import constants @@ -42,6 +46,9 @@ class LogRecordingCallback(mcpu.OpExecCbBase): self.processor.log_entries.append((log_type, log_msg)) + def SubmitManyJobs(self, jobs): + return mcpu.OpExecCbBase.SubmitManyJobs(self, jobs) + class ProcessorMock(mcpu.Processor): """Mocked opcode processor for tests. @@ -89,8 +96,8 @@ class ProcessorMock(mcpu.Processor): """Return a string with all log entries separated by a newline. """ - return "\n".join("%s: %s" % (type, msg) - for type, msg in self.GetLogEntries()) + return "\n".join("%s: %s" % (log_type, msg) + for log_type, msg in self.GetLogEntries()) def GetLogMessagesString(self): """Return a string with all log messages separated by a newline. @@ -107,8 +114,8 @@ class ProcessorMock(mcpu.Processor): @param expected_msg: the expected message """ - for type, msg in self.log_entries: - if type == expected_type and msg == expected_msg: + for log_type, msg in self.log_entries: + if log_type == expected_type and msg == expected_msg: return raise AssertionError( @@ -144,4 +151,4 @@ class ProcessorMock(mcpu.Processor): raise AssertionError( "Could not find '%s' in LU log messages. Log is:\n%s" % (expected_regex, self.GetLogMessagesString()) - ) \ No newline at end of file + ) diff --git a/test/py/cmdlib/testsupport/rpc_runner_mock.py b/test/py/cmdlib/testsupport/rpc_runner_mock.py index 9d9f6473d..23998c527 100644 --- a/test/py/cmdlib/testsupport/rpc_runner_mock.py +++ b/test/py/cmdlib/testsupport/rpc_runner_mock.py @@ -18,6 +18,10 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301, USA. + +"""Support for mocking the RPC runner""" + + import mock from ganeti import objects @@ -91,7 +95,7 @@ class RpcResultsBuilder(object): else: return node.uuid - def CreateSuccessfulNodeResult(self, node, data={}): + def CreateSuccessfulNodeResult(self, node, data=None): """@see L{RpcResultsBuilder} @param node: @see L{RpcResultsBuilder}. @@ -99,6 +103,8 @@ class RpcResultsBuilder(object): @param data: the data as returned by the RPC @rtype: L{rpc.RpcResult} """ + if data is None: + data = {} return rpc.RpcResult(data=(True, data), node=self._GetNodeId(node)) def CreateFailedNodeResult(self, node): @@ -127,7 +133,7 @@ class RpcResultsBuilder(object): """ 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}""" self._results.append(self.CreateSuccessfulNodeResult(node, data)) return self @@ -150,4 +156,4 @@ class RpcResultsBuilder(object): @rtype: dict """ - return dict((result.node, result) for result in self._results) \ No newline at end of file + return dict((result.node, result) for result in self._results) diff --git a/test/py/mocks.py b/test/py/mocks.py index 21481de7e..d06d6715a 100644 --- a/test/py/mocks.py +++ b/test/py/mocks.py @@ -24,7 +24,6 @@ import os -from ganeti import utils from ganeti import netutils @@ -37,7 +36,7 @@ FAKE_CLUSTER_KEY = ("AAAAB3NzaC1yc2EAAAABIwAAAQEAsuGLw70et3eApJ/ZEJkAVZogIrm" "vYdB2nQds7/+Bf40C/OpbvnAxna1kVtgFHAo18cQ==") -class FakeConfig: +class FakeConfig(object): """Fake configuration object""" def IsCluster(self): @@ -61,7 +60,7 @@ class FakeConfig: def GetMasterNodeName(self): return netutils.Hostname.GetSysName() - def GetDefaultIAllocator(Self): + def GetDefaultIAllocator(self): return "testallocator" def GetNodeName(self, node_uuid): @@ -74,7 +73,7 @@ class FakeConfig: return map(self.GetNodeName, node_uuids) -class FakeProc: +class FakeProc(object): """Fake processor object""" def Log(self, msg, *args, **kwargs): @@ -90,14 +89,14 @@ class FakeProc: pass -class FakeGLM: +class FakeGLM(object): """Fake global lock manager object""" - def list_owned(self, level): + def list_owned(self, _): return set() -class FakeContext: +class FakeContext(object): """Fake context object""" def __init__(self): diff --git a/test/py/testutils.py b/test/py/testutils.py index f0fe57b78..033daa85b 100644 --- a/test/py/testutils.py +++ b/test/py/testutils.py @@ -27,7 +27,6 @@ import stat import tempfile import unittest import logging -import types from ganeti import utils @@ -120,7 +119,7 @@ class GanetiTestCase(unittest.TestCase): while self._temp_files: try: utils.RemoveFile(self._temp_files.pop()) - except EnvironmentError, err: + except EnvironmentError: pass def assertFileContent(self, file_name, expected_content): @@ -207,8 +206,10 @@ def patch_object(*args, **kwargs): """ import mock try: + # pylint: disable=W0212 return mock._patch_object(*args, **kwargs) except AttributeError: + # pylint: disable=E1101 return mock.patch_object(*args, **kwargs) -- GitLab