Fixing environment tests

* As a side effect the bug related to oslo.messaging (AttributeError: ...)
  was also fixed by moving monkey patching to mistral/__init__.py. Monkey
  patching has to be done before importing oslo.messaging. Details can be
  found at https://bugs.launchpad.net/oslo.messaging/+bug/1288878
* Refactored environment engine tests
* Refactored and fixed environment API tests (there was a number of
  fragile dict string representation comparisons)

Closes-Bug: #1337705

Change-Id: Id725570dfc7148f010647c4b2487882174b0c56e

mistral/__init__.py

@@ -0,0 +1,25 @@
+# Copyright 2015 - Mirantis, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License");
+#    you may not use this file except in compliance with the License.
+#    You may obtain a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS,
+#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#    See the License for the specific language governing permissions and
+#    limitations under the License.
+
+import eventlet
+import sys
+
+
+eventlet.monkey_patch(
+    os=True,
+    select=True,
+    socket=True,
+    thread=False if '--use-debugger' in sys.argv else True,
+    time=True
+)

mistral/tests/unit/api/v2/test_environment.py

@@ -45,7 +45,7 @@ ENVIRONMENT = {
     'id': str(uuid.uuid4()),
     'name': 'test',
     'description': 'my test settings',
-    'variables': json.dumps(VARIABLES),
+    'variables': VARIABLES,
     'scope': 'private',
     'created_at': str(datetime.datetime.utcnow()),
     'updated_at': str(datetime.datetime.utcnow())
@@ -81,34 +81,50 @@ MOCK_DUPLICATE = mock.MagicMock(side_effect=exc.DBDuplicateEntry())
 MOCK_DELETE = mock.MagicMock(return_value=None)
 
 
+def _convert_vars_to_dict(env_dict):
+    """Converts 'variables' in the given environment dict into dictionary."""
+    if ('variables' in env_dict and
+            isinstance(env_dict.get('variables'), basestring)):
+        env_dict['variables'] = json.loads(env_dict['variables'])
+
+    return env_dict
+
+
+def _convert_vars_to_string(env_dict):
+    """Converts 'variables' in the given environment dict into string."""
+    if ('variables' in env_dict and
+            isinstance(env_dict.get('variables'), dict)):
+        env_dict['variables'] = json.dumps(env_dict['variables'])
+
+    return env_dict
+
+
 class TestEnvironmentController(base.FunctionalTest):
 
-    def _assert_dict_equal(self, actual, expected):
+    def _assert_dict_equal(self, expected, actual):
-        self.assertIsInstance(actual, dict)
         self.assertIsInstance(expected, dict)
+        self.assertIsInstance(actual, dict)
 
-        if (actual.get('variables') and
+        _convert_vars_to_dict(expected)
-                isinstance(actual.get('variables'), basestring)):
+        _convert_vars_to_dict(actual)
-            actual['variables'] = json.loads(actual['variables'])
 
-        if (expected.get('variables') and
+        self.assertDictEqual(expected, actual)
-                isinstance(expected.get('variables'), basestring)):
-            expected['variables'] = json.loads(expected['variables'])
-
-        self.assertDictEqual(actual, expected)
 
     def test_resource(self):
         resource = api.Environment(**copy.deepcopy(ENVIRONMENT))
 
-        actual = resource.to_dict()
+        self._assert_dict_equal(
-        expected = copy.deepcopy(ENVIRONMENT)
+            copy.deepcopy(ENVIRONMENT),
-
+            resource.to_dict()
-        self._assert_dict_equal(actual, expected)
+        )
 
     def test_resource_to_db_model(self):
-        resource = api.Environment(**copy.deepcopy(ENVIRONMENT))
+        resource = api.Environment(
+            **_convert_vars_to_string(copy.deepcopy(ENVIRONMENT))
+        )
 
         values = resource.to_dict()
+
         values['variables'] = json.loads(values['variables'])
         values['created_at'] = datetime.datetime.strptime(
             values['created_at'], DATETIME_FORMAT)
@@ -116,82 +132,86 @@ class TestEnvironmentController(base.FunctionalTest):
             values['updated_at'], DATETIME_FORMAT)
 
         db_model = db.Environment(**values)
-        with db_api.transaction():
-            db_api.create_environment(db_model)
 
-        self.assertEqual(db_model.id, values['id'])
+        db_api.create_environment(db_model)
-        self.assertEqual(db_model.name, values['name'])
+
+        self.assertEqual(values['id'], db_model.id)
+        self.assertEqual(values['name'], db_model.name)
         self.assertIsNone(db_model.project_id)
