From 5a46cde6b9de410ece11becb30ce69dc18f6a331 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Tue, 16 Mar 2021 23:57:19 +0100
Subject: [PATCH 1/3] Add API methods for skipping tests

---
 reframe/core/decorators.py               | 15 +++--
 reframe/core/exceptions.py               |  4 ++
 reframe/core/pipeline.py                 | 24 ++++++-
 reframe/frontend/executors/__init__.py   | 35 ++++++++--
 reframe/frontend/executors/policies.py   | 49 ++++++++++++--
 reframe/frontend/statistics.py           |  8 +++
 unittests/resources/checks/hellocheck.py | 11 ++++
 unittests/test_cli.py                    |  3 +-
 unittests/test_policies.py               | 81 +++++++++++++++++++++++-
 9 files changed, 209 insertions(+), 21 deletions(-)

diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py
index 72daa9a2d4..0a0b745430 100644
--- a/reframe/core/decorators.py
+++ b/reframe/core/decorators.py
@@ -20,7 +20,9 @@
 import traceback
 
 import reframe.utility.osext as osext
-from reframe.core.exceptions import ReframeSyntaxError, user_frame
+from reframe.core.exceptions import (ReframeSyntaxError,
+                                     SkipTestError,
+                                     user_frame)
 from reframe.core.logging import getlogger
 from reframe.core.pipeline import RegressionTest
 from reframe.utility.versioning import VersionValidator
@@ -54,12 +56,15 @@ def _instantiate_all():
 
             try:
                 ret.append(_instantiate(cls, args))
+            except SkipTestError as e:
+                getlogger().warning(f'skipping test {cls.__name__!r}: {e}')
             except Exception:
                 frame = user_frame(*sys.exc_info())
-                msg = "skipping test due to errors: %s: " % cls.__name__
-                msg += "use `-v' for more information\n"
-                msg += "  FILE: %s:%s" % (frame.filename, frame.lineno)
-                getlogger().warning(msg)
+                getlogger().warning(
+                    f"skipping test {cls.__name__!r} due to errors: "
+                    f"use `-v' for more information\n"
+                    f"    FILE: {frame.filename}:{frame.lineno}"
+                )
                 getlogger().verbose(traceback.format_exc())
 
         return ret
diff --git a/reframe/core/exceptions.py b/reframe/core/exceptions.py
index 16905345bd..ae6b594bb0 100644
--- a/reframe/core/exceptions.py
+++ b/reframe/core/exceptions.py
@@ -276,6 +276,10 @@ class DependencyError(ReframeError):
     '''Raised when a dependency problem is encountered.'''
 
 
