openstack
/
rally-openstack
Code Issues Proposed changes

Update hacking for Python3 

The repo is Python 3 now, so update hacking to version 3.0 which
supports Python 3.

Fix problems found.

Update local hacking checks for new flake8. Note that the repo has been
running hacking 2 for some time due to uncapped setup, and thus fixes
are needed for N350 - the uncapped setup disabled the local hacking
rules. They are now enabled again.

Change-Id: I25e5201933f65ba4d3045e6445f0e6419ea57f3e
This commit is contained in:
Andreas Jaeger 2020-04-02 09:41:25 +02:00
parent 771c922af5
commit 117544f62b
7 changed files with 94 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
rally_openstack/task/scenarios/mistral/utils.py
View File

@ -37,7 +37,7 @@ class MistralScenario(scenario.OpenStackScenario):
return self.clients("mistral").workbooks.list() return self.clients("mistral").workbooks.list()
@atomic.action_timer("mistral.create_workbook") @atomic.action_timer("mistral.create_workbook")
def _create_workbook(self, definition, namespace=''): def _create_workbook(self, definition, namespace=""):
"""Create a new workbook. """Create a new workbook.
:param definition: workbook description in string :param definition: workbook description in string
@ -56,7 +56,7 @@ class MistralScenario(scenario.OpenStackScenario):
) )
@atomic.action_timer("mistral.delete_workbook") @atomic.action_timer("mistral.delete_workbook")
def _delete_workbook(self, wb_name, namespace=''): def _delete_workbook(self, wb_name, namespace=""):
"""Delete the given workbook. """Delete the given workbook.
:param wb_name: the name of workbook that would be deleted. :param wb_name: the name of workbook that would be deleted.
@ -68,7 +68,7 @@ class MistralScenario(scenario.OpenStackScenario):
) )
@atomic.action_timer("mistral.create_workflow") @atomic.action_timer("mistral.create_workflow")
def _create_workflow(self, definition, namespace=''): def _create_workflow(self, definition, namespace=""):
"""creates a workflow in the given namespace. """creates a workflow in the given namespace.
:param definition: the definition of workflow :param definition: the definition of workflow
@ -80,7 +80,7 @@ class MistralScenario(scenario.OpenStackScenario):
) )
@atomic.action_timer("mistral.delete_workflow") @atomic.action_timer("mistral.delete_workflow")
def _delete_workflow(self, workflow_identifier, namespace=''): def _delete_workflow(self, workflow_identifier, namespace=""):
"""Delete the given workflow. """Delete the given workflow.
:param workflow_identifier: the identifier of workflow :param workflow_identifier: the identifier of workflow
@ -105,7 +105,7 @@ class MistralScenario(scenario.OpenStackScenario):
@atomic.action_timer("mistral.create_execution") @atomic.action_timer("mistral.create_execution")
def _create_execution(self, workflow_identifier, wf_input=None, def _create_execution(self, workflow_identifier, wf_input=None,
namespace='', **params): namespace="", **params):
"""Create a new execution. """Create a new execution.
:param workflow_identifier: name or id of the workflow to execute :param workflow_identifier: name or id of the workflow to execute

4
rally_openstack/task/ui/charts/osprofilerchart.py
View File

