Refactor service unit tests

Add a FakeService class, and refactor service unit tests
to use this class.

Change-Id: I650ad83386a58205ebe42274d2bf2f508436bfa6

Author: Tang Chen

openstackclient/tests/compute/v2/fakes.py

@@ -76,17 +76,6 @@
 QUOTA_columns = tuple(sorted(QUOTA))
 QUOTA_data = tuple(QUOTA[x] for x in sorted(QUOTA))
 
-service_host = 'host_test'
-service_binary = 'compute_test'
-service_status = 'enabled'
-service_disabled_reason = 'earthquake'
-SERVICE = {
-    'host': service_host,
-    'binary': service_binary,
-    'status': service_status,
-    'disabled_reason': service_disabled_reason,
-}
-
 
 class FakeAggregate(object):
     """Fake one aggregate."""
@@ -523,6 +512,54 @@ def get_servers(servers=None, count=2):
         return mock.MagicMock(side_effect=servers)
 
 
+class FakeService(object):
+    """Fake one or more services."""
+
+    @staticmethod
+    def create_one_service(attrs=None):
+        """Create a fake service.
+
+        :param Dictionary attrs:
+            A dictionary with all attributes
+        :return:
+            A FakeResource object, with id, name, ram, vcpus, properties
+        """
+        attrs = attrs or {}
+
+        # Set default attributes.
+        service_info = {
+            'host': 'host-' + uuid.uuid4().hex,
+            'binary': 'binary-' + uuid.uuid4().hex,
+            'status': 'enabled',
+            'disabled_reason': 'earthquake',
+        }
+
+        # Overwrite default attributes.
+        service_info.update(attrs)
+
+        service = fakes.FakeResource(info=copy.deepcopy(service_info),
+                                     loaded=True)
+
+        return service
+
+    @staticmethod
+    def create_services(attrs=None, count=2):
+        """Create multiple fake services.
+
+        :param Dictionary attrs:
+            A dictionary with all attributes
+        :param int count:
+            The number of services to fake
+        :return:
+            A list of FakeResource objects faking the services
+        """
+        services = []
+        for i in range(0, count):
+            services.append(FakeService.create_one_service(attrs))
+
+        return services
+
+
 class FakeFlavor(object):
     """Fake one or more flavors."""
 

openstackclient/tests/compute/v2/test_service.py

@@ -13,12 +13,10 @@
 #   under the License.
 #
 
-import copy
 import mock
 
 from openstackclient.compute.v2 import service
 from openstackclient.tests.compute.v2 import fakes as compute_fakes
-from openstackclient.tests import fakes
 
 
 class TestService(compute_fakes.TestComputev2):
@@ -36,24 +34,26 @@ class TestServiceDelete(TestService):
     def setUp(self):
         super(TestServiceDelete, self).setUp()
 
