Merge "Format aggregate command fields and de-race functional tests"

3 files changed, 117 insertions, 29 deletions

diff --git a/openstackclient/compute/v2/aggregate.py b/openstackclient/compute/v2/aggregate.py
index fa646478..3834de1f 100644
--- a/openstackclient/compute/v2/aggregate.py
+++ b/openstackclient/compute/v2/aggregate.py
@@ -18,6 +18,7 @@
 
 import logging
 
+from osc_lib.cli import format_columns
 from osc_lib.cli import parseractions
 from osc_lib.command import command
 from osc_lib import exceptions
@@ -58,6 +59,15 @@ class AddAggregateHost(command.ShowOne):
 
         info = {}
         info.update(data._info)
+
+        # Special mapping for columns to make the output easier to read:
+        # 'metadata' --> 'properties'
+        info.update(
+            {
+                'hosts': format_columns.ListColumn(info.pop('hosts')),
+                'properties': format_columns.DictColumn(info.pop('metadata')),
+            },
+        )
         return zip(*sorted(six.iteritems(info)))
 
 
@@ -101,8 +111,20 @@ class CreateAggregate(command.ShowOne):
                 parsed_args.property,
             )._info)
 
-        # TODO(dtroyer): re-format metadata field to properites as
-        #                in the set command
+        # Special mapping for columns to make the output easier to read:
+        # 'metadata' --> 'properties'
+        hosts = None
+        properties = None
+        if 'hosts' in info.keys():
+            hosts = format_columns.ListColumn(info.pop('hosts'))
+        if 'metadata' in info.keys():
+            properties = format_columns.DictColumn(info.pop('metadata'))
+        info.update(
+            {
+                'hosts': hosts,
+                'properties': properties,
+            },
+        )
         return zip(*sorted(six.iteritems(info)))
 
 
@@ -186,6 +208,10 @@ class ListAggregate(command.Lister):
         return (column_headers,
                 (utils.get_item_properties(
                     s, columns,
+                    formatters={
+                        'Hosts': format_columns.ListColumn,
+                        'Metadata': format_columns.DictColumn,
+                    },
                 ) for s in data))
 
 
@@ -220,6 +246,15 @@ class RemoveAggregateHost(command.ShowOne):
 
         info = {}
         info.update(data._info)
+
+        # Special mapping for columns to make the output easier to read:
+        # 'metadata' --> 'properties'
+        info.update(
+            {
+                'hosts': format_columns.ListColumn(info.pop('hosts')),
+                'properties': format_columns.DictColumn(info.pop('metadata')),
+            },
+        )
         return zip(*sorted(six.iteritems(info)))
 
 
@@ -326,7 +361,12 @@ class ShowAggregate(command.ShowOne):
         # 'metadata' --> 'properties'
         data._info.update(
             {
-                'properties': utils.format_dict(data._info.pop('metadata')),
+                'hosts': format_columns.ListColumn(
+                    data._info.pop('hosts')
+                ),
+                'properties': format_columns.DictColumn(
+                    data._info.pop('metadata')
+                ),
             },
         )
 
diff --git a/openstackclient/tests/functional/compute/v2/test_aggregate.py b/openstackclient/tests/functional/compute/v2/test_aggregate.py
index c591dd61..8a082e82 100644
--- a/openstackclient/tests/functional/compute/v2/test_aggregate.py
+++ b/openstackclient/tests/functional/compute/v2/test_aggregate.py
@@ -11,6 +11,7 @@
 #    under the License.
 
 import json
+import time
 import uuid
 
 from openstackclient.tests.functional import base
@@ -19,6 +20,32 @@ from openstackclient.tests.functional import base
 class AggregateTests(base.TestCase):
     """Functional tests for aggregate"""
 
+    @classmethod
+    def wait_for_status(cls, check_type, check_name, desired_status,
+                        wait=120, interval=5, failures=None):
+        current_status = "notset"
+        if failures is None:
+            failures = ['error']
+        total_sleep = 0
+        while total_sleep < wait:
+            output = json.loads(cls.openstack(
+                check_type + ' show -f json ' + check_name))
+            current_status = output['name']
+            if (current_status == desired_status):
+                print('{} {} now has status {}'
+                      .format(check_type, check_name, current_status))
+                return
+            print('Checking {} {} Waiting for {} current status: {}'
+                  .format(check_type, check_name,
+                          desired_status, current_status))
+            if current_status in failures:
+                raise Exception(
+                    'Current status {} of {} {} is one of failures {}'
+                    .format(current_status, check_type, check_name, failures))
+            time.sleep(interval)
+            total_sleep += interval
+        cls.assertOutput(desired_status, current_status)
+
     def test_aggregate_crud(self):
         """Test create, delete multiple"""
         name1 = uuid.uuid4().hex
@@ -36,12 +63,16 @@ class AggregateTests(base.TestCase):
             'nova',
             cmd_output['availability_zone']
         )
-        # TODO(dtroyer): enable the following once the properties field
-        #                is correctly formatted in the create output
-        # self.assertIn(
-        #     "a='b'",
-        #     cmd_output['properties']
-        # )
+        self.assertIn(
+            'a',
+            cmd_output['properties']
+        )
+        self.wait_for_status('aggregate', name1, name1)
+        self.addCleanup(
+            self.openstack,
+            'aggregate delete ' + name1,
+            fail_ok=True,
+        )
 
         name2 = uuid.uuid4().hex
         cmd_output = json.loads(self.openstack(
@@ -57,6 +88,12 @@ class AggregateTests(base.TestCase):
             'external',
             cmd_output['availability_zone']
         )
+        self.wait_for_status('aggregate', name2, name2)
+        self.addCleanup(
+            self.openstack,
+            'aggregate delete ' + name2,
+            fail_ok=True,
+        )
 
         # Test aggregate set
         name3 = uuid.uuid4().hex
@@ -69,6 +106,12 @@ class AggregateTests(base.TestCase):
             name1
         )
         self.assertOutput('', raw_output)
