openstack
/
nova
Code Issues Proposed changes

Tests: autospecs all the mock.patch usages 

By default, mock.patch's autospec argument is None, meaning that
there's no signature checking for the patched methods and functions.

oslotest.mock_fixture.patch_mock_module fixes a few issues within
mock.patch functions, as well as setting autospec=True by default,
unless otherwise specified or new_callable, create, spec arguments
are passed in.

Co-Authored-By: Claudiu Belu <cbelu@cloudbasesolutions.com>
Change-Id: I4484e63c97bd1cdde3d88855eabe7545784f365e
Closes-Bug: #1735588
This commit is contained in:
Takashi NATSUME 2019-08-19 18:11:48 +09:00 committed by Eric Fried
parent 8ea5e37338
commit 1eec451b1b
22 changed files with 81 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lower-constraints.txt
View File

@ -92,7 +92,7 @@ oslo.upgradecheck==0.1.1
oslo.utils==3.37.0
oslo.versionedobjects==1.35.0
oslo.vmware==2.17.0
oslotest==3.2.0
oslotest==3.8.0
osprofiler==1.4.0
ovs==2.10.0
ovsdbapp==0.15.0

2
nova/db/sqlalchemy/api.py
View File

@ -5818,7 +5818,7 @@ def instance_tag_set(context, instance_uuid, tags):
if to_add:
data = [
{'resource_id': instance_uuid, 'tag': tag} for tag in to_add]
context.session.execute(models.Tag.__table__.insert(), data)
context.session.execute(models.Tag.__table__.insert(None), data)
return context.session.query(models.Tag).filter_by(
resource_id=instance_uuid).all()

2
nova/network/manager.py
View File

@ -868,7 +868,7 @@ class NetworkManager(manager.Manager):
instance=instance)
address = str(fip.address)
cleanup.append(functools.partial(fip.disassociate, context))
cleanup.append(fip.disassociate)
# NOTE(melwitt): We recheck the quota after creating the object
# to prevent users from allocating more resources than their

2
nova/objects/aggregate.py
View File

@ -126,7 +126,7 @@ def _metadata_add_to_db(context, aggregate_id, metadata, max_retries=10,
"aggregate_id": aggregate_id})
if new_entries:
context.session.execute(
api_models.AggregateMetadata.__table__.insert(),
api_models.AggregateMetadata.__table__.insert(None),
new_entries)
return metadata

7
nova/test.py
View File

@ -153,6 +153,13 @@ class skipIf(object):
'classes')
# NOTE(claudiub): this needs to be called before any mock.patch calls are
# being done, and especially before any other test classes load. This fixes
# the mock.patch autospec issue:
# https://github.com/testing-cabal/mock/issues/396
mock_fixture.patch_mock_module()
class NovaExceptionReraiseFormatError(object):
real_log_exception = exception.NovaException._log_exception

14
nova/tests/functional/test_servers.py
View File