-        self.assertEqual(db_model.description, values['description'])
+        self.assertEqual(values['description'], db_model.description)
-        self.assertDictEqual(db_model.variables, values['variables'])
+        self.assertDictEqual(values['variables'], db_model.variables)
-        self.assertEqual(db_model.created_at, values['created_at'])
+        self.assertEqual(values['created_at'], db_model.created_at)
-        self.assertEqual(db_model.updated_at, values['updated_at'])
+        self.assertEqual(values['updated_at'], db_model.updated_at)
 
     @mock.patch.object(db_api, 'get_environments', MOCK_ENVIRONMENTS)
     def test_get_all(self):
         resp = self.app.get('/v2/environments')
 
-        self.assertEqual(resp.status_int, 200)
+        self.assertEqual(200, resp.status_int)
-        self.assertEqual(len(resp.json['environments']), 1)
+        self.assertEqual(1, len(resp.json['environments']))
 
     def test_get_all_empty(self):
         resp = self.app.get('/v2/environments')
 
-        self.assertEqual(resp.status_int, 200)
+        self.assertEqual(200, resp.status_int)
-        self.assertEqual(len(resp.json['environments']), 0)
+        self.assertEqual(0, len(resp.json['environments']))
 
     @mock.patch.object(db_api, 'get_environment', MOCK_ENVIRONMENT)
     def test_get(self):
         resp = self.app.get('/v2/environments/123')
 
-        self.assertEqual(resp.status_int, 200)
+        self.assertEqual(200, resp.status_int)
-        self.assertDictEqual(ENVIRONMENT, resp.json)
+        self._assert_dict_equal(ENVIRONMENT, resp.json)
 
     @mock.patch.object(db_api, "get_environment", MOCK_NOT_FOUND)
     def test_get_not_found(self):
         resp = self.app.get('/v2/environments/123', expect_errors=True)
 
-        self.assertEqual(resp.status_int, 404)
+        self.assertEqual(404, resp.status_int)
 
     @mock.patch.object(db_api, 'create_environment', MOCK_ENVIRONMENT)
     def test_post(self):
         resp = self.app.post_json(
             '/v2/environments',
-            copy.deepcopy(ENVIRONMENT))
+            _convert_vars_to_string(copy.deepcopy(ENVIRONMENT))
+        )
 
-        self.assertEqual(resp.status_int, 201)
+        self.assertEqual(201, resp.status_int)
 
-        self._assert_dict_equal(resp.json, copy.deepcopy(ENVIRONMENT))
+        self._assert_dict_equal(copy.deepcopy(ENVIRONMENT), resp.json)
 
     @mock.patch.object(db_api, 'create_environment', MOCK_DUPLICATE)
     def test_post_dup(self):
         resp = self.app.post_json(
             '/v2/environments',
-            copy.deepcopy(ENVIRONMENT),
+            _convert_vars_to_string(copy.deepcopy(ENVIRONMENT)),
-            expect_errors=True)
+            expect_errors=True
+        )
 
-        self.assertEqual(resp.status_int, 409)
+        self.assertEqual(409, resp.status_int)
 
     @mock.patch.object(db_api, 'create_environment', MOCK_ENVIRONMENT)
     def test_post_default_scope(self):
-        env = copy.deepcopy(ENVIRONMENT)
+        env = _convert_vars_to_string(copy.deepcopy(ENVIRONMENT))
+
         del env['scope']
 
         resp = self.app.post_json('/v2/environments', env)
 
-        self.assertEqual(resp.status_int, 201)
+        self.assertEqual(201, resp.status_int)
 
-        self._assert_dict_equal(resp.json, copy.deepcopy(ENVIRONMENT))
+        self._assert_dict_equal(copy.deepcopy(ENVIRONMENT), resp.json)
 
     @mock.patch.object(db_api, 'update_environment', MOCK_UPDATED_ENVIRONMENT)
     def test_put(self):
         resp = self.app.put_json(
             '/v2/environments',
-            copy.deepcopy(UPDATED_ENVIRONMENT))
+            copy.deepcopy(UPDATED_ENVIRONMENT)
+        )
 
-        self.assertEqual(resp.status_int, 200)
+        self.assertEqual(200, resp.status_int)
 
-        self._assert_dict_equal(resp.json, copy.deepcopy(UPDATED_ENVIRONMENT))
+        self._assert_dict_equal(copy.deepcopy(UPDATED_ENVIRONMENT), resp.json)
 
     @mock.patch.object(db_api, 'update_environment', MOCK_UPDATED_ENVIRONMENT)
     def test_put_default_scope(self):
@@ -200,27 +220,28 @@ class TestEnvironmentController(base.FunctionalTest):
 
         resp = self.app.put_json('/v2/environments', env)
 