@ -58,9 +58,9 @@ class OSProfilerChart(charts.OutputEmbeddedChart,
from osprofiler.drivers import base from osprofiler.drivers import base
from osprofiler import opts as osprofiler_opts from osprofiler import opts as osprofiler_opts
opts.register_opts(osprofiler_opts.list_opts()) opts.register_opts(osprofiler_opts.list_opts()) # noqa
try: try: # noqa
engine = base.get_driver(connection_str) engine = base.get_driver(connection_str)
except Exception: except Exception:
msg = "Error while fetching OSProfiler results." msg = "Error while fetching OSProfiler results."

2
test-requirements.txt
View File

@ -2,7 +2,7 @@
# of appearance. Changing the order has an impact on the overall integration # of appearance. Changing the order has an impact on the overall integration
# process, which may cause wedges in the gate later. # process, which may cause wedges in the gate later.
hacking>=0.12.0,!=0.13.0 # Apache Software License hacking>=3.0,<3.1.0 # Apache-2.0
pytest<=5.3 # MIT pytest<=5.3 # MIT
# py.test plugin for measuring coverage. # py.test plugin for measuring coverage.

104
tests/hacking/checks.py
View File

@ -28,6 +28,8 @@ import functools
import re import re
import tokenize import tokenize
from hacking import core
re_assert_equal_end_with_true_or_false = re.compile( re_assert_equal_end_with_true_or_false = re.compile(
r"assertEqual\(.*?, \s+(True|False)\)$") r"assertEqual\(.*?, \s+(True|False)\)$")
@ -79,12 +81,12 @@ re_log_warn = re.compile(r"(.)*LOG\.(warn)\(\s*('|\"|_)")
def skip_ignored_lines(func): def skip_ignored_lines(func):
@functools.wraps(func) @functools.wraps(func)
def wrapper(logical_line, physical_line, filename): def wrapper(physical_line, logical_line, filename):
line = physical_line.strip() line = physical_line.strip()
if not line or line.startswith("#") or line.endswith("# noqa"): if not line or line.startswith("#") or line.endswith("# noqa"):
return return
try: try:
for res in func(logical_line, physical_line, filename): for res in func(physical_line, logical_line, filename):
yield res yield res
except StopIteration: except StopIteration:
return return
@ -105,8 +107,9 @@ def _parse_assert_mock_str(line):
return None, None, None return None, None, None
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_assert_methods_from_mock(logical_line, physical_line, filename): def check_assert_methods_from_mock(physical_line, logical_line, filename):
"""Ensure that ``assert_*`` methods from ``mock`` library is used correctly """Ensure that ``assert_*`` methods from ``mock`` library is used correctly
N301 - base error number N301 - base error number
@ -158,8 +161,9 @@ def check_assert_methods_from_mock(logical_line, physical_line, filename):
"custom_msg": custom_msg}) "custom_msg": custom_msg})
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_import_of_logging(logical_line, physical_line, filename): def check_import_of_logging(physical_line, logical_line, filename):
"""Check correctness import of logging module """Check correctness import of logging module
N310 N310
@ -181,8 +185,9 @@ def check_import_of_logging(logical_line, physical_line, filename):
"use `rally.common.logging` instead.") "use `rally.common.logging` instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_import_of_config(logical_line, physical_line, filename): def check_import_of_config(physical_line, logical_line, filename):
"""Check correctness import of config module """Check correctness import of config module
N311 N311
@ -200,8 +205,9 @@ def check_import_of_config(logical_line, physical_line, filename):
"use `rally.common.cfg` instead.") "use `rally.common.cfg` instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def no_use_conf_debug_check(logical_line, physical_line, filename): def no_use_conf_debug_check(physical_line, logical_line, filename):
"""Check for "cfg.CONF.debug" """Check for "cfg.CONF.debug"
Rally has two DEBUG level: Rally has two DEBUG level:
@ -220,8 +226,9 @@ def no_use_conf_debug_check(logical_line, physical_line, filename):
"should be used instead.") "should be used instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def assert_true_instance(logical_line, physical_line, filename): def assert_true_instance(physical_line, logical_line, filename):
"""Check for assertTrue(isinstance(a, b)) sentences """Check for assertTrue(isinstance(a, b)) sentences
N320 N320
@ -231,8 +238,9 @@ def assert_true_instance(logical_line, physical_line, filename):
"you should use assertIsInstance(a, b) instead.") "you should use assertIsInstance(a, b) instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def assert_equal_type(logical_line, physical_line, filename): def assert_equal_type(physical_line, logical_line, filename):
"""Check for assertEqual(type(A), B) sentences """Check for assertEqual(type(A), B) sentences
N321 N321
@ -242,8 +250,9 @@ def assert_equal_type(logical_line, physical_line, filename):
"you should use assertIsInstance(a, b) instead.") "you should use assertIsInstance(a, b) instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def assert_equal_none(logical_line, physical_line, filename): def assert_equal_none(physical_line, logical_line, filename):
"""Check for assertEqual(A, None) or assertEqual(None, A) sentences """Check for assertEqual(A, None) or assertEqual(None, A) sentences
N322 N322
@ -256,8 +265,9 @@ def assert_equal_none(logical_line, physical_line, filename):
"instead.") "instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def assert_true_or_false_with_in(logical_line, physical_line, filename): def assert_true_or_false_with_in(physical_line, logical_line, filename):
"""Check assertTrue/False(A in/not in B) with collection contents """Check assertTrue/False(A in/not in B) with collection contents
Check for assertTrue/False(A in B), assertTrue/False(A not in B), Check for assertTrue/False(A in B), assertTrue/False(A not in B),
@ -275,8 +285,9 @@ def assert_true_or_false_with_in(logical_line, physical_line, filename):
" instead.") " instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def assert_equal_in(logical_line, physical_line, filename): def assert_equal_in(physical_line, logical_line, filename):
"""Check assertEqual(A in/not in B, True/False) with collection contents """Check assertEqual(A in/not in B, True/False) with collection contents
Check for assertEqual(A in B, True/False), assertEqual(True/False, A in B), Check for assertEqual(A in B, True/False), assertEqual(True/False, A in B),
@ -293,8 +304,9 @@ def assert_equal_in(logical_line, physical_line, filename):
"collection contents.") "collection contents.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def assert_not_equal_none(logical_line, physical_line, filename): def assert_not_equal_none(physical_line, logical_line, filename):
"""Check for assertNotEqual(A, None) or assertEqual(None, A) sentences """Check for assertNotEqual(A, None) or assertEqual(None, A) sentences
N325 N325
@ -307,8 +319,9 @@ def assert_not_equal_none(logical_line, physical_line, filename):
"instead.") "instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def assert_equal_true_or_false(logical_line, physical_line, filename): def assert_equal_true_or_false(physical_line, logical_line, filename):
"""Check for assertEqual(A, True/False) sentences """Check for assertEqual(A, True/False) sentences
Check for assertEqual(A, True/False) sentences or Check for assertEqual(A, True/False) sentences or
@ -324,8 +337,9 @@ def assert_equal_true_or_false(logical_line, physical_line, filename):
"you should use assertTrue(A) or assertFalse(A) instead.") "you should use assertTrue(A) or assertFalse(A) instead.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_no_direct_rally_objects_import(logical_line, physical_line, def check_no_direct_rally_objects_import(physical_line, logical_line,
filename): filename):
"""Check if rally.common.objects are properly imported. """Check if rally.common.objects are properly imported.
@ -347,8 +361,9 @@ def check_no_direct_rally_objects_import(logical_line, physical_line,
"After that you can use directly objects e.g. objects.Task") "After that you can use directly objects e.g. objects.Task")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_no_oslo_deprecated_import(logical_line, physical_line, filename): def check_no_oslo_deprecated_import(physical_line, logical_line, filename):
"""Check if oslo.foo packages are not imported instead of oslo_foo ones. """Check if oslo.foo packages are not imported instead of oslo_foo ones.
Libraries from oslo.foo namespace are deprecated because of namespace Libraries from oslo.foo namespace are deprecated because of namespace
@ -363,8 +378,9 @@ def check_no_oslo_deprecated_import(logical_line, physical_line, filename):
"instead") "instead")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_quotes(logical_line, physical_line, filename): def check_quotes(physical_line, logical_line, filename):
"""Check that single quotation marks are not used """Check that single quotation marks are not used
N350 N350
@ -416,8 +432,9 @@ def check_quotes(logical_line, physical_line, filename):
yield i, "N350 Remove Single quotes" yield i, "N350 Remove Single quotes"
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_no_constructor_data_struct(logical_line, physical_line, filename): def check_no_constructor_data_struct(physical_line, logical_line, filename):
"""Check that data structs (lists, dicts) are declared using literals """Check that data structs (lists, dicts) are declared using literals
N351 N351
@ -431,6 +448,7 @@ def check_no_constructor_data_struct(logical_line, physical_line, filename):
yield 0, "N351 Remove list() construct and use literal []" yield 0, "N351 Remove list() construct and use literal []"
@core.flake8ext
def check_dict_formatting_in_string(logical_line, tokens): def check_dict_formatting_in_string(logical_line, tokens):
"""Check that strings do not use dict-formatting with a single replacement """Check that strings do not use dict-formatting with a single replacement
@ -498,8 +516,9 @@ def check_dict_formatting_in_string(logical_line, tokens):
current_string = "" current_string = ""
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_using_unicode(logical_line, physical_line, filename): def check_using_unicode(physical_line, logical_line, filename):
"""Check crosspython unicode usage """Check crosspython unicode usage
N353 N353
@ -510,7 +529,8 @@ def check_using_unicode(logical_line, physical_line, filename):
"use 'str' instead.") "use 'str' instead.")
def check_raises(logical_line, physical_line, filename): @core.flake8ext
def check_raises(physical_line, logical_line, filename):
"""Check raises usage """Check raises usage
N354 N354
@ -524,8 +544,9 @@ def check_raises(logical_line, physical_line, filename):
"in docstrings.") "in docstrings.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_old_type_class(logical_line, physical_line, filename): def check_old_type_class(physical_line, logical_line, filename):
"""Use new-style Python classes """Use new-style Python classes
N355 N355
@ -537,8 +558,9 @@ def check_old_type_class(logical_line, physical_line, filename):
"``object`` or another new-style class.") "``object`` or another new-style class.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_datetime_alias(logical_line, physical_line, filename): def check_datetime_alias(physical_line, logical_line, filename):
"""Ensure using ``dt`` as alias for ``datetime`` """Ensure using ``dt`` as alias for ``datetime``
N356 N356
@ -547,8 +569,9 @@ def check_datetime_alias(logical_line, physical_line, filename):
yield 0, "N356 Please use ``dt`` as alias for ``datetime``." yield 0, "N356 Please use ``dt`` as alias for ``datetime``."
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_db_imports_in_cli(logical_line, physical_line, filename): def check_db_imports_in_cli(physical_line, logical_line, filename):
"""Ensure that CLI modules do not use ``rally.common.db`` """Ensure that CLI modules do not use ``rally.common.db``
N360 N360
@ -561,8 +584,9 @@ def check_db_imports_in_cli(logical_line, physical_line, filename):
"`rally.common.db``.") "`rally.common.db``.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_objects_imports_in_cli(logical_line, physical_line, filename): def check_objects_imports_in_cli(physical_line, logical_line, filename):
"""Ensure that CLI modules do not use ``rally.common.objects`` """Ensure that CLI modules do not use ``rally.common.objects``
N361 N361
@ -574,14 +598,16 @@ def check_objects_imports_in_cli(logical_line, physical_line, filename):
"`rally.common.objects``.") "`rally.common.objects``.")
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_log_warn(logical_line, physical_line, filename): def check_log_warn(physical_line, logical_line, filename):
if re_log_warn.search(logical_line): if re_log_warn.search(logical_line):
yield 0, "N313 LOG.warn is deprecated, please use LOG.warning" yield 0, "N313 LOG.warn is deprecated, please use LOG.warning"
@core.flake8ext
@skip_ignored_lines @skip_ignored_lines
def check_opts_import_path(logical_line, physical_line, filename): def check_opts_import_path(physical_line, logical_line, filename):
"""Ensure that we load opts from correct paths only """Ensure that we load opts from correct paths only
N342 N342
@ -590,7 +616,7 @@ def check_opts_import_path(logical_line, physical_line, filename):
"./rally/task/context.py", "./rally/task/context.py",
"./rally/task/scenario.py", "./rally/task/scenario.py",
"./rally/common/opts.py"] "./rally/common/opts.py"]
forbidden_methods = [".register_opts("] forbidden_methods = [r".register_opts("]
if filename not in excluded_files: if filename not in excluded_files:
for forbidden_method in forbidden_methods: for forbidden_method in forbidden_methods:
@ -598,29 +624,3 @@ def check_opts_import_path(logical_line, physical_line, filename):
yield (0, "N342 All options should be loaded from correct " yield (0, "N342 All options should be loaded from correct "
"paths only. For 'openstack' " "paths only. For 'openstack' "
"its './rally/plugin/openstack/cfg'") "its './rally/plugin/openstack/cfg'")
def factory(register):
register(check_assert_methods_from_mock)
register(check_import_of_logging)
register(check_import_of_config)
register(no_use_conf_debug_check)
register(assert_true_instance)
register(assert_equal_type)
register(assert_equal_none)
register(assert_true_or_false_with_in)
register(assert_equal_in)
register(assert_equal_true_or_false)
register(check_no_direct_rally_objects_import)
register(check_no_oslo_deprecated_import)
register(check_quotes)
register(check_no_constructor_data_struct)
register(check_dict_formatting_in_string)
register(check_using_unicode)
register(check_raises)
register(check_datetime_alias)
register(check_db_imports_in_cli)
register(check_objects_imports_in_cli)
register(check_old_type_class)
register(check_log_warn)
register(check_opts_import_path)