@ -4915,17 +4915,17 @@ class ConsumerGenerationConflictTest(
{'code': 'placement.concurrent_update',
'detail': 'consumer generation conflict'}]}))
def fake_put(_self, url, *args, **kwargs):
self.adapter_put_call_count = 0
def fake_put(_self, url, **kwargs):
self.adapter_put_call_count += 1
migration_uuid = self.get_migration_uuid_for_instance(server['id'])
if url == '/allocations/%s' % migration_uuid:
return rsp
else:
return orig_put(_self, url, *args, **kwargs)
with mock.patch('keystoneauth1.adapter.Adapter.put',
autospec=True) as mock_put:
mock_put.side_effect = fake_put
return orig_put(_self, url, **kwargs)
with mock.patch('keystoneauth1.adapter.Adapter.put', new=fake_put):
post = {
'os-migrateLive': {
'host': dest_hostname,
@ -4951,7 +4951,7 @@ class ConsumerGenerationConflictTest(
'instance.live_migration_post.end')
# 1 claim on destination, 1 normal delete on dest that fails,
self.assertEqual(2, mock_put.call_count)
self.assertEqual(2, self.adapter_put_call_count)
# As the cleanup on the source host failed Nova leaks the allocation
# held by the migration.

4
nova/tests/unit/api/openstack/compute/test_server_migrations.py
View File

@ -110,7 +110,7 @@ class ServerMigrationsTestsV21(test.NoDBTestCase):
self.controller._force_complete(self.req, '1', '1',
body={'force_complete': None})
live_migrate_force_complete.assert_called_once_with(
self.context, compute_api_get(), '1')
self.context, compute_api_get.return_value, '1')
_do_test()
def _test_force_complete_failed_with_exception(self, fake_exc,
@ -284,7 +284,7 @@ class ServerMigrationsTestsV224(ServerMigrationsTestsV21):
def _do_test(mock_get, mock_abort):
self.controller.delete(self.req, 'server-id', 'migration-id')
mock_abort.assert_called_once_with(self.context,
mock_get(),
mock_get.return_value,
'migration-id',
support_abort_in_queue=False)
_do_test()

24
nova/tests/unit/cmd/test_common.py
View File

@ -17,6 +17,8 @@
Unit tests for the common functions used by different CLI interfaces.
"""
import sys
import fixtures
import mock
from six.moves import StringIO
@ -68,31 +70,30 @@ class TestCmdCommon(test.NoDBTestCase):
self.assertNotIn('_private', method_names)
self.assertNotIn('foo', method_names)
@mock.patch.object(cmd_common, 'print')
@mock.patch.object(cmd_common, 'CONF')
def test_print_bash_completion_no_query_category(self, mock_CONF,
mock_print):
def test_print_bash_completion_no_query_category(self, mock_CONF):
self.useFixture(fixtures.MonkeyPatch('sys.stdout', StringIO()))
mock_CONF.category.query_category = None
categories = {'foo': mock.sentinel.foo, 'bar': mock.sentinel.bar}
cmd_common.print_bash_completion(categories)
mock_print.assert_called_once_with(' '.join(categories.keys()))
self.assertEqual(' '.join(categories.keys()) + '\n',
sys.stdout.getvalue())
@mock.patch.object(cmd_common, 'print')
@mock.patch.object(cmd_common, 'CONF')
def test_print_bash_completion_mismatch(self, mock_CONF, mock_print):
def test_print_bash_completion_mismatch(self, mock_CONF):
self.useFixture(fixtures.MonkeyPatch('sys.stdout', StringIO()))
mock_CONF.category.query_category = 'bar'
categories = {'foo': mock.sentinel.foo}
cmd_common.print_bash_completion(categories)
self.assertFalse(mock_print.called)
self.assertEqual('', sys.stdout.getvalue())
@mock.patch.object(cmd_common, 'methods_of')
@mock.patch.object(cmd_common, 'print')
@mock.patch.object(cmd_common, 'CONF')
def test_print_bash_completion(self, mock_CONF, mock_print,
mock_method_of):
def test_print_bash_completion(self, mock_CONF, mock_method_of):
self.useFixture(fixtures.MonkeyPatch('sys.stdout', StringIO()))
mock_CONF.category.query_category = 'foo'
actions = [('f1', mock.sentinel.f1), ('f2', mock.sentinel.f2)]
mock_method_of.return_value = actions
@ -103,7 +104,8 @@ class TestCmdCommon(test.NoDBTestCase):
mock_fn.assert_called_once_with()
mock_method_of.assert_called_once_with(mock_fn.return_value)
mock_print.assert_called_once_with(' '.join([k for k, v in actions]))
self.assertEqual(' '.join([k for k, v in actions]) + '\n',
sys.stdout.getvalue())
@mock.patch.object(cmd_common, 'validate_args')
@mock.patch.object(cmd_common, 'CONF')

2
nova/tests/unit/compute/fake_resource_tracker.py
View File

@ -28,5 +28,7 @@ class FakeResourceTracker(resource_tracker.ResourceTracker):
class RTMockMixin(object):
def _mock_rt(self, **kwargs):
if 'spec_set' in kwargs:
kwargs.update({'autospec': False})
return self.useFixture(fixtures.MockPatchObject(
self.compute, 'rt', **kwargs)).mock

2
nova/tests/unit/db/test_db_api.py
View File

@ -9247,7 +9247,7 @@ class TestDBInstanceTags(test.TestCase):
db.instance_tag_set(self.context, uuid, [tag1, tag2])
# Check insert() was called to insert 'tag1' and 'tag2'
mock_insert.assert_called_once_with()
mock_insert.assert_called_once_with(None)
mock_insert.reset_mock()
db.instance_tag_set(self.context, uuid, [tag1])

4
nova/tests/unit/network/test_manager.py
View File

@ -697,7 +697,7 @@ class FlatNetworkTestCase(test.TestCase):
# Verify we removed the fixed IP that was added after the first quota
# check passed.
disassociate.assert_called_once_with(self.context)
disassociate.assert_called_once_with()
@mock.patch('nova.objects.fixed_ip.FixedIP.associate_pool')
@mock.patch('nova.objects.instance.Instance.get_by_uuid')
@ -794,7 +794,7 @@ class FlatNetworkTestCase(test.TestCase):
mock.call(instance.uuid, '')
])
mock_fixedip_disassociate.assert_called_once_with(self.context)
mock_fixedip_disassociate.assert_called_once_with()
@mock.patch('nova.objects.instance.Instance.get_by_uuid')
@mock.patch('nova.objects.virtual_interface.VirtualInterface'

4
nova/tests/unit/network/test_neutronv2.py
View File

@ -3515,7 +3515,7 @@ class TestNeutronv2WithMock(TestNeutronv2Base):
}
test_ext_list = {'extensions': []}
mock_client = mock_get_client()
mock_client = mock_get_client.return_value
mock_client.show_port.return_value = test_port
mock_client.list_extensions.return_value = test_ext_list
result = self.api._get_port_vnic_info(
@ -4149,7 +4149,7 @@ class TestNeutronv2WithMock(TestNeutronv2Base):
mock_refresh_extensions):
api = neutronapi.API()
api.extensions = set([constants.VNIC_INDEX_EXT])
mock_client = mock_get_client()
mock_client = mock_get_client.return_value
mock_client.update_port.return_value = 'port'
instance = {'project_id': '9d049e4b60b64716978ab415e6fbd5c0',

4
nova/tests/unit/scheduler/fakes.py
View File

@ -183,5 +183,7 @@ class FakeHostState(host_manager.HostState):
class FakeScheduler(driver.Scheduler):
def select_destinations(self, context, spec_obj, instance_uuids):
def select_destinations(self, context, spec_obj, instance_uuids,
alloc_reqs_by_rp_uuid, provider_summaries,
allocation_request_version=None, return_alternates=False):
return []

3
nova/tests/unit/scheduler/test_scheduler.py
View File

@ -68,8 +68,7 @@ class SchedulerManagerTestCase(test.NoDBTestCase):
def setUp(self, mock_init_agg, mock_init_inst):
super(SchedulerManagerTestCase, self).setUp()
self.flags(driver=self.driver_plugin_name, group='scheduler')
with mock.patch.object(host_manager.HostManager, '_init_aggregates'):
self.manager = self.manager_cls()
self.manager = self.manager_cls()
self.context = context.RequestContext('fake_user', 'fake_project')
self.topic = 'fake_topic'
self.fake_args = (1, 2, 3)

5
nova/tests/unit/test_context.py
View File

@ -411,8 +411,9 @@ class ContextTestCase(test.NoDBTestCase):
mock_get_inst.side_effect = [mock.sentinel.instances,
test.TestingException()]
filters = {'deleted': False}
results = context.scatter_gather_cells(
ctxt, mappings, 30, objects.InstanceList.get_by_filters)
ctxt, mappings, 30, objects.InstanceList.get_by_filters, filters)
self.assertEqual(2, len(results))
self.assertIn(mock.sentinel.instances, results.values())
self.assertIsInstance(results[mapping1.uuid], Exception)
@ -425,7 +426,7 @@ class ContextTestCase(test.NoDBTestCase):
exception.NotFound()]
results = context.scatter_gather_cells(
ctxt, mappings, 30, objects.InstanceList.get_by_filters)
ctxt, mappings, 30, objects.InstanceList.get_by_filters, filters)
self.assertEqual(2, len(results))
self.assertIn(mock.sentinel.instances, results.values())
self.assertIsInstance(results[mapping1.uuid], exception.NovaException)

10
nova/tests/unit/test_json_ref.py
View File

@ -55,7 +55,7 @@ class TestJsonRef(test.NoDBTestCase):
d = copy.deepcopy(input)
@mock.patch('oslo_serialization.jsonutils.load')
@mock.patch('nova.tests.json_ref.open')
@mock.patch('six.moves.builtins.open', new_callable=mock.mock_open())
def test_resolve_ref(self, mock_open, mock_json_load):
mock_json_load.return_value = {'baz': 13}
@ -70,7 +70,7 @@ class TestJsonRef(test.NoDBTestCase):
mock_open.assert_called_once_with('some/base/path/another.json', 'r+b')
@mock.patch('oslo_serialization.jsonutils.load')
@mock.patch('nova.tests.json_ref.open')
@mock.patch('six.moves.builtins.open', new_callable=mock.mock_open())
def test_resolve_ref_recursively(self, mock_open, mock_json_load):
mock_json_load.side_effect = [
# this is the content of direct_ref.json
@ -94,7 +94,7 @@ class TestJsonRef(test.NoDBTestCase):
'r+b')
@mock.patch('oslo_serialization.jsonutils.load')
@mock.patch('nova.tests.json_ref.open')
@mock.patch('six.moves.builtins.open', new_callable=mock.mock_open())
def test_resolve_ref_with_override(self, mock_open, mock_json_load):
mock_json_load.return_value = {'baz': 13,
'boo': 42}
@ -112,7 +112,7 @@ class TestJsonRef(test.NoDBTestCase):
mock_open.assert_called_once_with('some/base/path/another.json', 'r+b')
@mock.patch('oslo_serialization.jsonutils.load')
@mock.patch('nova.tests.json_ref.open')
@mock.patch('six.moves.builtins.open', new_callable=mock.mock_open())
def test_resolve_ref_with_nested_override(self, mock_open, mock_json_load):
mock_json_load.return_value = {'baz': 13,
'boo': {'a': 1,
@ -134,7 +134,7 @@ class TestJsonRef(test.NoDBTestCase):
mock_open.assert_called_once_with('some/base/path/another.json', 'r+b')
@mock.patch('oslo_serialization.jsonutils.load')
@mock.patch('nova.tests.json_ref.open')
@mock.patch('six.moves.builtins.open', new_callable=mock.mock_open())
def test_resolve_ref_with_override_having_refs(self, mock_open,
mock_json_load):
mock_json_load.side_effect = [

12
nova/tests/unit/test_metadata.py
View File

@ -1396,7 +1396,7 @@ class MetadataHandlerTestCase(test.TestCase):
# with X-Metadata-Provider
proxy_lb_id = 'edge-x'
mock_client = mock_get_client()
mock_client = mock_get_client.return_value
mock_client.list_ports.return_value = {
'ports': [{'device_id': 'a-b-c-d', 'tenant_id': 'test'}]}
mock_client.list_subnets.return_value = {
@ -1417,7 +1417,7 @@ class MetadataHandlerTestCase(test.TestCase):
# with X-Metadata-Provider
proxy_lb_id = 'edge-x'
mock_client = mock_get_client()
mock_client = mock_get_client.return_value
mock_client.list_ports.return_value = {
'ports': [{'device_id': 'a-b-c-d', 'tenant_id': 'test'}]}
mock_client.list_subnets.return_value = {
@ -1478,7 +1478,7 @@ class MetadataHandlerTestCase(test.TestCase):
return {'ports':
[]}
mock_client = mock_get_client()
mock_client = mock_get_client.return_value
mock_client.list_ports.side_effect = fake_list_ports
mock_client.list_subnets.return_value = {
'subnets': [{'network_id': 'f-f-f-f'}]}
@ -1511,7 +1511,7 @@ class MetadataHandlerTestCase(test.TestCase):
encodeutils.to_utf8(proxy_lb_id),
hashlib.sha256).hexdigest()
mock_client = mock_get_client()
mock_client = mock_get_client.return_value
mock_client.list_ports.return_value = {
'ports': [{'device_id': 'a-b-c-d', 'tenant_id': 'test'}]}
mock_client.list_subnets.return_value = {
@ -1541,7 +1541,7 @@ class MetadataHandlerTestCase(test.TestCase):
# with X-Metadata-Provider
proxy_lb_id = 'edge-x'
mock_client = mock_get_client()
mock_client = mock_get_client.return_value
mock_client.list_ports.return_value = {
'ports': [{'device_id': 'a-b-c-d', 'tenant_id': 'test'}]}
mock_client.list_subnets.return_value = {
@ -1575,7 +1575,7 @@ class MetadataHandlerTestCase(test.TestCase):
encodeutils.to_utf8(proxy_lb_id),
hashlib.sha256).hexdigest()
mock_client = mock_get_client()
mock_client = mock_get_client.return_value
mock_client.list_ports.return_value = {
'ports': [{'device_id': 'a-b-c-d', 'tenant_id': 'test'}]}
mock_client.list_subnets.return_value = {

2
nova/tests/unit/virt/hyperv/test_vmops.py
View File

@ -989,7 +989,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
mock_get_configdrive_path.assert_has_calls(
expected_get_configdrive_path_calls)
mock_ConfigDriveBuilder.assert_called_with(
instance_md=mock_InstanceMetadata())
instance_md=mock_InstanceMetadata.return_value)
mock_make_drive = mock_ConfigDriveBuilder().__enter__().make_drive
mock_make_drive.assert_called_once_with(path_iso)
if not CONF.hyperv.config_drive_cdrom:

20
nova/tests/unit/virt/ironic/utils.py
View File

@ -150,8 +150,8 @@ def get_test_image_meta(**kw):
class FakeVolumeTargetClient(object):
def create(self, node_uuid, driver_volume_type, target_properties,
boot_index):
def create(self, node_uuid, volume_type, properties, boot_index,
volume_id):
pass
def delete(self, volume_target_id):
@ -160,7 +160,9 @@ class FakeVolumeTargetClient(object):
class FakePortClient(object):
def list(self, address=None, node=None):
def list(self, address=None, limit=None, marker=None, sort_key=None,
sort_dir=None, detail=False, fields=None, node=None,
portgroup=None):
pass
def get(self, port_uuid):
@ -172,13 +174,17 @@ class FakePortClient(object):
class FakePortgroupClient(object):
def list(self, node=None, detail=False):
def list(self, node=None, address=None, limit=None, marker=None,
sort_key=None, sort_dir=None, detail=False, fields=None):
pass
class FakeNodeClient(object):
def list(self, detail=False):
def list(self, associated=None, maintenance=None, marker=None, limit=None,
detail=False, sort_key=None, sort_dir=None, fields=None,
provision_state=None, driver=None, resource_class=None,
chassis=None):
return []
def get(self, node_uuid, fields=None):
@ -193,7 +199,9 @@ class FakeNodeClient(object):
def set_power_state(self, node_uuid, target, soft=False, timeout=None):
pass
def set_provision_state(self, node_uuid, target):
def set_provision_state(self, node_uuid, state, configdrive=None,
cleansteps=None, rescue_password=None,
os_ironic_api_version=None):
pass
def update(self, node_uuid, patch):

3
nova/tests/unit/virt/powervm/test_driver.py
View File

@ -523,7 +523,8 @@ class TestPowerVMDriver(test.NoDBTestCase):
'net_info')
mock_tf.return_value.add.assert_called_once_with(
mock_unplug_vifs.return_value)
mock_tf_run.assert_called_once_with(mock_tf.return_value, mock_inst)
mock_tf_run.assert_called_once_with(mock_tf.return_value,
instance=mock_inst)
# InstanceNotFound should pass
mock_tf_run.side_effect = exception.InstanceNotFound(instance_id='1')

11
nova/tests/unit/virt/xenapi/image/test_vdi_through_dev.py
View File

@ -92,20 +92,19 @@ class TestUploadToGlanceAsRawTgz(test.NoDBTestCase):
return_value=('readfile', 'writefile')),
mock.patch.object(store, '_get_virtual_size',
return_value='324'),
mock.patch.object(producer, 'get_metadata',
return_value='metadata'),
mock.patch.object(glance, 'UpdateGlanceImage',
return_value=consumer),
mock.patch.object(glance, 'UpdateGlanceImage',
return_value=consumer),
mock.patch.object(vdi_through_dev, 'TarGzProducer',
return_value=producer),
mock.patch.object(vdi_through_dev.eventlet, 'GreenPool',
return_value=pool)
) as (mock_create_pipe, mock_virtual_size, mock_metadata,
) as (mock_create_pipe, mock_virtual_size,
mock_upload, mock_TarGzProducer, mock_greenpool):
producer.get_metadata.return_value = "metadata"
store._perform_upload('devpath')
mock_metadata.assert_called_once_with()
producer.get_metadata.assert_called_once_with()
mock_virtual_size.assert_called_once_with()
mock_create_pipe.assert_called_once_with()
mock_TarGzProducer.assert_called_once_with(

2
test-requirements.txt
View File

@ -15,7 +15,7 @@ pycodestyle>=2.0.0 # MIT License
python-barbicanclient>=4.5.2 # Apache-2.0
python-ironicclient!=2.7.1,>=2.7.0 # Apache-2.0
requests-mock>=1.2.0 # Apache-2.0
oslotest>=3.2.0 # Apache-2.0
oslotest>=3.8.0 # Apache-2.0
stestr>=1.0.0 # Apache-2.0
osprofiler>=1.4.0 # Apache-2.0
testresources>=2.0.0 # Apache-2.0/BSD