From 3545caac56404e8ebe596f9ac11700f377b7c529 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Thu, 19 Oct 2017 17:44:33 -0400 Subject: [PATCH] Unit tests: ensure all threads complete A number of tests used to leave greenthreads that they had spawned running beyond the end of the test. This fixes those tests to not start unnecessary threads and/or to wait for all threads to complete before they finish. Change-Id: Ic0465dc25235ea3b6ec56646df1cb57878faf748 --- .../engine/service/test_service_engine.py | 10 ++--- .../tests/engine/service/test_stack_action.py | 2 +- heat/tests/engine/service/test_stack_adopt.py | 42 +++++++++---------- .../tests/engine/service/test_stack_create.py | 2 +- .../tests/engine/service/test_stack_delete.py | 22 ++++++---- .../tests/engine/service/test_stack_events.py | 2 +- .../engine/service/test_stack_snapshot.py | 2 +- 7 files changed, 41 insertions(+), 41 deletions(-) diff --git a/heat/tests/engine/service/test_service_engine.py b/heat/tests/engine/service/test_service_engine.py index b91aa9c2ba..8e00bda732 100644 --- a/heat/tests/engine/service/test_service_engine.py +++ b/heat/tests/engine/service/test_service_engine.py @@ -15,7 +15,6 @@ import datetime import mock from oslo_config import cfg -from oslo_service import threadgroup from oslo_utils import timeutils from heat.common import context @@ -309,6 +308,8 @@ class ServiceEngineTest(common.HeatTestCase): self.eng.thread_group_mgr.groups['sample-uuid2'] = dtg2 self.eng.service_id = 'sample-service-uuid' + self.patchobject(self.eng.manage_thread_grp, 'stop', + new=mock.Mock(wraps=self.eng.manage_thread_grp.stop)) orig_stop = self.eng.thread_group_mgr.stop with mock.patch.object(self.eng.thread_group_mgr, 'stop') as stop: @@ -328,7 +329,7 @@ class ServiceEngineTest(common.HeatTestCase): mock.call('sample-uuid2', True)] self.eng.thread_group_mgr.stop.assert_has_calls(calls, True) - # # Manage Thread group + # Manage Thread group self.eng.manage_thread_grp.stop.assert_called_with() # Service delete @@ -343,8 +344,6 @@ class ServiceEngineTest(common.HeatTestCase): '_stop_rpc_server') @mock.patch.object(worker.WorkerService, 'stop') - @mock.patch.object(threadgroup.ThreadGroup, - 'stop') @mock.patch('heat.common.context.get_admin_context', return_value=mock.Mock()) @mock.patch('heat.objects.service.Service.delete', @@ -353,7 +352,6 @@ class ServiceEngineTest(common.HeatTestCase): self, service_delete_method, admin_context_method, - thread_group_stop, worker_service_stop, rpc_server_stop): cfg.CONF.set_default('convergence_engine', True) @@ -363,7 +361,6 @@ class ServiceEngineTest(common.HeatTestCase): ) @mock.patch.object(service.EngineService, '_stop_rpc_server') - @mock.patch.object(threadgroup.ThreadGroup, 'stop') @mock.patch('heat.common.context.get_admin_context', return_value=mock.Mock()) @mock.patch('heat.objects.service.Service.delete', @@ -372,7 +369,6 @@ class ServiceEngineTest(common.HeatTestCase): self, service_delete_method, admin_context_method, - thread_group_stop, rpc_server_stop): cfg.CONF.set_default('convergence_engine', False) self._test_engine_service_stop( diff --git a/heat/tests/engine/service/test_stack_action.py b/heat/tests/engine/service/test_stack_action.py index 579c956171..1f01f3ac45 100644 --- a/heat/tests/engine/service/test_stack_action.py +++ b/heat/tests/engine/service/test_stack_action.py @@ -30,7 +30,7 @@ class StackServiceActionsTest(common.HeatTestCase): super(StackServiceActionsTest, self).setUp() self.ctx = utils.dummy_context() self.man = service.EngineService('a-host', 'a-topic') - self.man.create_periodic_tasks() + self.man.thread_group_mgr = service.ThreadGroupManager() @mock.patch.object(stack.Stack, 'load') @mock.patch.object(service.ThreadGroupManager, 'start') diff --git a/heat/tests/engine/service/test_stack_adopt.py b/heat/tests/engine/service/test_stack_adopt.py index 15b095db92..735d02023c 100644 --- a/heat/tests/engine/service/test_stack_adopt.py +++ b/heat/tests/engine/service/test_stack_adopt.py @@ -21,7 +21,6 @@ from heat.engine import service from heat.engine import stack as parser from heat.objects import stack as stack_object from heat.tests import common -from heat.tests.engine import tools from heat.tests import utils @@ -31,7 +30,7 @@ class StackServiceAdoptTest(common.HeatTestCase): super(StackServiceAdoptTest, self).setUp() self.ctx = utils.dummy_context() self.man = service.EngineService('a-host', 'a-topic') - self.man.thread_group_mgr = tools.DummyThreadGroupManager() + self.man.thread_group_mgr = service.ThreadGroupManager() def _get_adopt_data_and_template(self, environment=None): template = { @@ -55,14 +54,20 @@ class StackServiceAdoptTest(common.HeatTestCase): "metadata": {}}}} return template, adopt_data + def _do_adopt(self, stack_name, template, input_params, adopt_data): + result = self.man.create_stack(self.ctx, stack_name, + template, input_params, None, + {'adopt_stack_data': str(adopt_data)}) + self.man.thread_group_mgr.stop(result['stack_id'], graceful=True) + return result + def test_stack_adopt_with_params(self): cfg.CONF.set_override('enable_stack_adopt', True) cfg.CONF.set_override('convergence_engine', False) env = {'parameters': {"app_dbx": "test"}} template, adopt_data = self._get_adopt_data_and_template(env) - result = self.man.create_stack(self.ctx, "test_adopt_with_params", - template, {}, None, - {'adopt_stack_data': str(adopt_data)}) + result = self._do_adopt("test_adopt_with_params", template, {}, + adopt_data) stack = stack_object.Stack.get_by_id(self.ctx, result['stack_id']) self.assertEqual(template, stack.raw_template.template) @@ -78,9 +83,8 @@ class StackServiceAdoptTest(common.HeatTestCase): cfg.CONF.set_override('convergence_engine', True) env = {'parameters': {"app_dbx": "test"}} template, adopt_data = self._get_adopt_data_and_template(env) - result = self.man.create_stack(self.ctx, "test_adopt_with_params", - template, {}, None, - {'adopt_stack_data': str(adopt_data)}) + result = self._do_adopt("test_adopt_with_params", template, {}, + adopt_data) stack = stack_object.Stack.get_by_id(self.ctx, result['stack_id']) self.assertEqual(template, stack.raw_template.template) @@ -96,9 +100,8 @@ class StackServiceAdoptTest(common.HeatTestCase): "parameters": {"app_dbx": "bar"} } template, adopt_data = self._get_adopt_data_and_template(env) - result = self.man.create_stack(self.ctx, "test_adopt_saves_inputs", - template, input_params, None, - {'adopt_stack_data': str(adopt_data)}) + result = self._do_adopt("test_adopt_saves_inputs", template, + input_params, adopt_data) stack = stack_object.Stack.get_by_id(self.ctx, result['stack_id']) self.assertEqual(template, stack.raw_template.template) @@ -116,9 +119,8 @@ class StackServiceAdoptTest(common.HeatTestCase): "parameters": {"app_dbx": "bar"} } template, adopt_data = self._get_adopt_data_and_template(env) - result = self.man.create_stack(self.ctx, "test_adopt_saves_inputs", - template, input_params, None, - {'adopt_stack_data': str(adopt_data)}) + result = self._do_adopt("test_adopt_saves_inputs", template, + input_params, adopt_data) stack = stack_object.Stack.get_by_id(self.ctx, result['stack_id']) self.assertEqual(template, stack.raw_template.template) @@ -131,12 +133,11 @@ class StackServiceAdoptTest(common.HeatTestCase): cfg.CONF.set_override('convergence_engine', False) env = {'parameters': {"app_dbx": "test"}} template, adopt_data = self._get_adopt_data_and_template(env) - result = self.man.create_stack(self.ctx, "test_adopt_stack_state", - template, {}, None, - {'adopt_stack_data': str(adopt_data)}) + result = self._do_adopt("test_adopt_stack_state", template, {}, + adopt_data) stack = stack_object.Stack.get_by_id(self.ctx, result['stack_id']) - self.assertEqual((parser.Stack.ADOPT, parser.Stack.IN_PROGRESS), + self.assertEqual((parser.Stack.ADOPT, parser.Stack.COMPLETE), (stack.action, stack.status)) @mock.patch.object(parser.Stack, '_converge_create_or_update') @@ -147,9 +148,8 @@ class StackServiceAdoptTest(common.HeatTestCase): cfg.CONF.set_override('convergence_engine', True) env = {'parameters': {"app_dbx": "test"}} template, adopt_data = self._get_adopt_data_and_template(env) - result = self.man.create_stack(self.ctx, "test_adopt_stack_state", - template, {}, None, - {'adopt_stack_data': str(adopt_data)}) + result = self._do_adopt("test_adopt_stack_state", template, {}, + adopt_data) stack = stack_object.Stack.get_by_id(self.ctx, result['stack_id']) self.assertEqual((parser.Stack.ADOPT, parser.Stack.IN_PROGRESS), diff --git a/heat/tests/engine/service/test_stack_create.py b/heat/tests/engine/service/test_stack_create.py index 7a968fd83b..a58aac27a4 100644 --- a/heat/tests/engine/service/test_stack_create.py +++ b/heat/tests/engine/service/test_stack_create.py @@ -38,7 +38,7 @@ class StackCreateTest(common.HeatTestCase): super(StackCreateTest, self).setUp() self.ctx = utils.dummy_context() self.man = service.EngineService('a-host', 'a-topic') - self.man.create_periodic_tasks() + self.man.thread_group_mgr = service.ThreadGroupManager() @mock.patch.object(threadgroup, 'ThreadGroup') @mock.patch.object(stack.Stack, 'validate') diff --git a/heat/tests/engine/service/test_stack_delete.py b/heat/tests/engine/service/test_stack_delete.py index 418c9018e6..04ebd4aae0 100644 --- a/heat/tests/engine/service/test_stack_delete.py +++ b/heat/tests/engine/service/test_stack_delete.py @@ -33,7 +33,7 @@ class StackDeleteTest(common.HeatTestCase): super(StackDeleteTest, self).setUp() self.ctx = utils.dummy_context() self.man = service.EngineService('a-host', 'a-topic') - self.man.create_periodic_tasks() + self.man.thread_group_mgr = service.ThreadGroupManager() @mock.patch.object(parser.Stack, 'load') def test_stack_delete(self, mock_load): @@ -117,16 +117,20 @@ class StackDeleteTest(common.HeatTestCase): mock_load.return_value = stack mock_try.return_value = self.man.engine_id - mock_stop = self.patchobject(self.man.thread_group_mgr, 'stop') mock_send = self.patchobject(self.man.thread_group_mgr, 'send') mock_expired.side_effect = [False, True] - self.assertIsNone(self.man.delete_stack(self.ctx, stack.identifier())) - self.man.thread_group_mgr.groups[sid].wait() + with mock.patch.object(self.man.thread_group_mgr, 'stop') as mock_stop: + self.assertIsNone(self.man.delete_stack(self.ctx, + stack.identifier())) + self.man.thread_group_mgr.groups[sid].wait() + + mock_load.assert_called_with(self.ctx, stack=st) + mock_send.assert_called_once_with(stack.id, 'cancel') + mock_stop.assert_called_once_with(stack.id) + + self.man.thread_group_mgr.stop(sid, graceful=True) - mock_load.assert_called_with(self.ctx, stack=st) - mock_send.assert_called_once_with(stack.id, 'cancel') - mock_stop.assert_called_once_with(stack.id) self.assertEqual(2, len(mock_load.mock_calls)) mock_try.assert_called_with() mock_acquire.assert_called_once_with(True) @@ -197,7 +201,7 @@ class StackDeleteTest(common.HeatTestCase): return_value=None) self.assertIsNone(self.man.delete_stack(self.ctx, stack.identifier())) - self.man.thread_group_mgr.groups[sid].wait() + self.man.thread_group_mgr.stop(sid, graceful=True) self.assertEqual(2, len(mock_load.mock_calls)) mock_load.assert_called_with(self.ctx, stack=st) @@ -236,7 +240,7 @@ class StackDeleteTest(common.HeatTestCase): mock_expired.side_effect = [False, True] self.assertIsNone(self.man.delete_stack(self.ctx, stack.identifier())) - self.man.thread_group_mgr.groups[sid].wait() + self.man.thread_group_mgr.stop(sid, graceful=True) mock_load.assert_called_with(self.ctx, stack=st) mock_try.assert_called_with() diff --git a/heat/tests/engine/service/test_stack_events.py b/heat/tests/engine/service/test_stack_events.py index 8643b590cf..1c3bfa1e34 100644 --- a/heat/tests/engine/service/test_stack_events.py +++ b/heat/tests/engine/service/test_stack_events.py @@ -32,7 +32,7 @@ class StackEventTest(common.HeatTestCase): self.ctx = utils.dummy_context(tenant_id='stack_event_test_tenant') self.eng = service.EngineService('a-host', 'a-topic') - self.eng.create_periodic_tasks() + self.eng.thread_group_mgr = service.ThreadGroupManager() @tools.stack_context('service_event_list_test_stack') @mock.patch.object(service.EngineService, '_get_stack') diff --git a/heat/tests/engine/service/test_stack_snapshot.py b/heat/tests/engine/service/test_stack_snapshot.py index d0a2d6c554..86bae3ed7f 100644 --- a/heat/tests/engine/service/test_stack_snapshot.py +++ b/heat/tests/engine/service/test_stack_snapshot.py @@ -34,7 +34,7 @@ class SnapshotServiceTest(common.HeatTestCase): self.ctx = utils.dummy_context() self.engine = service.EngineService('a-host', 'a-topic') - self.engine.create_periodic_tasks() + self.engine.thread_group_mgr = service.ThreadGroupManager() def _create_stack(self, stack_name, files=None): t = template_format.parse(tools.wp_template)