commit 1b49965a9b31fada55a7a93af2881e8a1909f010
author Felipe Monteiro <felipe.monteiro@att.com> Wed Oct 31 20:44:50 2018 -0400
committer Felipe Monteiro <felipe.monteiro@att.com> Thu Nov 01 10:34:36 2018 -0400
tree f08dc3b8a1e06aa0881fd9e7e24d6f15c388fd87
parent 904a02bef8af64c2a6fb1e0aca67ea9db3f9883f

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

diff --git a/patrole_tempest_plugin/hacking/checks.py b/patrole_tempest_plugin/hacking/checks.py
index 853d65c..1f06258 100644
--- a/patrole_tempest_plugin/hacking/checks.py
+++ b/patrole_tempest_plugin/hacking/checks.py
@@ -36,7 +36,8 @@
 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 @@
 
     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):

patrole_tempest_plugin/tests/unit/test_hacking.py

diff --git a/patrole_tempest_plugin/tests/unit/test_hacking.py b/patrole_tempest_plugin/tests/unit/test_hacking.py
index f75bffe..d35b816 100644
--- a/patrole_tempest_plugin/tests/unit/test_hacking.py
+++ b/patrole_tempest_plugin/tests/unit/test_hacking.py
@@ -262,7 +262,7 @@
 
         # 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 @@
 
         # 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"))