From fc173b9f48c3f7330470c4064683baa4192fff1a Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Wed, 7 Mar 2018 21:14:19 -0500 Subject: [PATCH] Test: remove deprecated skip() method The skip() method has been deprecated for quite some time. Let's remove it for good and have users using the decorators and cancel() instead. Signed-off-by: Cleber Rosa --- avocado/core/decorators.py | 2 +- avocado/core/exceptions.py | 10 ----- avocado/core/test.py | 36 +---------------- docs/source/WritingTests.rst | 58 ++++----------------------- examples/tests/cancelonsetup.py | 4 -- examples/tests/vm-cleanup.py | 6 +-- selftests/.data/test_statuses.py | 8 ++-- selftests/functional/test_statuses.py | 27 ++++--------- 8 files changed, 25 insertions(+), 126 deletions(-) diff --git a/avocado/core/decorators.py b/avocado/core/decorators.py index 065ae844..48aea1b5 100644 --- a/avocado/core/decorators.py +++ b/avocado/core/decorators.py @@ -28,7 +28,7 @@ def fail_on(exceptions=None): :param exceptions: Tuple or single exception to be assumed as test fail [Exception] - :note: self.error and self.skip behavior remains intact + :note: self.error and self.cancel behavior remains intact :note: To allow simple usage param "exceptions" must not be callable """ func = False diff --git a/avocado/core/exceptions.py b/avocado/core/exceptions.py index 77ac0be3..0d99830f 100644 --- a/avocado/core/exceptions.py +++ b/avocado/core/exceptions.py @@ -119,16 +119,6 @@ class TestSkipError(TestBaseException): status = "SKIP" -class TestSetupSkip(TestBaseException): - - """ - Indicates that the test is skipped in setUp(). - - Should be thrown when skip() is used in setUp(). - """ - status = "SKIP" - - class TestFail(TestBaseException, AssertionError): """ diff --git a/avocado/core/test.py b/avocado/core/test.py index 5dde7acd..74ce57ed 100644 --- a/avocado/core/test.py +++ b/avocado/core/test.py @@ -807,7 +807,7 @@ class Test(unittest.TestCase, TestData): try: if skip_test is False: self.setUp() - except (exceptions.TestSetupSkip, exceptions.TestSkipError) as details: + except exceptions.TestSkipError as details: skip_test = True stacktrace.log_exc_info(sys.exc_info(), logger=LOG_JOB) raise exceptions.TestSkipError(details) @@ -821,16 +821,6 @@ class Test(unittest.TestCase, TestData): else: try: testMethod() - except exceptions.TestSetupSkip as details: - stacktrace.log_exc_info(sys.exc_info(), logger=LOG_JOB) - skip_illegal_msg = ('Calling skip() in places other than ' - 'setUp() is not allowed in avocado, you ' - 'must fix your test. Original skip exception: ' - '%s' % details) - raise exceptions.TestError(skip_illegal_msg) - except exceptions.TestSkipError as details: - stacktrace.log_exc_info(sys.exc_info(), logger=LOG_JOB) - raise except exceptions.TestCancel as details: stacktrace.log_exc_info(sys.exc_info(), logger=LOG_JOB) raise @@ -848,13 +838,6 @@ class Test(unittest.TestCase, TestData): try: if skip_test is False: self.tearDown() - except exceptions.TestSetupSkip as details: - stacktrace.log_exc_info(sys.exc_info(), logger=LOG_JOB) - skip_illegal_msg = ('Calling skip() in places other than ' - 'setUp() is not allowed in avocado, ' - 'you must fix your test. Original skip ' - 'exception: %s' % details) - raise exceptions.TestError(skip_illegal_msg) except exceptions.TestSkipError as details: stacktrace.log_exc_info(sys.exc_info(), logger=LOG_JOB) skip_illegal_msg = ('Using skip decorators in tearDown() ' @@ -1044,23 +1027,6 @@ class Test(unittest.TestCase, TestData): """ raise exceptions.TestError(message) - def skip(self, message=None): - """ - Skips the currently running test. - - This method should only be called from a test's setUp() method, not - anywhere else, since by definition, if a test gets to be executed, it - can't be skipped anymore. If you call this method outside setUp(), - avocado will mark your test status as ERROR, and instruct you to - fix your test in the error message. - - :param message: an optional message that will be recorded in the logs - :type message: str - """ - dep_msg = "WARNING: self.skip() will be deprecated. " \ - "Use 'self.cancel()' or the skip decorators" - raise exceptions.TestSetupSkip("[%s] %s" % (dep_msg, message)) - def cancel(self, message=None): """ Cancels the test. diff --git a/docs/source/WritingTests.rst b/docs/source/WritingTests.rst index 3e0409fc..8118c015 100644 --- a/docs/source/WritingTests.rst +++ b/docs/source/WritingTests.rst @@ -106,9 +106,8 @@ test. To support you Avocado supports several methods: Test methods ------------ -The simplest way to set the status is to use ``self.fail`` or -``self.error`` directly from test. One can also use ``self.skip`` -but only from the ``setUp`` method. +The simplest way to set the status is to use ``self.fail``, +``self.error`` or ``self.cancel`` directly from test. To remember a warning, one simply writes to ``self.log.warning`` logger. This won't interrupt the test execution, but it will @@ -1051,53 +1050,8 @@ sending a class:`signal.SIGTERM` to the test, making it raise a Skipping Tests ============== -Avocado offers some options for the test writers to skip a test: - -Test ``skip()`` Method ----------------------- - -.. warning:: `self.skip()` will be deprecated at the end of 2017. - Please adjust your tests to use the `self.cancel()` or the skip - decorators instead. - -Using the ``skip()`` method available in the Test API is only allowed -inside the ``setUp()`` method. Calling ``skip()`` from inside the test is not -allowed as, by concept, you cannot skip a test after it's already initiated. - -The test below:: - - import avocado - - class MyTestClass(avocado.Test): - - def setUp(self): - if self.check_condition(): - self.skip('Test skipped due to the condition.') - - def test(self): - pass - - def check_condition(self): - return True - -Will produce the following result:: - - $ avocado run test_skip_method.py - JOB ID : 1bd8642400e3b6c584979504cafc4318f7a5fb65 - JOB LOG : $HOME/avocado/job-results/job-2017-02-03T17.16-1bd8642/job.log - (1/1) test_skip_method.py:MyTestClass.test: SKIP - RESULTS : PASS 0 | ERROR 0 | FAIL 0 | SKIP 1 | WARN 0 | INTERRUPT 0 - JOB TIME : 0.10 s - JOB HTML : $HOME/avocado/job-results/job-2017-02-03T17.16-1bd8642/html/results.html - -Notice that the ``tearDown()`` will not be executed when ``skip()`` is used. -Any cleanup treatment has to be handled by the ``setUp()``, before the -call to ``skip()``. - -Avocado Skip Decorators ------------------------ - -Another way to skip tests is by using the Avocado skip decorators: +To skip tests is in Avocado, you must use one of the Avocado skip +decorators: - ``@avocado.skip(reason)``: Skips a test. - ``@avocado.skipIf(condition, reason)``: Skips a test if the condition is @@ -1142,6 +1096,10 @@ not ``False``. Using the skip decorators, nothing is actually executed. We will skip the `setUp()` method, the test method and the `tearDown()` method. +.. note:: It's an erroneous condition, reported with test status + ``ERROR``, to use any of the skip decorators on the + ``tearDown()`` method. + Cancelling Tests ================ diff --git a/examples/tests/cancelonsetup.py b/examples/tests/cancelonsetup.py index 43831b54..41ed3b26 100644 --- a/examples/tests/cancelonsetup.py +++ b/examples/tests/cancelonsetup.py @@ -11,10 +11,6 @@ class CancelOnSetupTest(Test): """ def setUp(self): - """ - self.skip() is under deprecation process. This should - end with CANCEL instead. - """ self.cancel('This should end with CANCEL.') def test_wont_be_executed(self): diff --git a/examples/tests/vm-cleanup.py b/examples/tests/vm-cleanup.py index d6ce3e4a..f628742d 100644 --- a/examples/tests/vm-cleanup.py +++ b/examples/tests/vm-cleanup.py @@ -52,9 +52,9 @@ class VMCleanup(Test): vm_domain = self.params.get("vm_domain", default=None) vm_host = self.params.get("vm_host", default=None) if vm_domain is None or vm_host is None: - self.skip('Either "vm_domain" or "vm_host" parameters have not ' - 'been given. Please edit the "vm-cleanup.yaml" file ' - 'with the appropriate parameters') + self.cancel('Either "vm_domain" or "vm_host" parameters have not ' + 'been given. Please edit the "vm-cleanup.yaml" file ' + 'with the appropriate parameters') self.tmpdir = tempfile.mkdtemp(prefix='avocado_' + __name__) clean_test = os.path.join(self.tmpdir, 'clean.py') diff --git a/selftests/.data/test_statuses.py b/selftests/.data/test_statuses.py index 922bd0e0..d2263835 100644 --- a/selftests/.data/test_statuses.py +++ b/selftests/.data/test_statuses.py @@ -1,14 +1,14 @@ import os import sys -from avocado import Test +from avocado import Test, skip class SkipSetup(Test): + @skip("from setUp()") def setUp(self): self.log.info('setup pre') - self.skip() self.log.info('setup post') def test(self): @@ -26,9 +26,9 @@ class Skip(Test): self.log.info('setup pre') self.log.info('setup post') + @skip("from test()") def test(self): self.log.info('test pre') - self.skip() self.log.info('test post') def tearDown(self): @@ -46,9 +46,9 @@ class SkipTeardown(Test): self.log.info('test pre') self.log.info('test post') + @skip("from tearDown()") def tearDown(self): self.log.info('teardown pre') - self.skip() self.log.info('teardown post') diff --git a/selftests/functional/test_statuses.py b/selftests/functional/test_statuses.py index 27df9728..51835a6b 100644 --- a/selftests/functional/test_statuses.py +++ b/selftests/functional/test_statuses.py @@ -24,30 +24,19 @@ ALL_MESSAGES = ['setup pre', # [msg_in, ...]), # ...} EXPECTED_RESULTS = {'SkipSetup.test': ('SKIP', - ['setup pre', - "[WARNING: self.skip() will be " - "deprecated. Use 'self.cancel()' " - "or the skip decorators]"]), - 'Skip.test': ('ERROR', - ['setup pre', - 'setup post', - 'test pre', - 'teardown pre', - 'teardown post', - "Calling skip() in places other " - "than setUp() is not allowed in " - "avocado, you must fix your " - "test."]), + ['TestSkipError: from setUp()']), + 'Skip.test': ('SKIP', + ['TestSkipError: from test()']), 'SkipTeardown.test': ('ERROR', ['setup pre', 'setup post', 'test pre', 'test post', - 'teardown pre', - "Calling skip() in places other " - "than setUp() is not allowed in " - "avocado, you must fix your " - "test."]), + 'TestError: Using skip decorators ' + 'in tearDown() is not allowed in ' + 'avocado, you must fix your test. ' + 'Original skip exception: from ' + 'tearDown()']), 'CancelSetup.test': ('CANCEL', ['setup pre', 'teardown pre', -- GitLab