diff --git a/HACKING.rst b/HACKING.rst index 2dfb28cb..5fb3faad 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -21,7 +21,6 @@ for more information: - [T102] Cannot import OpenStack python clients in ``patrole_tempest_plugin/tests/api`` - [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 - [T108] Check no hyphen at the end of rand_name() argument - [T109] Cannot use testtools.skip decorator; instead use diff --git a/patrole_tempest_plugin/hacking/checks.py b/patrole_tempest_plugin/hacking/checks.py index d7b772d1..48ef2695 100644 --- a/patrole_tempest_plugin/hacking/checks.py +++ b/patrole_tempest_plugin/hacking/checks.py @@ -17,6 +17,7 @@ import os import re +from hacking import core 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.*') SETUP_TEARDOWN_CLASS_DEFINITION = re.compile(r'^\s+def (setUp|tearDown)Class') SCENARIO_DECORATOR = re.compile(r'\s*@.*services\((.*)\)') -VI_HEADER_RE = re.compile(r"^#\s+vim?:.+") RAND_NAME_HYPHEN_RE = re.compile(r".*rand_name\(.+[\-\_][\"\']\)") MUTABLE_DEFAULT_ARGS = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") TESTTOOLS_SKIP_DECORATOR = re.compile(r'\s*@testtools\.skip\((.*)\)') @@ -42,6 +42,7 @@ EXT_RBAC_TEST = re.compile( have_rbac_decorator = False +@core.flake8ext def import_no_clients_in_api_tests(physical_line, filename): """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")) +@core.flake8ext def no_setup_teardown_class_for_tests(physical_line, filename): """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") -def no_vi_headers(physical_line, line_number, lines): - """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" - - +@core.flake8ext def service_tags_not_in_module_path(physical_line, filename): """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") +@core.flake8ext def no_hyphen_at_end_of_rand_name(logical_line, filename): """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 +@core.flake8ext def no_mutable_default_args(logical_line): """Check that mutable object isn't used as default argument @@ -122,6 +113,7 @@ def no_mutable_default_args(logical_line): yield (0, msg) +@core.flake8ext def no_testtools_skip_decorator(logical_line): """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") +@core.flake8ext def use_rand_uuid_instead_of_uuid4(logical_line, filename): """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) +@core.flake8ext def no_rbac_rule_validation_decorator(physical_line, filename): """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 +@core.flake8ext def no_rbac_suffix_in_test_filename(filename): """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" +@core.flake8ext def no_rbac_test_suffix_in_test_class_name(physical_line, filename): """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'" +@core.flake8ext def no_client_alias_in_test_cases(logical_line, filename): """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" +@core.flake8ext def no_extension_rbac_test_suffix_in_plugin_test_class_name(physical_line, filename): """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 " "'ExtRbacTest' base class") 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) diff --git a/patrole_tempest_plugin/tests/unit/test_hacking.py b/patrole_tempest_plugin/tests/unit/test_hacking.py index a0ace76c..edfaa7eb 100644 --- a/patrole_tempest_plugin/tests/unit/test_hacking.py +++ b/patrole_tempest_plugin/tests/unit/test_hacking.py @@ -62,12 +62,6 @@ class RBACHackingTestCase(base.TestCase): " def tearDownClass(cls):", "./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): self.assertTrue(checks.service_tags_not_in_module_path( "@utils.services('volume')", diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py index 05ad7b79..5c4efff3 100644 --- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py +++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py @@ -109,8 +109,8 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest): def test_policy(*args): raise exceptions.Forbidden() - test_re = ("User with roles \['member'\] was not allowed to perform " - "the following actions: \[%s\].*" % (mock.sentinel.action)) + test_re = (r"User with roles \['member'\] was not allowed to perform " + r"the following actions: \[%s\].*" % (mock.sentinel.action)) self.assertRaisesRegex( rbac_exceptions.RbacUnderPermissionException, test_re, test_policy, self.test_obj) @@ -164,8 +164,8 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest): def test_policy(*args): raise exception_cls(**kwargs) - test_re = (".*User with roles \[%s\] was not allowed to " - "perform the following actions: \[%s\].*" % ( + test_re = (r".*User with roles \[%s\] was not allowed to " + r"perform the following actions: \[%s\].*" % ( ', '.join("'%s'" % r for r in self.test_roles), mock.sentinel.action)) self.assertRaisesRegex( @@ -227,8 +227,8 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest): raise exceptions.NotFound() expected_errors = [ - ("User with roles \['member'\] was not allowed to perform the " - "following actions: \['%s'\].*" % policy_names[0]), + (r"User with roles \['member'\] was not allowed to perform the " + r"following actions: \['%s'\].*" % policy_names[0]), None ] @@ -282,7 +282,7 @@ class RBACRuleValidationTest(BaseRBACRuleValidationTest): for test_policy in ( 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, error_re, test_policy, self.test_obj) self.assertRegex(mock_log.error.mock_calls[0][1][0], error_re) @@ -380,8 +380,8 @@ class RBACMultiRoleRuleValidationTest(BaseRBACMultiRoleRuleValidationTest, def test_policy(*args): raise exceptions.Forbidden() - test_re = ("User with roles \['member', 'anotherrole'\] was not " - "allowed to perform the following actions: \[%s\].*" % + test_re = (r"User with roles \['member', 'anotherrole'\] was not " + r"allowed to perform the following actions: \[%s\].*" % (mock.sentinel.action)) self.assertRaisesRegex( rbac_exceptions.RbacUnderPermissionException, test_re, test_policy, @@ -409,8 +409,8 @@ class RBACMultiRoleRuleValidationTest(BaseRBACMultiRoleRuleValidationTest, raise exceptions.NotFound() expected_errors = [ - ("User with roles \['member', 'anotherrole'\] was not allowed to " - "perform the following actions: \['%s'\].*" % policy_names[0]), + (r"User with roles \['member', 'anotherrole'\] was not allowed to " + r"perform the following actions: \['%s'\].*" % policy_names[0]), None ] @@ -657,7 +657,7 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest): mock_authority.PolicyAuthority.return_value.allowed.side_effect = ( allowed_list) - error_re = ".*OverPermission: .* \[%s\]$" % fail_on_action + error_re = r".*OverPermission: .* \[%s\]$" % fail_on_action self.assertRaisesRegex( rbac_exceptions.RbacOverPermissionException, error_re, test_policy, self.test_obj) @@ -727,7 +727,7 @@ class RBACRuleValidationTestMultiPolicy(BaseRBACRuleValidationTest): error_re = ("User with roles ['member'] was not allowed to perform " "the following actions: %s. Expected allowed actions: %s. " "Expected disallowed actions: []." % - (rules, rules)).replace('[', '\[').replace(']', '\]') + (rules, rules)).replace('[', r'\[').replace(']', r'\]') self.assertRaisesRegex( rbac_exceptions.RbacUnderPermissionException, error_re, test_policy, self.test_obj) @@ -912,7 +912,7 @@ class RBACMultiRoleRuleValidationTestMultiPolicy( error_re = ("User with roles ['member', 'anotherrole'] was not " "allowed to perform the following actions: %s. Expected " "allowed actions: %s. Expected disallowed actions: []." % - (rules, rules)).replace('[', '\[').replace(']', '\]') + (rules, rules)).replace('[', r'\[').replace(']', r'\]') self.assertRaisesRegex( rbac_exceptions.RbacUnderPermissionException, error_re, test_policy, self.test_obj) diff --git a/test-requirements.txt b/test-requirements.txt index a08c27ae..63f290cc 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,7 @@ # 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 # 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 mock>=2.0.0 # BSD coverage!=4.4,>=4.0 # Apache-2.0 diff --git a/tox.ini b/tox.ini index af42f58b..f42a5b5a 100644 --- a/tox.ini +++ b/tox.ini @@ -99,12 +99,26 @@ show-source = True # H405 is another one that is good as a guideline, but sometimes # multiline doc strings just don't have a natural summary # line. Rejecting code for this reason is wrong. -ignore = E123,E125,H405 +# W504 line break after binary operator +ignore = E123,E125,H405,W504 builtins = _ exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build -[hacking] -local-check-factory = patrole_tempest_plugin.hacking.checks.factory +[flake8:local-plugins] +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] deps =