On Mon, Aug 5, 2013 at 2:41 PM, Thomas Thrainer <[email protected]> wrote:

>
>
>
> On Mon, Aug 5, 2013 at 1:20 PM, Michele Tartara <[email protected]>wrote:
>
>> On Thu, Aug 1, 2013 at 11:18 AM, Thomas Thrainer <[email protected]>wrote:
>>
>>> Some configuration objects are accessed quite often, so introduce
>>> shortcut properties for those.
>>>
>>> Signed-off-by: Thomas Thrainer <[email protected]>
>>> ---
>>>  test/py/cmdlib/cluster_unittest.py            | 46
>>> +++++++++++----------------
>>>  test/py/cmdlib/test_unittest.py               | 14 ++++----
>>>  test/py/cmdlib/testsupport/cmdlib_testcase.py |  7 ++++
>>>  3 files changed, 33 insertions(+), 34 deletions(-)
>>>
>>
>>
>> Shouldn't these be introduced earlier given that they are part of the
>> same patchset?
>>
>
> Yeah, they could. But actually I wanted to save some work by not faking
> history ;-).
> I would have to distribute this change over a couple of other commits,
> making sure in the progress that all tests continue to work after each
> commit. Given the fact that I changed and refined the test framework
> throughout the patch series again and again, I considered this change as a
> refinement too, which does not need to be hidden away...
>

I'm saying that just because sending already clean patches is the default
approach that has always been used in the past...


