Hi,

Yes, exactly. In addition, one or two "sunshine" (not failing) tests would
be great too.
The basic idea is 1) to test if your code does what it should and 2) to
actually execute the code during test time, so typos, type errors, etc. are
caught much quicker than in QA.

I usually look at the code coverage of my tests and aim for full coverage
when writing tests. You can use `make py-coverage` to produce the coverage
data, or look at the commands issued there to get quicker coverage data. My
IDE (IntelliJ IDEA / PyCharm) supports running tests from within the IDE
and displays the coverage in the editor immediately. Probably something
similar is doable in other IDE's as well.

I think there is not yet a node_unittest.py file, so you will have to
create that and add it to Makefile.am (right next to cluster_unittest.py).

Thanks,
Thomas


On Wed, Aug 21, 2013 at 8:20 AM, Sebastian Gebhard <[email protected]>wrote:

>  Hello Thomas,
>
> I am looking at it right now. How should this work in principle? From what
> I understand I should feed my code wrong input parameters (from what I see
> as wrong) and check if errors are raised, right?
> Something more I need to do?
>
> Cheers,
> Sebastian
>
>
> Am 20.08.2013 12:59, schrieb Thomas Thrainer:
>
> Hi,
>
>  I'm not reviewing as I don't really know the code, but I have a
> suggestion for this part. Right now I'm implementing a test framework for
> the cmdlib module which makes it rather easy to unit test such logic as you
> have implemented here. So it would be great if you could take a look at the
> tests (cluster_unittest.py for example) and implement some for your code as
> well.
>
>  Cheers,
> Thomas
>
>
> On Tue, Aug 20, 2013 at 12:44 PM, Sebastian Gebhard <[email protected]>wrote:
>
>> This patch adds functionality to LUNodeAdd to
>>   - check the arguments given. It will warn if no physical link is given
>>     and fail if OpenvSwitch is not enabled, but parameters are given
>>   - call the RPC to configure OpenvSwitch on the node
>> ---
>>  lib/cmdlib/node.py | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
>> index ca4a5a6..c62e691 100644
>> --- a/lib/cmdlib/node.py
>> +++ b/lib/cmdlib/node.py
>> @@ -112,6 +112,20 @@ class LUNodeAdd(LogicalUnit):
>>        raise errors.OpPrereqError("Cannot pass a node group when a node
>> is"
>>                                   " being readded", errors.ECODE_INVAL)
>>
>> +    # OpenvSwitch: Warn user if link is missing
>> +    if (self.op.ndparams[constants.ND_OVS] and not
>> +        self.op.ndparams[constants.ND_OVS_LINK]):
>> +      self.LogInfo("No physical interface for OpenvSwitch was given."
>> +                   " OpenvSwitch will not have an outside connection.
>> This"
>> +                   " might not be what you want.")
>> +    # OpenvSwitch: Fail if parameters are given, but OVS is not enabled.
>> +    if (not self.op.ndparams[constants.ND_OVS] and
>> +        (self.op.ndparams[constants.ND_OVS_NAME] or
>> +         self.op.ndparams[constants.ND_OVS_LINK])):
>> +      raise errors.OpPrereqError("OpenvSwitch name or link were given,
>> but"
>> +                                 " OpenvSwitch is not enabled. Please
>> enable"
>> +                                 " OpenvSwitch with --ovs",
>> errors.ECODE_INVAL)
>> +
>>    def BuildHooksEnv(self):
>>      """Build hooks env.
>>
>> @@ -375,6 +389,13 @@ class LUNodeAdd(LogicalUnit):
>>                        (verifier, nl_payload[failed]))
>>          raise errors.OpExecError("ssh/hostname verification failed")
>>
>> +    # OpenvSwitch initialization on the node
>> +    if self.new_node.ndparams[constants.ND_OVS]:
>> +      result = self.rpc.call_node_configure_ovs(
>> +                 self.new_node.name,
>> +                 self.new_node.ndparams[constants.ND_OVS_NAME],
>> +                 self.new_node.ndparams[constants.ND_OVS_LINK])
>> +
>>      if self.op.readd:
>>        self.context.ReaddNode(self.new_node)
>>        RedistributeAncillaryFiles(self)
>> --
>> 1.8.1.2
>>
>>
>
>
>  --
> 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
>
>
>
> --
> Fachschaft Elektrotechnik und Informationstechnik e.V.
> Technische Universität München   Tel: +49 89 289 22998
> Arcisstr. 21                     Fax: +49 89 289 25140
> 80333 München                : http://www.fs.ei.tum.de
>
>


-- 
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

Reply via email to