From 363802d9e957fbd68fea8dba9bfc962954346b8b Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Thu, 18 Jul 2019 04:11:19 +0900 Subject: [PATCH] Avoid using nova volume test data for cinder API mocking self.volumes from nova test data is used as return values of cinder API wrapper mock. Nova test data is based on novaclient.v2.volumes.Volume class. This is not correct. They should be based on cinderclient Volume class. This commit changes to use cinder test data for return values of openstack_dashboard.api.cinder mock. 'bootable' attribute is required, so it is added to cinder_data. This confusion comes from the naming of self.volumes. This commit renames self.volumes to self.nova_volumes to avoid further confusions. Note that api.nova.instance_volumes_list() calls get_server_volumes from novaclient and it is correct to use self.nova_volumes. Change-Id: I9c431dbb03f90bab84a4a6a3e3ea8fd5a5498b5b --- .../dashboards/admin/volumes/tests.py | 2 +- .../dashboards/project/backups/tests.py | 6 +-- .../dashboards/project/instances/tests.py | 40 +++++++++---------- .../dashboards/project/volumes/tests.py | 26 ++++++------ .../test/test_data/cinder_data.py | 6 ++- .../test/test_data/nova_data.py | 15 +++---- 6 files changed, 47 insertions(+), 48 deletions(-) diff --git a/openstack_dashboard/dashboards/admin/volumes/tests.py b/openstack_dashboard/dashboards/admin/volumes/tests.py index 3ada0ed55f..a0855355c5 100644 --- a/openstack_dashboard/dashboards/admin/volumes/tests.py +++ b/openstack_dashboard/dashboards/admin/volumes/tests.py @@ -188,7 +188,7 @@ class VolumeTests(test.BaseAdminViewTests): @test.create_mocks({api.cinder: ['volume_get', 'volume_reset_state']}) def test_update_volume_status(self): - volume = self.volumes.first() + volume = self.cinder_volumes.first() form_data = {'status': 'error'} self.mock_volume_reset_state.return_value = None diff --git a/openstack_dashboard/dashboards/project/backups/tests.py b/openstack_dashboard/dashboards/project/backups/tests.py index 5a1e6e076b..4f13b20dbd 100644 --- a/openstack_dashboard/dashboards/project/backups/tests.py +++ b/openstack_dashboard/dashboards/project/backups/tests.py @@ -112,7 +112,7 @@ class VolumeBackupsViewTests(test.TestCase): @test.create_mocks({api.cinder: ('volume_backup_create', 'volume_get')}) def test_create_backup_available(self): - volume = self.volumes.first() + volume = self.cinder_volumes.first() backup = self.cinder_volume_backups.first() self.mock_volume_get.return_value = volume @@ -145,8 +145,8 @@ class VolumeBackupsViewTests(test.TestCase): @test.create_mocks({api.cinder: ('volume_backup_create', 'volume_get')}) def test_create_backup_in_use(self): - # The second volume in the cinder test volume data is in-use - volume = self.volumes.list()[1] + # The third volume in the cinder test volume data is in-use + volume = self.cinder_volumes.list()[2] backup = self.cinder_volume_backups.first() self.mock_volume_get.return_value = volume diff --git a/openstack_dashboard/dashboards/project/instances/tests.py b/openstack_dashboard/dashboards/project/instances/tests.py index 31beb1ee5b..824acf6cbb 100644 --- a/openstack_dashboard/dashboards/project/instances/tests.py +++ b/openstack_dashboard/dashboards/project/instances/tests.py @@ -1457,7 +1457,7 @@ class InstanceDetailTests(InstanceTestBase): @helpers.create_mocks({api.neutron: ['is_extension_supported']}) def test_instance_details_volumes(self): server = self.servers.first() - volumes = [self.volumes.list()[1]] + volumes = [self.nova_volumes.list()[1]] security_groups = self.security_groups.list() self.mock_is_extension_supported.return_value = False @@ -1473,7 +1473,7 @@ class InstanceDetailTests(InstanceTestBase): @helpers.create_mocks({api.neutron: ['is_extension_supported']}) def test_instance_details_volume_sorting(self): server = self.servers.first() - volumes = self.volumes.list()[1:3] + volumes = self.nova_volumes.list()[1:3] security_groups = self.security_groups.list() self.mock_is_extension_supported.return_value = False @@ -2504,7 +2504,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, 'ConfigDrive': config_drive, 'ServerGroups': True, }) - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] @@ -2523,7 +2523,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, url = reverse('horizon:project:instances:launch') res = self.client.get(url) - bootable_volumes = [v.id for v in self.volumes.list() + bootable_volumes = [v.id for v in self.cinder_volumes.list() if (v.bootable == 'true' and v.status == 'available')] @@ -2730,7 +2730,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, flavor = self.flavors.first() keypair = self.keypairs.first() server = self.servers.first() - volume = self.volumes.first() + volume = self.cinder_volumes.first() sec_group = self.security_groups.first() avail_zone = self.availability_zones.first() customization_script = 'user data' @@ -2764,7 +2764,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, 'ConfigDrive': True, 'ServerGroups': False, }) - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] @@ -2862,7 +2862,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, flavor = self.flavors.first() keypair = self.keypairs.first() server = self.servers.first() - volume = self.volumes.first() + volume = self.cinder_volumes.first() sec_group = self.security_groups.first() avail_zone = self.availability_zones.first() customization_script = 'user data' @@ -2882,7 +2882,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, 'ServerGroups': True, }) self.mock_server_group_list.return_value = [] - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] @@ -3384,10 +3384,10 @@ class InstanceLaunchInstanceTests(InstanceTestBase, 'ConfigDrive': True, 'ServerGroups': False, }) - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE)] self.mock_volume_snapshot_list.return_value = volumes self.mock_flavor_list.return_value = self.flavors.list() @@ -3496,7 +3496,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, image = self.versioned_images.first() keypair = self.keypairs.first() server = self.servers.first() - volume = self.volumes.first() + volume = self.cinder_volumes.first() sec_group = self.security_groups.first() avail_zone = self.availability_zones.first() customization_script = 'user data' @@ -3512,7 +3512,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, 'ConfigDrive': True, 'ServerGroups': False, }) - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] @@ -3592,7 +3592,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, image = self.versioned_images.first() keypair = self.keypairs.first() server = self.servers.first() - volume = self.volumes.first() + volume = self.cinder_volumes.first() sec_group = self.security_groups.first() avail_zone = self.availability_zones.first() customization_script = 'user data' @@ -3613,7 +3613,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, 'ServerGroups': True, }) self.mock_server_group_list.return_value = self.server_groups.list() - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] @@ -3710,7 +3710,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, quotas: ('tenant_quota_usages',)}) def _launch_form_instance(self, image, flavor, keypair=None): server = self.servers.first() - volume = self.volumes.first() + volume = self.cinder_volumes.first() sec_group = self.security_groups.first() avail_zone = self.availability_zones.first() customization_script = 'user data' @@ -3725,7 +3725,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, 'ConfigDrive': True, 'ServerGroups': False, }) - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] @@ -3837,7 +3837,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, image = self.versioned_images.first() keypair = self.keypairs.first() server = self.servers.first() - volume = self.volumes.first() + volume = self.cinder_volumes.first() sec_group = self.security_groups.first() avail_zone = self.availability_zones.first() customization_script = 'user data' @@ -3867,7 +3867,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, self.networks.list()[1:], ] self.mock_port_list_with_trunk_types.return_value = self.ports.list() - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] @@ -4050,7 +4050,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, self._mock_glance_image_list_detailed(self.versioned_images.list()) self._mock_neutron_network_and_port_list() self.mock_server_group_list.return_value = self.server_groups.list() - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] @@ -4317,7 +4317,7 @@ class InstanceLaunchInstanceTests(InstanceTestBase, 'ServerGroups': True, }) self.mock_server_group_list.return_value = [] - volumes = [v for v in self.volumes.list() + volumes = [v for v in self.cinder_volumes.list() if (v.status == AVAILABLE and v.bootable == 'true')] self.mock_volume_list.return_value = volumes self.mock_volume_snapshot_list.return_value = [] diff --git a/openstack_dashboard/dashboards/project/volumes/tests.py b/openstack_dashboard/dashboards/project/volumes/tests.py index 4cf150f420..32ee346136 100644 --- a/openstack_dashboard/dashboards/project/volumes/tests.py +++ b/openstack_dashboard/dashboards/project/volumes/tests.py @@ -1833,13 +1833,13 @@ class VolumeViewTests(test.ResetImageAPIVersionMixin, test.TestCase): 'tenant_absolute_limits'], }) def _test_encryption(self, encryption): - volumes = self.volumes.list() + volumes = self.cinder_volumes.list() for volume in volumes: volume.encrypted = encryption limits = self.cinder_limits['absolute'] self.mock_volume_backup_supported.return_value = False - self.mock_volume_list_paged.return_value = [self.volumes.list(), + self.mock_volume_list_paged.return_value = [self.cinder_volumes.list(), False, False] self.mock_volume_snapshot_list.return_value = \ self.cinder_volume_snapshots.list() @@ -1854,23 +1854,23 @@ class VolumeViewTests(test.ResetImageAPIVersionMixin, test.TestCase): for row in rows: self.assertEqual(row.cells['encryption'].data, column_value) - self.assertEqual(8, self.mock_volume_backup_supported.call_count) + self.assertEqual(10, self.mock_volume_backup_supported.call_count) self.mock_volume_list_paged.assert_called_once_with( test.IsHttpRequest(), marker=None, sort_dir='desc', search_opts=None, paginate=True) self.mock_volume_snapshot_list.assert_called_once_with( test.IsHttpRequest(), search_opts=None) - self.assertEqual(11, self.mock_tenant_absolute_limits.call_count) + self.assertEqual(13, self.mock_tenant_absolute_limits.call_count) @mock.patch.object(quotas, 'tenant_quota_usages') @mock.patch.object(cinder, 'volume_get') def test_extend_volume_with_size_out_of_quota(self, mock_get, mock_quotas): - volume = self.volumes.first() + volume = self.cinder_volumes.first() usage_limit = self.cinder_quota_usages.first() usage_limit.add_quota(api.base.Quota('gigabytes', 100)) usage_limit.tally('gigabytes', 20) - usage_limit.tally('volumes', len(self.volumes.list())) + usage_limit.tally('volumes', len(self.cinder_volumes.list())) formData = {'name': u'A Volume I Am Making', 'orig_size': volume.size, @@ -1901,7 +1901,7 @@ class VolumeViewTests(test.ResetImageAPIVersionMixin, test.TestCase): limits = self.cinder_limits['absolute'] self.mock_volume_backup_supported.return_value = False - self.mock_volume_list_paged.return_value = [self.volumes.list(), + self.mock_volume_list_paged.return_value = [self.cinder_volumes.list(), False, False] self.mock_volume_snapshot_list.return_value = [] self.mock_server_list.return_value = [self.servers.list(), False] @@ -1917,7 +1917,7 @@ class VolumeViewTests(test.ResetImageAPIVersionMixin, test.TestCase): self.assertEqual('create_transfer' in actions, vol.status == 'available') - self.assertEqual(8, self.mock_volume_backup_supported.call_count) + self.assertEqual(10, self.mock_volume_backup_supported.call_count) self.mock_volume_list_paged.assert_called_once_with( test.IsHttpRequest(), marker=None, sort_dir='desc', search_opts=None, @@ -1926,12 +1926,12 @@ class VolumeViewTests(test.ResetImageAPIVersionMixin, test.TestCase): test.IsHttpRequest(), search_opts=None) self.mock_server_list.assert_called_once_with(test.IsHttpRequest(), search_opts=None) - self.assertEqual(11, self.mock_tenant_absolute_limits.call_count) + self.assertEqual(13, self.mock_tenant_absolute_limits.call_count) @mock.patch.object(cinder, 'transfer_get') @mock.patch.object(cinder, 'transfer_create') def test_create_transfer(self, mock_transfer_create, mock_transfer_get): - volumes = self.volumes.list() + volumes = self.cinder_volumes.list() volToTransfer = [v for v in volumes if v.status == 'available'][0] formData = {'volume_id': volToTransfer.id, 'name': u'any transfer name'} @@ -2061,7 +2061,7 @@ class VolumeViewTests(test.ResetImageAPIVersionMixin, test.TestCase): limits = self.cinder_limits['absolute'] self.mock_volume_backup_supported.return_value = True - self.mock_volume_list_paged.return_value = [self.volumes.list(), + self.mock_volume_list_paged.return_value = [self.cinder_volumes.list(), False, False] self.mock_volume_snapshot_list.return_value = [] self.mock_server_list.return_value = [self.servers.list(), False] @@ -2077,7 +2077,7 @@ class VolumeViewTests(test.ResetImageAPIVersionMixin, test.TestCase): self.assertEqual('backups' in actions, vol.status in ('available', 'in-use')) - self.assertEqual(8, self.mock_volume_backup_supported.call_count) + self.assertEqual(10, self.mock_volume_backup_supported.call_count) self.mock_volume_list_paged.assert_called_once_with( test.IsHttpRequest(), marker=None, sort_dir='desc', search_opts=None, @@ -2086,4 +2086,4 @@ class VolumeViewTests(test.ResetImageAPIVersionMixin, test.TestCase): test.IsHttpRequest(), search_opts=None) self.mock_server_list.assert_called_once_with(test.IsHttpRequest(), search_opts=None) - self.assertEqual(8, self.mock_volume_backup_supported.call_count) + self.assertEqual(13, self.mock_tenant_absolute_limits.call_count) diff --git a/openstack_dashboard/test/test_data/cinder_data.py b/openstack_dashboard/test/test_data/cinder_data.py index 98b29b915b..cbc5d03fcc 100644 --- a/openstack_dashboard/test/test_data/cinder_data.py +++ b/openstack_dashboard/test/test_data/cinder_data.py @@ -94,6 +94,7 @@ def data(TEST): 'display_description': 'Volume description', 'created_at': '2014-01-27 10:30:00', 'volume_type': None, + 'bootable': 'false', 'attachments': []}) nameless_volume = volumes.Volume( volumes.VolumeManager(None), @@ -105,6 +106,7 @@ def data(TEST): "device": "/dev/hda", "created_at": '2010-11-21 18:34:25', "volume_type": 'vol_type_1', + 'bootable': 'true', "attachments": []}) other_volume = volumes.Volume( volumes.VolumeManager(None), @@ -115,6 +117,7 @@ def data(TEST): 'display_description': '', 'created_at': '2013-04-01 10:30:00', 'volume_type': None, + 'bootable': 'true', 'attachments': [{"id": "1", "server_id": '1', "device": "/dev/hda"}]}) volume_with_type = volumes.Volume( @@ -127,6 +130,7 @@ def data(TEST): 'display_description': '', 'created_at': '2013-04-01 10:30:00', 'volume_type': 'vol_type_2', + 'bootable': 'false', 'attachments': [{"id": "2", "server_id": '2', "device": "/dev/hdb"}]}) non_bootable_volume = volumes.Volume( @@ -138,7 +142,7 @@ def data(TEST): 'display_description': '', 'created_at': '2013-04-01 10:30:00', 'volume_type': None, - 'bootable': False, + 'bootable': 'false', 'attachments': [{"id": "1", "server_id": '1', "device": "/dev/hda"}]}) diff --git a/openstack_dashboard/test/test_data/nova_data.py b/openstack_dashboard/test/test_data/nova_data.py index 24096a766e..0dbe89d917 100644 --- a/openstack_dashboard/test/test_data/nova_data.py +++ b/openstack_dashboard/test/test_data/nova_data.py @@ -158,7 +158,7 @@ def data(TEST): TEST.flavors = utils.TestDataContainer() TEST.flavor_access = utils.TestDataContainer() TEST.keypairs = utils.TestDataContainer() - TEST.volumes = utils.TestDataContainer() + TEST.nova_volumes = utils.TestDataContainer() TEST.quotas = utils.TestDataContainer() TEST.quota_usages = utils.TestDataContainer() TEST.usages = utils.TestDataContainer() @@ -216,15 +216,10 @@ def data(TEST): "volume_type": None, "attachments": []}) - volume.bootable = 'true' - nameless_volume.bootable = 'true' - attached_volume.bootable = 'true' - non_bootable_volume.bootable = 'false' - - TEST.volumes.add(volume) - TEST.volumes.add(nameless_volume) - TEST.volumes.add(attached_volume) - TEST.volumes.add(non_bootable_volume) + TEST.nova_volumes.add(volume) + TEST.nova_volumes.add(nameless_volume) + TEST.nova_volumes.add(attached_volume) + TEST.nova_volumes.add(non_bootable_volume) # Flavors flavor_1 = flavors.Flavor(flavors.FlavorManager(None),