Refactor unit test of "compute service list" command

Huanxuan Ao · Huanxuan Ao · commit 70f8ae753467 · 2016-06-22T19:02:39.000+08:00
The unit test of "compute service list" only checked the
"Disabled Reason" columns and its data. It is not enough.
This patch change the test to check all datas in the list.

Also, this patch modify the "Id" to "ID" in this command.

Change-Id: I988fd6365f2652185dd96d9417f294eba9c31cd9
diff --git a/openstackclient/compute/v2/service.py b/openstackclient/compute/v2/service.py
@@ -73,7 +73,7 @@ def take_action(self, parsed_args):
         compute_client = self.app.client_manager.compute
         if parsed_args.long:
             columns = (
-                "Id",
+                "ID",
                 "Binary",
                 "Host",
                 "Zone",
@@ -84,7 +84,7 @@ def take_action(self, parsed_args):
             )
         else:
             columns = (
-                "Id",
+                "ID",
                 "Binary",
                 "Host",
                 "Zone",
diff --git a/openstackclient/tests/compute/v2/fakes.py b/openstackclient/tests/compute/v2/fakes.py
@@ -648,15 +648,19 @@ def create_one_service(attrs=None):
         :param Dictionary attrs:
             A dictionary with all attributes
         :return:
-            A FakeResource object, with id, name, ram, vcpus, properties
+            A FakeResource object, with id, host, binary
         """
         attrs = attrs or {}
 
         # Set default attributes.
         service_info = {
+            'id': 'id-' + uuid.uuid4().hex,
             'host': 'host-' + uuid.uuid4().hex,
             'binary': 'binary-' + uuid.uuid4().hex,
             'status': 'enabled',
+            'zone': 'zone-' + uuid.uuid4().hex,
+            'state': 'state-' + uuid.uuid4().hex,
+            'updated_at': 'time-' + uuid.uuid4().hex,
             'disabled_reason': 'earthquake',
         }
 
diff --git a/openstackclient/tests/compute/v2/test_service.py b/openstackclient/tests/compute/v2/test_service.py
@@ -62,11 +62,35 @@ def test_service_delete_no_options(self):
 
 class TestServiceList(TestService):
 
+    service = compute_fakes.FakeService.create_one_service()
+
+    columns = (
+        'ID',
+        'Binary',
+        'Host',
+        'Zone',
+        'Status',
+        'State',
+        'Updated At',
+    )
+    columns_long = columns + (
+        'Disabled Reason',
+    )
+
+    data = [(
+        service.id,
+        service.binary,
+        service.host,
+        service.zone,
+        service.status,
+        service.state,
+        service.updated_at,
+    )]
+    data_long = [data[0] + (service.disabled_reason, )]
+
     def setUp(self):
         super(TestServiceList, self).setUp()
 
-        self.service = compute_fakes.FakeService.create_one_service()
-
         self.service_mock.list.return_value = [self.service]
 
         # Get the command object to test
@@ -93,8 +117,8 @@ def test_service_list(self):
             self.service.binary,
         )
 
-        self.assertNotIn("Disabled Reason", columns)
-        self.assertNotIn(self.service.disabled_reason, list(data)[0])
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, list(data))
 
     def test_service_list_with_long_option(self):
         arglist = [
@@ -114,8 +138,13 @@ def test_service_list_with_long_option(self):
         # containing the data to be listed.
         columns, data = self.cmd.take_action(parsed_args)
 
-        self.assertIn("Disabled Reason", columns)
-        self.assertIn(self.service.disabled_reason, list(data)[0])
+        self.service_mock.list.assert_called_with(
+            self.service.host,
+            self.service.binary,
+        )
+
+        self.assertEqual(self.columns_long, columns)
+        self.assertEqual(self.data_long, list(data))
 
 
 class TestServiceSet(TestService):
