RFE: unittest: Dynamically allow skipping individual tests

[mulkieran]

Feature or enhancement

Proposal:

The motivation is laid out in this PR: https://github.com/stratis-storage/stratis-cli/pull/1091 .

The problem I'm addressing is that of not knowing what tests you will want to skip when you are writing the code.

We do some legacy tests on nightly. These legacy tests run older versions of the CLI against the current version of stratisd to try to detect if some failure has crept in. But sometimes, the tests are too precise, and FAIL with the new version of stratisd even when we consider the stratis-cli behavior to be correct. This can happen if, for example, we change the exception that is raised from StratisCliEngineError to StratisCliUserError, that is, we run a pre-check in stratis-cli of what we previously expected the engine to return an error for. In both cases, stratis-cli will exit with an error, and we consider this acceptable.

This approach will put in place a framework that will work for skipping any test that is desired to skip in future. It will be necessary to annotate every test with the skipIf decorator now and for future tests so that this approach can work in future.

The changes in the PR show how it would have to be implemented: https://github.com/stratis-storage/stratis-cli/pull/1091/files .

diff --git a/tests/whitebox/_misc.py b/tests/whitebox/_misc.py
index d15c42bc..3634ed3b 100644
--- a/tests/whitebox/_misc.py
+++ b/tests/whitebox/_misc.py
@@ -39,6 +39,11 @@ from stratis_cli._errors import StratisCliActionError

 _OK = StratisCliErrorCodes.OK

+_STRATIS_SKIP_TESTS_VALUE = os.getenv("STRATIS_SKIP_TESTS")
+_STRATIS_SKIP_TESTS = (
+    [] if _STRATIS_SKIP_TESTS_VALUE is None else _STRATIS_SKIP_TESTS_VALUE.spli
t(";")
+)
+

 def device_name_list(min_devices=0, max_devices=10, unique=False):
     """
@@ -336,6 +341,14 @@ def test_runner(command_line, stdin=None):
 TEST_RUNNER = test_runner

+def skip_if_requested(test_id):
+    """
+    Return true if the test id is included in the environment variable
+    STRATIS_SKIP_TESTS.
+    """
+    return any(x.endswith(test_id) for x in _STRATIS_SKIP_TESTS)
+
+

diff --git a/tests/whitebox/integration/report/test_get_report.py b/tests/whiteb
ox/integration/report/test_get_report.py
index 334eda52..06a8758a 100644
--- a/tests/whitebox/integration/report/test_get_report.py
+++ b/tests/whitebox/integration/report/test_get_report.py
@@ -16,14 +16,13 @@ Test 'stratis report'.
 """

 # isort: STDLIB
-import os
 import unittest

 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
 from stratis_cli._stratisd_constants import ReportKey

-from .._misc import TEST_RUNNER, SimTestCase
+from .._misc import TEST_RUNNER, SimTestCase, skip_if_requested

 _ERROR = StratisCliErrorCodes.ERROR

@@ -35,24 +34,35 @@ class ReportTestCase(SimTestCase):

     _MENU = ["--propagate", "report"]

+    @unittest.skipIf(skip_if_requested("ReportTestCase.test_report"), "skip requested")
     def test_report(self):
         """
         Test getting stopped pool report.
         """
         TEST_RUNNER(self._MENU + [ReportKey.STOPPED_POOLS.value])

+    @unittest.skipIf(
+        skip_if_requested("ReportTestCase.test_report_no_name"), "skip requested"
+    )
     def test_report_no_name(self):
         """
         Test getting engine state report when no name specified.
         """
         TEST_RUNNER(self._MENU)

+    @unittest.skipIf(
+        skip_if_requested("ReportTestCase.test_engine_state_report"), "skip requested"
+    )
     def test_engine_state_report(self):

The problem is obvious: every test method must be annotated with a skipIf decorator in case, sometime in the future, it is desired to skip that tests explicitly and it is not possible to change the code, i.e., to encode the skip condition statically. Moreover, each annotation must be different and match the name of the function; there is no general way to obtain an identifier of the test function from the decorator.

I think it would be reasonable for unittest to provide one of the following solutions:

1. Every test is automatically decorated with a skipIf decorator that checks an enivronment variable that specifies tests to skip.
2. unittest implements a way to specify a list of tests to skip in the most general way possible.
3. unittest implements a change to how decorators work so that the decorators have an easy access to the name or id of the function that they decorate. This would allow a decorator written by a unittest user to be fully generic.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

I could not find any previous discussion of this feature on the linked Discourse channel. I do not know if it would be considered small or large, that depends on the implementation.

[blhsing]

I think we can add a command-line option --skip that specifies a glob pattern of tests to skip. The option can be specified multiple times to skip tests of multiple name patterns.