diff --git a/HACKING.rst b/HACKING.rst index 28a977d6..87e3b1ff 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -33,12 +33,15 @@ for more information: The following are Patrole's specific Commandments: - [P100] The ``rbac_rule_validation.action`` decorator must be applied to - an RBAC test + all RBAC tests - [P101] RBAC test filenames must end with "_rbac.py"; for example, test_servers_rbac.py, not test_servers.py - [P102] RBAC test class names must end in 'RbacTest' - [P103] ``self.client`` must not be used as a client alias; this allows for code that is more maintainable and easier to read +- [P104] RBAC `plugin test class`_ names must end in 'PluginRbacTest' + +.. _plugin test class: https://github.com/openstack/patrole/tree/master/patrole_tempest_plugin/tests/api/network#neutron-plugin-tests Role Overriding --------------- diff --git a/patrole_tempest_plugin/hacking/checks.py b/patrole_tempest_plugin/hacking/checks.py index d106da83..853d65c2 100644 --- a/patrole_tempest_plugin/hacking/checks.py +++ b/patrole_tempest_plugin/hacking/checks.py @@ -36,6 +36,7 @@ RBAC_CLASS_NAME_RE = re.compile(r'class .+RbacTest') RULE_VALIDATION_DECORATOR = re.compile( r'\s*@rbac_rule_validation.action\(.*') IDEMPOTENT_ID_DECORATOR = re.compile(r'\s*@decorators\.idempotent_id\((.*)\)') +PLUGIN_RBAC_TEST = re.compile(r"class .+\(.+PluginRbacTest\)") have_rbac_decorator = False @@ -211,6 +212,20 @@ def no_client_alias_in_test_cases(logical_line, filename): return 0, "Do not use 'self.client' as a service client alias" +def no_plugin_rbac_test_suffix_in_plugin_test_class_name(physical_line, + filename): + """Check that Plugin RBAC class names end with "PluginRbacTest" + + P104 + """ + if "patrole_tempest_plugin/tests/api" in filename: + if PLUGIN_RBAC_TEST.match(physical_line): + subclass = physical_line.split('(')[0] + if not subclass.endswith("PluginRbacTest"): + error = "Plugin RBAC test classes must end in 'PluginRbacTest'" + return len(subclass) - 1, error + + def factory(register): register(import_no_clients_in_api_tests) register(no_setup_teardown_class_for_tests) @@ -223,3 +238,4 @@ def factory(register): register(no_rbac_rule_validation_decorator) register(no_rbac_suffix_in_test_filename) register(no_rbac_test_suffix_in_test_class_name) + register(no_plugin_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 6096c244..f75bffe6 100644 --- a/patrole_tempest_plugin/tests/unit/test_hacking.py +++ b/patrole_tempest_plugin/tests/unit/test_hacking.py @@ -256,3 +256,30 @@ class RBACHackingTestCase(base.TestCase): self.assertTrue(checks.no_client_alias_in_test_cases( " cls.client", "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + + def test_no_plugin_rbac_test_suffix_in_plugin_test_class_name(self): + check = checks.no_plugin_rbac_test_suffix_in_plugin_test_class_name + + # Passing cases: these do not inherit from "PluginRbacTest" base class. + self.assertFalse(check( + "class FakeRbacTest", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + self.assertFalse(check( + "class FakeRbacTest(base.BaseFakeRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + + # Passing cases: these **do** end in correct test class suffix. + self.assertFalse(check( + "class FakePluginRbacTest", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + self.assertFalse(check( + "class FakePluginRbacTest(base.BaseFakeRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + + # Failing cases: these **do not** end in correct test class suffix. + self.assertTrue(check( + "class FakeRbacTest(BaseFakePluginRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + self.assertTrue(check( + "class FakeRbacTest(BaseFakeNetworkPluginRbacTest)", + "./patrole_tempest_plugin/tests/api/network/fake_test_rbac.py"))