On Mon, Aug 26, 2013 at 9:45 AM, Thomas Thrainer <[email protected]> wrote: > > > > On Mon, Aug 26, 2013 at 8:35 AM, Sebastian Gebhard <[email protected]> > wrote: >> >> This patch enables patching the rpc module to create a mocked version >> which can be used to mock a rpc.DnsOnlyRunner(). This is needed for >> unit testing LUNodeAdd, as it need to run RPCs against nodes not yet >> present in the configuration. >> >> Signed-off-by: Sebastian Gebhard <[email protected]> >> --- >> Makefile.am | 1 + >> test/py/cmdlib/testsupport/cmdlib_testcase.py | 28 >> ++++++++++++++++++++++++++- >> test/py/cmdlib/testsupport/rpc_runner_mock.py | 26 >> +++++++++++++++++++++++++ >> test/py/cmdlib/testsupport/util.py | 4 ++-- >> 4 files changed, 56 insertions(+), 3 deletions(-) >> >> diff --git a/Makefile.am b/Makefile.am >> index e99707e..55c833b 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -1280,6 +1280,7 @@ python_tests = \ >> test/py/cmdlib/group_unittest.py \ >> test/py/cmdlib/instance_unittest.py \ >> test/py/cmdlib/instance_storage_unittest.py \ >> + test/py/cmdlib/node_unittest.py \ >> test/py/cmdlib/test_unittest.py \ >> test/py/cfgupgrade_unittest.py \ >> test/py/docs_unittest.py \ >> diff --git a/test/py/cmdlib/testsupport/cmdlib_testcase.py >> b/test/py/cmdlib/testsupport/cmdlib_testcase.py >> index 811d455..6901d05 100644 >> --- a/test/py/cmdlib/testsupport/cmdlib_testcase.py >> +++ b/test/py/cmdlib/testsupport/cmdlib_testcase.py >> @@ -34,11 +34,12 @@ from cmdlib.testsupport.netutils_mock import >> patchNetutils, \ >> SetupDefaultNetutilsMock >> from cmdlib.testsupport.processor_mock import ProcessorMock >> from cmdlib.testsupport.rpc_runner_mock import CreateRpcRunnerMock, \ >> - RpcResultsBuilder >> + RpcResultsBuilder, patchRpcModule, SetupDefaultRpcModuleMock >> from cmdlib.testsupport.ssh_mock import patchSsh >> >> from ganeti.cmdlib.base import LogicalUnit >> from ganeti import errors >> +from ganeti import locking >> from ganeti import objects >> from ganeti import opcodes >> from ganeti import runtime >> @@ -57,6 +58,19 @@ class GanetiContextMock(object): >> def __init__(self, test_case): >> self._test_case = test_case >> >> + def AddNode(self, node, ec_id): >> + self._test_case.cfg.AddNode(node, ec_id) >> + self._test_case.glm.add(locking.LEVEL_NODE, node.uuid) >> + self._test_case.glm.add(locking.LEVEL_NODE_RES, node.uuid) >> + >> + def ReaddNode(self, node): >> + pass >> + >> + def RemoveNode(self, node): >> + self._test_case.cfg.RemoveNode(node.uuid) >> + self._test_case.glm.remove(locking.LEVEL_NODE, node.uuid) >> + self._test_case.glm.remove(locking.LEVEL_NODE_RES, node.uuid) >> + >> >> class MockLU(LogicalUnit): >> def BuildHooksNodes(self): >> @@ -109,6 +123,7 @@ class CmdlibTestCase(testutils.GanetiTestCase): >> self._iallocator_patcher = None >> self._netutils_patcher = None >> self._ssh_patcher = None >> + self._rpc_patcher = None >> >> try: >> runtime.InitArchInfo() >> @@ -128,6 +143,9 @@ class CmdlibTestCase(testutils.GanetiTestCase): >> if self._ssh_patcher is not None: >> self._ssh_patcher.stop() >> self._ssh_patcher = None >> + if self._rpc_patcher is not None: >> + self._rpc_patcher.stop() >> + self._rpc_patcher = None >> >> def tearDown(self): >> super(CmdlibTestCase, self).tearDown() >> @@ -178,6 +196,14 @@ class CmdlibTestCase(testutils.GanetiTestCase): >> # this test module does not use ssh, no patching performed >> self._ssh_patcher = None >> >> + try: >> + self._rpc_patcher = patchRpcModule(self._GetTestModule()) >> + self.rpc_mod = self._rpc_patcher.start() >> + SetupDefaultRpcModuleMock(self.rpc_mod) >> + except (ImportError, AttributeError): >> + # this test module does not use rpc, no patching performed >> + self._rpc_patcher = None >> + >> def GetMockLU(self): >> """Creates a mock L{LogialUnit} with access to the mocked config etc. >> >> diff --git a/test/py/cmdlib/testsupport/rpc_runner_mock.py >> b/test/py/cmdlib/testsupport/rpc_runner_mock.py >> index 93845fb..484f178 100644 >> --- a/test/py/cmdlib/testsupport/rpc_runner_mock.py >> +++ b/test/py/cmdlib/testsupport/rpc_runner_mock.py >> @@ -27,6 +27,8 @@ import mock >> from ganeti import objects >> from ganeti import rpc >> >> +from cmdlib.testsupport.util import patchModule >> + >> >> def CreateRpcRunnerMock(): >> """Creates a new L{mock.MagicMock} tailored for L{rpc.RpcRunner} >> @@ -179,3 +181,27 @@ class RpcResultsBuilder(object): >> @rtype: dict >> """ >> return dict((result.node, result) for result in self._results) >> + >> + >> +def patchRpcModule(module_under_test): >> + """Patches the L{ganeti.rpc} module for tests. >> + >> + This function is meant to be used as a decorator for test methods. >> + >> + @type module_under_test: string >> + @param module_under_test: the module within cmdlib which is tested. The >> + "ganeti.cmdlib" prefix is optional. >> + >> + """ >> + return patchModule(module_under_test, "rpc", wraps=rpc) >> + >> + >> +def SetupDefaultRpcModuleMock(rpc_mod): >> + """Configures the given rpc_mod. >> + >> + All relevant functions in rpc_mod are stubbed in a sensible way. >> + >> + @param rpc_mod: the mock module to configure >> + >> + """ >> + rpc_mod.DnsOnlyRunner.return_value = CreateRpcRunnerMock() >> \ No newline at end of file > > > Newline missing. Did 'make pylint', 'make pylint-qa' and 'make pylint-test' > run through? > >> >> diff --git a/test/py/cmdlib/testsupport/util.py >> b/test/py/cmdlib/testsupport/util.py >> index e2354ce..61305c9 100644 >> --- a/test/py/cmdlib/testsupport/util.py >> +++ b/test/py/cmdlib/testsupport/util.py >> @@ -26,7 +26,7 @@ import mock >> >> >> # pylint: disable=C0103 >> -def patchModule(module_under_test, mock_module): >> +def patchModule(module_under_test, mock_module, **kwargs): >> """Computes the module prefix required to mock parts of the Ganeti >> code. >> >> @type module_under_test: string >> @@ -38,4 +38,4 @@ def patchModule(module_under_test, mock_module): >> """ >> if not module_under_test.startswith("ganeti.cmdlib"): >> module_under_test = "ganeti.cmdlib." + module_under_test >> - return mock.patch("%s.%s" % (module_under_test, mock_module)) >> + return mock.patch("%s.%s" % (module_under_test, mock_module), **kwargs) >> -- >> 1.8.1.2 >> > > The rest looks good to me, but I can't quite LGTM it, as parts of it are > written by me ;-).
In this case it would be useful to have this noted in the commit message, if separating your code from Sebastian's is too complex/not worthy. Thanks, Guido
