Hacking checks for negative test cases
This patchset adds 2 hacking checks for making sure negative
tests have correct conventions applied.
* T117: Check that each negative test has the
``@decorators.attr(type=['negative'])`` applied
This patch set adds both hacking checks, adds unit tests
and updates HACKING.rst documentation with both new checks.
Closes-Bug: 1781044
Change-Id: I46df351187d22090861150c84fa0a0c1054ae3d6
diff --git a/HACKING.rst b/HACKING.rst
index 2a7ae1d..5b9c0f1 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -25,6 +25,8 @@
- [T115] Check that admin tests should exist under admin path
- [N322] Method's default argument shouldn't be mutable
- [T116] Unsupported 'message' Exception attribute in PY3
+- [T117] Check negative tests have ``@decorators.attr(type=['negative'])``
+ applied.
Test Data/Configuration
-----------------------
@@ -146,11 +148,6 @@
This attribute must be applied to each test that belongs to a negative test
class, i.e. a test class name ending with "Negative.*" substring.
-.. todo::
-
- Add a hacking check for ensuring that all classes that contain substring
- "Negative" have the negative attribute decorator applied above each test.
-
Slow Attribute
^^^^^^^^^^^^^^
The ``type='slow'`` attribute is used to signify that a test takes a long time
diff --git a/tempest/hacking/checks.py b/tempest/hacking/checks.py
index a57a360..a72675e 100644
--- a/tempest/hacking/checks.py
+++ b/tempest/hacking/checks.py
@@ -34,6 +34,9 @@
METHOD_DELETE_RESOURCE = re.compile(r"^\s*def delete_.+")
CLASS = re.compile(r"^class .+")
EX_ATTRIBUTE = re.compile(r'(\s+|\()(e|ex|exc|exception).message(\s+|\))')
+NEGATIVE_TEST_DECORATOR = re.compile(
+ r'\s*@decorators\.attr\(type=.*negative.*\)')
+_HAVE_NEGATIVE_DECORATOR = False
def import_no_clients_in_api_and_scenario_tests(physical_line, filename):
@@ -306,6 +309,29 @@
yield(0, msg)
+def negative_test_attribute_always_applied_to_negative_tests(physical_line,
+ filename):
+ """Check ``@decorators.attr(type=['negative'])`` applied to negative tests.
+
+ T117
+ """
+ global _HAVE_NEGATIVE_DECORATOR
+
+ if re.match(r'.\/tempest\/api\/.*_negative.*', filename):
+
+ if NEGATIVE_TEST_DECORATOR.match(physical_line):
+ _HAVE_NEGATIVE_DECORATOR = True
+ return
+
+ if TEST_DEFINITION.match(physical_line):
+ if not _HAVE_NEGATIVE_DECORATOR:
+ return (
+ 0, "T117: Must apply `@decorators.attr(type=['negative'])`"
+ " to all negative API tests"
+ )
+ _HAVE_NEGATIVE_DECORATOR = False
+
+
def factory(register):
register(import_no_clients_in_api_and_scenario_tests)
register(scenario_tests_need_service_tags)
@@ -322,3 +348,4 @@
register(use_rand_uuid_instead_of_uuid4)
register(dont_put_admin_tests_on_nonadmin_path)
register(unsupported_exception_attribute_PY3)
+ register(negative_test_attribute_always_applied_to_negative_tests)
diff --git a/tempest/tests/test_hacking.py b/tempest/tests/test_hacking.py
index bc3a753..9534ce8 100644
--- a/tempest/tests/test_hacking.py
+++ b/tempest/tests/test_hacking.py
@@ -193,3 +193,60 @@
"raise TestCase.failureException(exception.message)"))), 1)
self.assertEqual(len(list(checks.unsupported_exception_attribute_PY3(
"raise TestCase.failureException(ee.message)"))), 0)
+
+ def _test_no_negatve_test_attribute_applied_to_negative_test(
+ self, filename, with_other_decorators=False,
+ with_negative_decorator=True, expected_success=True):
+ check = checks.negative_test_attribute_always_applied_to_negative_tests
+ other_decorators = [
+ "@decorators.idempotent_id(123)",
+ "@utils.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
+ # @decorators.attr(type=['negative']) decorator.
+ for decorator in other_decorators:
+ self.assertIsNone(check(" %s" % decorator, filename))
+ if with_negative_decorator:
+ self.assertIsNone(
+ check("@decorators.attr(type=['negative'])", 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 @decorators.attr(type=['negative']) decorator.
+ for decorator in other_decorators:
+ self.assertIsNone(check(" %s" % decorator, filename))
+ final_result = check(" def test_some_negative_case", filename)
+ if expected_success:
+ self.assertIsNone(final_result)
+ else:
+ self.assertIsInstance(final_result, tuple)
+ self.assertFalse(final_result[0])
+
+ def test_no_negatve_test_attribute_applied_to_negative_test(self):
+ # Check negative filename, negative decorator passes
+ self._test_no_negatve_test_attribute_applied_to_negative_test(
+ "./tempest/api/test_something_negative.py")
+ # Check negative filename, negative decorator, other decorators passes
+ self._test_no_negatve_test_attribute_applied_to_negative_test(
+ "./tempest/api/test_something_negative.py",
+ with_other_decorators=True)
+
+ # Check non-negative filename skips check, causing pass
+ self._test_no_negatve_test_attribute_applied_to_negative_test(
+ "./tempest/api/test_something.py")
+
+ # Check negative filename, no negative decorator fails
+ self._test_no_negatve_test_attribute_applied_to_negative_test(
+ "./tempest/api/test_something_negative.py",
+ with_negative_decorator=False,
+ expected_success=False)
+ # Check negative filename, no negative decorator, other decorators
+ # fails
+ self._test_no_negatve_test_attribute_applied_to_negative_test(
+ "./tempest/api/test_something_negative.py",
+ with_other_decorators=True,
+ with_negative_decorator=False,
+ expected_success=False)