+        self.service = compute_fakes.FakeService.create_one_service()
+
         self.service_mock.delete.return_value = None
 
         # Get the command object to test
         self.cmd = service.DeleteService(self.app, None)
 
     def test_service_delete_no_options(self):
         arglist = [
-            compute_fakes.service_binary,
+            self.service.binary,
         ]
         verifylist = [
-            ('service', compute_fakes.service_binary),
+            ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
 
         self.service_mock.delete.assert_called_with(
-            compute_fakes.service_binary,
+            self.service.binary,
         )
         self.assertIsNone(result)
 
@@ -63,23 +63,21 @@ class TestServiceList(TestService):
     def setUp(self):
         super(TestServiceList, self).setUp()
 
-        self.service_mock.list.return_value = [fakes.FakeResource(
-            None,
-            copy.deepcopy(compute_fakes.SERVICE),
-            loaded=True,
-        )]
+        self.service = compute_fakes.FakeService.create_one_service()
+
+        self.service_mock.list.return_value = [self.service]
 
         # Get the command object to test
         self.cmd = service.ListService(self.app, None)
 
     def test_service_list(self):
         arglist = [
-            '--host', compute_fakes.service_host,
-            '--service', compute_fakes.service_binary,
+            '--host', self.service.host,
+            '--service', self.service.binary,
         ]
         verifylist = [
-            ('host', compute_fakes.service_host),
-            ('service', compute_fakes.service_binary),
+            ('host', self.service.host),
+            ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -89,22 +87,22 @@ def test_service_list(self):
         columns, data = self.cmd.take_action(parsed_args)
 
         self.service_mock.list.assert_called_with(
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary,
         )
 
         self.assertNotIn("Disabled Reason", columns)
-        self.assertNotIn(compute_fakes.service_disabled_reason, list(data)[0])
+        self.assertNotIn(self.service.disabled_reason, list(data)[0])
 
     def test_service_list_with_long_option(self):
         arglist = [
-            '--host', compute_fakes.service_host,
-            '--service', compute_fakes.service_binary,
+            '--host', self.service.host,
+            '--service', self.service.binary,
             '--long'
         ]
         verifylist = [
-            ('host', compute_fakes.service_host),
-            ('service', compute_fakes.service_binary),
+            ('host', self.service.host),
+            ('service', self.service.binary),
             ('long', True)
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -115,67 +113,60 @@ def test_service_list_with_long_option(self):
         columns, data = self.cmd.take_action(parsed_args)
 
         self.assertIn("Disabled Reason", columns)
-        self.assertIn(compute_fakes.service_disabled_reason, list(data)[0])
+        self.assertIn(self.service.disabled_reason, list(data)[0])
 
 
 class TestServiceSet(TestService):
 
     def setUp(self):
         super(TestServiceSet, self).setUp()
 
-        self.service_mock.enable.return_value = [fakes.FakeResource(
-            None,
-            copy.deepcopy(compute_fakes.SERVICE),
-            loaded=True,
-        )]
+        self.service = compute_fakes.FakeService.create_one_service()
 
-        self.service_mock.disable.return_value = [fakes.FakeResource(
-            None,
-            copy.deepcopy(compute_fakes.SERVICE),
-            loaded=True,
-        )]
+        self.service_mock.enable.return_value = self.service
+        self.service_mock.disable.return_value = self.service
 
         self.cmd = service.SetService(self.app, None)
 
     def test_service_set_enable(self):
         arglist = [
             '--enable',
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary,
         ]
         verifylist = [
             ('enabled', True),
-            ('host', compute_fakes.service_host),
-            ('service', compute_fakes.service_binary),
+            ('host', self.service.host),
+            ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
 
         self.service_mock.enable.assert_called_with(
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary
         )
         self.assertIsNone(result)
 
     def test_service_set_disable(self):
         arglist = [
             '--disable',
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary,
         ]
         verifylist = [
             ('enabled', False),
-            ('host', compute_fakes.service_host),
-            ('service', compute_fakes.service_binary),
+            ('host', self.service.host),
+            ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
 
         self.service_mock.disable.assert_called_with(
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary
         )
         self.assertIsNone(result)
 
@@ -184,22 +175,22 @@ def test_service_set_disable_with_reason(self):
         arglist = [
             '--disable',
             '--disable-reason', reason,
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary,
         ]
         verifylist = [
             ('enabled', False),
             ('disable_reason', reason),
-            ('host', compute_fakes.service_host),
-            ('service', compute_fakes.service_binary),
+            ('host', self.service.host),
+            ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
 
         self.service_mock.disable_log_reason.assert_called_with(
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary,
             reason
         )
         self.assertIsNone(result)
@@ -208,14 +199,14 @@ def test_service_set_only_with_disable_reason(self):
         reason = 'earthquake'
         arglist = [
             '--disable-reason', reason,
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary,
         ]
         verifylist = [
             ('enabled', True),
             ('disable_reason', reason),
-            ('host', compute_fakes.service_host),
-            ('service', compute_fakes.service_binary),
+            ('host', self.service.host),
+            ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -226,8 +217,8 @@ def test_service_set_only_with_disable_reason(self):
             mock_log.assert_called_once_with(msg)
 
         self.service_mock.enable.assert_called_with(
-            compute_fakes.service_host,
-            compute_fakes.service_binary
+            self.service.host,
+            self.service.binary
         )
         self.assertIsNone(result)
 
@@ -236,14 +227,14 @@ def test_service_set_enable_with_disable_reason(self):
         arglist = [
             '--enable',
             '--disable-reason', reason,
-            compute_fakes.service_host,
-            compute_fakes.service_binary,
+            self.service.host,
+            self.service.binary,
         ]
         verifylist = [
             ('enabled', True),
             ('disable_reason', reason),
-            ('host', compute_fakes.service_host),
-            ('service', compute_fakes.service_binary),
+            ('host', self.service.host),
+            ('service', self.service.binary),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -254,7 +245,7 @@ def test_service_set_enable_with_disable_reason(self):
             mock_log.assert_called_once_with(msg)
 
         self.service_mock.enable.assert_called_with(
-            compute_fakes.service_host,
-            compute_fakes.service_binary
+            self.service.host,
+            self.service.binary
         )
         self.assertIsNone(result)