+        self.wait_for_status('aggregate', name3, name3)
+        self.addCleanup(
+            self.openstack,
+            'aggregate delete ' + name3,
+            fail_ok=True,
+        )
 
         cmd_output = json.loads(self.openstack(
             'aggregate show -f json ' +
@@ -83,11 +126,11 @@ class AggregateTests(base.TestCase):
             cmd_output['availability_zone']
         )
         self.assertIn(
-            "c='d'",
+            'c',
             cmd_output['properties']
         )
         self.assertNotIn(
-            "a='b'",
+            'a',
             cmd_output['properties']
         )
 
diff --git a/openstackclient/tests/unit/compute/v2/test_aggregate.py b/openstackclient/tests/unit/compute/v2/test_aggregate.py
index 3efe0dbd..0937047c 100644
--- a/openstackclient/tests/unit/compute/v2/test_aggregate.py
+++ b/openstackclient/tests/unit/compute/v2/test_aggregate.py
@@ -16,6 +16,7 @@
 import mock
 from mock import call
 
+from osc_lib.cli import format_columns
 from osc_lib import exceptions
 from osc_lib import utils
 
@@ -31,16 +32,16 @@ class TestAggregate(compute_fakes.TestComputev2):
         'availability_zone',
         'hosts',
         'id',
-        'metadata',
         'name',
+        'properties',
     )
 
     data = (
         fake_ag.availability_zone,
-        fake_ag.hosts,
+        format_columns.ListColumn(fake_ag.hosts),
         fake_ag.id,
-        fake_ag.metadata,
         fake_ag.name,
+        format_columns.DictColumn(fake_ag.metadata),
     )
 
     def setUp(self):
@@ -75,7 +76,7 @@ class TestAggregateAddHost(TestAggregate):
         self.aggregate_mock.add_host.assert_called_once_with(self.fake_ag,
                                                              parsed_args.host)
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
 
 class TestAggregateCreate(TestAggregate):
@@ -99,7 +100,7 @@ class TestAggregateCreate(TestAggregate):
         self.aggregate_mock.create.assert_called_once_with(parsed_args.name,
                                                            None)
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_aggregate_create_with_zone(self):
         arglist = [
@@ -116,7 +117,7 @@ class TestAggregateCreate(TestAggregate):
         self.aggregate_mock.create.assert_called_once_with(parsed_args.name,
                                                            parsed_args.zone)
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
     def test_aggregate_create_with_property(self):
         arglist = [
@@ -135,7 +136,7 @@ class TestAggregateCreate(TestAggregate):
         self.aggregate_mock.set_metadata.assert_called_once_with(
             self.fake_ag, parsed_args.property)
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
 
 class TestAggregateDelete(TestAggregate):
@@ -234,8 +235,11 @@ class TestAggregateList(TestAggregate):
         TestAggregate.fake_ag.id,
         TestAggregate.fake_ag.name,
         TestAggregate.fake_ag.availability_zone,
-        {key: value for key, value in TestAggregate.fake_ag.metadata.items()
-         if key != 'availability_zone'},
+        format_columns.DictColumn({
+            key: value
+            for key, value in TestAggregate.fake_ag.metadata.items()
+            if key != 'availability_zone'
+        }),
     ), )
 
     def setUp(self):
@@ -250,7 +254,7 @@ class TestAggregateList(TestAggregate):
         columns, data = self.cmd.take_action(parsed_args)
 
         self.assertEqual(self.list_columns, columns)
-        self.assertEqual(self.list_data, tuple(data))
+        self.assertItemEqual(self.list_data, tuple(data))
 
     def test_aggregate_list_with_long(self):
         arglist = [
@@ -263,7 +267,7 @@ class TestAggregateList(TestAggregate):
         columns, data = self.cmd.take_action(parsed_args)
 
         self.assertEqual(self.list_columns_long, columns)
-        self.assertEqual(self.list_data_long, tuple(data))
+        self.assertListItemEqual(self.list_data_long, tuple(data))
 
 
 class TestAggregateRemoveHost(TestAggregate):
@@ -290,7 +294,7 @@ class TestAggregateRemoveHost(TestAggregate):
         self.aggregate_mock.remove_host.assert_called_once_with(
             self.fake_ag, parsed_args.host)
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, data)
 
 
 class TestAggregateSet(TestAggregate):
@@ -440,13 +444,14 @@ class TestAggregateShow(TestAggregate):
 
     data = (
         TestAggregate.fake_ag.availability_zone,
-        TestAggregate.fake_ag.hosts,
+        format_columns.ListColumn(TestAggregate.fake_ag.hosts),
         TestAggregate.fake_ag.id,
         TestAggregate.fake_ag.name,
-        utils.format_dict(
-            {key: value
-             for key, value in TestAggregate.fake_ag.metadata.items()
-             if key != 'availability_zone'}),
+        format_columns.DictColumn({
+            key: value
+            for key, value in TestAggregate.fake_ag.metadata.items()
+            if key != 'availability_zone'
+        }),
     )
 
     def setUp(self):
@@ -467,7 +472,7 @@ class TestAggregateShow(TestAggregate):
         self.aggregate_mock.get.assert_called_once_with(parsed_args.aggregate)
 
         self.assertEqual(self.columns, columns)
-        self.assertEqual(self.data, data)
+        self.assertItemEqual(self.data, tuple(data))
 
 
 class TestAggregateUnset(TestAggregate):