Merge pull request #1243 from vkarak/bugfix/spawned_process_error_formatting

[bugfix] Fix formatting of the error message when an OS command fails

Author: Vasileios Karakasis

reframe/core/exceptions.py

@@ -144,12 +144,21 @@ def __init__(self, stdout, stderr):
 class SpawnedProcessError(ReframeError):
     '''Raised when a spawned OS command has failed.'''
 
-    def __init__(self, command, stdout, stderr, exitcode):
+    def __init__(self, args, stdout, stderr, exitcode):
         super().__init__()
 
-        # Format message and put it in args
+        if isinstance(args, str):
+            self._command = args
+        else:
+            self._command = ' '.join(args)
+
+        self._stdout = stdout
+        self._stderr = stderr
+        self._exitcode = exitcode
+
+        # Format message
         lines = [
-            "command '%s' failed with exit code %s:" % (command, exitcode)
+            f"command '{self.command}' failed with exit code {self.exitcode}:"
         ]
         lines.append('=== STDOUT ===')
         if stdout:
@@ -160,10 +169,6 @@ def __init__(self, command, stdout, stderr, exitcode):
             lines.append(stderr)
 
         self._message = '\n'.join(lines)
-        self._command = command
-        self._stdout = stdout
-        self._stderr = stderr
-        self._exitcode = exitcode
 
     @property
     def command(self):
@@ -185,10 +190,12 @@ def exitcode(self):
 class SpawnedProcessTimeout(SpawnedProcessError):
     '''Raised when a spawned OS command has timed out.'''
 
-    def __init__(self, command, stdout, stderr, timeout):
-        super().__init__(command, stdout, stderr, None)
+    def __init__(self, args, stdout, stderr, timeout):
+        super().__init__(args, stdout, stderr, None)
+        self._timeout = timeout
 
-        lines = ["command '%s' timed out after %ss:" % (command, timeout)]
+        # Format message
+        lines = [f"command '{self.command}' timed out after {self.timeout}s:"]
         lines.append('=== STDOUT ===')
         if self._stdout:
             lines.append(self._stdout)
@@ -198,7 +205,6 @@ def __init__(self, command, stdout, stderr, timeout):
             lines.append(self._stderr)
 
         self._message = '\n'.join(lines)
-        self._timeout = timeout
 
     @property
     def timeout(self):

reframe/utility/os_ext.py

@@ -51,25 +51,6 @@ def run_command(cmd, check=False, timeout=None, shell=False, log=True):
     return completed
 
 
-def grep_command_output(cmd, pattern, where='stdout'):
-    completed = subprocess.run(shlex.split(cmd),
-                               stdout=subprocess.PIPE,
-                               stderr=subprocess.PIPE,
-                               universal_newlines=True)
-    if where == 'stdout':
-        outlist = [completed.stdout]
-    elif where == 'stderr':
-        outlist = [completed.stderr]
-    else:
-        outlist = [completed.stdout, completed.stderr]
-
-    for out in outlist:
-        if re.search(pattern, out, re.MULTILINE):
-            return True
-
-    return False
-
-
 def run_command_async(cmd,
                       stdout=subprocess.PIPE,
                       stderr=subprocess.PIPE,

unittests/test_exceptions.py

@@ -57,12 +57,28 @@ def test_spawned_process_error(self):
         exc_args = ('foo bar', 'partial output', 'error message', 1)
         e = exc.SpawnedProcessError(*exc_args)
         with pytest.raises(
-            exc.ReframeError,
-            match=r"command 'foo bar' failed with exit code 1:\n"
-                  r"=== STDOUT ===\n"
-                  r'partial output\n'
-                  r"=== STDERR ===\n"
-                  r"error message"):
+                exc.ReframeError,
+                match=(r"command 'foo bar' failed with exit code 1:\n"
+                       r"=== STDOUT ===\n"
+                       r'partial output\n'
+                       r"=== STDERR ===\n"
+                       r"error message")
+        ):
+            raise_exc(e)
+
+        assert exc_args == e.args
+
+    def test_spawned_process_error_list_args(self):
+        exc_args = (['foo', 'bar'], 'partial output', 'error message', 1)
+        e = exc.SpawnedProcessError(*exc_args)
+        with pytest.raises(
+                exc.ReframeError,
+                match=(r"command 'foo bar' failed with exit code 1:\n"
+                       r"=== STDOUT ===\n"
+                       r'partial output\n'
+                       r"=== STDERR ===\n"
+                       r"error message")
+        ):
             raise_exc(e)
 
         assert exc_args == e.args
