Update hacking for Python3

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

Update hacking extension for newer flake8. Remove vi header check, this
is enabled as H106 already.

Fix problems found.

Change-Id: Ie4ccf0a1075995f5624a838388b6b0b46343129a
This commit is contained in:
Andreas Jaeger 2020-03-28 19:25:15 +01:00 committed by Felipe Monteiro
parent 84cb426c16
commit 2aad808184
6 changed files with 45 additions and 55 deletions

View File

@ -21,7 +21,6 @@ for more information:
- [T102] Cannot import OpenStack python clients in - [T102] Cannot import OpenStack python clients in
``patrole_tempest_plugin/tests/api`` ``patrole_tempest_plugin/tests/api``
- [T105] Tests cannot use setUpClass/tearDownClass - [T105] Tests cannot use setUpClass/tearDownClass
- [T106] vim configuration should not be kept in source files.
- [T107] Check that a service tag isn't in the module path - [T107] Check that a service tag isn't in the module path
- [T108] Check no hyphen at the end of rand_name() argument - [T108] Check no hyphen at the end of rand_name() argument
- [T109] Cannot use testtools.skip decorator; instead use - [T109] Cannot use testtools.skip decorator; instead use

View File

@ -17,6 +17,7 @@
import os import os
import re import re
from hacking import core
import pycodestyle import pycodestyle
@ -27,7 +28,6 @@ PYTHON_CLIENT_RE = re.compile('import (%s)client' % '|'.join(PYTHON_CLIENTS))
TEST_DEFINITION = re.compile(r'^\s*def test.*') TEST_DEFINITION = re.compile(r'^\s*def test.*')
SETUP_TEARDOWN_CLASS_DEFINITION = re.compile(r'^\s+def (setUp|tearDown)Class') SETUP_TEARDOWN_CLASS_DEFINITION = re.compile(r'^\s+def (setUp|tearDown)Class')
SCENARIO_DECORATOR = re.compile(r'\s*@.*services\((.*)\)') SCENARIO_DECORATOR = re.compile(r'\s*@.*services\((.*)\)')
VI_HEADER_RE = re.compile(r"^#\s+vim?:.+")
RAND_NAME_HYPHEN_RE = re.compile(r".*rand_name\(.+[\-\_][\"\']\)") RAND_NAME_HYPHEN_RE = re.compile(r".*rand_name\(.+[\-\_][\"\']\)")
MUTABLE_DEFAULT_ARGS = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") MUTABLE_DEFAULT_ARGS = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
TESTTOOLS_SKIP_DECORATOR = re.compile(r'\s*@testtools\.skip\((.*)\)') TESTTOOLS_SKIP_DECORATOR = re.compile(r'\s*@testtools\.skip\((.*)\)')
@ -42,6 +42,7 @@ EXT_RBAC_TEST = re.compile(
have_rbac_decorator = False have_rbac_decorator = False
@core.flake8ext
def import_no_clients_in_api_tests(physical_line, filename): def import_no_clients_in_api_tests(physical_line, filename):
"""Check for client imports from patrole_tempest_plugin/tests/api """Check for client imports from patrole_tempest_plugin/tests/api
@ -56,6 +57,7 @@ def import_no_clients_in_api_tests(physical_line, filename):
"patrole_tempest_plugin/tests/scenario/* tests")) "patrole_tempest_plugin/tests/scenario/* tests"))
@core.flake8ext
def no_setup_teardown_class_for_tests(physical_line, filename): def no_setup_teardown_class_for_tests(physical_line, filename):
"""Check that tests do not use setUpClass/tearDownClass """Check that tests do not use setUpClass/tearDownClass
@ -69,20 +71,7 @@ def no_setup_teardown_class_for_tests(physical_line, filename):
"T105: (setUp|tearDown)Class can not be used in tests") "T105: (setUp|tearDown)Class can not be used in tests")
def no_vi_headers(physical_line, line_number, lines): @core.flake8ext
"""Check for vi editor configuration in source files.
By default vi modelines can only appear in the first or
last 5 lines of a source file.
T106
"""
# NOTE(gilliard): line_number is 1-indexed
if line_number <= 5 or line_number > len(lines) - 5:
if VI_HEADER_RE.match(physical_line):
return 0, "T106: Don't put vi configuration in source files"
def service_tags_not_in_module_path(physical_line, filename): def service_tags_not_in_module_path(physical_line, filename):
"""Check that a service tag isn't in the module path """Check that a service tag isn't in the module path
@ -102,6 +91,7 @@ def service_tags_not_in_module_path(physical_line, filename):
"T107: service tag should not be in path") "T107: service tag should not be in path")
@core.flake8ext
def no_hyphen_at_end_of_rand_name(logical_line, filename): def no_hyphen_at_end_of_rand_name(logical_line, filename):
"""Check no hyphen at the end of rand_name() argument """Check no hyphen at the end of rand_name() argument
@ -112,6 +102,7 @@ def no_hyphen_at_end_of_rand_name(logical_line, filename):
return 0, msg return 0, msg
@core.flake8ext
def no_mutable_default_args(logical_line): def no_mutable_default_args(logical_line):
"""Check that mutable object isn't used as default argument """Check that mutable object isn't used as default argument
@ -122,6 +113,7 @@ def no_mutable_default_args(logical_line):
yield (0, msg) yield (0, msg)
@core.flake8ext
def no_testtools_skip_decorator(logical_line): def no_testtools_skip_decorator(logical_line):
"""Check that methods do not have the testtools.skip decorator """Check that methods do not have the testtools.skip decorator
@ -132,6 +124,7 @@ def no_testtools_skip_decorator(logical_line):
"decorators.skip_because from tempest.lib") "decorators.skip_because from tempest.lib")
@core.flake8ext
def use_rand_uuid_instead_of_uuid4(logical_line, filename): def use_rand_uuid_instead_of_uuid4(logical_line, filename):
"""Check that tests use data_utils.rand_uuid() instead of uuid.uuid4() """Check that tests use data_utils.rand_uuid() instead of uuid.uuid4()
@ -145,6 +138,7 @@ def use_rand_uuid_instead_of_uuid4(logical_line, filename):
yield (0, msg) yield (0, msg)
@core.flake8ext
def no_rbac_rule_validation_decorator(physical_line, filename): def no_rbac_rule_validation_decorator(physical_line, filename):
"""Check that each test has the ``rbac_rule_validation.action`` decorator. """Check that each test has the ``rbac_rule_validation.action`` decorator.
@ -174,6 +168,7 @@ def no_rbac_rule_validation_decorator(physical_line, filename):
have_rbac_decorator = False have_rbac_decorator = False
@core.flake8ext
def no_rbac_suffix_in_test_filename(filename): def no_rbac_suffix_in_test_filename(filename):
"""Check that RBAC filenames end with "_rbac" suffix. """Check that RBAC filenames end with "_rbac" suffix.
@ -188,6 +183,7 @@ def no_rbac_suffix_in_test_filename(filename):
return 0, "RBAC test filenames must end in _rbac suffix" return 0, "RBAC test filenames must end in _rbac suffix"
@core.flake8ext
def no_rbac_test_suffix_in_test_class_name(physical_line, filename): def no_rbac_test_suffix_in_test_class_name(physical_line, filename):
"""Check that RBAC class names end with "RbacTest" """Check that RBAC class names end with "RbacTest"
@ -203,6 +199,7 @@ def no_rbac_test_suffix_in_test_class_name(physical_line, filename):
return 0, "RBAC test class names must end in 'RbacTest'" return 0, "RBAC test class names must end in 'RbacTest'"
@core.flake8ext
def no_client_alias_in_test_cases(logical_line, filename): def no_client_alias_in_test_cases(logical_line, filename):
"""Check that test cases don't use "self.client" to define a client. """Check that test cases don't use "self.client" to define a client.
@ -213,6 +210,7 @@ def no_client_alias_in_test_cases(logical_line, filename):
return 0, "Do not use 'self.client' as a service client alias" return 0, "Do not use 'self.client' as a service client alias"
@core.flake8ext
def no_extension_rbac_test_suffix_in_plugin_test_class_name(physical_line, def no_extension_rbac_test_suffix_in_plugin_test_class_name(physical_line,
filename): filename):
"""Check that Extension RBAC class names end with "ExtRbacTest" """Check that Extension RBAC class names end with "ExtRbacTest"
@ -249,18 +247,3 @@ def no_extension_rbac_test_suffix_in_plugin_test_class_name(physical_line,
error = ("Plugin RBAC test subclasses must inherit from a " error = ("Plugin RBAC test subclasses must inherit from a "
"'ExtRbacTest' base class") "'ExtRbacTest' base class")
return len(superclass) - 1, error return len(superclass) - 1, error
def factory(register):
register(import_no_clients_in_api_tests)
register(no_setup_teardown_class_for_tests)
register(no_vi_headers)
register(no_hyphen_at_end_of_rand_name)
register(no_mutable_default_args)
register(no_testtools_skip_decorator)
register(use_rand_uuid_instead_of_uuid4)
register(service_tags_not_in_module_path)
register(no_rbac_rule_validation_decorator)
register(no_rbac_suffix_in_test_filename)
register(no_rbac_test_suffix_in_test_class_name)
register(no_extension_rbac_test_suffix_in_plugin_test_class_name)

View File

@ -62,12 +62,6 @@ class RBACHackingTestCase(base.TestCase):
" def tearDownClass(cls):", " def tearDownClass(cls):",
"./patrole_tempest_plugin/tests/scenario/fake_test.py")) "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
def test_no_vi_headers(self):
self.assertTrue(checks.no_vi_headers(
"# vim: tabstop=4", 1, range(250)))
self.assertTrue(checks.no_vi_headers(
"# vim: tabstop=4", 249, range(250)))
def test_service_tags_not_in_module_path(self): def test_service_tags_not_in_module_path(self):
self.assertTrue(checks.service_tags_not_in_module_path( self.assertTrue(checks.service_tags_not_in_module_path(
"@utils.services('volume')", "@utils.services('volume')",

View File

@ -109,8 +109,8 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
def test_policy(*args): def test_policy(*args):
raise exceptions.Forbidden() raise exceptions.Forbidden()
test_re = ("User with roles \['member'\] was not allowed to perform " test_re = (r"User with roles \['member'\] was not allowed to perform "
"the following actions: \[%s\].*" % (mock.sentinel.action)) r"the following actions: \[%s\].*" % (mock.sentinel.action))
self.assertRaisesRegex( self.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, test_re, test_policy, rbac_exceptions.RbacUnderPermissionException, test_re, test_policy,
self.test_obj) self.test_obj)
@ -164,8 +164,8 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
def test_policy(*args): def test_policy(*args):
raise exception_cls(**kwargs) raise exception_cls(**kwargs)
test_re = (".*User with roles \[%s\] was not allowed to " test_re = (r".*User with roles \[%s\] was not allowed to "
"perform the following actions: \[%s\].*" % ( r"perform the following actions: \[%s\].*" % (
', '.join("'%s'" % r for r in self.test_roles), ', '.join("'%s'" % r for r in self.test_roles),
mock.sentinel.action)) mock.sentinel.action))
self.assertRaisesRegex( self.assertRaisesRegex(
@ -227,8 +227,8 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
raise exceptions.NotFound() raise exceptions.NotFound()
expected_errors = [ expected_errors = [
("User with roles \['member'\] was not allowed to perform the " (r"User with roles \['member'\] was not allowed to perform the "
"following actions: \['%s'\].*" % policy_names[0]), r"following actions: \['%s'\].*" % policy_names[0]),
None None
] ]
@ -282,7 +282,7 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest):
for test_policy in ( for test_policy in (
test_policy_expect_forbidden, test_policy_expect_not_found): test_policy_expect_forbidden, test_policy_expect_not_found):
error_re = ".*OverPermission: .* \[%s\]$" % mock.sentinel.action error_re = r".*OverPermission: .* \[%s\]$" % mock.sentinel.action
self.assertRaisesRegex(rbac_exceptions.RbacOverPermissionException, self.assertRaisesRegex(rbac_exceptions.RbacOverPermissionException,
error_re, test_policy, self.test_obj) error_re, test_policy, self.test_obj)
self.assertRegex(mock_log.error.mock_calls[0][1][0], error_re) self.assertRegex(mock_log.error.mock_calls[0][1][0], error_re)
@ -380,8 +380,8 @@ class RBACMultiRoleRuleValidationTest(BaseRBACMultiRoleRuleValidationTest,
def test_policy(*args): def test_policy(*args):
raise exceptions.Forbidden() raise exceptions.Forbidden()
test_re = ("User with roles \['member', 'anotherrole'\] was not " test_re = (r"User with roles \['member', 'anotherrole'\] was not "
"allowed to perform the following actions: \[%s\].*" % r"allowed to perform the following actions: \[%s\].*" %
(mock.sentinel.action)) (mock.sentinel.action))
self.assertRaisesRegex( self.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, test_re, test_policy, rbac_exceptions.RbacUnderPermissionException, test_re, test_policy,
@ -409,8 +409,8 @@ class RBACMultiRoleRuleValidationTest(BaseRBACMultiRoleRuleValidationTest,
raise exceptions.NotFound() raise exceptions.NotFound()
expected_errors = [ expected_errors = [
("User with roles \['member', 'anotherrole'\] was not allowed to " (r"User with roles \['member', 'anotherrole'\] was not allowed to "
"perform the following actions: \['%s'\].*" % policy_names[0]), r"perform the following actions: \['%s'\].*" % policy_names[0]),
None None
] ]
@ -657,7 +657,7 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
mock_authority.PolicyAuthority.return_value.allowed.side_effect = ( mock_authority.PolicyAuthority.return_value.allowed.side_effect = (
allowed_list) allowed_list)
error_re = ".*OverPermission: .* \[%s\]$" % fail_on_action error_re = r".*OverPermission: .* \[%s\]$" % fail_on_action
self.assertRaisesRegex( self.assertRaisesRegex(
rbac_exceptions.RbacOverPermissionException, error_re, rbac_exceptions.RbacOverPermissionException, error_re,
test_policy, self.test_obj) test_policy, self.test_obj)
@ -727,7 +727,7 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest):
error_re = ("User with roles ['member'] was not allowed to perform " error_re = ("User with roles ['member'] was not allowed to perform "
"the following actions: %s. Expected allowed actions: %s. " "the following actions: %s. Expected allowed actions: %s. "
"Expected disallowed actions: []." % "Expected disallowed actions: []." %
(rules, rules)).replace('[', '\[').replace(']', '\]') (rules, rules)).replace('[', r'\[').replace(']', r'\]')
self.assertRaisesRegex( self.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, error_re, rbac_exceptions.RbacUnderPermissionException, error_re,
test_policy, self.test_obj) test_policy, self.test_obj)
@ -912,7 +912,7 @@ class RBACMultiRoleRuleValidationTestMultiPolicy(
error_re = ("User with roles ['member', 'anotherrole'] was not " error_re = ("User with roles ['member', 'anotherrole'] was not "
"allowed to perform the following actions: %s. Expected " "allowed to perform the following actions: %s. Expected "
"allowed actions: %s. Expected disallowed actions: []." % "allowed actions: %s. Expected disallowed actions: []." %
(rules, rules)).replace('[', '\[').replace(']', '\]') (rules, rules)).replace('[', r'\[').replace(']', r'\]')
self.assertRaisesRegex( self.assertRaisesRegex(
rbac_exceptions.RbacUnderPermissionException, error_re, rbac_exceptions.RbacUnderPermissionException, error_re,
test_policy, self.test_obj) test_policy, self.test_obj)

View File

@ -1,7 +1,7 @@
# The order of packages is significant, because pip processes them in the order # The order of packages is significant, because pip processes them in the order
# 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>=1.1.0,<1.2.0 # Apache-2.0 hacking>=3.0,<3.1.0 # Apache-2.0
fixtures>=3.0.0 # Apache-2.0/BSD fixtures>=3.0.0 # Apache-2.0/BSD
mock>=2.0.0 # BSD mock>=2.0.0 # BSD
coverage!=4.4,>=4.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0

20
tox.ini
View File

@ -99,12 +99,26 @@ show-source = True
# H405 is another one that is good as a guideline, but sometimes # H405 is another one that is good as a guideline, but sometimes
# multiline doc strings just don't have a natural summary # multiline doc strings just don't have a natural summary
# line. Rejecting code for this reason is wrong. # line. Rejecting code for this reason is wrong.
ignore = E123,E125,H405 # W504 line break after binary operator
ignore = E123,E125,H405,W504
builtins = _ builtins = _
exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build
[hacking] [flake8:local-plugins]
local-check-factory = patrole_tempest_plugin.hacking.checks.factory extension =
T102 = checks:import_no_clients_in_api_tests
T105 = checks:no_setup_teardown_class_for_tests
T107 = checks:service_tags_not_in_module_path
T108 = checks:no_hyphen_at_end_of_rand_name
N322 = checks:no_mutable_default_args
T109 = checks:no_testtools_skip_decorator
T113 = checks:use_rand_uuid_instead_of_uuid4
P100 = checks:no_rbac_rule_validation_decorator
P101 = checks:no_rbac_suffix_in_test_filename
P102 = checks:no_rbac_test_suffix_in_test_class_name
P103 = checks:no_client_alias_in_test_cases
P104 = checks:no_extension_rbac_test_suffix_in_plugin_test_class_name
paths = ./patrole_tempest_plugin/hacking
[testenv:lower-constraints] [testenv:lower-constraints]
deps = deps =