Proposal to fix #154 (v2)

- The one way to generate ExecUtilException - RaiseError.UtilityExitedWithNonZeroCode - RaiseError is added - ExecUtilException::error is added (it contains the error data) - ExecUtilException::__str__ is updated - PostgresNode::psql and PostgresNode::safe_psql are updated - TestLocalOperations::test_exec_command_failure is updated - TestRemoteOperations::test_exec_command_failure is updated - TestRemoteOperations::test_makedirs_and_rmdirs_failure is updated
postgrespro · dmitry-lipetsk · Dec 10, 2024 · Dec 8, 2024 · Dec 9, 2024 · Dec 9, 2024
commit f9ddd043aceb3ac86f6b63cd7fba00575e0d44b1
diff --git a/testgres/exceptions.py b/testgres/exceptions.py
@@ -9,13 +9,14 @@ class TestgresException(Exception):
 
 @six.python_2_unicode_compatible
 class ExecUtilException(TestgresException):
-    def __init__(self, message=None, command=None, exit_code=0, out=None):
+    def __init__(self, message=None, command=None, exit_code=0, out=None, error=None):
         super(ExecUtilException, self).__init__(message)
 
         self.message = message
         self.command = command
         self.exit_code = exit_code
         self.out = out
+        self.error = error
 
     def __str__(self):
         msg = []
@@ -24,13 +25,17 @@ def __str__(self):
             msg.append(self.message)
 
         if self.command:
-            msg.append(u'Command: {}'.format(self.command))
+            command_s = ' '.join(self.command) if isinstance(self.command, list) else self.command,
+            msg.append(u'Command: {}'.format(command_s))
 
         if self.exit_code:
             msg.append(u'Exit code: {}'.format(self.exit_code))
 
+        if self.error:
+            msg.append(u'---- Error:\n{}'.format(self.error))
+
         if self.out:
-            msg.append(u'----\n{}'.format(self.out))
+            msg.append(u'---- Out:\n{}'.format(self.out))
 
         return self.convert_and_join(msg)
 

diff --git a/testgres/helpers/raise_error.py b/testgres/helpers/raise_error.py
@@ -0,0 +1,45 @@
+from ..exceptions import ExecUtilException
+
+
+class RaiseError:
+    def UtilityExitedWithNonZeroCode(cmd, exit_code, msg_arg, error, out):
+        assert type(exit_code) == int  # noqa: E721
+
+        msg_arg_s = __class__._TranslateDataIntoString(msg_arg).strip()
+        assert type(msg_arg_s) == str  # noqa: E721
+
+        if msg_arg_s == "":
+            msg_arg_s = "#no_error_message"
+
+        message = "Utility exited with non-zero code. Error: `" + msg_arg_s + "`"
+        raise ExecUtilException(
+            message=message,
+            command=cmd,
+            exit_code=exit_code,
+            out=out,
+            error=error)
+
+    def _TranslateDataIntoString(data):
+        if type(data) == bytes:  # noqa: E721
+            return __class__._TranslateDataIntoString__FromBinary(data)
+
+        return str(data)
+
+    def _TranslateDataIntoString__FromBinary(data):
+        assert type(data) == bytes  # noqa: E721
+
+        try:
+            return data.decode('utf-8')
+        except UnicodeDecodeError:
+            pass
+
+        return "#cannot_decode_text"
+
+    def _BinaryIsASCII(data):
+        assert type(data) == bytes  # noqa: E721
+
+        for b in data:
+            if not (b >= 0 and b <= 127):
+                return False
+
+        return True
diff --git a/testgres/node.py b/testgres/node.py
@@ -3,7 +3,6 @@
 import os
 import random
 import signal
-import subprocess
 import threading
 import tempfile
 from queue import Queue
