Improve an internal ipaddress test, add a comment explaining why treating networks as containers of interfaces rather than addresses would prove confusing

2 files changed, 31 insertions, 17 deletions

diff --git a/Lib/ipaddress.py b/Lib/ipaddress.py
index abbdcf8..05ea453 100644
--- a/Lib/ipaddress.py
+++ b/Lib/ipaddress.py
@@ -416,6 +416,11 @@ class _IPAddressBase:
         """Return the shorthand version of the IP address as a string."""
         return str(self)
 
+    @property
+    def version(self):
+        msg = '%200s has no version specified' % (type(self),)
+        raise NotImplementedError(msg)
+
     def _ip_int_from_prefix(self, prefixlen=None):
         """Turn the prefix length netmask into a int for comparison.
 
@@ -555,10 +560,6 @@ class _BaseAddress(_IPAddressBase):
     def _get_address_key(self):
         return (self._version, self)
 
-    @property
-    def version(self):
-        raise NotImplementedError('BaseIP has no version')
-
 
 class _BaseNetwork(_IPAddressBase):
 
@@ -727,12 +728,12 @@ class _BaseNetwork(_IPAddressBase):
         return int(self.broadcast_address) - int(self.network_address) + 1
 
     @property
-    def version(self):
-        raise NotImplementedError('BaseNet has no version')
-
-    @property
     def _address_class(self):
-        raise NotImplementedError('BaseNet has no associated address class')
+        # Returning bare address objects (rather than interfaces) allows for
+        # more consistent behaviour across the network address, broadcast
+        # address and individual host addresses.
+        msg = '%200s has no associated address class' % (type(self),)
+        raise NotImplementedError(msg)
 
     @property
     def prefixlen(self):
@@ -1333,7 +1334,6 @@ class IPv4Network(_BaseV4, _BaseNetwork):
 
     """
     # Class to use when creating address objects
-    # TODO (ncoghlan): Investigate using IPv4Interface instead
     _address_class = IPv4Address
 
     def __init__(self, address, strict=True):
@@ -1945,7 +1945,6 @@ class IPv6Network(_BaseV6, _BaseNetwork):
     """
 
     # Class to use when creating address objects
-    # TODO (ncoghlan): Investigate using IPv6Interface instead
     _address_class = IPv6Address
 
     def __init__(self, address, strict=True):
diff --git a/Lib/test/test_ipaddress.py b/Lib/test/test_ipaddress.py
index cbd656c..22caa6d 100644
--- a/Lib/test/test_ipaddress.py
+++ b/Lib/test/test_ipaddress.py
@@ -150,16 +150,31 @@ class IpaddrUnitTest(unittest.TestCase):
         self.assertEqual(first, last)
         self.assertEqual(0, ipaddress._get_prefix_length(2**32, 0, 32))
         self.assertEqual(128, ipaddress._count_righthand_zero_bits(0, 128))
-        base_ip = ipaddress._BaseAddress('127.0.0.1')
-        try:
-            base_ip.version
-            self.fail('_BaseAddress.version didn\'t raise NotImplementedError')
-        except NotImplementedError:
-            pass
         self.assertEqual("IPv4Network('1.2.3.0/24')", repr(self.ipv4_network))
         self.assertEqual('0x1020318', hex(self.ipv4_network))
         self.assertRaises(TypeError, self.ipv4_network.__eq__, object())
 
+    def testMissingAddressVersion(self):
+        class Broken(ipaddress._BaseAddress):
+            pass
+        broken = Broken('127.0.0.1')
+        with self.assertRaisesRegex(NotImplementedError, "Broken.*version"):
+            broken.version
+
+    def testMissingNetworkVersion(self):
+        class Broken(ipaddress._BaseNetwork):
+            pass
+        broken = Broken('127.0.0.1')
+        with self.assertRaisesRegex(NotImplementedError, "Broken.*version"):
+            broken.version
+
+    def testMissingAddressClass(self):
+        class Broken(ipaddress._BaseNetwork):
+            pass
+        broken = Broken('127.0.0.1')
+        with self.assertRaisesRegex(NotImplementedError, "Broken.*address"):
+            broken._address_class
+
     def testGetNetwork(self):
         self.assertEqual(int(self.ipv4_network.network_address), 16909056)
         self.assertEqual(str(self.ipv4_network.network_address), '1.2.3.0')