@@ -71,33 +87,35 @@ def test_spawned_process_error_nostdout(self):
         exc_args = ('foo bar', '', 'error message', 1)
         e = exc.SpawnedProcessError(*exc_args)
         with pytest.raises(
-            exc.ReframeError,
-            match=r"command 'foo bar' failed with exit code 1:\n"
-                  r"=== STDOUT ===\n"
-                  r"=== STDERR ===\n"
-                  r"error message"):
+                exc.ReframeError,
+                match=(r"command 'foo bar' failed with exit code 1:\n"
+                       r"=== STDOUT ===\n"
+                       r"=== STDERR ===\n"
+                       r"error message")
+        ):
             raise_exc(e)
 
     def test_spawned_process_error_nostderr(self):
         exc_args = ('foo bar', 'partial output', '', 1)
         e = exc.SpawnedProcessError(*exc_args)
         with pytest.raises(
-            exc.ReframeError,
-            match=r"command 'foo bar' failed with exit code 1:\n"
-                  r"=== STDOUT ===\n"
-                  r'partial output\n'
-                  r"=== STDERR ==="):
+                exc.ReframeError,
+                match=(r"command 'foo bar' failed with exit code 1:\n"
+                       r"=== STDOUT ===\n"
+                       r'partial output\n'
+                       r"=== STDERR ===")
+        ):
             raise_exc(e)
 
     def test_spawned_process_timeout(self):
         exc_args = ('foo bar', 'partial output', 'partial error', 10)
         e = exc.SpawnedProcessTimeout(*exc_args)
         with pytest.raises(exc.ReframeError,
-                           match=r"command 'foo bar' timed out after 10s:\n"
-                                 r"=== STDOUT ===\n"
-                                 r'partial output\n'
-                                 r"=== STDERR ===\n"
-                                 r"partial error"):
+                           match=(r"command 'foo bar' timed out after 10s:\n"
+                                  r"=== STDOUT ===\n"
+                                  r'partial output\n'
+                                  r"=== STDERR ===\n"
+                                  r"partial error")):
             raise_exc(e)
 
         assert exc_args == e.args
@@ -106,20 +124,20 @@ def test_spawned_process_timeout_nostdout(self):
         exc_args = ('foo bar', '', 'partial error', 10)
         e = exc.SpawnedProcessTimeout(*exc_args)
         with pytest.raises(exc.ReframeError,
-                           match=r"command 'foo bar' timed out after 10s:\n"
-                                 r"=== STDOUT ===\n"
-                                 r"=== STDERR ===\n"
-                                 r"partial error"):
+                           match=(r"command 'foo bar' timed out after 10s:\n"
+                                  r"=== STDOUT ===\n"
+                                  r"=== STDERR ===\n"
+                                  r"partial error")):
             raise_exc(e)
 
     def test_spawned_process_timeout_nostderr(self):
         exc_args = ('foo bar', 'partial output', '', 10)
         e = exc.SpawnedProcessTimeout(*exc_args)
         with pytest.raises(exc.ReframeError,
-                           match=r"command 'foo bar' timed out after 10s:\n"
-                                 r"=== STDOUT ===\n"
-                                 r'partial output\n'
-                                 r"=== STDERR ==="):
+                           match=(r"command 'foo bar' timed out after 10s:\n"
+                                  r"=== STDOUT ===\n"
+                                  r'partial output\n'
+                                  r"=== STDERR ===")):
             raise_exc(e)
 
     def test_job_error(self):

unittests/test_utility.py

@@ -27,18 +27,21 @@ def test_command_success(self):
         assert completed.stdout == 'foobar\n'
 
     def test_command_error(self):
-        with pytest.raises(SpawnedProcessError):
+        with pytest.raises(SpawnedProcessError,
+                           match=r"command 'false' failed with exit code 1"):
             os_ext.run_command('false', check=True)
 
     def test_command_timeout(self):
-        try:
+        with pytest.raises(
+                SpawnedProcessTimeout,
+                match=r"command 'sleep 3' timed out after 2s"
+        ) as exc_info:
             os_ext.run_command('sleep 3', timeout=2)
-        except SpawnedProcessTimeout as e:
-            assert e.timeout == 2
-            # Try to get the string repr. of the exception: see bug #658
-            s = str(e)
-        else:
-            pytest.fail('expected timeout')
+
+        assert exc_info.value.timeout == 2
+
+        # Try to get the string repr. of the exception: see bug #658
+        s = str(exc_info.value)
 
     def test_command_async(self):
         from datetime import datetime
@@ -55,10 +58,6 @@ def test_command_async(self):
         assert t_launch.seconds < 1
         assert t_sleep.seconds >= 1
 
-    def test_grep(self):
-        assert os_ext.grep_command_output(cmd='echo hello', pattern='hello')
-        assert not os_ext.grep_command_output(cmd='echo hello', pattern='foo')
-
     def test_copytree(self):
         dir_src = tempfile.mkdtemp()
         dir_dst = tempfile.mkdtemp()