From 4d011af928eac40ebe8fafafa6bb77a0874b66d6 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 18 Jul 2018 00:11:48 -0400 Subject: [PATCH] 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 --- HACKING.rst | 7 ++--- tempest/hacking/checks.py | 27 +++++++++++++++++ tempest/tests/test_hacking.py | 57 +++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 2a7ae1d104..5b9c0f1836 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -25,6 +25,8 @@ Tempest Specific Commandments - [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 @@ should be applied to all negative test scenarios. 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 a57a360880..a72675e61e 100644 --- a/tempest/hacking/checks.py +++ b/tempest/hacking/checks.py @@ -34,6 +34,9 @@ METHOD_GET_RESOURCE = re.compile(r"^\s*def (list|show)\_.+") 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 @@ def unsupported_exception_attribute_PY3(logical_line): 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 @@ def factory(register): 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 bc3a753f32..9534ce8df5 100644 --- a/tempest/tests/test_hacking.py +++ b/tempest/tests/test_hacking.py @@ -193,3 +193,60 @@ class HackingTestCase(base.TestCase): "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)