From 269d92b19cf646308fe3ad93feac2bff0e3ad9c7 Mon Sep 17 00:00:00 2001 From: Kaifeng Wang Date: Mon, 6 Aug 2018 17:03:36 +0800 Subject: [PATCH] Fix lock leaks in unit tests Following message was seen during unit tests: "BUG: node lock was not released by the moment node info object is deleted." Though the bug was marked invalid before, apparently it had never been resolved. Tests involved in this patch were located by saving stack at NodeInfo.__init__ and printing out at NodeInfo.__del__, without promises, but hopefully covers all tests in current repo. Change-Id: I678407b350bd07926eb98db30f24cc83747fa84c Story: #1533595 Task: #11296 --- ironic_inspector/test/unit/test_node_cache.py | 9 ++++++++- ironic_inspector/test/unit/test_process.py | 5 +++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index 3b8d27928..8bddf32c8 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/ironic_inspector/test/unit/test_node_cache.py @@ -237,6 +237,7 @@ class TestNodeCacheFind(test_base.NodeTest): def test_bmc(self): res = node_cache.find_node(bmc_address='1.2.3.4') + self.addCleanup(res.release_lock) self.assertEqual(self.uuid, res.uuid) self.assertTrue( datetime.datetime.utcnow() - datetime.timedelta(seconds=60) @@ -251,8 +252,10 @@ class TestNodeCacheFind(test_base.NodeTest): bmc_address='1.2.3.4', mac=self.macs2) res = node_cache.find_node(bmc_address='1.2.3.4', mac=self.macs) + self.addCleanup(res.release_lock) self.assertEqual(self.uuid, res.uuid) res = node_cache.find_node(bmc_address='1.2.3.4', mac=self.macs2) + self.addCleanup(res.release_lock) self.assertEqual(uuid2, res.uuid) def test_same_bmc_raises(self): @@ -265,6 +268,7 @@ class TestNodeCacheFind(test_base.NodeTest): def test_macs(self): res = node_cache.find_node(mac=['11:22:33:33:33:33', self.macs[1]]) + self.addCleanup(res.release_lock) self.assertEqual(self.uuid, res.uuid) self.assertTrue( datetime.datetime.utcnow() - datetime.timedelta(seconds=60) @@ -287,6 +291,7 @@ class TestNodeCacheFind(test_base.NodeTest): def test_both(self): res = node_cache.find_node(bmc_address='1.2.3.4', mac=self.macs) + self.addCleanup(res.release_lock) self.assertEqual(self.uuid, res.uuid) self.assertTrue( datetime.datetime.utcnow() - datetime.timedelta(seconds=60) @@ -459,7 +464,7 @@ class TestNodeCacheGetNode(test_base.NodeTest): state=istate.States.starting, started_at=started_at).save(session) info = node_cache.get_node(self.uuid, locked=True) - + self.addCleanup(info.release_lock) self.assertEqual(self.uuid, info.uuid) self.assertEqual(started_at, info.started_at) self.assertIsNone(info.finished_at) @@ -852,6 +857,7 @@ class TestNodeCacheGetByPath(test_base.NodeTest): class TestLock(test_base.NodeTest): def test_acquire(self, get_lock_mock): node_info = node_cache.NodeInfo(self.uuid) + self.addCleanup(node_info.release_lock) self.assertFalse(node_info._locked) get_lock_mock.assert_called_once_with(self.uuid) self.assertFalse(get_lock_mock.return_value.acquire.called) @@ -875,6 +881,7 @@ class TestLock(test_base.NodeTest): def test_acquire_non_blocking(self, get_lock_mock): node_info = node_cache.NodeInfo(self.uuid) + self.addCleanup(node_info.release_lock) self.assertFalse(node_info._locked) get_lock_mock.return_value.acquire.side_effect = iter([False, True]) diff --git a/ironic_inspector/test/unit/test_process.py b/ironic_inspector/test/unit/test_process.py index 2ace74eba..a23d0b193 100644 --- a/ironic_inspector/test/unit/test_process.py +++ b/ironic_inspector/test/unit/test_process.py @@ -74,6 +74,11 @@ class BaseProcessTest(BaseTest): self.cli.node.get.return_value = self.node self.process_mock = self.process_fixture.mock self.process_mock.return_value = self.fake_result_json + self.addCleanup(self._cleanup_lock, self.node_info) + + def _cleanup_lock(self, node_info): + if node_info._locked: + node_info.release_lock() class TestProcess(BaseProcessTest):