[pylint] Fix/ignore pylint errors in test modules

- Add ignores to pylint false positives in the
  test modules.
- Remove unnecessary fake data
- Fix wrong mock methods used in tests

Change-Id: I64ffff15cc546c67e7e545b1da7ec0efa002bdc5
This commit is contained in:
Goutham Pacha Ravi 2019-02-19 19:10:24 -08:00
parent 1efbf6f0c6
commit 3bd1e5054a
21 changed files with 49 additions and 64 deletions

View File

@ -22,15 +22,11 @@ import webob.dec
import webob.request import webob.request
from manila.api import common as api_common from manila.api import common as api_common
from manila.api.middleware import auth
from manila.api.middleware import fault
from manila.api.openstack import api_version_request as api_version from manila.api.openstack import api_version_request as api_version
from manila.api.openstack import wsgi as os_wsgi from manila.api.openstack import wsgi as os_wsgi
from manila.api import urlmap from manila.api import urlmap
from manila.api.v1 import limits
from manila.api.v1 import router as router_v1 from manila.api.v1 import router as router_v1
from manila.api.v2 import router as router_v2 from manila.api.v2 import router as router_v2
from manila.api import versions
from manila.common import constants from manila.common import constants
from manila import context from manila import context
from manila import exception from manila import exception
@ -61,31 +57,6 @@ def fake_wsgi(self, req):
return self.application return self.application
def wsgi_app(inner_app_v2=None, fake_auth=True, fake_auth_context=None,
use_no_auth=False, ext_mgr=None):
if not inner_app_v2:
inner_app_v2 = router_v2.APIRouter(ext_mgr)
if fake_auth:
if fake_auth_context is not None:
ctxt = fake_auth_context
else:
ctxt = context.RequestContext('fake', 'fake', auth_token=True)
api_v2 = fault.FaultWrapper(auth.InjectContext(ctxt,
inner_app_v2))
elif use_no_auth:
api_v2 = fault.FaultWrapper(auth.NoAuthMiddleware(
limits.RateLimitingMiddleware(inner_app_v2)))
else:
api_v2 = fault.FaultWrapper(auth.AuthMiddleware(
limits.RateLimitingMiddleware(inner_app_v2)))
mapper = urlmap.URLMap()
mapper['/v2'] = api_v2
mapper['/'] = fault.FaultWrapper(versions.Versions())
return mapper
class FakeToken(object): class FakeToken(object):
id_count = 0 id_count = 0
@ -95,6 +66,7 @@ class FakeToken(object):
def __init__(self, **kwargs): def __init__(self, **kwargs):
FakeToken.id_count += 1 FakeToken.id_count += 1
self.id = FakeToken.id_count self.id = FakeToken.id_count
self.token_hash = None
for k, v in kwargs.items(): for k, v in kwargs.items():
setattr(self, k, v) setattr(self, k, v)

View File

@ -269,7 +269,7 @@ class ExperimentalAPITestCase(test.TestCase):
return {'fake_key': 'fake_value'} return {'fake_key': 'fake_value'}
@wsgi.Controller.api_version('2.1', '2.1', experimental=True) # noqa @wsgi.Controller.api_version('2.1', '2.1', experimental=True) # noqa
def index(self, req): # pylint: disable=E0102 def index(self, req): # pylint: disable=function-redefined
return {'fake_key': 'fake_value'} return {'fake_key': 'fake_value'}
def setUp(self): def setUp(self):

View File

@ -369,7 +369,7 @@ class ParseLimitsTest(BaseLimitTestSuite):
'(POST, /bar*, /bar.*, 5, second);' '(POST, /bar*, /bar.*, 5, second);'
'(Say, /derp*, /derp.*, 1, day)') '(Say, /derp*, /derp.*, 1, day)')
except ValueError as e: except ValueError as e:
assert False, six.text_types(e) assert False, six.text_type(e)
# Make sure the number of returned limits are correct # Make sure the number of returned limits are correct
self.assertEqual(4, len(l)) self.assertEqual(4, len(l))

