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
diff --git a/HACKING.rst b/HACKING.rst
index 28a977d..87e3b1f 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -33,12 +33,15 @@
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 d106da8..853d65c 100644
--- a/patrole_tempest_plugin/hacking/checks.py
+++ b/patrole_tempest_plugin/hacking/checks.py
@@ -36,6 +36,7 @@
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 @@
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 @@
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 6096c24..f75bffe 100644
--- a/patrole_tempest_plugin/tests/unit/test_hacking.py
+++ b/patrole_tempest_plugin/tests/unit/test_hacking.py
@@ -256,3 +256,30 @@
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"))