From 904a02bef8af64c2a6fb1e0aca67ea9db3f9883f Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Sun, 21 Oct 2018 12:54:46 -0400 Subject: [PATCH] hacking: Add hacking rule for plugin rbac test class names This patch set introduces a new hacking check called `no_plugin_rbac_test_suffix_in_plugin_test_class_name` which is responsible for enforcing that all plugin rbac test classes end in the correct suffix in order to avoid issues like [0]. Basically, some network plugin rbac tests were skipping because the regex in .zuul.yaml was not selecting them because the classes were improperly named. This is to avoid that regression. Updates documentation with P104 - alias for this new hacking rule - and adds unit tests to validate its logic. [0] https://review.openstack.org/#/c/612197/ Change-Id: Ia50edbe5aeb25e57756e9579da8270396bba718c --- HACKING.rst | 5 +++- patrole_tempest_plugin/hacking/checks.py | 16 +++++++++++ .../tests/unit/test_hacking.py | 27 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) 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"))