View File

@ -43,7 +43,8 @@ class ClientAuthTestCase(test.TestCase):
'foo_group') 'foo_group')
self.fake_client.assert_called_once_with( self.fake_client.assert_called_once_with(
session=mock_load_session(), session=mock_load_session(),
auth=auth.load_auth_from_conf_options()) auth=auth.load_auth_from_conf_options(
client_auth.CONF, 'foo_group'))
def test_get_client_admin_false(self): def test_get_client_admin_false(self):
self.mock_object(auth, 'load_session_from_conf_options') self.mock_object(auth, 'load_session_from_conf_options')

View File

@ -2100,6 +2100,7 @@ class ShareGroupSnapshotMemberNewProviderLocationColumnChecks(
self.test_case.assertTrue(hasattr(sgsm, 'provider_location')) self.test_case.assertTrue(hasattr(sgsm, 'provider_location'))
# Check that we can write string data to the new field # Check that we can write string data to the new field
# pylint: disable=no-value-for-parameter
engine.execute(sgsm_table.update().where( engine.execute(sgsm_table.update().where(
sgsm_table.c.id == self.share_group_snapshot_member_id, sgsm_table.c.id == self.share_group_snapshot_member_id,
).values({ ).values({
@ -2151,12 +2152,14 @@ class ShareGroupNewConsistentSnapshotSupportColumnChecks(BaseMigrationChecks):
# Check that we can write proper enum data to the new field # Check that we can write proper enum data to the new field
for value in (None, 'pool', 'host'): for value in (None, 'pool', 'host'):
# pylint: disable=no-value-for-parameter
engine.execute(sg_table.update().where( engine.execute(sg_table.update().where(
sg_table.c.id == self.share_group_id, sg_table.c.id == self.share_group_id,
).values({self.new_attr_name: value})) ).values({self.new_attr_name: value}))
# Check that we cannot write values that are not allowed by enum. # Check that we cannot write values that are not allowed by enum.
for value in ('', 'fake', 'pool1', 'host1', '1pool', '1host'): for value in ('', 'fake', 'pool1', 'host1', '1pool', '1host'):
# pylint: disable=no-value-for-parameter
self.test_case.assertRaises( self.test_case.assertRaises(
oslo_db_exc.DBError, oslo_db_exc.DBError,
engine.execute, engine.execute,
@ -2241,6 +2244,7 @@ class ShareGroupNewAvailabilityZoneIDColumnChecks(BaseMigrationChecks):
# Check that we can write proper data to the new field # Check that we can write proper data to the new field
for value in (None, self.availability_zone_id): for value in (None, self.availability_zone_id):
# pylint: disable=no-value-for-parameter
engine.execute(sg_table.update().where( engine.execute(sg_table.update().where(
sg_table.c.id == self.share_group_id, sg_table.c.id == self.share_group_id,
).values({self.new_attr_name: value})) ).values({self.new_attr_name: value}))
@ -2337,6 +2341,7 @@ class SquashSGSnapshotMembersAndSSIModelsChecks(BaseMigrationChecks):
self.test_case.assertTrue(hasattr(ssi, key)) self.test_case.assertTrue(hasattr(ssi, key))
# Check that we can write string data to the new fields # Check that we can write string data to the new fields
# pylint: disable=no-value-for-parameter
engine.execute(ssi_table.update().where( engine.execute(ssi_table.update().where(
ssi_table.c.id == self.share_group_snapshot_member_id, ssi_table.c.id == self.share_group_snapshot_member_id,
).values({ ).values({

View File

@ -43,6 +43,7 @@ class FakeServer(object):
class FakeKeypair(object): class FakeKeypair(object):
def __init__(self, **kwargs): def __init__(self, **kwargs):
self.id = kwargs.pop('id', 'fake_keypair_id') self.id = kwargs.pop('id', 'fake_keypair_id')
self.name = None
for key, value in kwargs.items(): for key, value in kwargs.items():
setattr(self, key, value) setattr(self, key, value)

View File

@ -116,7 +116,7 @@ def fake_snapshot(create_instance=False, **kwargs):
'name': 'fakesnapshotname', 'name': 'fakesnapshotname',
'share_size': 1, 'share_size': 1,
'share_proto': 'fake_proto', 'share_proto': 'fake_proto',
'instance': None, 'instance': {},
'share': 'fake_share', 'share': 'fake_share',
'aggregate_status': aggregate_status, 'aggregate_status': aggregate_status,
'project_id': 'fakeprojectid', 'project_id': 'fakeprojectid',

View File

@ -24,10 +24,10 @@ from manila import utils
class BaseChild(interface.LinuxInterfaceDriver): class BaseChild(interface.LinuxInterfaceDriver):
def plug(*args): def plug(self, *args):
pass pass
def unplug(*args): def unplug(self, *args):
pass pass
@ -74,6 +74,7 @@ class TestABCDriver(TestBase):
pass pass
try: try:
# pylint: disable=abstract-class-instantiated
ICanNotBeInstancetiated() ICanNotBeInstancetiated()
except TypeError: except TypeError:
pass pass

View File

@ -751,7 +751,9 @@ class NeutronBindNetworkPluginTest(test.TestCase):
self.fake_context, self.fake_context,
fake_network_allocation) fake_network_allocation)
self.bind_plugin._wait_for_ports_bind.assert_called_once_with( self.bind_plugin._wait_for_ports_bind.assert_called_once_with(
[db_api.network_allocation_create()], fake_share_server) [db_api.network_allocation_create(
self.fake_context, fake_network_allocation)],
fake_share_server)
@mock.patch.object(db_api, 'network_allocation_create', @mock.patch.object(db_api, 'network_allocation_create',
mock.Mock(return_values=fake_network_allocation_multi)) mock.Mock(return_values=fake_network_allocation_multi))
@ -1270,7 +1272,9 @@ class NeutronBindSingleNetworkPluginTest(test.TestCase):
self.fake_context, self.fake_context,
fake_network_allocation) fake_network_allocation)
self.bind_plugin._wait_for_ports_bind.assert_called_once_with( self.bind_plugin._wait_for_ports_bind.assert_called_once_with(
[db_api.network_allocation_create()], fake_share_server) [db_api.network_allocation_create(
self.fake_context, fake_network_allocation)],
fake_share_server)
@ddt.data({ @ddt.data({
'neutron_binding_profiles': None, 'neutron_binding_profiles': None,

View File

@ -73,7 +73,7 @@ class SchedulerManagerTestCase(test.TestCase):
self.fake_args = (1, 2, 3) self.fake_args = (1, 2, 3)
self.fake_kwargs = {'cat': 'meow', 'dog': 'woof'} self.fake_kwargs = {'cat': 'meow', 'dog': 'woof'}
def raise_no_valid_host(*args, **kwargs): def raise_no_valid_host(self, *args, **kwargs):
raise exception.NoValidHost(reason="") raise exception.NoValidHost(reason="")
def test_1_correct_init(self): def test_1_correct_init(self):

View File

@ -46,6 +46,7 @@ class FakeSchedulerOptions(scheduler_options.SchedulerOptions):
def _get_file_handle(self, filename): def _get_file_handle(self, filename):
self.file_was_loaded = True self.file_was_loaded = True
if six.PY2: if six.PY2:
# pylint: disable=import-error
import StringIO import StringIO
return StringIO.StringIO(self._file_data) return StringIO.StringIO(self._file_data)
else: else:

View File

@ -90,7 +90,7 @@ class CapacityWeigherTestCase(test.TestCase):
@ddt.unpack @ddt.unpack
def test_default_of_spreading_first(self, cap_thin, cap_thin_key, def test_default_of_spreading_first(self, cap_thin, cap_thin_key,
winner): winner):
hostinfo_list = self._get_all_hosts() hosts = self._get_all_hosts() # pylint: disable=no-value-for-parameter
# Results for the 1st test # Results for the 1st test
# {'capabilities:thin_provisioning': '<is> True'}: # {'capabilities:thin_provisioning': '<is> True'}:
@ -131,16 +131,16 @@ class CapacityWeigherTestCase(test.TestCase):
} }
} }
weighed_host = self._get_weighed_host( weighed_host = self._get_weighed_host(
hostinfo_list, hosts,
weight_properties=weight_properties) weight_properties=weight_properties)
self.assertEqual(1.0, weighed_host.weight) self.assertEqual(1.0, weighed_host.weight)
self.assertEqual( self.assertEqual(
winner, utils.extract_host(weighed_host.obj.host)) winner, utils.extract_host(weighed_host.obj.host))
def test_unknown_is_last(self): def test_unknown_is_last(self):
hostinfo_list = self._get_all_hosts() hosts = self._get_all_hosts() # pylint: disable=no-value-for-parameter
last_host = self._get_weighed_host(hostinfo_list, index=-1) last_host = self._get_weighed_host(hosts, index=-1)
self.assertEqual( self.assertEqual(
'host6', utils.extract_host(last_host.obj.host)) 'host6', utils.extract_host(last_host.obj.host))
self.assertEqual(0.0, last_host.weight) self.assertEqual(0.0, last_host.weight)
@ -173,7 +173,7 @@ class CapacityWeigherTestCase(test.TestCase):
cap_thin_key, cap_thin_key,
winner): winner):
self.flags(capacity_weight_multiplier=-1.0) self.flags(capacity_weight_multiplier=-1.0)
hostinfo_list = self._get_all_hosts() hosts = self._get_all_hosts() # pylint: disable=no-value-for-parameter
# Results for the 1st test # Results for the 1st test
# {'capabilities:thin_provisioning': '<is> True'}: # {'capabilities:thin_provisioning': '<is> True'}:
@ -220,7 +220,7 @@ class CapacityWeigherTestCase(test.TestCase):
} }
} }
weighed_host = self._get_weighed_host( weighed_host = self._get_weighed_host(
hostinfo_list, hosts,
weight_properties=weight_properties) weight_properties=weight_properties)
self.assertEqual(0.0, weighed_host.weight) self.assertEqual(0.0, weighed_host.weight)
self.assertEqual( self.assertEqual(
@ -253,7 +253,7 @@ class CapacityWeigherTestCase(test.TestCase):
def test_capacity_weight_multiplier_2(self, cap_thin, cap_thin_key, def test_capacity_weight_multiplier_2(self, cap_thin, cap_thin_key,
winner): winner):
self.flags(capacity_weight_multiplier=2.0) self.flags(capacity_weight_multiplier=2.0)
hostinfo_list = self._get_all_hosts() hosts = self._get_all_hosts() # pylint: disable=no-value-for-parameter
# Results for the 1st test # Results for the 1st test
# {'capabilities:thin_provisioning': '<is> True'}: # {'capabilities:thin_provisioning': '<is> True'}:
@ -299,8 +299,7 @@ class CapacityWeigherTestCase(test.TestCase):
} }
} }
weighed_host = self._get_weighed_host( weighed_host = self._get_weighed_host(
hostinfo_list, hosts, weight_properties=weight_properties)
weight_properties=weight_properties)
self.assertEqual(2.0, weighed_host.weight) self.assertEqual(2.0, weighed_host.weight)
self.assertEqual( self.assertEqual(
winner, utils.extract_host(weighed_host.obj.host)) winner, utils.extract_host(weighed_host.obj.host))

View File

@ -16,8 +16,8 @@
from eventlet import greenthread from eventlet import greenthread
import mock import mock
from oslo_concurrency import processutils from oslo_concurrency import processutils
from six.moves.urllib import error as url_error # pylint: disable=E0611 from six.moves.urllib import error as url_error
from six.moves.urllib import request as url_request # pylint: disable=E0611 from six.moves.urllib import request as url_request
from manila import exception from manila import exception
from manila.share import configuration as conf from manila.share import configuration as conf

View File

@ -20,7 +20,7 @@ from manila.share.drivers.dell_emc.plugins.unity import utils
from manila import test from manila import test
class MockPort(object): class MockSP(object):
def __init__(self, sp_id): def __init__(self, sp_id):
self.sp_id = sp_id self.sp_id = sp_id
@ -28,8 +28,8 @@ class MockPort(object):
return self.sp_id return self.sp_id
SPA = MockPort('spa') SPA = MockSP('spa')
SPB = MockPort('spb') SPB = MockSP('spb')
class MockPort(object): class MockPort(object):

View File

@ -798,7 +798,8 @@ class MapRFSNativeShareDriverTestCase(test.TestCase):
self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool)) self.mock_object(utils, 'SSHPool', mock.Mock(return_value=ssh_pool))
self.mock_object(processutils, 'ssh_execute', self.mock_object(processutils, 'ssh_execute',
mock.Mock(return_value=ssh_output)) mock.Mock(return_value=ssh_output))
result = self._driver._maprfs_util._run_ssh(self.local_ip, cmd_list) result = self._driver._maprfs_util._run_ssh(
self.local_ip, cmd_list, check_exit_code=False)
utils.SSHPool.assert_called_once_with( utils.SSHPool.assert_called_once_with(
self._driver.configuration.maprfs_clinode_ip[0], self._driver.configuration.maprfs_clinode_ip[0],
self._driver.configuration.maprfs_ssh_port, self._driver.configuration.maprfs_ssh_port,

View File

@ -90,7 +90,7 @@ class NetAppDriverUtilsTestCase(test.TestCase):
api_trace_pattern=regex) api_trace_pattern=regex)
@na_utils.trace @na_utils.trace
def _trace_test_method(*args, **kwargs): def _trace_test_method(self, *args, **kwargs):
return 'OK' return 'OK'
def test_trace_no_tracing(self): def test_trace_no_tracing(self):

View File

@ -492,13 +492,13 @@ class GenericShareDriverTestCase(test.TestCase):
def test_add_mount_permanently(self): def test_add_mount_permanently(self):
self.mock_object(self._driver, '_ssh_exec') self.mock_object(self._driver, '_ssh_exec')
self._driver._add_mount_permanently(self.share.id, self.server) self._driver._add_mount_permanently(self.share.id, self.server)
self._driver._ssh_exec.has_calls( self._driver._ssh_exec.assert_has_calls([
mock.call( mock.call(
self.server, self.server,
['grep', self.share.id, const.MOUNT_FILE_TEMP, ['grep', self.share.id, const.MOUNT_FILE_TEMP,
'|', 'sudo', 'tee', '-a', const.MOUNT_FILE]), '|', 'sudo', 'tee', '-a', const.MOUNT_FILE]),
mock.call(self.server, ['sudo', 'mount', '-a']) mock.call(self.server, ['sudo', 'mount', '-a'])
) ])
def test_add_mount_permanently_raise_error_on_add(self): def test_add_mount_permanently_raise_error_on_add(self):
self.mock_object( self.mock_object(

View File

@ -1387,7 +1387,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
{'status': 'ACTIVE'}] {'status': 'ACTIVE'}]
# Note that in this case, although the status is active, the # Note that in this case, although the status is active, the
# 'networks' field is missing. # 'networks' field is missing.
self._test_wait_for_instance( self._test_wait_for_instance( # pylint: disable=no-value-for-parameter
server_get_side_eff=server_get_side_eff, server_get_side_eff=server_get_side_eff,
expected_exc=exception.ServiceInstanceException, expected_exc=exception.ServiceInstanceException,
expected_try_count=3, expected_try_count=3,
@ -1395,7 +1395,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
def test_wait_for_instance_error_state(self): def test_wait_for_instance_error_state(self):
mock_instance = {'status': 'ERROR'} mock_instance = {'status': 'ERROR'}
self._test_wait_for_instance( self._test_wait_for_instance( # pylint: disable=no-value-for-parameter
server_get_side_eff=[mock_instance], server_get_side_eff=[mock_instance],
expected_exc=exception.ServiceInstanceException, expected_exc=exception.ServiceInstanceException,
expected_try_count=1) expected_try_count=1)
@ -1403,7 +1403,7 @@ class ServiceInstanceManagerTestCase(test.TestCase):
def test_wait_for_instance_available(self): def test_wait_for_instance_available(self):
mock_instance = {'status': 'ACTIVE', mock_instance = {'status': 'ACTIVE',
'networks': mock.sentinel.networks} 'networks': mock.sentinel.networks}
self._test_wait_for_instance( self._test_wait_for_instance( # pylint: disable=no-value-for-parameter
server_get_side_eff=[mock_instance], server_get_side_eff=[mock_instance],
expected_try_count=1, expected_try_count=1,
expected_ret_val=mock_instance) expected_ret_val=mock_instance)

View File

@ -95,7 +95,7 @@ class FakeTempDir(object):
def __enter__(self, *args, **kwargs): def __enter__(self, *args, **kwargs):
return '/foo/path' return '/foo/path'
def __exit__(*args, **kwargs): def __exit__(self, *args, **kwargs):
pass pass

View File

@ -220,7 +220,7 @@ foo_res opt_3 some_value local"""
self.driver.zfs('foo', 'bar') self.driver.zfs('foo', 'bar')
self.assertEqual(0, self.driver.execute_with_retry.call_count) self.assertEqual(0, self.driver.execute_with_retry.call_count)
self.driver.execute.asssert_called_once_with( self.driver.execute.assert_called_once_with(
'sudo', 'zfs', 'foo', 'bar') 'sudo', 'zfs', 'foo', 'bar')
def test_zfs_with_retry(self): def test_zfs_with_retry(self):
@ -230,7 +230,7 @@ foo_res opt_3 some_value local"""
self.driver.zfs_with_retry('foo', 'bar') self.driver.zfs_with_retry('foo', 'bar')
self.assertEqual(0, self.driver.execute.call_count) self.assertEqual(0, self.driver.execute.call_count)
self.driver.execute_with_retry.asssert_called_once_with( self.driver.execute_with_retry.assert_called_once_with(
'sudo', 'zfs', 'foo', 'bar') 'sudo', 'zfs', 'foo', 'bar')

View File

@ -167,7 +167,7 @@ class ShareDriverTestCase(test.TestCase):
conf = configuration.Configuration(None) conf = configuration.Configuration(None)
self.mock_object(conf, 'safe_get', mock.Mock(return_value=opt)) self.mock_object(conf, 'safe_get', mock.Mock(return_value=opt))
share_driver = driver.ShareDriver(allowed, configuration=conf) share_driver = driver.ShareDriver(allowed, configuration=conf)
self.assertTrue(conf.safe_get.celled) self.assertTrue(conf.safe_get.called)
self.assertEqual(opt, share_driver.driver_handles_share_servers) self.assertEqual(opt, share_driver.driver_handles_share_servers)
@ddt.data( @ddt.data(
@ -187,7 +187,7 @@ class ShareDriverTestCase(test.TestCase):
self.assertRaises( self.assertRaises(
exception.ManilaException, exception.ManilaException,
driver.ShareDriver, allowed, configuration=conf) driver.ShareDriver, allowed, configuration=conf)
self.assertTrue(conf.safe_get.celled) self.assertTrue(conf.safe_get.called)
def test_setup_server_handling_disabled(self): def test_setup_server_handling_disabled(self):
share_driver = self._instantiate_share_driver(None, False) share_driver = self._instantiate_share_driver(None, False)