@@ -987,6 +986,25 @@ def psql(self,
             >>> psql(query='select 3', ON_ERROR_STOP=1)
         """
 
+        return self._psql(
+            ignore_errors=True,
+            query=query,
+            filename=filename,
+            dbname=dbname,
+            username=username,
+            input=input,
+            **variables
+        )
+
+    def _psql(
+            self,
+            ignore_errors,
+            query=None,
+            filename=None,
+            dbname=None,
+            username=None,
+            input=None,
+            **variables):
         dbname = dbname or default_dbname()
 
         psql_params = [
@@ -1017,20 +1035,8 @@ def psql(self,
 
         # should be the last one
         psql_params.append(dbname)
-        if not self.os_ops.remote:
-            # start psql process
-            process = subprocess.Popen(psql_params,
-                                       stdin=subprocess.PIPE,
-                                       stdout=subprocess.PIPE,
-                                       stderr=subprocess.PIPE)
-
-            # wait until it finishes and get stdout and stderr
-            out, err = process.communicate(input=input)
-            return process.returncode, out, err
-        else:
-            status_code, out, err = self.os_ops.exec_command(psql_params, verbose=True, input=input)
 
-            return status_code, out, err
+        return self.os_ops.exec_command(psql_params, verbose=True, input=input, ignore_errors=ignore_errors)
 
     @method_decorator(positional_args_hack(['dbname', 'query']))
     def safe_psql(self, query=None, expect_error=False, **kwargs):
@@ -1051,21 +1057,17 @@ def safe_psql(self, query=None, expect_error=False, **kwargs):
         Returns:
             psql's output as str.
         """
+        assert type(kwargs) == dict  # noqa: E721
+        assert not ("ignore_errors" in kwargs.keys())
 
         # force this setting
         kwargs['ON_ERROR_STOP'] = 1
         try:
-            ret, out, err = self.psql(query=query, **kwargs)
+            ret, out, err = self._psql(ignore_errors=False, query=query, **kwargs)
         except ExecUtilException as e:
-            ret = e.exit_code
-            out = e.out
-            err = e.message
-        if ret:
-            if expect_error:
-                out = (err or b'').decode('utf-8')
-            else:
-                raise QueryException((err or b'').decode('utf-8'), query)
-        elif expect_error:
+            raise QueryException(e.message, query)
+
+        if expect_error:
             assert False, "Exception was expected, but query finished successfully: `{}` ".format(query)
 
         return out

diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py
@@ -11,6 +11,7 @@
 
 from ..exceptions import ExecUtilException
 from .os_ops import ConnectionParams, OsOperations, pglib, get_default_encoding
+from ..helpers.raise_error import RaiseError
 
 try:
     from shutil import which as find_executable
@@ -47,14 +48,6 @@ def __init__(self, conn_params=None):
         self.remote = False
         self.username = conn_params.username or getpass.getuser()
 
-    @staticmethod
-    def _raise_exec_exception(message, command, exit_code, output):
-        """Raise an ExecUtilException."""
-        raise ExecUtilException(message=message.format(output),
-                                command=' '.join(command) if isinstance(command, list) else command,
-                                exit_code=exit_code,
-                                out=output)
-
     @staticmethod
     def _process_output(encoding, temp_file_path):
         """Process the output of a command from a temporary file."""
@@ -120,11 +113,20 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False,
         """
         Execute a command in a subprocess and handle the output based on the provided parameters.
         """
+        assert type(expect_error) == bool  # noqa: E721
+        assert type(ignore_errors) == bool  # noqa: E721
+
         process, output, error = self._run_command(cmd, shell, input, stdin, stdout, stderr, get_process, timeout, encoding)
         if get_process:
             return process
         if not ignore_errors and ((process.returncode != 0 or has_errors(output=output, error=error)) and not expect_error):
-            self._raise_exec_exception('Utility exited with non-zero code. Error `{}`', cmd, process.returncode, error or output)
+            RaiseError.UtilityExitedWithNonZeroCode(
+                cmd=cmd,
+                exit_code=process.returncode,
+           
F438
     msg_arg=error or output,
+                error=error,
+                out=output
+            )
 
         if verbose:
             return process.returncode, output, error

diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py
@@ -14,6 +14,7 @@
         raise ImportError("You must have psycopg2 or pg8000 modules installed")
 
 from ..exceptions import ExecUtilException
+from ..helpers.raise_error import RaiseError
 from .os_ops import OsOperations, ConnectionParams, get_default_encoding
 
 error_markers = [b'error', b'Permission denied', b'fatal', b'No such file or directory']
@@ -66,6 +67,9 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False,
         Args:
         - cmd (str): The command to be executed.
         """
+        assert type(expect_error) == bool  # noqa: E721
+        assert type(ignore_errors) == bool  # noqa: E721
+
         ssh_cmd = []
         if isinstance(cmd, str):
             ssh_cmd = ['ssh', self.ssh_dest] + self.ssh_args + [cmd]
@@ -100,10 +104,12 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False,
             error = error.decode(encoding)
 
         if not ignore_errors and error_found and not expect_error:
-            error = normalize_error(error)
-            assert type(error) == str  # noqa: E721
-            message = "Utility exited with non-zero code. Error: " + error
-            raise ExecUtilException(message=message, command=cmd, exit_code=exit_status, out=result)
+            RaiseError.UtilityExitedWithNonZeroCode(
+                cmd=cmd,
+                exit_code=exit_status,
+                msg_arg=error,
+                error=error,
+                out=result)
 
         if verbose:
             return exit_status, result, error

diff --git a/tests/test_local.py b/tests/test_local.py
@@ -37,7 +37,7 @@ def test_exec_command_failure(self):
                 error = e.message
                 break
             raise Exception("We wait an exception!")
-        assert error == "Utility exited with non-zero code. Error `b'/bin/sh: 1: nonexistent_command: not found\\n'`"
+        assert error == "Utility exited with non-zero code. Error: `/bin/sh: 1: nonexistent_command: not found`"
 
     def test_exec_command_failure__expect_error(self):
         """

diff --git a/tests/test_remote.py b/tests/test_remote.py
@@ -37,7 +37,7 @@ def test_exec_command_failure(self):
                 error = e.message
                 break
             raise Exception("We wait an exception!")
-        assert error == b'Utility exited with non-zero code. Error: bash: line 1: nonexistent_command: command not found\n'
+        assert error == 'Utility exited with non-zero code. Error: `bash: line 1: nonexistent_command: command not found`'
 
     def test_exec_command_failure__expect_error(self):
         """
@@ -98,11 +98,14 @@ def test_makedirs_and_rmdirs_failure(self):
             self.operations.makedirs(path)
 
         # Test rmdirs
-        try:
-            exit_status, result, error = self.operations.rmdirs(path, verbose=True)
-        except ExecUtilException as e:
-            error = e.message
-        assert error == b"Utility exited with non-zero code. Error: rm: cannot remove '/root/test_dir': Permission denied\n"
+        while True:
+            try:
+                self.operations.rmdirs(path, verbose=True)
+            except ExecUtilException as e:
+                error = e.message
+                break
+            raise Exception("We wait an exception!")
+        assert error == "Utility exited with non-zero code. Error: `rm: cannot remove '/root/test_dir': Permission denied`"
 
     def test_listdir(self):
         """