From 31ae635ffe0a2c37ab2bda394d2b072d7e5acff9 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 14 Dec 2022 11:37:00 +0000 Subject: [PATCH] tests: Use SDK objects where expected We had not migrated a number of tests to use SDK objects instead of fake novaclient-like objects when migrating the commands themselves. Address this now. Change-Id: Ib0da07fd9d793968b111986bd36a6d4311469d4e Signed-off-by: Stephen Finucane --- .../tests/unit/compute/v2/fakes.py | 164 +++++++----------- .../tests/unit/compute/v2/test_aggregate.py | 28 ++- .../tests/unit/compute/v2/test_keypair.py | 70 ++++++-- .../tests/unit/compute/v2/test_server.py | 1 - .../unit/compute/v2/test_server_backup.py | 8 - .../unit/compute/v2/test_server_image.py | 12 +- 6 files changed, 133 insertions(+), 150 deletions(-) diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index f73b692774..acee68e4e0 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -19,16 +19,19 @@ from unittest import mock import uuid from novaclient import api_versions +from openstack.compute.v2 import aggregate as _aggregate from openstack.compute.v2 import flavor as _flavor from openstack.compute.v2 import hypervisor as _hypervisor +from openstack.compute.v2 import keypair as _keypair from openstack.compute.v2 import migration as _migration from openstack.compute.v2 import server as _server from openstack.compute.v2 import server_action as _server_action from openstack.compute.v2 import server_group as _server_group from openstack.compute.v2 import server_interface as _server_interface from openstack.compute.v2 import server_migration as _server_migration -from openstack.compute.v2 import service -from openstack.compute.v2 import volume_attachment +from openstack.compute.v2 import service as _service +from openstack.compute.v2 import usage as _usage +from openstack.compute.v2 import volume_attachment as _volume_attachment from openstackclient.api import compute_v2 from openstackclient.tests.unit import fakes @@ -187,10 +190,8 @@ class TestComputev2(utils.TestCommand): def create_one_aggregate(attrs=None): """Create a fake aggregate. - :param dict attrs: - A dictionary with all attributes - :return: - A FakeResource object, with id and other attributes + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.aggregate.Aggregate object """ attrs = attrs or {} @@ -209,21 +210,16 @@ def create_one_aggregate(attrs=None): # Overwrite default attributes. aggregate_info.update(attrs) - aggregate = fakes.FakeResource( - info=copy.deepcopy(aggregate_info), loaded=True - ) + aggregate = _aggregate.Aggregate(**aggregate_info) return aggregate def create_aggregates(attrs=None, count=2): """Create multiple fake aggregates. - :param dict attrs: - A dictionary with all attributes - :param int count: - The number of aggregates to fake - :return: - A list of FakeResource objects faking the aggregates + :param dict attrs: A dictionary with all attributes + :param int count: The number of aggregates to fake + :return: A list of fake openstack.compute.v2.aggregate.Aggregate objects """ aggregates = [] for i in range(0, count): @@ -238,12 +234,9 @@ def get_aggregates(aggregates=None, count=2): If aggregates list is provided, then initialize the Mock object with the list. Otherwise create one. - :param List aggregates: - A list of FakeResource objects faking aggregates - :param int count: - The number of aggregates to fake - :return: - An iterable Mock object with side_effect set to a list of faked + :return: A list of fake openstack.compute.v2.aggregate.Aggregate objects + :param int count: The number of aggregates to fake + :return: An iterable Mock object with side_effect set to a list of faked aggregates """ if aggregates is None: @@ -491,19 +484,13 @@ def create_servers(attrs=None, methods=None, count=2): return servers -def create_one_sdk_server(attrs=None, methods=None): +def create_one_sdk_server(attrs=None): """Create a fake server for testing migration to sdk - :param dict attrs: - A dictionary with all attributes - :param dict methods: - A dictionary with all methods - :return: - A openstack.compute.v2.server.Server object, - with id, name, metadata, and so on + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.server.Server object, """ attrs = attrs or {} - methods = methods or {} # Set default attributes. server_info = { @@ -529,22 +516,16 @@ def create_one_sdk_server(attrs=None, methods=None): return server -def create_sdk_servers(attrs=None, methods=None, count=2): +def create_sdk_servers(attrs=None, count=2): """Create multiple fake servers for testing migration to sdk - :param dict attrs: - A dictionary with all attributes - :param dict methods: - A dictionary with all methods - :param int count: - The number of servers to fake - :return: - A list of openstack.compute.v2.server.Server objects - faking the servers + :param dict attrs: A dictionary with all attributes + :param int count: The number of servers to fake + :return: A list of fake openstack.compute.v2.server.Server objects """ servers = [] for i in range(0, count): - servers.append(create_one_sdk_server(attrs, methods)) + servers.append(create_one_sdk_server(attrs)) return servers @@ -555,12 +536,11 @@ def get_servers(servers=None, count=2): If servers list is provided, then initialize the Mock object with the list. Otherwise create one. - :param List servers: - A list of FakeResource objects faking servers + :param list servers: A list of fake openstack.compute.v2.server.Server + objects :param int count: The number of servers to fake - :return: - An iterable Mock object with side_effect set to a list of faked + :return: An iterable Mock object with side_effect set to a list of faked servers """ if servers is None: @@ -614,10 +594,8 @@ def create_one_server_action(attrs=None): def create_one_service(attrs=None): """Create a fake service. - :param dict attrs: - A dictionary with all attributes - :return: - A fake Service object, with id, host, binary, and so on + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.service.Service object """ attrs = attrs or {} @@ -638,7 +616,7 @@ def create_one_service(attrs=None): # Overwrite default attributes. service_info.update(attrs) - return service.Service(**service_info) + return _service.Service(**service_info) def create_services(attrs=None, count=2): @@ -661,10 +639,8 @@ def create_services(attrs=None, count=2): def create_one_flavor(attrs=None): """Create a fake flavor. - :param dict attrs: - A dictionary with all attributes - :return: - A FakeResource object, with id, name, ram, vcpus, and so on + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.flavor.Flavor object """ attrs = attrs or {} @@ -695,12 +671,9 @@ def create_one_flavor(attrs=None): def create_flavors(attrs=None, count=2): """Create multiple fake flavors. - :param dict attrs: - A dictionary with all attributes - :param int count: - The number of flavors to fake - :return: - A list of FakeResource objects faking the flavors + :param dict attrs: A dictionary with all attributes + :param int count: The number of flavors to fake + :return: A list of fake openstack.compute.v2.flavor.Flavor objects """ flavors = [] for i in range(0, count): @@ -715,12 +688,10 @@ def get_flavors(flavors=None, count=2): If flavors list is provided, then initialize the Mock object with the list. Otherwise create one. - :param List flavors: - A list of FakeResource objects faking flavors - :param int count: - The number of flavors to fake - :return: - An iterable Mock object with side_effect set to a list of faked + :param list flavors: A list of fake openstack.compute.v2.flavor.Flavor + objects + :param int count: The number of flavors to fake + :return: An iterable Mock object with side_effect set to a list of faked flavors """ if flavors is None: @@ -757,10 +728,8 @@ def create_one_flavor_access(attrs=None): def create_one_keypair(attrs=None): """Create a fake keypair - :param dict attrs: - A dictionary with all attributes - :return: - A FakeResource object, name, fingerprint, and so on + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.keypair.Keypair object """ attrs = attrs or {} @@ -776,20 +745,15 @@ def create_one_keypair(attrs=None): # Overwrite default attributes. keypair_info.update(attrs) - keypair = fakes.FakeResource(info=copy.deepcopy(keypair_info), loaded=True) - - return keypair + return _keypair.Keypair(**keypair_info) def create_keypairs(attrs=None, count=2): """Create multiple fake keypairs. - :param dict attrs: - A dictionary with all attributes - :param int count: - The number of keypairs to fake - :return: - A list of FakeResource objects faking the keypairs + :param dict attrs: A dictionary with all attributes + :param int count: The number of keypairs to fake + :return: A list of fake openstack.compute.v2.keypair.Keypair objects """ keypairs = [] @@ -805,12 +769,10 @@ def get_keypairs(keypairs=None, count=2): If keypairs list is provided, then initialize the Mock object with the list. Otherwise create one. - :param List keypairs: - A list of FakeResource objects faking keypairs - :param int count: - The number of keypairs to fake - :return: - An iterable Mock object with side_effect set to a list of faked + :param list keypairs: A list of fake openstack.compute.v2.keypair.Keypair + objects + :param int count: The number of keypairs to fake + :return: An iterable Mock object with side_effect set to a list of faked keypairs """ if keypairs is None: @@ -1122,7 +1084,7 @@ def create_one_usage(attrs=None): # Set default attributes. usage_info = { - 'tenant_id': 'usage-tenant-id-' + uuid.uuid4().hex, + 'project_id': 'usage-tenant-id-' + uuid.uuid4().hex, 'total_memory_mb_usage': 512.0, 'total_vcpus_usage': 1.0, 'total_local_gb_usage': 1.0, @@ -1145,9 +1107,7 @@ def create_one_usage(attrs=None): # Overwrite default attributes. usage_info.update(attrs) - usage = fakes.FakeResource(info=copy.deepcopy(usage_info), loaded=True) - - return usage + return _usage.Usage(**usage_info) def create_usages(attrs=None, count=2): @@ -1511,7 +1471,7 @@ def create_one_volume_attachment(attrs=None): # Overwrite default attributes. volume_attachment_info.update(attrs) - return volume_attachment.VolumeAttachment(**volume_attachment_info) + return _volume_attachment.VolumeAttachment(**volume_attachment_info) def create_volume_attachments(attrs=None, count=2): @@ -1532,10 +1492,8 @@ def create_volume_attachments(attrs=None, count=2): def create_one_hypervisor(attrs=None): """Create a fake hypervisor. - :param dict attrs: - A dictionary with all attributes - :return: - A FakeResource object, with id, hypervisor_hostname, and so on + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.hypervisor.Hypervisor object """ attrs = attrs or {} @@ -1579,12 +1537,9 @@ def create_one_hypervisor(attrs=None): def create_hypervisors(attrs=None, count=2): """Create multiple fake hypervisors. - :param dict attrs: - A dictionary with all attributes - :param int count: - The number of hypervisors to fake - :return: - A list of FakeResource objects faking the hypervisors + :param dict attrs: A dictionary with all attributes + :param int count: The number of hypervisors to fake + :return: A list of fake openstack.compute.v2.hypervisor.Hypervisor objects """ hypervisors = [] for i in range(0, count): @@ -1596,10 +1551,8 @@ def create_hypervisors(attrs=None, count=2): def create_one_server_group(attrs=None): """Create a fake server group - :param dict attrs: - A dictionary with all attributes - :return: - A fake ServerGroup object, with id and other attributes + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.server_group.ServerGroup object """ if attrs is None: attrs = {} @@ -1626,7 +1579,8 @@ def create_one_server_interface(attrs=None): :param dict attrs: A dictionary with all attributes :param dict methods: A dictionary with all methods - :return: A fake ServerInterface object with various attributes set + :return: A fake openstack.compute.v2.server_interface.ServerInterface + object """ attrs = attrs or {} diff --git a/openstackclient/tests/unit/compute/v2/test_aggregate.py b/openstackclient/tests/unit/compute/v2/test_aggregate.py index 4185e64c14..4e7456a012 100644 --- a/openstackclient/tests/unit/compute/v2/test_aggregate.py +++ b/openstackclient/tests/unit/compute/v2/test_aggregate.py @@ -31,18 +31,28 @@ class TestAggregate(compute_fakes.TestComputev2): columns = ( 'availability_zone', + 'created_at', + 'deleted_at', 'hosts', 'id', + 'is_deleted', 'name', 'properties', + 'updated_at', + 'uuid', ) data = ( fake_ag.availability_zone, + fake_ag.created_at, + fake_ag.deleted_at, format_columns.ListColumn(fake_ag.hosts), fake_ag.id, + fake_ag.is_deleted, fake_ag.name, format_columns.DictColumn(fake_ag.metadata), + fake_ag.updated_at, + fake_ag.uuid, ) def setUp(self): @@ -488,24 +498,28 @@ class TestAggregateSet(TestAggregate): class TestAggregateShow(TestAggregate): columns = ( 'availability_zone', + 'created_at', + 'deleted_at', 'hosts', 'id', + 'is_deleted', 'name', 'properties', + 'updated_at', + 'uuid', ) data = ( TestAggregate.fake_ag.availability_zone, + TestAggregate.fake_ag.created_at, + TestAggregate.fake_ag.deleted_at, format_columns.ListColumn(TestAggregate.fake_ag.hosts), TestAggregate.fake_ag.id, + TestAggregate.fake_ag.is_deleted, TestAggregate.fake_ag.name, - format_columns.DictColumn( - { - key: value - for key, value in TestAggregate.fake_ag.metadata.items() - if key != 'availability_zone' - } - ), + format_columns.DictColumn(TestAggregate.fake_ag.metadata), + TestAggregate.fake_ag.updated_at, + TestAggregate.fake_ag.uuid, ) def setUp(self): diff --git a/openstackclient/tests/unit/compute/v2/test_keypair.py b/openstackclient/tests/unit/compute/v2/test_keypair.py index f3618cefe6..fb085c358b 100644 --- a/openstackclient/tests/unit/compute/v2/test_keypair.py +++ b/openstackclient/tests/unit/compute/v2/test_keypair.py @@ -57,9 +57,20 @@ class TestKeypairCreate(TestKeypair): self.keypair = compute_fakes.create_one_keypair() - self.columns = ('fingerprint', 'name', 'type', 'user_id') + self.columns = ( + 'created_at', + 'fingerprint', + 'id', + 'is_deleted', + 'name', + 'type', + 'user_id', + ) self.data = ( + self.keypair.created_at, self.keypair.fingerprint, + self.keypair.id, + self.keypair.is_deleted, self.keypair.name, self.keypair.type, self.keypair.user_id, @@ -75,7 +86,7 @@ class TestKeypairCreate(TestKeypair): '_generate_keypair', return_value=keypair.Keypair('private', 'public'), ) - def test_key_pair_create_no_options(self, mock_generate): + def test_keypair_create_no_options(self, mock_generate): arglist = [ self.keypair.name, ] @@ -96,7 +107,10 @@ class TestKeypairCreate(TestKeypair): def test_keypair_create_public_key(self): self.data = ( + self.keypair.created_at, self.keypair.fingerprint, + self.keypair.id, + self.keypair.is_deleted, self.keypair.name, self.keypair.type, self.keypair.user_id, @@ -173,7 +187,10 @@ class TestKeypairCreate(TestKeypair): self.sdk_client.create_keypair.return_value = self.keypair self.data = ( + self.keypair.created_at, self.keypair.fingerprint, + self.keypair.id, + self.keypair.is_deleted, self.keypair.name, self.keypair.type, self.keypair.user_id, @@ -291,7 +308,7 @@ class TestKeypairDelete(TestKeypair): keypairs = compute_fakes.create_keypairs(count=2) def setUp(self): - super(TestKeypairDelete, self).setUp() + super().setUp() self.cmd = keypair.DeleteKeypair(self.app, None) @@ -397,7 +414,7 @@ class TestKeypairList(TestKeypair): keypairs = compute_fakes.create_keypairs(count=1) def setUp(self): - super(TestKeypairList, self).setUp() + super().setUp() self.sdk_client.keypairs.return_value = self.keypairs @@ -661,24 +678,22 @@ class TestKeypairList(TestKeypair): class TestKeypairShow(TestKeypair): - keypair = compute_fakes.create_one_keypair() - def setUp(self): - super(TestKeypairShow, self).setUp() + super().setUp() - self.sdk_client.find_keypair.return_value = self.keypair + self.columns = ( + 'created_at', + 'fingerprint', + 'id', + 'is_deleted', + 'name', + 'private_key', + 'type', + 'user_id', + ) self.cmd = keypair.ShowKeypair(self.app, None) - self.columns = ("fingerprint", "name", "type", "user_id") - - self.data = ( - self.keypair.fingerprint, - self.keypair.name, - self.keypair.type, - self.keypair.user_id, - ) - def test_keypair_show_no_options(self): arglist = [] verifylist = [] @@ -693,11 +708,16 @@ class TestKeypairShow(TestKeypair): ) def test_keypair_show(self): + self.keypair = compute_fakes.create_one_keypair() self.sdk_client.find_keypair.return_value = self.keypair self.data = ( + self.keypair.created_at, self.keypair.fingerprint, + self.keypair.id, + self.keypair.is_deleted, self.keypair.name, + self.keypair.private_key, self.keypair.type, self.keypair.user_id, ) @@ -716,6 +736,9 @@ class TestKeypairShow(TestKeypair): self.assertEqual(self.data, data) def test_keypair_show_public(self): + self.keypair = compute_fakes.create_one_keypair() + self.sdk_client.find_keypair.return_value = self.keypair + arglist = ['--public-key', self.keypair.name] verifylist = [('public_key', True), ('name', self.keypair.name)] @@ -728,11 +751,16 @@ class TestKeypairShow(TestKeypair): @mock.patch.object(sdk_utils, 'supports_microversion', return_value=True) def test_keypair_show_with_user(self, sm_mock): + self.keypair = compute_fakes.create_one_keypair() self.sdk_client.find_keypair.return_value = self.keypair self.data = ( + self.keypair.created_at, self.keypair.fingerprint, + self.keypair.id, + self.keypair.is_deleted, self.keypair.name, + self.keypair.private_key, self.keypair.type, self.keypair.user_id, ) @@ -762,6 +790,7 @@ class TestKeypairShow(TestKeypair): @mock.patch.object(sdk_utils, 'supports_microversion', return_value=False) def test_keypair_show_with_user_pre_v210(self, sm_mock): + self.keypair = compute_fakes.create_one_keypair() arglist = [ '--user', identity_fakes.user_name, @@ -774,8 +803,11 @@ class TestKeypairShow(TestKeypair): parsed_args = self.check_parser(self.cmd, arglist, verifylist) ex = self.assertRaises( - exceptions.CommandError, self.cmd.take_action, parsed_args + exceptions.CommandError, + self.cmd.take_action, + parsed_args, ) self.assertIn( - '--os-compute-api-version 2.10 or greater is required', str(ex) + '--os-compute-api-version 2.10 or greater is required', + str(ex), ) diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 92ff5b139f..a4414af2cc 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -161,7 +161,6 @@ class TestServer(compute_fakes.TestComputev2): def setup_sdk_servers_mock(self, count): servers = compute_fakes.create_sdk_servers( attrs=self.attrs, - methods=self.methods, count=count, ) diff --git a/openstackclient/tests/unit/compute/v2/test_server_backup.py b/openstackclient/tests/unit/compute/v2/test_server_backup.py index 0bbc77c8ba..7b2c252b81 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_backup.py +++ b/openstackclient/tests/unit/compute/v2/test_server_backup.py @@ -38,13 +38,9 @@ class TestServerBackup(compute_fakes.TestComputev2): # Set object attributes to be tested. Could be overwritten in subclass. self.attrs = {} - # Set object methods to be tested. Could be overwritten in subclass. - self.methods = {} - def setup_servers_mock(self, count): servers = compute_fakes.create_sdk_servers( attrs=self.attrs, - methods=self.methods, count=count, ) @@ -89,10 +85,6 @@ class TestServerBackupCreate(TestServerBackup): # Get the command object to test self.cmd = server_backup.CreateServerBackup(self.app, None) - self.methods = { - 'backup': None, - } - def setup_images_mock(self, count, servers=None): if servers: images = image_fakes.create_images( diff --git a/openstackclient/tests/unit/compute/v2/test_server_image.py b/openstackclient/tests/unit/compute/v2/test_server_image.py index f231be4ced..cc1ee90ad3 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_image.py +++ b/openstackclient/tests/unit/compute/v2/test_server_image.py @@ -23,7 +23,7 @@ from openstackclient.tests.unit.image.v2 import fakes as image_fakes class TestServerImage(compute_fakes.TestComputev2): def setUp(self): - super(TestServerImage, self).setUp() + super().setUp() # Get a shortcut to the compute client ServerManager Mock self.app.client_manager.sdk_connection = mock.Mock() @@ -37,13 +37,9 @@ class TestServerImage(compute_fakes.TestComputev2): # Set object attributes to be tested. Could be overwritten in subclass. self.attrs = {} - # Set object methods to be tested. Could be overwritten in subclass. - self.methods = {} - def setup_servers_mock(self, count): servers = compute_fakes.create_sdk_servers( attrs=self.attrs, - methods=self.methods, count=count, ) @@ -82,15 +78,11 @@ class TestServerImageCreate(TestServerImage): return datalist def setUp(self): - super(TestServerImageCreate, self).setUp() + super().setUp() # Get the command object to test self.cmd = server_image.CreateServerImage(self.app, None) - self.methods = { - 'create_image': None, - } - def setup_images_mock(self, count, servers=None): if servers: images = image_fakes.create_images(