Merge pull request #266 from vkarak/bugfix/serial-policy-crash

Fix serial execution policy crash on keyboard interrupt

Author: vkarak

reframe/frontend/executors/__init__.py

@@ -110,7 +110,7 @@ def abort(self, cause=None):
         try:
             # FIXME: we should perhaps extend the RegressionTest interface
             # for supporting job cancelling
-            if not self.zombie:
+            if not self.zombie and self._check.job:
                 self._check.job.cancel()
         except JobNotStartedError:
             self.fail((type(exc), exc, None))
@@ -212,10 +212,10 @@ def _retry_failed(self, checks, system):
             self._current_run += 1
             self._stats.next_run()
             if self._stats.current_run != self._current_run:
-                    raise AssertionError('current_run variable out of sync'
-                                         '(Runner: %d; TestStats: %d)' %
-                                         self._current_run,
-                                         self._stats.current_run)
+                raise AssertionError('current_run variable out of sync'
+                                     '(Runner: %d; TestStats: %d)' %
+                                     self._current_run,
+                                     self._stats.current_run)
 
             self._printer.separator(
                 'short double line',

unittests/resources/frontend_checks.py

@@ -31,17 +31,6 @@ def setup(self, system, environ, **job_opts):
 
 
 class BadSetupCheckEarly(BaseFrontendCheck):
-    def __init__(self, **kwargs):
-        super().__init__(type(self).__name__, **kwargs)
-
-        self.valid_systems = ['*']
-        self.valid_prog_environs = ['*']
-
-    def setup(self, system, environ, **job_opts):
-        raise ReframeError('Setup failure')
-
-
-class BadSetupCheckEarlyNonLocal(BaseFrontendCheck):
     def __init__(self, **kwargs):
         super().__init__(type(self).__name__, **kwargs)
         self.valid_systems = ['*']
@@ -111,10 +100,11 @@ def __init__(self, phase='wait', **kwargs):
         self.phase = phase
 
     def setup(self, system, environ, **job_opts):
-        super().setup(system, environ, **job_opts)
         if self.phase == 'setup':
             raise KeyboardInterrupt
 
+        super().setup(system, environ, **job_opts)
+
     def wait(self):
         # We do our nasty stuff in wait() to make things more complicated
         if self.phase == 'wait':
@@ -176,7 +166,7 @@ def __init__(self, run_to_pass, filename, **kwargs):
 def _get_checks(**kwargs):
     return [BadSetupCheck(**kwargs),
             BadSetupCheckEarly(**kwargs),
-            BadSetupCheckEarlyNonLocal(**kwargs),
+            KeyboardInterruptCheck(phase='setup', **kwargs),
             NoSystemCheck(**kwargs),
             NoPrgEnvCheck(**kwargs),
             SanityFailureCheck(**kwargs),

unittests/test_cli.py

@@ -168,10 +168,22 @@ def test_check_failure(self):
 
     def test_check_setup_failure(self):
         self.checkpath = ['unittests/resources/frontend_checks.py']
-        self.more_options = ['-t', 'BadSetupCheckEarlyNonLocal']
+        self.more_options = ['-t', 'BadSetupCheckEarly']
         self.local = False
 
         returncode, stdout, stderr = self._run_reframe()
+        self.assertNotIn('Traceback', stdout)
+        self.assertNotIn('Traceback', stderr)
+        self.assertIn('FAILED', stdout)
+        self.assertNotEqual(returncode, 0)
+
+    def test_check_kbd_interrupt(self):
+        self.checkpath = ['unittests/resources/frontend_checks.py']
+        self.more_options = ['-t', 'KeyboardInterruptCheck']
+        self.local = False
+
+        returncode, stdout, stderr = self._run_reframe()
+        self.assertNotIn('Traceback', stdout)
         self.assertNotIn('Traceback', stderr)
         self.assertIn('FAILED', stdout)
         self.assertNotEqual(returncode, 0)
@@ -184,6 +196,7 @@ def test_check_sanity_failure(self):
         self.assertIn('FAILED', stdout)
 
         # This is a normal failure, it should not raise any exception
+        self.assertNotIn('Traceback', stdout)
         self.assertNotIn('Traceback', stderr)
         self.assertNotEqual(returncode, 0)
         self.assertTrue(self._stage_exists('SanityFailureCheck',
@@ -197,6 +210,7 @@ def test_performance_check_failure(self):
         self.assertIn('FAILED', stdout)
 
         # This is a normal failure, it should not raise any exception
+        self.assertNotIn('Traceback', stdout)
         self.assertNotIn('Traceback', stderr)
         self.assertNotEqual(0, returncode)
         self.assertTrue(self._stage_exists('PerformanceFailureCheck',
@@ -284,6 +298,7 @@ def test_execution_modes(self):
         self.mode = 'unittest'
 
         returncode, stdout, stderr = self._run_reframe()
+        self.assertNotIn('Traceback', stdout)
         self.assertNotIn('Traceback', stderr)
         self.assertNotIn('FAILED', stdout)
         self.assertIn('PASSED', stdout)
@@ -302,4 +317,3 @@ def test_no_ignore_check_conflicts(self):
         self.action = 'list'
         returncode, *_ = self._run_reframe()
         self.assertNotEqual(0, returncode)
-