starlingx/config
Code Issues Proposed changes

Adding pylint checking to the sysinv unit tests

Browse Source 
pylint checks were excluding the unit tests folder.
It is now included.

The following pylint errors needed to be fixed in the unit tests
 - wildcard import (tests.db)
 - duplicate-key (fornodeid was listed twice in one test structure)
 - function-redefined (test_storage_tier.py)
 - dangerous-default-value (using a list as a default)
 - no-member (rename a class to end in Mixin to avoid these checks)

Change-Id: I1095d54542e1e7b3d07865e1e7b4cd46f14dc407
Story: 2007082
Task: 38081
Signed-off-by: Al Bailey <Al.Bailey@windriver.com>
This commit is contained in:
Al Bailey
2020-01-10 14:05:19 -06:00
parent 7867465026
commit 6258c07dde
11 changed files with 73 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
sysinv/sysinv/sysinv/pylint.rc
View File

@@ -7,7 +7,7 @@ rcfile=pylint.rc
#init-hook=
# Add files or directories to the blacklist. Should be base names, not paths.
ignore=tests
ignore=
# Pickle collected data for later comparisons.
persistent=yes

4
sysinv/sysinv/sysinv/sysinv/tests/api/base.py
View File

@@ -139,7 +139,9 @@ class FunctionalTest(base.TestCase):
return response
def get_json(self, path, expect_errors=False, headers=None,
extra_environ=None, q=[], path_prefix=PATH_PREFIX, **params):
extra_environ=None, q=None, path_prefix=PATH_PREFIX, **params):
if q is None:
q = []
full_path = path_prefix + path
query_params = {'q.field': [],
'q.value': [],

2
sysinv/sysinv/sysinv/sysinv/tests/api/test_acl.py
View File

@@ -36,7 +36,7 @@ class TestACL(base.FunctionalTest):
self.dbapi = db_api.get_instance()
self.node_path = '/ihosts/%s' % self.fake_node['uuid']
def get_json(self, path, expect_errors=False, headers=None, q=[], **param):
def get_json(self, path, expect_errors=False, headers=None, q=None, **param):
return super(TestACL, self).get_json(path,
expect_errors=expect_errors,
headers=headers,

23
sysinv/sysinv/sysinv/sysinv/tests/api/test_label.py
View File

@@ -4,17 +4,14 @@
#
import mock
import platform
from six.moves import http_client
from six.moves.urllib.parse import urlencode
from sysinv.common import constants
from sysinv.db import api as dbapi
from sysinv.tests.api import base
from sysinv.tests.db import utils as dbutils
if platform.python_version().startswith('2.7'):
from urllib import urlencode
else:
from urllib.parse import urlencode
HEADER = {'User-Agent': 'sysinv'}
es_labels = {'elastic-data': 'enabled',
@@ -30,6 +27,15 @@ def mock_helm_override_get(dbapi, app_name, chart_name, namespace):
return True
def mock_get_system_enabled_k8s_plugins_return_plugins():
return {"intel-gpu-plugin": "intelgpu=enabled",
"intel-qat-plugin": "intelqat=enabled"}
def mock_get_system_enabled_k8s_plugins_return_none():
return None
class LabelTestCase(base.FunctionalTest):
def setUp(self):
super(LabelTestCase, self).setUp()
@@ -167,13 +173,6 @@ class LabelAssignTestCase(LabelTestCase):
}
self.assign_labels_failure(host_uuid, topology_mgr_label)
def mock_get_system_enabled_k8s_plugins_return_plugins():
return {"intel-gpu-plugin": "intelgpu=enabled",
"intel-qat-plugin": "intelqat=enabled"}
def mock_get_system_enabled_k8s_plugins_return_none():
return None
@mock.patch('sysinv.api.controllers.v1.label._get_system_enabled_k8s_plugins',
mock_get_system_enabled_k8s_plugins_return_plugins)
def test_create_plugin_labels_on_supported_node(self):

8
sysinv/sysinv/sysinv/sysinv/tests/api/test_storage_tier.py
View File

@@ -759,12 +759,12 @@ class StorageTierDependentTCs(base.FunctionalTest):
mock.patch.object(rpcapi.ConductorAPI, 'configure_osd_istor')) as (
mock_mon_status, mock_backend_configured, mock_osd):
def fake_configure_osd_istor(context, istor_obj):
def fake_configure_osd_istor_1(context, istor_obj):
istor_obj['osdid'] = 1
return istor_obj
mock_mon_status.return_value = [3, 2, ['controller-0', 'controller-1', 'storage-0']]
mock_osd.side_effect = fake_configure_osd_istor
mock_osd.side_effect = fake_configure_osd_istor_1
response = self.post_json('/istors', values, expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
@@ -783,12 +783,12 @@ class StorageTierDependentTCs(base.FunctionalTest):
mock.patch.object(rpcapi.ConductorAPI, 'configure_osd_istor')) as (
mock_mon_status, mock_backend_configured, mock_osd):
def fake_configure_osd_istor(context, istor_obj):
def fake_configure_osd_istor_2(context, istor_obj):
istor_obj['osdid'] = 1
return istor_obj
mock_mon_status.return_value = [3, 2, ['controller-0', 'controller-1', 'storage-0']]
mock_osd.side_effect = fake_configure_osd_istor
mock_osd.side_effect = fake_configure_osd_istor_2
response = self.post_json('/istors', values, expect_errors=True)
self.assertEqual(http_client.OK, response.status_int)

1
sysinv/sysinv/sysinv/sysinv/tests/db/__init__.py
View File

@@ -13,4 +13,3 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from sysinv.tests.db import *

35
sysinv/sysinv/sysinv/sysinv/tests/db/sqlalchemy/test_migrations.py
View File

@@ -48,6 +48,7 @@ from six.moves.urllib.parse import urlparse
import mock
import sqlalchemy
import sqlalchemy.exc
import subprocess
from migrate.versioning import repository
from oslo_db.sqlalchemy import utils as db_utils
@@ -200,13 +201,11 @@ class BaseMigrationTestCase(test_utils.BaseTestCase):
super(BaseMigrationTestCase, self).tearDown()
def execute_cmd(self, cmd=None):
from future import standard_library
standard_library.install_aliases()
from subprocess import getstatusoutput
status, output = getstatusoutput(cmd)
process = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
output = process.communicate()[0]
LOG.debug(output)
self.assertEqual(0, status,
self.assertEqual(0, process.returncode,
"Failed to run: %s\n%s" % (cmd, output))
@lockutils.synchronized('pgadmin', 'tests-', external=True)
@@ -419,8 +418,8 @@ class TestWalkVersions(test_utils.BaseTestCase, WalkVersionsMixin):
self._post_downgrade_043.assert_called_with(self.engine)
@mock.patch.object(WalkVersionsMixin, '_migrate_up')
@mock.patch.object(WalkVersionsMixin, '_migrate_down')
@mock.patch.object(WalkVersionsMixin, '_migrate_up')
def test_walk_versions_all_default(self, _migrate_up, _migrate_down):
self.REPOSITORY.latest = 20
self.migration_api.db_version.return_value = self.INIT_VERSION
@@ -434,13 +433,13 @@ class TestWalkVersions(test_utils.BaseTestCase, WalkVersionsMixin):
versions = range(self.INIT_VERSION + 1, self.REPOSITORY.latest + 1)
upgraded = [mock.call(None, v, with_data=True) for v in versions]
self.assertEqual(self._migrate_up.call_args_list, upgraded)
self.assertEqual(_migrate_up.call_args_list, upgraded)
downgraded = [mock.call(None, v - 1) for v in reversed(versions)]
self.assertEqual(self._migrate_down.call_args_list, downgraded)
self.assertEqual(_migrate_down.call_args_list, downgraded)
@mock.patch.object(WalkVersionsMixin, '_migrate_up')
@mock.patch.object(WalkVersionsMixin, '_migrate_down')
@mock.patch.object(WalkVersionsMixin, '_migrate_up')
def test_walk_versions_all_true(self, _migrate_up, _migrate_down):
self.REPOSITORY.latest = 20
self.migration_api.db_version.return_value = self.INIT_VERSION
@@ -455,7 +454,7 @@ class TestWalkVersions(test_utils.BaseTestCase, WalkVersionsMixin):
upgraded.extend(
[mock.call(self.engine, v) for v in reversed(versions)]
)
self.assertEqual(upgraded, self._migrate_up.call_args_list)
self.assertEqual(upgraded, _migrate_up.call_args_list)
downgraded_1 = [
mock.call(self.engine, v - 1, with_data=True) for v in versions
@@ -465,10 +464,10 @@ class TestWalkVersions(test_utils.BaseTestCase, WalkVersionsMixin):
downgraded_2.append(mock.call(self.engine, v - 1))
downgraded_2.append(mock.call(self.engine, v - 1))
downgraded = downgraded_1 + downgraded_2
self.assertEqual(self._migrate_down.call_args_list, downgraded)
self.assertEqual(_migrate_down.call_args_list, downgraded)
@mock.patch.object(WalkVersionsMixin, '_migrate_up')
@mock.patch.object(WalkVersionsMixin, '_migrate_down')
@mock.patch.object(WalkVersionsMixin, '_migrate_up')
def test_walk_versions_true_false(self, _migrate_up, _migrate_down):
self.REPOSITORY.latest = 20
self.migration_api.db_version.return_value = self.INIT_VERSION
@@ -481,15 +480,15 @@ class TestWalkVersions(test_utils.BaseTestCase, WalkVersionsMixin):
for v in versions:
upgraded.append(mock.call(self.engine, v, with_data=True))
upgraded.append(mock.call(self.engine, v))
self.assertEqual(upgraded, self._migrate_up.call_args_list)
self.assertEqual(upgraded, _migrate_up.call_args_list)
downgraded = [
mock.call(self.engine, v - 1, with_data=True) for v in versions
]
self.assertEqual(self._migrate_down.call_args_list, downgraded)
self.assertEqual(_migrate_down.call_args_list, downgraded)
@mock.patch.object(WalkVersionsMixin, '_migrate_up')
@mock.patch.object(WalkVersionsMixin, '_migrate_down')
@mock.patch.object(WalkVersionsMixin, '_migrate_up')
def test_walk_versions_all_false(self, _migrate_up, _migrate_down):
self.REPOSITORY.latest = 20
self.migration_api.db_version.return_value = self.INIT_VERSION
@@ -501,7 +500,7 @@ class TestWalkVersions(test_utils.BaseTestCase, WalkVersionsMixin):
upgraded = [
mock.call(self.engine, v, with_data=True) for v in versions
]
self.assertEqual(upgraded, self._migrate_up.call_args_list)
self.assertEqual(upgraded, _migrate_up.call_args_list)
class TestMigrations(BaseMigrationTestCase, WalkVersionsMixin):
@@ -746,7 +745,7 @@ class TestMigrations(BaseMigrationTestCase, WalkVersionsMixin):
'forinodeid': 'Integer', 'core': 'Integer', 'thread': 'Integer',
'cpu_family': 'String', 'cpu_model': 'String', 'allocated_function': 'String',
'capabilities': 'Text', 'forihostid': 'Integer', # 'coProcessors': 'String',
'forinodeid': 'Integer', 'deleted_at': 'DateTime',
'deleted_at': 'DateTime',
'created_at': 'DateTime', 'updated_at': 'DateTime'
}
for col, coltype in cpus_col.items():

12
sysinv/sysinv/sysinv/sysinv/tests/helm/test_helm.py
View File

@@ -18,14 +18,14 @@ class StxOpenstackAppMixin(object):
app_name = 'stx-openstack'
class HelmOperatorTestSuite(helm_base.HelmTestCaseMixin):
"""When HelmOperatorTestSuite is added as a Mixin
class HelmOperatorTestSuiteMixin(helm_base.HelmTestCaseMixin):
"""When HelmOperatorTestSuiteMixin is added as a Mixin
alongside a subclass of BaseHostTestCase
these testcases are added to it
This also requires an AppMixin to provide app_name
"""
def setUp(self):
super(HelmOperatorTestSuite, self).setUp()
super(HelmOperatorTestSuiteMixin, self).setUp()
self.app = dbutils.create_test_app(name=self.app_name)
# If a ceph keyring entry is missing, a subprocess will be invoked
# so a fake keyring password is being supplied here.
@@ -54,7 +54,7 @@ class HelmOperatorTestSuite(helm_base.HelmTestCaseMixin):
self.addCleanup(write_file.stop)
def tearDown(self):
super(HelmOperatorTestSuite, self).tearDown()
super(HelmOperatorTestSuiteMixin, self).tearDown()
@mock.patch.object(HelmOperator, '_write_chart_overrides')
def test_generate_helm_chart_overrides(self, mock_write_chart):
@@ -73,7 +73,7 @@ class HelmOperatorTestSuite(helm_base.HelmTestCaseMixin):
class HelmSTXOpenstackControllerTestCase(StxOpenstackAppMixin,
dbbase.BaseIPv6Mixin,
dbbase.BaseCephStorageBackendMixin,
HelmOperatorTestSuite,
HelmOperatorTestSuiteMixin,
dbbase.ControllerHostTestCase):
pass
@@ -85,6 +85,6 @@ class HelmSTXOpenstackControllerTestCase(StxOpenstackAppMixin,
# - stx-openstack app
class HelmSTXOpenstackAIOTestCase(StxOpenstackAppMixin,
dbbase.BaseCephStorageBackendMixin,
HelmOperatorTestSuite,
HelmOperatorTestSuiteMixin,
dbbase.AIOSimplexHostTestCase):
pass

6
sysinv/sysinv/sysinv/sysinv/tests/objects/test_objects.py
View File

@@ -230,7 +230,7 @@ def things_temporarily_local():
base.SysinvObject.indirection_api = _api
class _TestObject(object):
class _TestObjectMixin(object):
def test_hydration_type_error(self):
primitive = {'sysinv_object.name': 'MyObj',
'sysinv_object.namespace': 'sysinv',
@@ -298,7 +298,7 @@ class _TestObject(object):
raised = False
ex_out = ""
try:
obj.foobar
obj.foobar # pylint: disable=no-member
except NotImplementedError as ex:
ex_out = str(ex)
raised = True
@@ -443,7 +443,7 @@ class _TestObject(object):
self.assertFalse('does_not_exist' in obj)
class TestObject(_LocalTest, _TestObject):
class TestObject(_LocalTest, _TestObjectMixin):
pass

25
sysinv/sysinv/sysinv/sysinv/tests/puppet/test_interface.py
View File

@@ -298,8 +298,9 @@ class InterfaceTestCaseMixin(base.PuppetTestCaseMixin):
@puppet.puppet_context
def _update_context(self):
# interface is added as an operator by systemconfig.puppet_plugins
self.context = \
self.operator.interface._create_interface_context(self.host)
self.operator.interface._create_interface_context(self.host) # pylint: disable=no-member
# Update the puppet context with generated interface context
self.operator.context.update(self.context)
@@ -354,43 +355,43 @@ class InterfaceTestCase(InterfaceTestCaseMixin, dbbase.BaseHostTestCase):
self.assertFalse(result)
def test_get_port_interface_id_index(self):
index = self.operator.interface._get_port_interface_id_index(self.host)
index = self.operator.interface._get_port_interface_id_index(self.host) # pylint: disable=no-member
for port in self.ports:
self.assertTrue(port['interface_id'] in index)
self.assertEqual(index[port['interface_id']], port)
def test_get_port_pciaddr_index(self):
index = self.operator.interface._get_port_pciaddr_index(self.host)
index = self.operator.interface._get_port_pciaddr_index(self.host) # pylint: disable=no-member
for port in self.ports:
self.assertTrue(port['pciaddr'] in index)
self.assertIn(port, index[port['pciaddr']])
def test_get_interface_name_index(self):
index = self.operator.interface._get_interface_name_index(self.host)
index = self.operator.interface._get_interface_name_index(self.host) # pylint: disable=no-member
for iface in self.interfaces:
self.assertTrue(iface['ifname'] in index)
self.assertEqual(index[iface['ifname']], iface)
def test_get_network_type_index(self):
index = self.operator.interface._get_network_type_index()
index = self.operator.interface._get_network_type_index() # pylint: disable=no-member
for network in self.networks:
self.assertTrue(network['type'] in index)
self.assertEqual(index[network['type']], network)
def test_get_address_interface_name_index(self):
index = self.operator.interface._get_address_interface_name_index(self.host)
index = self.operator.interface._get_address_interface_name_index(self.host) # pylint: disable=no-member
for address in self.addresses:
self.assertTrue(address['ifname'] in index)
self.assertIn(address, index[address['ifname']])
def test_get_routes_interface_name_index(self):
index = self.operator.interface._get_routes_interface_name_index(self.host)
index = self.operator.interface._get_routes_interface_name_index(self.host) # pylint: disable=no-member
for route in self.routes:
self.assertTrue(route['ifname'] in index)
self.assertIn(route, index[route['ifname']])
def test_get_gateway_index(self):
index = self.operator.interface._get_gateway_index()
index = self.operator.interface._get_gateway_index() # pylint: disable=no-member
self.assertEqual(len(index), 2)
self.assertEqual(index[constants.NETWORK_TYPE_MGMT],
str(self.mgmt_gateway_address))
@@ -1072,7 +1073,9 @@ class InterfaceTestCase(InterfaceTestCaseMixin, dbbase.BaseHostTestCase):
def _get_sriov_config(self, ifname='default',
vf_driver=constants.SRIOV_DRIVER_TYPE_VFIO,
vf_addrs=[""]):
vf_addrs=None):
if vf_addrs is None:
vf_addrs = [""]
config = {'ifname': ifname,
'vf_driver': vf_driver,
'vf_addrs': vf_addrs}
@@ -1548,12 +1551,12 @@ class InterfaceHostTestCase(InterfaceTestCaseMixin, dbbase.BaseHostTestCase):
hieradata_directory = self._create_hieradata_directory()
config_filename = self._get_config_filename(hieradata_directory)
with open(config_filename, 'w') as config_file:
config = self.operator.interface.get_host_config(self.host)
config = self.operator.interface.get_host_config(self.host) # pylint: disable=no-member
self.assertIsNotNone(config)
yaml.dump(config, config_file, default_flow_style=False)
def test_create_interface_context(self):
context = self.operator.interface._create_interface_context(self.host)
context = self.operator.interface._create_interface_context(self.host) # pylint: disable=no-member
self.assertIn('personality', context)
self.assertIn('subfunctions', context)
self.assertIn('devices', context)

25
sysinv/sysinv/sysinv/sysinv/tests/puppet/test_puppet.py
View File

@@ -7,8 +7,8 @@ from sysinv.tests.db import base as dbbase
from sysinv.tests.puppet import base
class PuppetOperatorTestSuite(base.PuppetTestCaseMixin):
"""When PuppetOperatorTestSuite is added as a Mixin
class PuppetOperatorTestSuiteMixin(base.PuppetTestCaseMixin):
"""When PuppetOperatorTestSuiteMixin is added as a Mixin
to a testcase which is a subclass of BaseHostTestCase
these testcases are added to it
"""
@@ -29,60 +29,61 @@ class PuppetOperatorTestSuite(base.PuppetTestCaseMixin):
self.operator.update_secure_system_config()
assert self.mock_write_config.called
# self.host is defined in BaseHostTestCase
def test_update_host_config(self):
self.operator.update_host_config(self.host)
self.operator.update_host_config(self.host) # pylint: disable=no-member
assert self.mock_write_config.called
# ============= IPv4 environment tests ==============
# Tests all puppet operations for a Controller (defaults to IPv4)
class PlatformIPv4ControllerHostTestCase(PuppetOperatorTestSuite,
class PlatformIPv4ControllerHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.ControllerHostTestCase):
pass
# Tests all puppet operations for a Worker (defaults to IPv4)
class PlatformIPv4WorkerHostTestCase(PuppetOperatorTestSuite,
class PlatformIPv4WorkerHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.WorkerHostTestCase):
pass
# Tests all puppet operations for a Storage Host (defaults to IPv4)
class PlatformIPv4StorageHostTestCase(PuppetOperatorTestSuite,
class PlatformIPv4StorageHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.StorageHostTestCase):
pass
# Tests all puppet operations for an AIO Host (defaults to IPv4)
class PlatformIPv4AIOHostTestCase(PuppetOperatorTestSuite,
class PlatformIPv4AIOHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.AIOHostTestCase):
pass
# ============= IPv6 environment tests ==============
# Tests all puppet operations for a Controller using IPv6
class PlatformIPv6ControllerHostTestCase(PuppetOperatorTestSuite,
class PlatformIPv6ControllerHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.BaseIPv6Mixin,
dbbase.ControllerHostTestCase):
pass
# Tests all puppet operations for a Worker using IPv6
class PlatformIPv6WorkerHostTestCase(PuppetOperatorTestSuite,
class PlatformIPv6WorkerHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.BaseIPv6Mixin,
dbbase.WorkerHostTestCase):
pass
# Tests all puppet operations for a Storage Host using IPv6
class PlatformIPv6StorageHostTestCase(PuppetOperatorTestSuite,
class PlatformIPv6StorageHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.BaseIPv6Mixin,
dbbase.StorageHostTestCase):
pass
# Tests all puppet operations for an AIO Host using IPv6
class PlatformIPv6AIOHostTestCase(PuppetOperatorTestSuite,
class PlatformIPv6AIOHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.BaseIPv6Mixin,
dbbase.AIOHostTestCase):
pass
@@ -90,7 +91,7 @@ class PlatformIPv6AIOHostTestCase(PuppetOperatorTestSuite,
# ============= Ceph Backend environment tests ==============
# Tests all puppet operations for an AIO Host using IPv4 and Ceph Backend
class PlatformCephBackendAIOHostTestCase(PuppetOperatorTestSuite,
class PlatformCephBackendAIOHostTestCase(PuppetOperatorTestSuiteMixin,
dbbase.BaseCephStorageBackendMixin,
dbbase.AIOHostTestCase):
pass