-        self.assertEqual(resp.status_int, 200)
+        self.assertEqual(200, resp.status_int)
 
-        self._assert_dict_equal(resp.json, copy.deepcopy(UPDATED_ENVIRONMENT))
+        self._assert_dict_equal(copy.deepcopy(UPDATED_ENVIRONMENT), resp.json)
 
     @mock.patch.object(db_api, 'update_environment', MOCK_NOT_FOUND)
     def test_put_not_found(self):
         resp = self.app.put_json(
             '/v2/environments/test',
             copy.deepcopy(UPDATED_ENVIRONMENT),
-            expect_errors=True)
+            expect_errors=True
+        )
 
-        self.assertEqual(resp.status_int, 404)
+        self.assertEqual(404, resp.status_int)
 
     @mock.patch.object(db_api, 'delete_environment', MOCK_DELETE)
     def test_delete(self):
         resp = self.app.delete('/v2/environments/123')
 
-        self.assertEqual(resp.status_int, 204)
+        self.assertEqual(204, resp.status_int)
 
     @mock.patch.object(db_api, 'delete_environment', MOCK_NOT_FOUND)
     def test_delete_not_found(self):
         resp = self.app.delete('/v2/environments/123', expect_errors=True)
 
-        self.assertEqual(resp.status_int, 404)
+        self.assertEqual(404, resp.status_int)

mistral/tests/unit/engine1/base.py

@@ -12,8 +12,6 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
-import sys
-
 import eventlet
 from oslo.config import cfg
 from oslo import messaging
@@ -28,14 +26,6 @@ from mistral.tests import base
 from mistral.workflow import states
 
 
-eventlet.monkey_patch(
-    os=True,
-    select=True,
-    socket=True,
-    thread=False if '--use-debugger' in sys.argv else True,
-    time=True
-)
-
 LOG = logging.getLogger(__name__)
 
 

mistral/tests/unit/engine1/test_environment.py

@@ -21,6 +21,7 @@ from mistral.openstack.common import log as logging
 from mistral.services import workbooks as wb_service
 from mistral.tests.unit.engine1 import base
 
+
 LOG = logging.getLogger(__name__)
 
 # Use the set_default method to set value otherwise in certain test cases
@@ -33,38 +34,47 @@ TARGET = '10.1.15.251'
 WORKBOOK = """
 ---
 version: '2.0'
+
 name: my_wb
+
 workflows:
   wf1:
     type: reverse
+
     input:
       - param1
       - param2
+
     output:
       final_result: $.final_result
+
     tasks:
       task1:
         action: std.echo output='{$.param1}'
         target: $.__env.var1
         publish:
           result1: $
+
       task2:
+        requires: [task1]
         action: std.echo output="'{$.result1} & {$.param2}'"
         target: $.__env.var1
         publish:
           final_result: $
-        requires: [task1]
+
   wf2:
     type: direct
+
     output:
       slogan: $.slogan
+
     tasks:
       task1:
         workflow: wf1
         input:
-            param1: $.__env.var2
+          param1: $.__env.var2
-            param2: $.__env.var3
+          param2: $.__env.var3
-            task_name: "task2"
+          task_name: task2
         publish:
           slogan: "{$.final_result} is a cool {$.__env.var4}!"
 """
@@ -94,9 +104,11 @@ class SubworkflowsTest(base.EngineTestCase):
 
     @mock.patch.object(rpc.ExecutorClient, 'run_action', MOCK_RUN_AT_TARGET)
     def _test_subworkflow(self, env):
-        exec1_db = self.engine.start_workflow('my_wb.wf2',
+        exec1_db = self.engine.start_workflow(
-                                              None,
+            'my_wb.wf2',
-                                              environment=env)
+            None,
+            environment=env
+        )
 
         # Execution 1.
         self.assertIsNotNone(exec1_db)
@@ -148,13 +160,18 @@ class SubworkflowsTest(base.EngineTestCase):
 
         # Check if target is resolved.
         tasks_exec2 = db_api.get_tasks(execution_id=exec2_db.id)
-        self._assert_single_item(tasks_exec2, name="task1")
+
-        self._assert_single_item(tasks_exec2, name="task2")
+        self._assert_single_item(tasks_exec2, name='task1')
+        self._assert_single_item(tasks_exec2, name='task2')
 
         for task in tasks_exec2:
             rpc.ExecutorClient.run_action.assert_any_call(
-                task.id, 'mistral.actions.std_actions.EchoAction', {},
+                task.id,
-                task.input, TARGET)
+                'mistral.actions.std_actions.EchoAction',
+                {},
+                task.input,
+                TARGET
+            )
 
     def test_subworkflow_env_task_input(self):
         env = {