>
>
>>
>>
>>>
>>> diff --git a/test/py/cmdlib/cluster_unittest.py
>>> b/test/py/cmdlib/cluster_unittest.py
>>> index 95b4c9f..6061440 100644
>>> --- a/test/py/cmdlib/cluster_unittest.py
>>> +++ b/test/py/cmdlib/cluster_unittest.py
>>> @@ -273,21 +273,19 @@ class
>>> TestLUClusterActivateMasterIp(CmdlibTestCase):
>>>
>>>      self.rpc.call_node_activate_master_ip.return_value = \
>>>        RpcResultsBuilder(cfg=self.cfg) \
>>> -        .CreateSuccessfulNodeResult(self.cfg.GetMasterNode())
>>> +        .CreateSuccessfulNodeResult(self.master)
>>>
>>>      self.ExecOpCode(op)
>>>
>>>      self.rpc.call_node_activate_master_ip.assert_called_once_with(
>>> -      self.cfg.GetMasterNode(),
>>> -      self.cfg.GetMasterNetworkParameters(),
>>> -      False)
>>> +      self.master_uuid, self.cfg.GetMasterNetworkParameters(), False)
>>>
>>>    def testFailure(self):
>>>      op = opcodes.OpClusterActivateMasterIp()
>>>
>>>      self.rpc.call_node_activate_master_ip.return_value = \
>>>        RpcResultsBuilder(cfg=self.cfg) \
>>> -        .CreateFailedNodeResult(self.cfg.GetMasterNode()) \
>>> +        .CreateFailedNodeResult(self.master) \
>>>
>>>      self.ExecOpCodeExpectOpExecError(op)
>>>
>>> @@ -298,21 +296,19 @@ class
>>> TestLUClusterDeactivateMasterIp(CmdlibTestCase):
>>>
>>>      self.rpc.call_node_deactivate_master_ip.return_value = \
>>>        RpcResultsBuilder(cfg=self.cfg) \
>>> -        .CreateSuccessfulNodeResult(self.cfg.GetMasterNode())
>>> +        .CreateSuccessfulNodeResult(self.master)
>>>
>>>      self.ExecOpCode(op)
>>>
>>>      self.rpc.call_node_deactivate_master_ip.assert_called_once_with(
>>> -      self.cfg.GetMasterNode(),
>>> -      self.cfg.GetMasterNetworkParameters(),
>>> -      False)
>>> +      self.master_uuid, self.cfg.GetMasterNetworkParameters(), False)
>>>
>>>    def testFailure(self):
>>>      op = opcodes.OpClusterDeactivateMasterIp()
>>>
>>>      self.rpc.call_node_deactivate_master_ip.return_value = \
>>>        RpcResultsBuilder(cfg=self.cfg) \
>>> -        .CreateFailedNodeResult(self.cfg.GetMasterNode()) \
>>> +        .CreateFailedNodeResult(self.master) \
>>>
>>>      self.ExecOpCodeExpectOpExecError(op)
>>>
>>> @@ -328,7 +324,7 @@ class TestLUClusterConfigQuery(CmdlibTestCase):
>>>
>>>      self.rpc.call_get_watcher_pause.return_value = \
>>>        RpcResultsBuilder(self.cfg) \
>>> -        .CreateSuccessfulNodeResult(self.cfg.GetMasterNode(), -1)
>>> +        .CreateSuccessfulNodeResult(self.master, -1)
>>>
>>>      ret = self.ExecOpCode(op)
>>>
>>> @@ -365,7 +361,7 @@ class TestLUClusterDestroy(CmdlibTestCase):
>>>
>>>      self.ExecOpCode(op)
>>>
>>> -    self.assertSingleHooksCall([self.cfg.GetMasterNodeName()],
>>> +    self.assertSingleHooksCall([self.master.name],
>>>                                 "cluster-destroy",
>>>                                 constants.HOOKS_PHASE_POST)
>>>
>>> @@ -376,7 +372,7 @@ class TestLUClusterPostInit(CmdlibTestCase):
>>>
>>>      self.ExecOpCode(op)
>>>
>>> -    self.assertSingleHooksCall([self.cfg.GetMasterNodeName()],
>>> +    self.assertSingleHooksCall([self.master.name],
>>>                                 "cluster-init",
>>>                                 constants.HOOKS_PHASE_POST)
>>>
>>> @@ -390,7 +386,7 @@ class TestLUClusterQuery(CmdlibTestCase):
>>>    def testIPv6Cluster(self):
>>>      op = opcodes.OpClusterQuery()
>>>
>>> -    self.cfg.GetClusterInfo().primary_ip_family =
>>> netutils.IP6Address.family
>>> +    self.cluster.primary_ip_family = netutils.IP6Address.family
>>>
>>>      self.ExecOpCode(op)
>>>
>>> @@ -430,18 +426,14 @@ class TestLUClusterRename(CmdlibTestCase):
>>>
>>>      self.assertEqual(1, self.ssh_mod.WriteKnownHostsFile.call_count)
>>>      self.rpc.call_node_deactivate_master_ip.assert_called_once_with(
>>> -      self.cfg.GetMasterNode(),
>>> -      self.cfg.GetMasterNetworkParameters(),
>>> -      False)
>>> +      self.master_uuid, self.cfg.GetMasterNetworkParameters(), False)
>>>      self.rpc.call_node_activate_master_ip.assert_called_once_with(
>>> -      self.cfg.GetMasterNode(),
>>> -      self.cfg.GetMasterNetworkParameters(),
>>> -      False)
>>> +      self.master_uuid, self.cfg.GetMasterNetworkParameters(), False)
>>>
>>>    def testRenameOfflineMaster(self):
>>>      op = opcodes.OpClusterRename(name=self.NEW_NAME)
>>>
>>> -    self.cfg.GetMasterNodeInfo().offline = True
>>> +    self.master.offline = True
>>>      self.netutils_mod.GetHostname.return_value = \
>>>        HostnameMock(self.NEW_NAME, self.NEW_IP)
>>>
>>> @@ -455,7 +447,7 @@ class TestLUClusterRepairDiskSizes(CmdlibTestCase):
>>>      self.ExecOpCode(op)
>>>
>>>    def _SetUpInstanceSingleDisk(self, dev_type=constants.LD_LV):
>>> -    pnode = self.cfg.GetMasterNodeInfo()
>>> +    pnode = self.master
>>>      snode = self.cfg.AddNewNode()
>>>
>>>      inst = self.cfg.AddNewInstance()
>>> @@ -472,7 +464,7 @@ class TestLUClusterRepairDiskSizes(CmdlibTestCase):
>>>
>>>      self.rpc.call_blockdev_getdimensions.return_value = \
>>>        RpcResultsBuilder(cfg=self.cfg) \
>>> -        .CreateFailedNodeResult(self.cfg.GetMasterNode())
>>> +        .CreateFailedNodeResult(self.master)
>>>
>>>      self.ExecOpCode(op)
>>>
>>> @@ -484,7 +476,7 @@ class TestLUClusterRepairDiskSizes(CmdlibTestCase):
>>>
>>>      self.rpc.call_blockdev_getdimensions.return_value = \
>>>        RpcResultsBuilder(cfg=self.cfg) \
>>> -        .CreateSuccessfulNodeResult(self.cfg.GetMasterNode(), node_data)
>>> +        .CreateSuccessfulNodeResult(self.master, node_data)
>>>
>>>      return self.ExecOpCode(op)
>>>
>>> @@ -522,7 +514,7 @@ class TestLUClusterRepairDiskSizes(CmdlibTestCase):
>>>
>>>    def testExclusiveStorageInvalidResultData(self):
>>>      self._SetUpInstanceSingleDisk()
>>> -
>>>  self.cfg.GetMasterNodeInfo().ndparams[constants.ND_EXCLUSIVE_STORAGE] =
>>> True
>>> +    self.master.ndparams[constants.ND_EXCLUSIVE_STORAGE] = True
>>>      self._ExecOpClusterRepairDiskSizes([(1024 * 1024 * 1024, None)])
>>>
>>>      self.mcpu.assertLogContainsRegex(
>>> @@ -531,13 +523,13 @@ class TestLUClusterRepairDiskSizes(CmdlibTestCase):
>>>    def testExclusiveStorageCorrectSpindles(self):
>>>      (_, disk) = self._SetUpInstanceSingleDisk()
>>>      disk.spindles = 1
>>> -
>>>  self.cfg.GetMasterNodeInfo().ndparams[constants.ND_EXCLUSIVE_STORAGE] =
>>> True
>>> +    self.master.ndparams[constants.ND_EXCLUSIVE_STORAGE] = True
>>>      changed = self._ExecOpClusterRepairDiskSizes([(1024 * 1024 * 1024,
>>> 1)])
>>>      self.assertEqual(0, len(changed))
>>>
>>>    def testExclusiveStorageWrongSpindles(self):
>>>      self._SetUpInstanceSingleDisk()
>>> -
>>>  self.cfg.GetMasterNodeInfo().ndparams[constants.ND_EXCLUSIVE_STORAGE] =
>>> True
>>> +    self.master.ndparams[constants.ND_EXCLUSIVE_STORAGE] = True
>>>      changed = self._ExecOpClusterRepairDiskSizes([(1024 * 1024 * 1024,
>>> 1)])
>>>      self.assertEqual(1, len(changed))
>>>
>>> diff --git a/test/py/cmdlib/test_unittest.py
>>> b/test/py/cmdlib/test_unittest.py
>>> index 9abc7bf..37e2b6b 100644
>>> --- a/test/py/cmdlib/test_unittest.py
>>> +++ b/test/py/cmdlib/test_unittest.py
>>> @@ -48,7 +48,7 @@ class TestLUTestDelay(CmdlibTestCase):
>>>      self.ExecOpCodeExpectOpExecError(op)
>>>
>>>    def testOnNodeUuid(self):
>>> -    node_uuids = [self.cfg.GetMasterNode()]
>>> +    node_uuids = [self.master_uuid]
>>>      op = opcodes.OpTestDelay(duration=DELAY_DURATION,
>>>                               on_node_uuids=node_uuids)
>>>      self.ExecOpCode(op)
>>> @@ -57,19 +57,19 @@ class TestLUTestDelay(CmdlibTestCase):
>>>
>>>    def testOnNodeName(self):
>>>      op = opcodes.OpTestDelay(duration=DELAY_DURATION,
>>> -                             on_nodes=[self.cfg.GetMasterNodeName()])
>>> +                             on_nodes=[self.master.name])
>>>      self.ExecOpCode(op)
>>>
>>> -
>>>  
>>> self.rpc.call_test_delay.assert_called_once_with([self.cfg.GetMasterNode()],
>>> +    self.rpc.call_test_delay.assert_called_once_with([self.master_uuid],
>>>                                                       DELAY_DURATION)
>>>
>>>    def testSuccessfulRpc(self):
>>>      op = opcodes.OpTestDelay(duration=DELAY_DURATION,
>>> -                             on_nodes=[self.cfg.GetMasterNodeName()])
>>> +                             on_nodes=[self.master.name])
>>>
>>>      self.rpc.call_test_delay.return_value = \
>>>        RpcResultsBuilder(cfg=self.cfg) \
>>> -        .AddSuccessfulNode(self.cfg.GetMasterNode()) \
>>> +        .AddSuccessfulNode(self.master) \
>>>          .Build()
>>>
>>>      self.ExecOpCode(op)
>>> @@ -78,11 +78,11 @@ class TestLUTestDelay(CmdlibTestCase):
>>>
>>>    def testFailingRpc(self):
>>>      op = opcodes.OpTestDelay(duration=DELAY_DURATION,
>>> -                             on_nodes=[self.cfg.GetMasterNodeName()])
>>> +                             on_nodes=[self.master.name])
>>>
>>>      self.rpc.call_test_delay.return_value = \
>>>        RpcResultsBuilder(cfg=self.cfg) \
>>> -        .AddFailedNode(self.cfg.GetMasterNode()) \
>>> +        .AddFailedNode(self.master) \
>>>          .Build()
>>>
>>>      self.ExecOpCodeExpectOpExecError(op)
>>> diff --git a/test/py/cmdlib/testsupport/cmdlib_testcase.py
>>> b/test/py/cmdlib/testsupport/cmdlib_testcase.py
>>> index fb22a06..a52a958 100644
>>> --- a/test/py/cmdlib/testsupport/cmdlib_testcase.py
>>> +++ b/test/py/cmdlib/testsupport/cmdlib_testcase.py
>>> @@ -71,6 +71,13 @@ class CmdlibTestCase(testutils.GanetiTestCase):
>>>    REMOVE = object()
>>>    IGNORE = object()
>>>
>>> +  cluster = property(fget=lambda self: self.cfg.GetClusterInfo(),
>>> +                     doc="Cluster configuration object")
>>> +  master = property(fget=lambda self: self.cfg.GetMasterNodeInfo(),
>>> +                    doc="Master node")
>>> +  master_uuid = property(fget=lambda self: self.cfg.GetMasterNode(),
>>> +                         doc="Master node UUID")
>>> +
>>>    def setUp(self):
>>>      super(CmdlibTestCase, self).setUp()
>>>      self._iallocator_patcher = None
>>> --
>>> 1.8.3
>>>
>>>
>>
>>
>> Thanks,
>> Michele
>>
>> --
>> Google Germany GmbH
>> Dienerstr. 12
>> 80331 München
>>
>> Registergericht und -nummer: Hamburg, HRB 86891
>> Sitz der Gesellschaft: Hamburg
>> Geschäftsführer: Graham Law, Christine Elizabeth Flores
>>
>
>
>
> --
> Thomas Thrainer | Software Engineer | [email protected] |
>
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
>
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
>

Michele

-- 
Google Germany GmbH
Dienerstr. 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores

Reply via email to