12
tests/unit/task/scenarios/mistral/test_utils.py
View File

@ -54,7 +54,7 @@ class MistralScenarioTestCase(test.ScenarioTestCase):
scenario._delete_workbook("wb_name") scenario._delete_workbook("wb_name")
self.clients("mistral").workbooks.delete.assert_called_once_with( self.clients("mistral").workbooks.delete.assert_called_once_with(
"wb_name", "wb_name",
namespace='' namespace=""
) )
self._test_atomic_action_timer( self._test_atomic_action_timer(
scenario.atomic_actions(), scenario.atomic_actions(),
@ -76,7 +76,7 @@ class MistralScenarioTestCase(test.ScenarioTestCase):
def test_create_execution(self): def test_create_execution(self):
scenario = utils.MistralScenario(context=self.context) scenario = utils.MistralScenario(context=self.context)
namespace = 'namespace' namespace = "namespace"
wf_name = "fake_wf_name" wf_name = "fake_wf_name"
mock_wait_for_status = self.mock_wait_for_status.mock mock_wait_for_status = self.mock_wait_for_status.mock
@ -118,7 +118,7 @@ class MistralScenarioTestCase(test.ScenarioTestCase):
mock_create_exec.assert_called_once_with( mock_create_exec.assert_called_once_with(
wf_name, wf_name,
workflow_input=INPUT_EXAMPLE, workflow_input=INPUT_EXAMPLE,
namespace='' namespace=""
) )
def test_create_execution_with_params(self): def test_create_execution_with_params(self):
@ -136,7 +136,7 @@ class MistralScenarioTestCase(test.ScenarioTestCase):
mock_create_exec.assert_called_once_with( mock_create_exec.assert_called_once_with(
wf_name, wf_name,
workflow_input=None, workflow_input=None,
namespace='', namespace="",
**PARAMS_EXAMPLE **PARAMS_EXAMPLE
) )
@ -189,8 +189,8 @@ wf:
) )
def test_delete_workflow(self): def test_delete_workflow(self):
wf_identifier = 'wf_identifier' wf_identifier = "wf_identifier"
namespace = 'delete_wf_test' namespace = "delete_wf_test"
scenario = utils.MistralScenario(context=self.context) scenario = utils.MistralScenario(context=self.context)
scenario._delete_workflow(wf_identifier, namespace=namespace) scenario._delete_workflow(wf_identifier, namespace=namespace)

