From 9f1fe2f5eedac2871a84470ab50135dd56df1a50 Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Thu, 11 Apr 2019 08:04:56 +0000 Subject: [PATCH 1/6] Fix SkipUnless logic to be based on programs capable of introspection --- Lib/test/test_uuid.py | 20 ++++++---- Lib/uuid.py | 38 +++++++++++++------ .../2019-04-11-07-59-43.bpo-28009.s85urF.rst | 3 ++ 3 files changed, 42 insertions(+), 19 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 992ef0cbf8048c..8adf94dec3a541 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -462,8 +462,7 @@ def test_uuid1_eui64(self): with unittest.mock.patch.multiple( self.uuid, _node=None, # Ignore any cached node value. - _NODE_GETTERS_WIN32=[too_large_getter], - _NODE_GETTERS_UNIX=[too_large_getter], + _NODE_GETTERS=[too_large_getter], ): node = self.uuid.getnode() self.assertTrue(0 < node < (1 << 48), '%012x' % node) @@ -673,7 +672,7 @@ class TestUUIDWithExtModule(BaseTestUUID, unittest.TestCase): class BaseTestInternals: - uuid = None + _uuid = py_uuid @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_find_mac(self): @@ -708,27 +707,32 @@ def check_node(self, node, requires=None): self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) - @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipUnless(_uuid._ifconfig_getnode in _uuid._NODE_GETTERS, + "ifconfig is not used for introspection on this platform") def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() self.check_node(node, 'ifconfig') - @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipUnless(_uuid._ip_getnode in _uuid._NODE_GETTERS, + "ip is not used for introspection on this platform") def test_ip_getnode(self): node = self.uuid._ip_getnode() self.check_node(node, 'ip') - @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipUnless(_uuid._arp_getnode in _uuid._NODE_GETTERS, + "arp is not used for introspection on this platform") def test_arp_getnode(self): node = self.uuid._arp_getnode() self.check_node(node, 'arp') - @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipUnless(_uuid._lanscan_getnode in _uuid._NODE_GETTERS, + "lanscan is not used for introspection on this platform") def test_lanscan_getnode(self): node = self.uuid._lanscan_getnode() self.check_node(node, 'lanscan') - @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipUnless(_uuid._netstat_getnode in _uuid._NODE_GETTERS, + "netstat is not used for introspection on this platform") def test_netstat_getnode(self): node = self.uuid._netstat_getnode() self.check_node(node, 'netstat') diff --git a/Lib/uuid.py b/Lib/uuid.py index ddc63ccd082c0b..3d8361a7c3f8b9 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -45,6 +45,7 @@ """ import os +import platform import sys from enum import Enum @@ -52,6 +53,12 @@ __author__ = 'Ka-Ping Yee ' +# The recognized platforms - known behaviors +_AIX = platform.system() == 'AIX' +_DARWIN = platform.system() == 'Darwin' +_LINUX = platform.system() == 'Linux' +_WINDOWS = platform.system() == 'Windows' + RESERVED_NCS, RFC_4122, RESERVED_MICROSOFT, RESERVED_FUTURE = [ 'reserved for NCS compatibility', 'specified in RFC 4122', 'reserved for Microsoft compatibility', 'reserved for future definition'] @@ -673,12 +680,26 @@ def _random_getnode(): return random.getrandbits(48) | (1 << 40) -_node = None - -_NODE_GETTERS_WIN32 = [_windll_getnode, _netbios_getnode, _ipconfig_getnode] +if _LINUX: + _GETTERS = [_ifconfig_getnode, _ip_getnode, + _arp_getnode, _lanscan_getnode] +elif _DARWIN: + _GETTERS = [_arp_getnode, _ifconfig_getnode, _netstat_getnode] +elif _WINDOWS: + _GETTERS = [_windll_getnode, _netbios_getnode, _ipconfig_getnode] +elif _AIX: + _GETTERS = [_netstat_getnode] +else: + _GETTERS = [_ifconfig_getnode, _ip_getnode, + _arp_getnode, _lanscan_getnode, _netstat_getnode] +if os.name == 'posix': + _NODE_GETTERS = [_unix_getnode] + _GETTERS +elif os.name == 'nt': + _NODE_GETTERS = [_windll_getnode] + _GETTERS +else: + _NODE_GETTERS = _GETTERS -_NODE_GETTERS_UNIX = [_unix_getnode, _ifconfig_getnode, _ip_getnode, - _arp_getnode, _lanscan_getnode, _netstat_getnode] +_node = None def getnode(*, getters=None): """Get the hardware address as a 48-bit positive integer. @@ -692,12 +713,7 @@ def getnode(*, getters=None): if _node is not None: return _node - if sys.platform == 'win32': - getters = _NODE_GETTERS_WIN32 - else: - getters = _NODE_GETTERS_UNIX - - for getter in getters + [_random_getnode]: + for getter in _NODE_GETTERS + [_random_getnode]: try: _node = getter() except: diff --git a/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst b/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst new file mode 100644 index 00000000000000..8cc96b7eb90879 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst @@ -0,0 +1,3 @@ +Fix the SkipUnless logic to be based on whether a program +is available AND can be used to get a MACADDR as basis for an UUID. +Patch by M. Felt From ccbb61b80e2b460a74f06342f627c256665d9709 Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Thu, 11 Apr 2019 13:55:31 +0000 Subject: [PATCH 2/6] reformat to one-line where possible --- Lib/uuid.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 3d8361a7c3f8b9..7228f5170b96e0 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -681,8 +681,7 @@ def _random_getnode(): if _LINUX: - _GETTERS = [_ifconfig_getnode, _ip_getnode, - _arp_getnode, _lanscan_getnode] + _GETTERS = [_ifconfig_getnode, _ip_getnode, _arp_getnode, _lanscan_getnode] elif _DARWIN: _GETTERS = [_arp_getnode, _ifconfig_getnode, _netstat_getnode] elif _WINDOWS: @@ -690,8 +689,8 @@ def _random_getnode(): elif _AIX: _GETTERS = [_netstat_getnode] else: - _GETTERS = [_ifconfig_getnode, _ip_getnode, - _arp_getnode, _lanscan_getnode, _netstat_getnode] + _GETTERS = [_ifconfig_getnode, _arp_getnode, _netstat_getnode, + _lanscan_getnode, _ip_getnode] if os.name == 'posix': _NODE_GETTERS = [_unix_getnode] + _GETTERS elif os.name == 'nt': From c8b96a0014dd502bf5c57e6adf974638e22b073f Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Mon, 15 Apr 2019 13:42:58 +0000 Subject: [PATCH 3/6] reword blurb --- Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst b/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst index 8cc96b7eb90879..3a3775f8484e08 100644 --- a/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst +++ b/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst @@ -1,3 +1,3 @@ -Fix the SkipUnless logic to be based on whether a program +Fix the uuid test skipping logic based on whether a program is available AND can be used to get a MACADDR as basis for an UUID. Patch by M. Felt From a1792359c9cc65f183b1946274a4dff7152eba45 Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Wed, 17 Apr 2019 10:56:12 +0000 Subject: [PATCH 4/6] correct typo --- Lib/uuid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 7228f5170b96e0..023188855da58d 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -685,7 +685,7 @@ def _random_getnode(): elif _DARWIN: _GETTERS = [_arp_getnode, _ifconfig_getnode, _netstat_getnode] elif _WINDOWS: - _GETTERS = [_windll_getnode, _netbios_getnode, _ipconfig_getnode] + _GETTERS = [_netbios_getnode, _ipconfig_getnode] elif _AIX: _GETTERS = [_netstat_getnode] else: From e0f222956ab74a21132aeb7a8d559fe7a25e737f Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Wed, 12 Jun 2019 15:14:22 +0000 Subject: [PATCH 5/6] improve blurb and variable naming for GETTERS --- Lib/test/test_uuid.py | 12 ++++++------ Lib/uuid.py | 18 +++++++++--------- .../2019-04-11-07-59-43.bpo-28009.s85urF.rst | 4 ++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 8adf94dec3a541..92642d239b92cd 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -462,7 +462,7 @@ def test_uuid1_eui64(self): with unittest.mock.patch.multiple( self.uuid, _node=None, # Ignore any cached node value. - _NODE_GETTERS=[too_large_getter], + _GETTERS=[too_large_getter], ): node = self.uuid.getnode() self.assertTrue(0 < node < (1 << 48), '%012x' % node) @@ -707,31 +707,31 @@ def check_node(self, node, requires=None): self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) - @unittest.skipUnless(_uuid._ifconfig_getnode in _uuid._NODE_GETTERS, + @unittest.skipUnless(_uuid._ifconfig_getnode in _uuid._GETTERS, "ifconfig is not used for introspection on this platform") def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() self.check_node(node, 'ifconfig') - @unittest.skipUnless(_uuid._ip_getnode in _uuid._NODE_GETTERS, + @unittest.skipUnless(_uuid._ip_getnode in _uuid._GETTERS, "ip is not used for introspection on this platform") def test_ip_getnode(self): node = self.uuid._ip_getnode() self.check_node(node, 'ip') - @unittest.skipUnless(_uuid._arp_getnode in _uuid._NODE_GETTERS, + @unittest.skipUnless(_uuid._arp_getnode in _uuid._GETTERS, "arp is not used for introspection on this platform") def test_arp_getnode(self): node = self.uuid._arp_getnode() self.check_node(node, 'arp') - @unittest.skipUnless(_uuid._lanscan_getnode in _uuid._NODE_GETTERS, + @unittest.skipUnless(_uuid._lanscan_getnode in _uuid._GETTERS, "lanscan is not used for introspection on this platform") def test_lanscan_getnode(self): node = self.uuid._lanscan_getnode() self.check_node(node, 'lanscan') - @unittest.skipUnless(_uuid._netstat_getnode in _uuid._NODE_GETTERS, + @unittest.skipUnless(_uuid._netstat_getnode in _uuid._GETTERS, "netstat is not used for introspection on this platform") def test_netstat_getnode(self): node = self.uuid._netstat_getnode() diff --git a/Lib/uuid.py b/Lib/uuid.py index 023188855da58d..914e5ad1c969ba 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -681,22 +681,22 @@ def _random_getnode(): if _LINUX: - _GETTERS = [_ifconfig_getnode, _ip_getnode, _arp_getnode, _lanscan_getnode] + _OS_GETTERS = [_ifconfig_getnode, _ip_getnode, _arp_getnode, _lanscan_getnode] elif _DARWIN: - _GETTERS = [_arp_getnode, _ifconfig_getnode, _netstat_getnode] + _OS_GETTERS = [_arp_getnode, _ifconfig_getnode, _netstat_getnode] elif _WINDOWS: - _GETTERS = [_netbios_getnode, _ipconfig_getnode] + _OS_GETTERS = [_netbios_getnode, _ipconfig_getnode] elif _AIX: - _GETTERS = [_netstat_getnode] + _OS_GETTERS = [_netstat_getnode] else: - _GETTERS = [_ifconfig_getnode, _arp_getnode, _netstat_getnode, + _OS_GETTERS = [_ifconfig_getnode, _arp_getnode, _netstat_getnode, _lanscan_getnode, _ip_getnode] if os.name == 'posix': - _NODE_GETTERS = [_unix_getnode] + _GETTERS + _GETTERS = [_unix_getnode] + _OS_GETTERS elif os.name == 'nt': - _NODE_GETTERS = [_windll_getnode] + _GETTERS + _GETTERS = [_windll_getnode] + _OS_GETTERS else: - _NODE_GETTERS = _GETTERS + _GETTERS = _OS_GETTERS _node = None @@ -712,7 +712,7 @@ def getnode(*, getters=None): if _node is not None: return _node - for getter in _NODE_GETTERS + [_random_getnode]: + for getter in _GETTERS + [_random_getnode]: try: _node = getter() except: diff --git a/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst b/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst index 3a3775f8484e08..233640716d152b 100644 --- a/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst +++ b/Misc/NEWS.d/next/Tests/2019-04-11-07-59-43.bpo-28009.s85urF.rst @@ -1,3 +1,3 @@ -Fix the uuid test skipping logic based on whether a program -is available AND can be used to get a MACADDR as basis for an UUID. +Modify the test_uuid logic to test when a program is available +AND can be used to obtain a MACADDR as basis for an UUID. Patch by M. Felt From 2487d1b6059880836e05be44ce4f96c2be741546 Mon Sep 17 00:00:00 2001 From: Michael Felt Date: Thu, 13 Jun 2019 10:14:00 +0000 Subject: [PATCH 6/6] add documentation to code block --- Lib/uuid.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 914e5ad1c969ba..7aa01bb5c35506 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -680,17 +680,23 @@ def _random_getnode(): return random.getrandbits(48) | (1 << 40) +# _OS_GETTERS, when known, are targetted for a specific OS or platform. +# The order is by 'common practice' on the specified platform. +# Note: 'posix' and 'windows' _OS_GETTERS are prefixed by a dll/dlload() method +# which, when successful, means none of these "external" methods are called. +# _GETTERS is (also) used by test_uuid.py to SkipUnless(), e.g., +# @unittest.skipUnless(_uuid._ifconfig_getnode in _uuid._GETTERS, ...) if _LINUX: - _OS_GETTERS = [_ifconfig_getnode, _ip_getnode, _arp_getnode, _lanscan_getnode] + _OS_GETTERS = [_ip_getnode, _ifconfig_getnode] elif _DARWIN: - _OS_GETTERS = [_arp_getnode, _ifconfig_getnode, _netstat_getnode] + _OS_GETTERS = [_ifconfig_getnode, _arp_getnode, _netstat_getnode] elif _WINDOWS: _OS_GETTERS = [_netbios_getnode, _ipconfig_getnode] elif _AIX: _OS_GETTERS = [_netstat_getnode] else: - _OS_GETTERS = [_ifconfig_getnode, _arp_getnode, _netstat_getnode, - _lanscan_getnode, _ip_getnode] + _OS_GETTERS = [_ifconfig_getnode, _ip_getnode, _arp_getnode, + _netstat_getnode, _lanscan_getnode] if os.name == 'posix': _GETTERS = [_unix_getnode] + _OS_GETTERS elif os.name == 'nt':