From cb1e856b9f04e17a7465a2cce5eb422d72736836 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 10 Jan 2023 14:29:37 +0100 Subject: [PATCH] Rename NodeInfo._lock to avoid conflict with Mock._lock in tests Starting with https://github.com/python/cpython/pull/98797, Python's Mock has its own _lock. I hope they rename it to something really private (e.g. __lock), but for now rename our attribute (and hope that no downstream plugins relied on it, sigh). Change-Id: I7ba858fb3f259b8e7a3becde94b7ba6b90615287 --- ironic_inspector/node_cache.py | 16 ++++---- ironic_inspector/test/unit/test_introspect.py | 1 + ironic_inspector/test/unit/test_node_cache.py | 38 +++++++++---------- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 82444bec..49a241c8 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -75,16 +75,16 @@ def __init__(self, uuid, version_id=None, state=None, started_at=None, # equivalent to True actually. self._manage_boot = manage_boot if manage_boot is not None else True # This is a lock on a node UUID, not on a NodeInfo object - self._lock = locking.get_lock(uuid) + self._node_lock = locking.get_lock(uuid) # Whether lock was acquired using this NodeInfo object self._fsm = None self._options = None def __del__(self): - if self._lock.is_locked(): + if self._node_lock.is_locked(): LOG.warning('BUG: node lock was not released by the moment ' 'node info object is deleted') - self._lock.release() + self._node_lock.release() def __str__(self): """Self represented as an UUID and a state.""" @@ -103,13 +103,13 @@ def acquire_lock(self, blocking=True): return immediately. :returns: boolean value, whether lock was acquired successfully """ - if self._lock.is_locked(): + if self._node_lock.is_locked(): LOG.debug('Attempting to acquire lock already held', node_info=self) return True LOG.debug('Attempting to acquire lock', node_info=self) - if self._lock.acquire(blocking): + if self._node_lock.acquire(blocking): LOG.debug('Successfully acquired lock', node_info=self) return True else: @@ -121,9 +121,9 @@ def release_lock(self): Does nothing if lock was not acquired using this NodeInfo object. """ - if self._lock.is_locked(): + if self._node_lock.is_locked(): LOG.debug('Successfully released lock', node_info=self) - self._lock.release() + self._node_lock.release() @property def version_id(self): @@ -640,7 +640,7 @@ def inner(node_info, *args, **kwargs): finally: # FIXME(milan) hacking the test cases to work # with release_lock.assert_called_once... - if node_info._lock.is_locked(): + if node_info._node_lock.is_locked(): node_info.release_lock() return inner diff --git a/ironic_inspector/test/unit/test_introspect.py b/ironic_inspector/test/unit/test_introspect.py index 8f8d6396..2f9e7653 100644 --- a/ironic_inspector/test/unit/test_introspect.py +++ b/ironic_inspector/test/unit/test_introspect.py @@ -41,6 +41,7 @@ def setUp(self): self.node_info = mock.Mock(uuid=self.uuid, options={}) self.node_info.ports.return_value = self.ports_dict self.node_info.node.return_value = self.node + driver_fixture = self.useFixture(fixtures.MockPatchObject( pxe_filter, 'driver', autospec=True)) driver_mock = driver_fixture.mock.return_value diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index 0c068ffb..cd8a8bab 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/ironic_inspector/test/unit/test_node_cache.py @@ -55,7 +55,7 @@ def test_add_node(self): (datetime.datetime.utcnow() - datetime.timedelta(seconds=60) < node.started_at < datetime.datetime.utcnow() + datetime.timedelta(seconds=60))) - self.assertFalse(node._lock.is_locked()) + self.assertFalse(node._node_lock.is_locked()) res = set((r.uuid, r.started_at) for r in db.get_nodes()) @@ -196,7 +196,7 @@ def test_bmc(self): datetime.datetime.utcnow() - datetime.timedelta(seconds=60) < res.started_at < datetime.datetime.utcnow() + datetime.timedelta(seconds=1)) - self.assertTrue(res._lock.is_locked()) + self.assertTrue(res._node_lock.is_locked()) def test_same_bmc_different_macs(self): uuid2 = uuidutils.generate_uuid() @@ -227,7 +227,7 @@ def test_macs(self): datetime.datetime.utcnow() - datetime.timedelta(seconds=60) < res.started_at < datetime.datetime.utcnow() + datetime.timedelta(seconds=1)) - self.assertTrue(res._lock.is_locked()) + self.assertTrue(res._node_lock.is_locked()) def test_macs_not_found(self): self.assertRaises(utils.Error, node_cache.find_node, @@ -250,7 +250,7 @@ def test_both(self): datetime.datetime.utcnow() - datetime.timedelta(seconds=60) < res.started_at < datetime.datetime.utcnow() + datetime.timedelta(seconds=1)) - self.assertTrue(res._lock.is_locked()) + self.assertTrue(res._node_lock.is_locked()) def test_inconsistency(self): db.delete_node(uuid=self.uuid) @@ -388,7 +388,7 @@ def test_ok(self): self.assertEqual(started_at, info.started_at) self.assertIsNone(info.finished_at) self.assertIsNone(info.error) - self.assertFalse(info._lock.is_locked()) + self.assertFalse(info._node_lock.is_locked()) def test_not_found(self): self.assertRaises(utils.Error, node_cache.get_node, @@ -410,7 +410,7 @@ def test_with_name(self): self.assertEqual(started_at, info.started_at) self.assertIsNone(info.finished_at) self.assertIsNone(info.error) - self.assertFalse(info._lock.is_locked()) + self.assertFalse(info._node_lock.is_locked()) ironic.get_node.assert_called_once_with('name') @@ -448,7 +448,7 @@ def test_error(self): def test_release_lock(self): self.node_info.acquire_lock() self.node_info.finished(istate.Events.finish) - self.assertFalse(self.node_info._lock.is_locked()) + self.assertFalse(self.node_info._node_lock.is_locked()) class TestNodeInfoOptions(test_base.NodeTestBase): @@ -812,40 +812,40 @@ class TestInternalLock(test_base.NodeTestBase): def test_acquire(self, lock_mock): node_info = node_cache.NodeInfo(self.uuid) self.addCleanup(node_info.release_lock) - self.assertFalse(node_info._lock.is_locked()) + self.assertFalse(node_info._node_lock.is_locked()) lock_mock.assert_called_once_with('node-{}'.format(self.uuid), semaphores=mock.ANY) self.assertFalse(lock_mock.return_value.acquire.called) self.assertTrue(node_info.acquire_lock()) - self.assertTrue(node_info._lock.is_locked()) + self.assertTrue(node_info._node_lock.is_locked()) self.assertTrue(node_info.acquire_lock()) - self.assertTrue(node_info._lock.is_locked()) + self.assertTrue(node_info._node_lock.is_locked()) lock_mock.return_value.acquire.assert_called_once_with(blocking=True) def test_release(self, lock_mock): node_info = node_cache.NodeInfo(self.uuid) node_info.acquire_lock() - self.assertTrue(node_info._lock.is_locked()) + self.assertTrue(node_info._node_lock.is_locked()) node_info.release_lock() - self.assertFalse(node_info._lock.is_locked()) + self.assertFalse(node_info._node_lock.is_locked()) node_info.release_lock() - self.assertFalse(node_info._lock.is_locked()) + self.assertFalse(node_info._node_lock.is_locked()) lock_mock.return_value.acquire.assert_called_once_with(blocking=True) lock_mock.return_value.release.assert_called_once_with() def test_acquire_non_blocking(self, lock_mock): node_info = node_cache.NodeInfo(self.uuid) self.addCleanup(node_info.release_lock) - self.assertFalse(node_info._lock.is_locked()) + self.assertFalse(node_info._node_lock.is_locked()) lock_mock.return_value.acquire.side_effect = iter([False, True]) self.assertFalse(node_info.acquire_lock(blocking=False)) - self.assertFalse(node_info._lock.is_locked()) + self.assertFalse(node_info._node_lock.is_locked()) self.assertTrue(node_info.acquire_lock(blocking=False)) - self.assertTrue(node_info._lock.is_locked()) + self.assertTrue(node_info._node_lock.is_locked()) self.assertTrue(node_info.acquire_lock(blocking=False)) - self.assertTrue(node_info._lock.is_locked()) + self.assertTrue(node_info._node_lock.is_locked()) lock_mock.return_value.acquire.assert_called_with(blocking=False) self.assertEqual(2, lock_mock.return_value.acquire.call_count) @@ -1123,7 +1123,7 @@ def function(node_info): def test_unlock(self): @node_cache.release_lock def func(node_info): - self.assertTrue(node_info._lock.is_locked()) + self.assertTrue(node_info._node_lock.is_locked()) self.node_info.acquire_lock(blocking=True) with mock.patch.object(self.node_info, 'release_lock', @@ -1134,7 +1134,7 @@ def func(node_info): def test_unlock_unlocked(self): @node_cache.release_lock def func(node_info): - self.assertFalse(node_info._lock.is_locked()) + self.assertFalse(node_info._node_lock.is_locked()) self.node_info.release_lock() with mock.patch.object(self.node_info, 'release_lock',