4
tests/unit/verification/tempest/test_context.py
View File

@ -292,8 +292,8 @@ class TempestContextTestCase(test.TestCase):
_name, pos, _kwargs = client.create_router.mock_calls[0] _name, pos, _kwargs = client.create_router.mock_calls[0]
router = pos[0]["router"] router = pos[0]["router"]
external_gateway_info = router["external_gateway_info"] external_gateway_info = router["external_gateway_info"]
self.assertEqual('my-uuid', external_gateway_info["network_id"]) self.assertEqual("my-uuid", external_gateway_info["network_id"])
self.assertEqual(True, external_gateway_info["enable_snat"]) self.assertTrue(external_gateway_info["enable_snat"])
def test__cleanup_tempest_roles(self): def test__cleanup_tempest_roles(self):
self.context._created_roles = [fakes.FakeRole(), fakes.FakeRole()] self.context._created_roles = [fakes.FakeRole(), fakes.FakeRole()]

28
tox.ini
View File

@ -76,8 +76,32 @@ ignore = H105,E731,W503
show-source = true show-source = true
exclude=.venv,.git,.tox,dist,*lib/python*,*egg,tools,build,setup.py exclude=.venv,.git,.tox,dist,*lib/python*,*egg,tools,build,setup.py
[hacking] [flake8:local-plugins]
local-check-factory = tests.hacking.checks.factory extension =
N301 = checks:check_assert_methods_from_mock
N310 = checks:check_import_of_logging
N311 = checks:check_import_of_config
N312 = checks:no_use_conf_debug_check
N313 = checks:check_log_warn
N320 = checks:assert_true_instance
N321 = checks:assert_equal_type
N322 = checks:assert_equal_none
N323 = checks:assert_true_or_false_with_in
N324 = checks:assert_equal_in
N326 = checks:assert_equal_true_or_false
N340 = checks:check_no_direct_rally_objects_import
N341 = checks:check_no_oslo_deprecated_import
N342 = checks:check_opts_import_path
N350 = checks:check_quotes
N351 = checks:check_no_constructor_data_struct
N352 = checks:check_dict_formatting_in_string
N353 = checks:check_using_unicode
N354 = checks:check_raises
N355 = checks:check_old_type_class
N356 = checks:check_datetime_alias
N360 = checks:check_db_imports_in_cli
N361 = checks:check_objects_imports_in_cli
paths = ./tests/hacking
[testenv:bindep] [testenv:bindep]
# Do not install any requirements. We want this to be fast and work even if # Do not install any requirements. We want this to be fast and work even if