launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/nodegroupinterface-validation-fallout into lp:maas.

Commit message:
NodeGroupInterface.network is not defined until both the broadcast address and subnet mask are defined.

Previously they both had to be not None, but that was not enough because the field constraint allows them to be the empty string.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~allenap/maas/nodegroupinterface-validation-fallout/+merge/127360

Fixes a build failure, http://10.189.74.2:8080/job/maas-trunk/898/
-- 
https://code.launchpad.net/~allenap/maas/nodegroupinterface-validation-fallout/+merge/127360
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/nodegroupinterface-validation-fallout into lp:maas.

=== modified file 'src/maasserver/models/nodegroupinterface.py'
--- src/maasserver/models/nodegroupinterface.py	2012-10-01 15:17:40 +0000
+++ src/maasserver/models/nodegroupinterface.py	2012-10-01 19:58:51 +0000
@@ -76,7 +76,7 @@
 
         :return: :class:`IPNetwork`
         """
-        if self.broadcast_ip is not None and self.subnet_mask is not None:
+        if self.broadcast_ip and self.subnet_mask:
             return IPNetwork("%s/%s" % (self.broadcast_ip, self.subnet_mask))
         return None
 

=== added file 'src/maasserver/tests/test_nodegroupinterface.py'
--- src/maasserver/tests/test_nodegroupinterface.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/tests/test_nodegroupinterface.py	2012-10-01 19:58:51 +0000
@@ -0,0 +1,55 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for :class:`NodeGroupInterface`."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+from maasserver.enum import (
+    NODEGROUP_STATUS,
+    NODEGROUPINTERFACE_MANAGEMENT,
+    )
+from maasserver.testing.factory import factory
+from maasserver.testing.testcase import TestCase
+from netaddr import IPNetwork
+
+
+def make_interface():
+    nodegroup = factory.make_node_group(
+        status=NODEGROUP_STATUS.ACCEPTED,
+        management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+    return nodegroup.get_managed_interface()
+
+
+class TestNodeGroupInterface(TestCase):
+
+    def test_network_is_defined_when_broadcast_and_mask_are(self):
+        interface = make_interface()
+        self.assertIsInstance(interface.network, IPNetwork)
+
+    def test_network_is_undefined_when_broadcast_is_None(self):
+        interface = make_interface()
+        interface.broadcast_ip = None
+        self.assertIsNone(interface.network)
+
+    def test_network_is_undefined_when_broadcast_is_empty(self):
+        interface = make_interface()
+        interface.broadcast_ip = ""
+        self.assertIsNone(interface.network)
+
+    def test_network_is_undefined_when_subnet_mask_is_None(self):
+        interface = make_interface()
+        interface.subnet_mask = None
+        self.assertIsNone(interface.network)
+
+    def test_network_is_undefined_subnet_mask_is_empty(self):
+        interface = make_interface()
+        interface.subnet_mask = ""
+        self.assertIsNone(interface.network)