From 1b49965a9b31fada55a7a93af2881e8a1909f010 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 31 Oct 2018 20:44:50 -0400 Subject: [PATCH] trivial: Correct base class name in hacking check This patch set fixes the base class name in a hacking check unit test for PluginRbacTest Suffix (P104). The passing case should be: class FakePluginRbacTest(base.BaseFakePluginRbacTest) Not: class FakePluginRbacTest(base.BaseFakeRbacTest) Because the point is to check that all subclasses that inherit from class .+PluginRbacTest also end in that suffix. Change-Id: Ic6306b97bb68c42f51e796d876893f9ac91c67a4 --- patrole_tempest_plugin/hacking/checks.py | 35 ++++++++++++++++--- .../tests/unit/test_hacking.py | 29 +++++++++++++-- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/patrole_tempest_plugin/hacking/checks.py b/patrole_tempest_plugin/hacking/checks.py index 853d65c2..1f062584 100644 --- a/patrole_tempest_plugin/hacking/checks.py +++ b/patrole_tempest_plugin/hacking/checks.py @@ -36,7 +36,8 @@ 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\)") +PLUGIN_RBAC_TEST = re.compile( + r"class .+\(.+PluginRbacTest\)|class .+PluginRbacTest\(.+\)") have_rbac_decorator = False @@ -218,12 +219,36 @@ def no_plugin_rbac_test_suffix_in_plugin_test_class_name(physical_line, P104 """ + suffix = "PluginRbacTest" 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 + subclass, superclass = physical_line.split('(') + subclass = subclass.split('class')[1].strip() + superclass = superclass.split(')')[0].strip() + if "." in superclass: + superclass = superclass.split(".")[1] + + both_have = all( + clazz.endswith(suffix) for clazz in [subclass, superclass]) + none_have = not any( + clazz.endswith(suffix) for clazz in [subclass, superclass]) + + if not (both_have or none_have): + if (subclass.startswith("Base") and + superclass.startswith("Base")): + return + + # Case 1: Subclass of "BasePluginRbacTest" must end in `suffix` + # Case 2: Subclass that ends in `suffix` must inherit from base + # class ending in `suffix`. + if not subclass.endswith(suffix): + error = ("Plugin RBAC test subclasses must end in " + "'PluginRbacTest'") + return len(subclass) - 1, error + elif not superclass.endswith(suffix): + error = ("Plugin RBAC test subclasses must inherit from a " + "'PluginRbacTest' base class") + return len(superclass) - 1, error def factory(register): diff --git a/patrole_tempest_plugin/tests/unit/test_hacking.py b/patrole_tempest_plugin/tests/unit/test_hacking.py index f75bffe6..d35b8166 100644 --- a/patrole_tempest_plugin/tests/unit/test_hacking.py +++ b/patrole_tempest_plugin/tests/unit/test_hacking.py @@ -262,7 +262,7 @@ class RBACHackingTestCase(base.TestCase): # Passing cases: these do not inherit from "PluginRbacTest" base class. self.assertFalse(check( - "class FakeRbacTest", + "class FakeRbacTest(BaseFakeRbacTest)", "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) self.assertFalse(check( "class FakeRbacTest(base.BaseFakeRbacTest)", @@ -270,16 +270,39 @@ class RBACHackingTestCase(base.TestCase): # Passing cases: these **do** end in correct test class suffix. self.assertFalse(check( - "class FakePluginRbacTest", + "class FakePluginRbacTest(BaseFakePluginRbacTest)", "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) self.assertFalse(check( - "class FakePluginRbacTest(base.BaseFakeRbacTest)", + "class FakePluginRbacTest(base.BaseFakePluginRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + + # Passing cases: plugin base class inherits from another base class. + self.assertFalse(check( + "class BaseFakePluginRbacTest(base.BaseFakeRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + self.assertFalse(check( + "class BaseFakePluginRbacTest(BaseFakeRbacTest)", "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) # Failing cases: these **do not** end in correct test class suffix. + # Case 1: RbacTest subclass doesn't end in PluginRbacTest. + self.assertTrue(check( + "class FakeRbacTest(base.BaseFakePluginRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) 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")) + # Case 2: PluginRbacTest subclass doesn't inherit from + # BasePluginRbacTest. + self.assertTrue(check( + "class FakePluginRbacTest(base.BaseFakeRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + self.assertTrue(check( + "class FakePluginRbacTest(BaseFakeRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")) + self.assertTrue(check( + "class FakeNeutronPluginRbacTest(BaseFakeNeutronRbacTest)", + "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))