From 03a2accb2f13f82b1090e1ae6f154bf263e7e252 Mon Sep 17 00:00:00 2001 From: Dean Troyer Date: Fri, 17 May 2019 20:06:44 -0500 Subject: [PATCH] Format aggregate command fields and de-race functional tests Rename metadata to property in all aggregate commands Beef up functional tests to reduce street racing Change-Id: I4598da73b85a954f3e6a3981db21891b45d9548c Signed-off-by: Dean Troyer --- openstackclient/compute/v2/aggregate.py | 46 ++++++++++++++- .../functional/compute/v2/test_aggregate.py | 59 ++++++++++++++++--- .../tests/unit/compute/v2/test_aggregate.py | 41 +++++++------ 3 files changed, 117 insertions(+), 29 deletions(-) diff --git a/openstackclient/compute/v2/aggregate.py b/openstackclient/compute/v2/aggregate.py index fa64647899..3834de1f0b 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 c591dd61fd..8a082e82ad 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 3efe0dbd36..0937047cf0 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):