Merge "Adds unit tests for hacking checks"
diff --git a/HACKING.rst b/HACKING.rst
index f89910b..3bc8270 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -32,8 +32,7 @@
The following are Patrole's specific Commandments:
- [P100] The ``rbac_rule_validation.action`` decorator must be applied to
- an RBAC test (the check fails if the decorator is not one of the
- two decorators directly above the function declaration)
+ an RBAC test
- [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'
diff --git a/patrole_tempest_plugin/hacking/checks.py b/patrole_tempest_plugin/hacking/checks.py
index a3ef01f..eb73ef1 100644
--- a/patrole_tempest_plugin/hacking/checks.py
+++ b/patrole_tempest_plugin/hacking/checks.py
@@ -31,14 +31,13 @@
RAND_NAME_HYPHEN_RE = re.compile(r".*rand_name\(.+[\-\_][\"\']\)")
MUTABLE_DEFAULT_ARGS = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
TESTTOOLS_SKIP_DECORATOR = re.compile(r'\s*@testtools\.skip\((.*)\)')
-TEST_METHOD = re.compile(r"^ def test_.+")
CLASS = re.compile(r"^class .+")
RBAC_CLASS_NAME_RE = re.compile(r'class .+RbacTest')
RULE_VALIDATION_DECORATOR = re.compile(
- r'\s*@.*rbac_rule_validation.action\((.*)\)')
+ r'\s*@rbac_rule_validation.action\(.*')
IDEMPOTENT_ID_DECORATOR = re.compile(r'\s*@decorators\.idempotent_id\((.*)\)')
-previous_decorator = None
+have_rbac_decorator = False
def import_no_clients_in_api_tests(physical_line, filename):
@@ -144,8 +143,7 @@
yield (0, msg)
-def no_rbac_rule_validation_decorator(physical_line, filename,
- previous_logical):
+def no_rbac_rule_validation_decorator(physical_line, filename):
"""Check that each test has the ``rbac_rule_validation.action`` decorator.
Checks whether the test function has "@rbac_rule_validation.action"
@@ -157,22 +155,24 @@
P100
"""
- global previous_decorator
+ global have_rbac_decorator
- if "patrole_tempest_plugin/tests/api" in filename:
+ if ("patrole_tempest_plugin/tests/api" in filename or
+ "patrole_tempest_plugin/tests/scenario" in filename):
- if IDEMPOTENT_ID_DECORATOR.match(physical_line):
- previous_decorator = previous_logical
+ if RULE_VALIDATION_DECORATOR.match(physical_line):
+ have_rbac_decorator = True
return
- if TEST_METHOD.match(physical_line):
- if not RULE_VALIDATION_DECORATOR.match(previous_logical) and \
- not RULE_VALIDATION_DECORATOR.match(previous_decorator):
- return (0, "Must use rbac_rule_validation.action "
- "decorator for API and scenario tests")
+ if TEST_DEFINITION.match(physical_line):
+ if not have_rbac_decorator:
+ return (0, "Must use rbac_rule_validation.action "
+ "decorator for API and scenario tests")
+
+ have_rbac_decorator = False
-def no_rbac_suffix_in_test_filename(physical_line, filename, previous_logical):
+def no_rbac_suffix_in_test_filename(filename):
"""Check that RBAC filenames end with "_rbac" suffix.
P101
@@ -186,8 +186,7 @@
return 0, "RBAC test filenames must end in _rbac suffix"
-def no_rbac_test_suffix_in_test_class_name(physical_line, filename,
- previous_logical):
+def no_rbac_test_suffix_in_test_class_name(physical_line, filename):
"""Check that RBAC class names end with "RbacTest"
P102
@@ -202,7 +201,7 @@
return 0, "RBAC test class names must end in 'RbacTest'"
-def no_client_alias_in_test_cases(filename, logical_line):
+def no_client_alias_in_test_cases(logical_line, filename):
"""Check that test cases don't use "self.client" to define a client.
P103
diff --git a/patrole_tempest_plugin/tests/unit/test_hacking.py b/patrole_tempest_plugin/tests/unit/test_hacking.py
new file mode 100644
index 0000000..021602b
--- /dev/null
+++ b/patrole_tempest_plugin/tests/unit/test_hacking.py
@@ -0,0 +1,258 @@
+# Copyright 2017 AT&T Corporation.
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from tempest.tests import base
+
+from patrole_tempest_plugin.hacking import checks
+
+
+class RBACHackingTestCase(base.TestCase):
+
+ def test_import_no_clients_in_api_tests(self):
+ for client in checks.PYTHON_CLIENTS:
+ import_string = "import " + client + "client"
+ self.assertTrue(checks.import_no_clients_in_api_tests(
+ import_string,
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))
+ self.assertFalse(checks.import_no_clients_in_api_tests(
+ import_string,
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+ self.assertFalse(checks.import_no_clients_in_api_tests(
+ import_string,
+ "./patrole_tempest_plugin/tests/unit/fake_test.py"))
+ self.assertFalse(checks.import_no_clients_in_api_tests(
+ import_string,
+ "./patrole_tempest_plugin/fake_test.py"))
+
+ def test_no_setup_teardown_class_for_tests(self):
+ self.assertTrue(checks.no_setup_teardown_class_for_tests(
+ " def setUpClass(cls):",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertIsNone(checks.no_setup_teardown_class_for_tests(
+ " def setUpClass(cls): # noqa",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertTrue(checks.no_setup_teardown_class_for_tests(
+ " def setUpClass(cls):",
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))
+ self.assertTrue(checks.no_setup_teardown_class_for_tests(
+ " def setUpClass(cls):",
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+ self.assertTrue(checks.no_setup_teardown_class_for_tests(
+ " def tearDownClass(cls):",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertIsNone(checks.no_setup_teardown_class_for_tests(
+ " def tearDownClass(cls): # noqa",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertTrue(checks.no_setup_teardown_class_for_tests(
+ " def tearDownClass(cls):",
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))
+ self.assertTrue(checks.no_setup_teardown_class_for_tests(
+ " def tearDownClass(cls):",
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+
+ def test_no_vi_headers(self):
+ self.assertTrue(checks.no_vi_headers(
+ "# vim: tabstop=4", 1, range(250)))
+ self.assertTrue(checks.no_vi_headers(
+ "# vim: tabstop=4", 249, range(250)))
+
+ def test_service_tags_not_in_module_path(self):
+ self.assertTrue(checks.service_tags_not_in_module_path(
+ "@test.services('volume')",
+ "./patrole_tempest_plugin/tests/api/volume/fake_test_rbac.py"))
+ self.assertFalse(checks.service_tags_not_in_module_path(
+ "@test.services('image')",
+ "./patrole_tempest_plugin/tests/api/volume/fake_test_rbac.py"))
+
+ def test_no_hyphen_at_end_of_rand_name(self):
+ self.assertIsNone(checks.no_hyphen_at_end_of_rand_name(
+ "data_utils.rand_name('test')",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertIsNone(checks.no_hyphen_at_end_of_rand_name(
+ "data_utils.rand_name('test')",
+ "./patrole_tempest_plugin/tests/api/compute/fake_test_rbac.py"))
+ self.assertIsNone(checks.no_hyphen_at_end_of_rand_name(
+ "data_utils.rand_name('test')",
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+ self.assertIsNone(checks.no_hyphen_at_end_of_rand_name(
+ "data_utils.rand_name('test')",
+ "./patrole_tempest_plugin/tests/unit/fake_test.py"))
+ self.assertTrue(checks.no_hyphen_at_end_of_rand_name(
+ "data_utils.rand_name('test-')",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertTrue(checks.no_hyphen_at_end_of_rand_name(
+ "data_utils.rand_name('test-')",
+ "./patrole_tempest_plugin/tests/api/compute/fake_test_rbac.py"))
+ self.assertTrue(checks.no_hyphen_at_end_of_rand_name(
+ "data_utils.rand_name('test-')",
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+ self.assertTrue(checks.no_hyphen_at_end_of_rand_name(
+ "data_utils.rand_name('test-')",
+ "./patrole_tempest_plugin/tests/unit/fake_test.py"))
+
+ def test_no_mutable_default_args(self):
+ self.assertEqual(0, len(list(checks.no_mutable_default_args(
+ " def test_function(test_param_1, test_param_2"))))
+ self.assertEqual(1, len(list(checks.no_mutable_default_args(
+ " def test_function(test_param_1, test_param_2={}"))))
+
+ def test_no_testtools_skip_decorator(self):
+ self.assertEqual(1, len(list(checks.no_testtools_skip_decorator(
+ " @testtools.skip('Bug')"))))
+ self.assertEqual(0, len(list(checks.no_testtools_skip_decorator(
+ " @testtools.skipTest('reason')"))))
+ self.assertEqual(0, len(list(checks.no_testtools_skip_decorator(
+ " @testtools.skipUnless(reason, 'message')"))))
+ self.assertEqual(0, len(list(checks.no_testtools_skip_decorator(
+ " @testtools.skipIf(reason, 'message')"))))
+
+ def test_use_rand_uuid_instead_of_uuid4(self):
+ self.assertTrue(checks.use_rand_uuid_instead_of_uuid4(
+ "new_uuid = uuid.uuid4()",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertTrue(checks.use_rand_uuid_instead_of_uuid4(
+ "new_hex_uuid = uuid.uuid4().hex",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertIsNotNone(checks.use_rand_uuid_instead_of_uuid4(
+ "new_uuid = data_utils.rand_uuid()",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertIsNotNone(checks.use_rand_uuid_instead_of_uuid4(
+ "new_hex_uuid = data_utils.rand_uuid_hex()",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+
+ def _test_no_rbac_rule_validation_decorator(
+ self, filename, with_other_decorators=True,
+ with_rbac_decorator=True, expected_success=True):
+ other_decorators = [
+ "@decorators.idempotent_id(123)",
+ "@decorators.attr(type=['slow'])",
+ "@test.requires_ext(extension='ext', service='svc')"
+ ]
+
+ if with_other_decorators:
+ # Include multiple decorators to verify that this check works with
+ # arbitrarily many decorators. These insert decorators above the
+ # rbac_rule_validation decorator.
+ for decorator in other_decorators:
+ self.assertIsNone(checks.no_rbac_rule_validation_decorator(
+ " %s" % decorator, filename))
+ if with_rbac_decorator:
+ self.assertIsNone(checks.no_rbac_rule_validation_decorator(
+ " @rbac_rule_validation.action('rule')",
+ filename))
+ if with_other_decorators:
+ # Include multiple decorators to verify that this check works with
+ # arbitrarily many decorators. These insert decorators between
+ # the test and the @rbac_rule_validation decorator.
+ for decorator in other_decorators:
+ self.assertIsNone(checks.no_rbac_rule_validation_decorator(
+ " %s" % decorator, filename))
+ final_result = checks.no_rbac_rule_validation_decorator(
+ " def test_rbac_test",
+ filename)
+ if expected_success:
+ self.assertIsNone(final_result)
+ else:
+ self.assertIsInstance(final_result, tuple)
+ self.assertFalse(final_result[0])
+
+ def test_no_rbac_rule_validation_decorator(self):
+ self._test_no_rbac_rule_validation_decorator(
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py")
+ self._test_no_rbac_rule_validation_decorator(
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py",
+ False)
+ self._test_no_rbac_rule_validation_decorator(
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py",
+ with_other_decorators=True, with_rbac_decorator=False,
+ expected_success=False)
+ self._test_no_rbac_rule_validation_decorator(
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py",
+ with_other_decorators=False, with_rbac_decorator=False,
+ expected_success=False)
+
+ self._test_no_rbac_rule_validation_decorator(
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py")
+ self._test_no_rbac_rule_validation_decorator(
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py",
+ False)
+ self._test_no_rbac_rule_validation_decorator(
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py",
+ with_other_decorators=True, with_rbac_decorator=False,
+ expected_success=False)
+ self._test_no_rbac_rule_validation_decorator(
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py",
+ with_other_decorators=False, with_rbac_decorator=False,
+ expected_success=False)
+
+ def test_no_rbac_suffix_in_test_filename(self):
+ self.assertFalse(checks.no_rbac_suffix_in_test_filename(
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertFalse(checks.no_rbac_suffix_in_test_filename(
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+ self.assertFalse(checks.no_rbac_suffix_in_test_filename(
+ "./patrole_tempest_plugin/tests/unit/fake_test.py"))
+ self.assertFalse(checks.no_rbac_suffix_in_test_filename(
+ "./patrole_tempest_plugin/tests/api/fake_rbac_base.py"))
+ self.assertFalse(checks.no_rbac_suffix_in_test_filename(
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))
+ self.assertTrue(checks.no_rbac_suffix_in_test_filename(
+ "./patrole_tempest_plugin/tests/api/fake_test.py"))
+
+ def test_no_rbac_test_suffix_in_test_class_name(self):
+ self.assertFalse(checks.no_rbac_test_suffix_in_test_class_name(
+ "class FakeTest",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertFalse(checks.no_rbac_test_suffix_in_test_class_name(
+ "class FakeTest",
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+ self.assertFalse(checks.no_rbac_test_suffix_in_test_class_name(
+ "class FakeTest",
+ "./patrole_tempest_plugin/tests/unit/fake_test.py"))
+ self.assertFalse(checks.no_rbac_test_suffix_in_test_class_name(
+ "class FakeTest",
+ "./patrole_tempest_plugin/tests/api/fake_rbac_base.py"))
+ self.assertFalse(checks.no_rbac_test_suffix_in_test_class_name(
+ "class FakeRbacTest",
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))
+ self.assertTrue(checks.no_rbac_test_suffix_in_test_class_name(
+ "class FakeTest",
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))
+
+ def test_no_client_alias_in_test_cases(self):
+ self.assertFalse(checks.no_client_alias_in_test_cases(
+ " self.client",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertFalse(checks.no_client_alias_in_test_cases(
+ " cls.client",
+ "./patrole_tempest_plugin/tests/fake_test.py"))
+ self.assertFalse(checks.no_client_alias_in_test_cases(
+ " self.client",
+ "./patrole_tempest_plugin/tests/unit/fake_test.py"))
+ self.assertFalse(checks.no_client_alias_in_test_cases(
+ " cls.client",
+ "./patrole_tempest_plugin/tests/unit/fake_test.py"))
+ self.assertFalse(checks.no_client_alias_in_test_cases(
+ " self.client",
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+ self.assertFalse(checks.no_client_alias_in_test_cases(
+ " cls.client",
+ "./patrole_tempest_plugin/tests/scenario/fake_test.py"))
+ self.assertTrue(checks.no_client_alias_in_test_cases(
+ " self.client",
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))
+ self.assertTrue(checks.no_client_alias_in_test_cases(
+ " cls.client",
+ "./patrole_tempest_plugin/tests/api/fake_test_rbac.py"))