+class SkipTestError(ReframeError):
+    '''Raised when a test needs to be skipped.'''
+
+
 def user_frame(exc_type, exc_value, tb):
     '''Return a user frame from the exception's traceback.
 
diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py
index 6545f97aea..81c1541441 100644
--- a/reframe/core/pipeline.py
+++ b/reframe/core/pipeline.py
@@ -37,8 +37,8 @@
 from reframe.core.containers import ContainerPlatformField
 from reframe.core.deferrable import _DeferredExpression
 from reframe.core.exceptions import (BuildError, DependencyError,
-                                     PipelineError, SanityError,
-                                     PerformanceError)
+                                     PerformanceError, PipelineError,
+                                     SanityError, SkipTestError)
 from reframe.core.meta import RegressionTestMeta
 from reframe.core.schedulers import Job
 from reframe.core.warnings import user_deprecation_warning
@@ -1841,6 +1841,26 @@ def getdep(self, target, environ=None, part=None):
         raise DependencyError(f'could not resolve dependency to ({target!r}, '
                               f'{part!r}, {environ!r})')
 
+    def skip(self, msg=None):
+        '''Skip test.
+
+        :arg msg: A message explaining why the test was skipped.
+
+        .. versionadded:: 3.5.1
+        '''
+        raise SkipTestError(msg)
+
+    def skip_if(self, cond, msg=None):
+        '''Skip test if condition is true.
+
+        :arg cond: The condition to check for skipping the test.
+        :arg msg: A message explaining why the test was skipped.
+
+        .. versionadded:: 3.5.1
+        '''
+        if cond:
+            self.skip(msg)
+
     def __str__(self):
         return "%s(name='%s', prefix='%s')" % (type(self).__name__,
                                                self.name, self.prefix)
diff --git a/reframe/frontend/executors/__init__.py b/reframe/frontend/executors/__init__.py
index 3173b52c0d..6f2fd3283d 100644
--- a/reframe/frontend/executors/__init__.py
+++ b/reframe/frontend/executors/__init__.py
@@ -19,6 +19,7 @@
                                      JobNotStartedError,
                                      FailureLimitError,
                                      ForceExitError,
+                                     SkipTestError,
                                      TaskExit)
 from reframe.core.schedulers.local import LocalJobScheduler
 from reframe.frontend.printer import PrettyPrinter
@@ -131,6 +132,7 @@ def __init__(self, case, listeners=[]):
         self._current_stage = 'startup'
         self._exc_info = (None, None, None)
         self._listeners = list(listeners)
+        self._skipped = False
 
         # Reference count for dependent tests; safe to cleanup the test only
         # if it is zero
@@ -212,7 +214,8 @@ def exc_info(self):
 
     @property
     def failed(self):
-        return self._failed_stage is not None and not self._aborted
+        return (self._failed_stage is not None and
+                not self._aborted and not self._skipped)
 
     @property
     def failed_stage(self):
@@ -230,6 +233,10 @@ def completed(self):
     def aborted(self):
         return self._aborted
 
+    @property
+    def skipped(self):
+        return self._skipped
+
     def _notify_listeners(self, callback_name):
         for l in self._listeners:
             callback = getattr(l, callback_name)
@@ -260,7 +267,12 @@ def __exit__(this, exc_type, exc_value, traceback):
                 logger.debug(f'Entering stage: {self._current_stage}')
                 with update_timestamps():
                     return fn(*args, **kwargs)
-
+        except SkipTestError as e:
+            if not self.succeeded:
+                # Only skip a test if it hasn't finished yet;
+                # This practically ignores skipping during the cleanup phase
+                self.skip()
+                raise TaskExit from e
         except ABORT_REASONS:
             self.fail()
             raise
@@ -321,6 +333,12 @@ def fail(self, exc_info=None):
         self._exc_info = exc_info or sys.exc_info()
         self._notify_listeners('on_task_failure')
 
+    def skip(self, exc_info=None):
+        self._skipped = True
+        self._failed_stage = self._current_stage
+        self._exc_info = exc_info or sys.exc_info()
+        self._notify_listeners('on_task_skip')
+
     def abort(self, cause=None):
         if self.failed or self._aborted:
             return
@@ -355,6 +373,10 @@ def on_task_run(self, task):
     def on_task_exit(self, task):
         '''Called whenever a RegressionTask finishes.'''
 
+    @abc.abstractmethod
+    def on_task_skip(self, task):
+        '''Called whenever a RegressionTask is skipped.'''
+
     @abc.abstractmethod
     def on_task_failure(self, task):
         '''Called when a regression test has failed.'''
@@ -415,18 +437,21 @@ def runall(self, testcases, restored_cases=None):
             # Print the summary line
             num_failures = len(self._stats.failed())
             num_completed = len(self._stats.completed())
+            num_skipped = len(self._stats.skipped())
             num_tasks = len(self._stats.tasks())
-            if num_failures > 0 or num_completed < num_tasks:
+            if num_failures > 0 or num_completed + num_skipped < num_tasks:
                 status = 'FAILED'
             else:
                 status = 'PASSED'
 
             total_run = len(testcases) - num_tasks + num_completed
+            total_completed = len(self._stats.completed(0))
+            total_skipped = len(self._stats.skipped(0))
             self._printer.status(
                 status,
-                f'Ran {num_completed}/{total_run}'
+                f'Ran {total_completed}/{total_run}'
                 f' test case(s) from {num_checks} check(s) '
-                f'({num_failures} failure(s))',
+                f'({num_failures} failure(s), {total_skipped} skipped)',
                 just='center'
             )
             self._printer.timestamp('Finished on', 'short double line')
diff --git a/reframe/frontend/executors/policies.py b/reframe/frontend/executors/policies.py
index a23e24aeb8..27e57e53dd 100644
--- a/reframe/frontend/executors/policies.py
+++ b/reframe/frontend/executors/policies.py
@@ -10,6 +10,7 @@
 import time
 
 from reframe.core.exceptions import (FailureLimitError,
+                                     SkipTestError,
                                      TaskDependencyError,
                                      TaskExit)
 from reframe.core.logging import getlogger
@@ -100,6 +101,14 @@ def runcase(self, case):
                    for c in case.deps if c in self._task_index):
                 raise TaskDependencyError('dependencies failed')
 
+            if any(self._task_index[c].skipped
+                   for c in case.deps if c in self._task_index):
+                try:
+                    raise SkipTestError('skipped due to skipped dependencies')
+                except SkipTestError as e:
+                    task.skip()
+                    raise TaskExit from e
+
             partname = task.testcase.partition.fullname
             task.setup(task.testcase.partition,
                        task.testcase.environ,
@@ -132,12 +141,10 @@ def runcase(self, case):
 
             self._retired_tasks.append(task)
             task.finalize()
-
         except TaskExit:
             return
         except ABORT_REASONS as e:
             task.abort(e)
-
             raise
         except BaseException:
             task.fail(sys.exc_info())
@@ -151,6 +158,10 @@ def on_task_run(self, task):
     def on_task_exit(self, task):
         pass
 
+    def on_task_skip(self, task):
+        msg = str(task.exc_info[1])
+        self.printer.status('SKIP', msg, just='right')
+
     def on_task_failure(self, task):
         self._num_failed_tasks += 1
         timings = task.pipeline_timings(['compile_complete',
@@ -247,6 +258,8 @@ def _remove_from_running(self, task):
             getlogger().debug2('Task was not running')
             pass
 
+    # FIXME: The following functions are very similar and they are also reused
+    # in the serial policy; we should refactor them
     def deps_failed(self, task):
         # NOTE: Restored dependencies are not in the task_index
         return any(self._task_index[c].failed
@@ -257,6 +270,11 @@ def deps_succeeded(self, task):
         return all(self._task_index[c].succeeded
                    for c in task.testcase.deps if c in self._task_index)
 
+    def deps_skipped(self, task):
+        # NOTE: Restored dependencies are not in the task_index
+        return any(self._task_index[c].skipped
+                   for c in task.testcase.deps if c in self._task_index)
+
     def on_task_setup(self, task):
         partname = task.check.current_partition.fullname
         self._ready_tasks[partname].append(task)
@@ -265,6 +283,17 @@ def on_task_run(self, task):
         partname = task.check.current_partition.fullname
         self._running_tasks[partname].append(task)
 
+    def on_task_skip(self, task):
+        # Remove the task from the running list if it was skipped after the
+        # run phase
+        if task.check.current_partition:
+            partname = task.check.current_partition.fullname
+            if task.failed_stage in ('run_complete', 'run_wait'):
+                self._running_tasks[partname].remove(task)
+
+        msg = str(task.exc_info[1])
+        self.printer.status('SKIP', msg, just='right')
+
     def on_task_failure(self, task):
         if task.aborted:
             return
@@ -308,7 +337,13 @@ def on_task_exit(self, task):
         self._completed_tasks.append(task)
 
     def _setup_task(self, task):
-        if self.deps_succeeded(task):
+        if self.deps_skipped(task):
+            try:
+                raise SkipTestError('skipped due to skipped dependencies')
+            except SkipTestError as e:
+                task.skip()
+                return False
+        elif self.deps_succeeded(task):
             try:
                 task.setup(task.testcase.partition,
                            task.testcase.environ,
@@ -346,7 +381,7 @@ def runcase(self, case):
         try:
             partname = partition.fullname
             if not self._setup_task(task):
-                if not task.failed:
+                if not task.skipped and not task.failed:
                     self.printer.status(
                         'DEP', '%s on %s using %s' %
                         (check.name, partname, environ.name),
@@ -371,7 +406,7 @@ def runcase(self, case):
             else:
                 self.printer.status('HOLD', task.check.info(), just='right')
         except TaskExit:
-            if not task.failed:
+            if not task.failed or not task.skipped:
                 with contextlib.suppress(TaskExit):
                     self._reschedule(task)
 
@@ -380,7 +415,6 @@ def runcase(self, case):
             # If abort was caused due to failure elsewhere, abort current
             # task as well
             task.abort(e)
-
             self._failall(e)
             raise
 
@@ -416,7 +450,8 @@ def split_jobs(tasks):
     def _setup_all(self):
         still_waiting = []
         for task in self._waiting_tasks:
-            if not self._setup_task(task) and not task.failed:
+            if (not self._setup_task(task) and
+                not task.failed and not task.skipped):
                 still_waiting.append(task)
 
         self._waiting_tasks[:] = still_waiting
diff --git a/reframe/frontend/statistics.py b/reframe/frontend/statistics.py
index 77412ecdcf..16db93d2d5 100644
--- a/reframe/frontend/statistics.py
+++ b/reframe/frontend/statistics.py
@@ -35,6 +35,9 @@ def tasks(self, run=-1):
     def failed(self, run=-1):
         return [t for t in self.tasks(run) if t.failed]
 
+    def skipped(self, run=-1):
+        return [t for t in self.tasks(run) if t.skipped]
+
     def aborted(self, run=-1):
         return [t for t in self.tasks(run) if t.aborted]
 
@@ -83,6 +86,7 @@ def json(self, force=False):
             testcases = []
             num_failures = 0
             num_aborted = 0
+            num_skipped = 0
             for t in run:
                 check = t.check
                 partition = check.current_partition
@@ -158,6 +162,9 @@ def json(self, force=False):
                             'traceback': t.exc_info[2]
                         }
                         entry['fail_severe'] = errors.is_severe(*t.exc_info)
+                elif t.skipped:
+                    entry['result'] = 'skipped'
+                    num_skipped += 1
                 else:
                     entry['result'] = 'success'
                     entry['outputdir'] = check.outputdir
@@ -183,6 +190,7 @@ def json(self, force=False):
                 'num_cases': len(run),
                 'num_failures': num_failures,
                 'num_aborted': num_aborted,
+                'num_skipped': num_skipped,
                 'runid': runid,
                 'testcases': testcases
             })
diff --git a/unittests/resources/checks/hellocheck.py b/unittests/resources/checks/hellocheck.py
index a78efd8023..187238c448 100644
--- a/unittests/resources/checks/hellocheck.py
+++ b/unittests/resources/checks/hellocheck.py
@@ -32,3 +32,14 @@ def __init__(self):
         self.valid_prog_environs = ['*']
         self.sourcepath = 'hello.c'
         self.sanity_patterns = sn.assert_not_found(r'(?i)error', self.stdout)
+
+
+@rfm.simple_test
+class SkipTest(rfm.RunOnlyRegressionTest):
+    '''Test to be always skipped'''
+    valid_systems = ['*']
+    valid_prog_environs = ['*']
+    sanity_patterns = sn.assert_true(1)
+
+    def __init__(self):
+        self.skip_if(True, 'unsupported')
diff --git a/unittests/test_cli.py b/unittests/test_cli.py
index 5f789c443e..fff6223531 100644
--- a/unittests/test_cli.py
+++ b/unittests/test_cli.py
@@ -734,7 +734,8 @@ def test_maxfail_option(run_reframe):
     )
     assert 'Traceback' not in stdout
     assert 'Traceback' not in stderr
-    assert 'Ran 2/2 test case(s) from 2 check(s) (0 failure(s))' in stdout
+    assert ('Ran 2/2 test case(s) from 2 check(s) '
+            '(0 failure(s), 0 skipped)') in stdout
     assert returncode == 0
 
 
diff --git a/unittests/test_policies.py b/unittests/test_policies.py
index 5afc19197b..f6b3fe3862 100644
--- a/unittests/test_policies.py
+++ b/unittests/test_policies.py
@@ -119,11 +119,53 @@ def _make_cases(checks=None, sort=False, *args, **kwargs):
     return _make_cases
 
 
+@pytest.fixture(params=['pre_setup', 'post_setup',
+                        'pre_compile', 'post_compile',
+                        'pre_run', 'post_run',
+                        'pre_sanity', 'post_sanity',
+                        'pre_performance', 'post_performance',
+                        'pre_cleanup', 'post_cleanup'])
+def make_cases_for_skipping(request):
+    import reframe as rfm
+    import reframe.utility.sanity as sn
+
+    def _make_cases():
+        @fixtures.custom_prefix('unittests/resources/checks')
+        class _T0(rfm.RegressionTest):
+            valid_systems = ['*']
+            valid_prog_environs = ['*']
+            sourcepath = 'hello.c'
+            executable = 'echo'
+            sanity_patterns = sn.assert_true(1)
+
+            def check_and_skip(self):
+                self.skip_if(True)
+
+            # Attach the hook manually based on the request.param
+            when, stage = request.param.split('_', maxsplit=1)
+            hook = rfm.run_before if when == 'pre' else rfm.run_after
+            check_and_skip = hook(stage)(check_and_skip)
+
+        class _T1(rfm.RunOnlyRegressionTest):
+            valid_systems = ['*']
+            valid_prog_environs = ['*']
+            sanity_patterns = sn.assert_true(1)
+
+            def __init__(self):
+                self.depends_on(_T0.__qualname__)
+
+        cases = executors.generate_testcases([_T0(), _T1()])
+        depgraph, _ = dependencies.build_deps(cases)
+        return dependencies.toposort(depgraph), request.param
+
+    return _make_cases
+
+
 def assert_runall(runner):
     # Make sure that all cases finished, failed or
     # were aborted
     for t in runner.stats.tasks():
-        assert t.succeeded or t.failed or t.aborted
+        assert t.succeeded or t.failed or t.aborted or t.skipped
 
 
 def assert_all_dead(runner):
@@ -299,6 +341,40 @@ def test_strict_performance_check(make_runner, make_cases, common_exec_ctx):
     assert 1 == num_failures_stage(runner, 'cleanup')
 
 
+def test_runall_skip_tests(make_runner, make_cases,
+                           make_cases_for_skipping,
+                           common_exec_ctx):
+    runner = make_runner(max_retries=1)
+    more_cases, stage = make_cases_for_skipping()
+    cases = make_cases() + more_cases
+    runner.runall(cases)
+
+    def assert_reported_skipped(num_skipped):
+        report = runner.stats.json()
+        assert report[0]['num_skipped'] == num_skipped
+
+        num_reported = 0
+        for tc in report[0]['testcases']:
+            if tc['result'] == 'skipped':
+                num_reported += 1
+
+        assert num_reported == num_skipped
+
+    assert_runall(runner)
+    assert 11 == runner.stats.num_cases(0)
+    assert 5 == runner.stats.num_cases(1)
+    assert 5 == len(runner.stats.failed(0))
+    assert 5 == len(runner.stats.failed(1))
+    if stage.endswith('cleanup'):
+        assert 0 == len(runner.stats.skipped(0))
+        assert 0 == len(runner.stats.skipped(1))
+        assert_reported_skipped(0)
+    else:
+        assert 2 == len(runner.stats.skipped(0))
+        assert 0 == len(runner.stats.skipped(1))
+        assert_reported_skipped(2)
+
+
 # We explicitly ask for a system with a non-local scheduler here, to make sure
 # that the execution policies behave correctly with forced local tests
 def test_force_local_execution(make_runner, make_cases, testsys_exec_ctx):
@@ -468,6 +544,9 @@ def on_task_success(self, task):
     def on_task_failure(self, task):
         pass
 
+    def on_task_skip(self, task):
+        pass
+
     def on_task_setup(self, task):
         pass
 

From 50e84ce0013472013f285a5547f11a8e72cb478e Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Mon, 22 Mar 2021 18:51:09 +0100
Subject: [PATCH 2/3] Address PR comments

---
 reframe/frontend/executors/policies.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/reframe/frontend/executors/policies.py b/reframe/frontend/executors/policies.py
index 27e57e53dd..902ed1a1c2 100644
--- a/reframe/frontend/executors/policies.py
+++ b/reframe/frontend/executors/policies.py
@@ -103,6 +103,9 @@ def runcase(self, case):
 
             if any(self._task_index[c].skipped
                    for c in case.deps if c in self._task_index):
+
+                # We raise the SkipTestError here and catch it immediately in
+                # order for `skip()` to get the correct exception context.
                 try:
                     raise SkipTestError('skipped due to skipped dependencies')
                 except SkipTestError as e:

From 4b0da8754f994b08a2c2efe611ff126aa69998b8 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Mon, 22 Mar 2021 21:36:01 +0100
Subject: [PATCH 3/3] Address PR comments

---
 reframe/frontend/executors/__init__.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/reframe/frontend/executors/__init__.py b/reframe/frontend/executors/__init__.py
index 6f2fd3283d..350a33bdd0 100644
--- a/reframe/frontend/executors/__init__.py
+++ b/reframe/frontend/executors/__init__.py
@@ -422,7 +422,6 @@ def stats(self):
         return self._stats
 
     def runall(self, testcases, restored_cases=None):
-        abort_reason = None
         num_checks = len({tc.check.name for tc in testcases})
         self._printer.separator('short double line',
                                 'Running %d check(s)' % num_checks)
@@ -444,7 +443,7 @@ def runall(self, testcases, restored_cases=None):
             else:
                 status = 'PASSED'
 
-            total_run = len(testcases) - num_tasks + num_completed
+            total_run = len(testcases)
             total_completed = len(self._stats.completed(0))
             total_skipped = len(self._stats.skipped(0))
             self._printer.status(