From f9ddd043aceb3ac86f6b63cd7fba00575e0d44b1 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sun, 8 Dec 2024 22:06:29 +0300 Subject: [PATCH 01/13] 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 --- testgres/exceptions.py | 11 +++++-- testgres/helpers/raise_error.py | 45 ++++++++++++++++++++++++++++ testgres/node.py | 50 ++++++++++++++++--------------- testgres/operations/local_ops.py | 20 +++++++------ testgres/operations/remote_ops.py | 14 ++++++--- tests/test_local.py | 2 +- tests/test_remote.py | 15 ++++++---- 7 files changed, 110 insertions(+), 47 deletions(-) create mode 100644 testgres/helpers/raise_error.py diff --git a/testgres/exceptions.py b/testgres/exceptions.py index ee329031..ff4381f4 100644 --- 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 new file mode 100644 index 00000000..c67833dd --- /dev/null +++ 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 index 48a100a9..8300d493 100644 --- 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 index 5b7972ae..14c408c9 100644 --- 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, + 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 index 88394eb7..4340ec11 100644 --- 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 index da26468b..cb96a3bc 100644 --- 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 index 565163f7..c1a91bc6 100755 --- 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): """ From 2bb38dc45b69c4d5d4c93a65fea4b64b81511287 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 9 Dec 2024 11:30:51 +0300 Subject: [PATCH 02/13] [BUG FIX] PostgresNode::safe_psql did not respect "expect_error" parameter --- testgres/node.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/testgres/node.py b/testgres/node.py index 8300d493..11f73af2 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1059,13 +1059,22 @@ def safe_psql(self, query=None, expect_error=False, **kwargs): """ assert type(kwargs) == dict # noqa: E721 assert not ("ignore_errors" in kwargs.keys()) + assert not ("expect_error" in kwargs.keys()) # force this setting kwargs['ON_ERROR_STOP'] = 1 try: ret, out, err = self._psql(ignore_errors=False, query=query, **kwargs) except ExecUtilException as e: - raise QueryException(e.message, query) + if not expect_error: + raise QueryException(e.message, query) + + if type(e.error) == bytes: # noqa: E721 + return e.error.decode("utf-8") # throw + + # [2024-12-09] This situation is not expected + assert False + return e.error if expect_error: assert False, "Exception was expected, but query finished successfully: `{}` ".format(query) From 45b8dc024dc39994bed029ebb1d363686ae71cbf Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 9 Dec 2024 15:07:21 +0300 Subject: [PATCH 03/13] [BUG FIX] A problem in psql/safe_psql and 'input' data was fixed [local_op] Both LocalOperations::exec_command and RemoteOperations::exec_command were updated. --- testgres/node.py | 12 ++++++++++++ testgres/operations/helpers.py | 15 +++++++++++++++ testgres/operations/local_ops.py | 9 ++++++++- testgres/operations/remote_ops.py | 7 ++++++- 4 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 testgres/operations/helpers.py diff --git a/testgres/node.py b/testgres/node.py index 11f73af2..68e9b0eb 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1005,6 +1005,18 @@ def _psql( username=None, input=None, **variables): + assert type(variables) == dict # noqa: E721 + + # + # We do not support encoding. It may be added later. Ok? + # + if input is None: + pass + elif type(input) == bytes: # noqa: E721 + pass + else: + raise Exception("Input data must be None or bytes.") + dbname = dbname or default_dbname() psql_params = [ diff --git a/testgres/operations/helpers.py b/testgres/operations/helpers.py new file mode 100644 index 00000000..d714d336 --- /dev/null +++ b/testgres/operations/helpers.py @@ -0,0 +1,15 @@ +class Helpers: + def PrepareProcessInput(input, encoding): + if not input: + return None + + if type(input) == str: # noqa: E721 + if encoding is None: + return input.encode() + + assert type(encoding) == str # noqa: E721 + return input.encode(encoding) + + # It is expected! + assert type(input) == bytes # noqa: E721 + return input diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 14c408c9..833d1fd2 100644 --- 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 import Helpers from ..helpers.raise_error import RaiseError try: @@ -58,6 +59,8 @@ def _process_output(encoding, temp_file_path): return output, None # In Windows stderr writing in stdout def _run_command__nt(self, cmd, shell, input, stdin, stdout, stderr, get_process, timeout, encoding): + # TODO: why don't we use the data from input? + with tempfile.NamedTemporaryFile(mode='w+b', delete=False) as temp_file: stdout = temp_file stderr = subprocess.STDOUT @@ -79,6 +82,10 @@ def _run_command__nt(self, cmd, shell, input, stdin, stdout, stderr, get_process return process, output, error def _run_command__generic(self, cmd, shell, input, stdin, stdout, stderr, get_process, timeout, encoding): + input_prepared = None + if not get_process: + input_prepared = Helpers.PrepareProcessInput(input, encoding) # throw + process = subprocess.Popen( cmd, shell=shell, @@ -89,7 +96,7 @@ def _run_command__generic(self, cmd, shell, input, stdin, stdout, stderr, get_pr if get_process: return process, None, None try: - output, error = process.communicate(input=input.encode(encoding) if input else None, timeout=timeout) + output, error = process.communicate(input=input_prepared, timeout=timeout) if encoding: output = output.decode(encoding) error = error.decode(encoding) diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 4340ec11..cbaeb62b 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -16,6 +16,7 @@ from ..exceptions import ExecUtilException from ..helpers.raise_error import RaiseError from .os_ops import OsOperations, ConnectionParams, get_default_encoding +from .helpers import Helpers error_markers = [b'error', b'Permission denied', b'fatal', b'No such file or directory'] @@ -70,6 +71,10 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, assert type(expect_error) == bool # noqa: E721 assert type(ignore_errors) == bool # noqa: E721 + input_prepared = None + if not get_process: + input_prepared = Helpers.PrepareProcessInput(input, encoding) # throw + ssh_cmd = [] if isinstance(cmd, str): ssh_cmd = ['ssh', self.ssh_dest] + self.ssh_args + [cmd] @@ -80,7 +85,7 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, return process try: - result, error = process.communicate(input, timeout=timeout) + result, error = process.communicate(input=input_prepared, timeout=timeout) except subprocess.TimeoutExpired: process.kill() raise ExecUtilException("Command timed out after {} seconds.".format(timeout)) From f848a63fd3dce4a50d1a994f2b5ee1ce9e15a7b6 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 9 Dec 2024 17:50:50 +0300 Subject: [PATCH 04/13] PostgresNode::safe_psql raises InvalidOperationException If expect_error is True and no error is detected, safe_psql raises InvalidOperationException exception. Tests (local, remote) are added. --- testgres/__init__.py | 3 ++- testgres/exceptions.py | 3 +++ testgres/node.py | 5 +++-- tests/test_simple.py | 21 ++++++++++++++++++++- tests/test_simple_remote.py | 20 +++++++++++++++++++- 5 files changed, 47 insertions(+), 5 deletions(-) diff --git a/testgres/__init__.py b/testgres/__init__.py index 8d0e38c6..69d2ab4a 100644 --- a/testgres/__init__.py +++ b/testgres/__init__.py @@ -23,7 +23,8 @@ CatchUpException, \ StartNodeException, \ InitNodeException, \ - BackupException + BackupException, \ + InvalidOperationException from .enums import \ XLogMethod, \ diff --git a/testgres/exceptions.py b/testgres/exceptions.py index ff4381f4..b4d9f76b 100644 --- a/testgres/exceptions.py +++ b/testgres/exceptions.py @@ -103,3 +103,6 @@ class InitNodeException(TestgresException): class BackupException(TestgresException): pass + +class InvalidOperationException(TestgresException): + pass diff --git a/testgres/node.py b/testgres/node.py index 68e9b0eb..ae52f21b 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -73,7 +73,8 @@ TimeoutException, \ InitNodeException, \ TestgresException, \ - BackupException + BackupException, \ + InvalidOperationException from .logger import TestgresLogger @@ -1089,7 +1090,7 @@ def safe_psql(self, query=None, expect_error=False, **kwargs): return e.error if expect_error: - assert False, "Exception was expected, but query finished successfully: `{}` ".format(query) + raise InvalidOperationException("Exception was expected, but query finished successfully: `{}`.".format(query)) return out diff --git a/tests/test_simple.py b/tests/test_simple.py index 51cdc896..c4200c6f 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -23,7 +23,9 @@ BackupException, \ QueryException, \ TimeoutException, \ - TestgresException, NodeApp + TestgresException, \ + InvalidOperationException, \ + NodeApp from testgres import \ TestgresConfig, \ @@ -310,6 +312,23 @@ def test_psql(self): with self.assertRaises(QueryException): node.safe_psql('select 1') + def test_safe_psql__expect_error(self): + with get_new_node().init().start() as node: + err = node.safe_psql('select_or_not_select 1', expect_error=True) + self.assertTrue(type(err) == str) # noqa: E721 + self.assertIn('select_or_not_select', err) + self.assertIn('ERROR: syntax error at or near "select_or_not_select"', err) + + # --------- + with self.assertRaises(InvalidOperationException) as ctx: + node.safe_psql("select 1;", expect_error=True) + + self.assertEqual(str(ctx.exception), "Exception was expected, but query finished successfully: `select 1;`.") + + # --------- + res = node.safe_psql("select 1;", expect_error=False) + self.assertEqual(res, b'1\n') + def test_transactions(self): with get_new_node().init().start() as node: diff --git a/tests/test_simple_remote.py b/tests/test_simple_remote.py index 936c31f2..26ac7c61 100755 --- a/tests/test_simple_remote.py +++ b/tests/test_simple_remote.py @@ -23,7 +23,8 @@ BackupException, \ QueryException, \ TimeoutException, \ - TestgresException + TestgresException, \ + InvalidOperationException from testgres.config import \ TestgresConfig, \ @@ -295,6 +296,23 @@ def test_psql(self): with self.assertRaises(QueryException): node.safe_psql('select 1') + def test_safe_psql__expect_error(self): + with get_remote_node(conn_params=conn_params).init().start() as node: + err = node.safe_psql('select_or_not_select 1', expect_error=True) + self.assertTrue(type(err) == str) # noqa: E721 + self.assertIn('select_or_not_select', err) + self.assertIn('ERROR: syntax error at or near "select_or_not_select"', err) + + # --------- + with self.assertRaises(InvalidOperationException) as ctx: + node.safe_psql("select 1;", expect_error=True) + + self.assertEqual(str(ctx.exception), "Exception was expected, but query finished successfully: `select 1;`.") + + # --------- + res = node.safe_psql("select 1;", expect_error=False) + self.assertEqual(res, b'1\n') + def test_transactions(self): with get_remote_node(conn_params=conn_params).init().start() as node: with node.connect() as con: From db0744e7a575ebd5d7263c2a9fe2a193f7716c48 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 9 Dec 2024 18:02:12 +0300 Subject: [PATCH 05/13] A problem with InvalidOperationException and flake8 is fixed --- testgres/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testgres/__init__.py b/testgres/__init__.py index 69d2ab4a..665548d6 100644 --- a/testgres/__init__.py +++ b/testgres/__init__.py @@ -61,7 +61,7 @@ "NodeBackup", "testgres_config", "TestgresConfig", "configure_testgres", "scoped_config", "push_config", "pop_config", "NodeConnection", "DatabaseError", "InternalError", "ProgrammingError", "OperationalError", - "TestgresException", "ExecUtilException", "QueryException", "TimeoutException", "CatchUpException", "StartNodeException", "InitNodeException", "BackupException", + "TestgresException", "ExecUtilException", "QueryException", "TimeoutException", "CatchUpException", "StartNodeException", "InitNodeException", "BackupException", "InvalidOperationException", "XLogMethod", "IsolationLevel", "NodeStatus", "ProcessType", "DumpFormat", "PostgresNode", "NodeApp", "reserve_port", "release_port", "bound_ports", "get_bin_path", "get_pg_config", "get_pg_version", From 5bb1510bdb8a48d84bd2b5ea0c4a20f6951a8edf Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 9 Dec 2024 18:27:29 +0300 Subject: [PATCH 06/13] A code style is fixed [flake8] --- testgres/exceptions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testgres/exceptions.py b/testgres/exceptions.py index b4d9f76b..d61d4691 100644 --- a/testgres/exceptions.py +++ b/testgres/exceptions.py @@ -104,5 +104,6 @@ class InitNodeException(TestgresException): class BackupException(TestgresException): pass + class InvalidOperationException(TestgresException): pass From 31c7bce1e21e796d996873f3c9ae270e52992f88 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 9 Dec 2024 19:56:32 +0300 Subject: [PATCH 07/13] [BUG FIX] Wrappers for psql use subprocess.PIPE for stdout and stderr It fixes a problem with Windows. --- testgres/node.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/testgres/node.py b/testgres/node.py index ae52f21b..1037aca2 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -5,6 +5,7 @@ import signal import threading import tempfile +import subprocess from queue import Queue import time @@ -1049,7 +1050,13 @@ def _psql( # should be the last one psql_params.append(dbname) - return self.os_ops.exec_command(psql_params, verbose=True, input=input, ignore_errors=ignore_errors) + return self.os_ops.exec_command( + psql_params, + verbose=True, + input=input, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE, + ignore_errors=ignore_errors) @method_decorator(positional_args_hack(['dbname', 'query'])) def safe_psql(self, query=None, expect_error=False, **kwargs): From b013801e5aff06d1daa13e805d6080906f953868 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Mon, 9 Dec 2024 20:23:56 +0300 Subject: [PATCH 08/13] [BUG FIX] TestgresTests::test_safe_psql__expect_error uses rm_carriage_returns It fixes a problem with this test on Windows. --- tests/test_simple.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_simple.py b/tests/test_simple.py index c4200c6f..fade468c 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -327,7 +327,7 @@ def test_safe_psql__expect_error(self): # --------- res = node.safe_psql("select 1;", expect_error=False) - self.assertEqual(res, b'1\n') + self.assertEqual(rm_carriage_returns(res), b'1\n') def test_transactions(self): with get_new_node().init().start() as node: From c49ee4cf5979b83dbcecb085a2b8473f32474271 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 10 Dec 2024 11:22:28 +0300 Subject: [PATCH 09/13] node.py is updated [formatting] The previous order of imports is restored for minimization number of changes. --- testgres/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testgres/node.py b/testgres/node.py index 1037aca2..32b1e244 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -3,9 +3,9 @@ import os import random import signal +import subprocess import threading import tempfile -import subprocess from queue import Queue import time From 6a0e71495740caa32cbd54cd4b2a3819e13de539 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 10 Dec 2024 11:47:48 +0300 Subject: [PATCH 10/13] Formatting --- testgres/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testgres/node.py b/testgres/node.py index 32b1e244..b5cbab27 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -74,7 +74,7 @@ TimeoutException, \ InitNodeException, \ TestgresException, \ - BackupException, \ + BackupException, \ InvalidOperationException from .logger import TestgresLogger From 3cc19d2afdd390699ec234f83496cece996f2fe0 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 10 Dec 2024 14:55:46 +0300 Subject: [PATCH 11/13] raise_error.py is moved into testgres/operations from testgres/helpers Let's store our things on one place. We use RaiseError only in testgres/operations structures currently. --- testgres/operations/local_ops.py | 2 +- testgres/{helpers => operations}/raise_error.py | 0 testgres/operations/remote_ops.py | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename testgres/{helpers => operations}/raise_error.py (100%) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 833d1fd2..d6daaa3b 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -11,8 +11,8 @@ from ..exceptions import ExecUtilException from .os_ops import ConnectionParams, OsOperations, pglib, get_default_encoding +from .raise_error import RaiseError from .helpers import Helpers -from ..helpers.raise_error import RaiseError try: from shutil import which as find_executable diff --git a/testgres/helpers/raise_error.py b/testgres/operations/raise_error.py similarity index 100% rename from testgres/helpers/raise_error.py rename to testgres/operations/raise_error.py diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index cbaeb62b..c48f867b 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -14,8 +14,8 @@ 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 +from .raise_error import RaiseError from .helpers import Helpers error_markers = [b'error', b'Permission denied', b'fatal', b'No such file or directory'] From 7b70e9e7a0b22fb999e308dab8bccc46e8e22a65 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 10 Dec 2024 16:21:39 +0300 Subject: [PATCH 12/13] Helpers.GetDefaultEncoding is added This function is equal to os_ops.get_default_encoding and is used in: - Helpers.PrepareProcessInput - RaiseError._TranslateDataIntoString__FromBinary --- testgres/operations/helpers.py | 39 +++++++++++++++++++++++++++++- testgres/operations/raise_error.py | 3 ++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/testgres/operations/helpers.py b/testgres/operations/helpers.py index d714d336..b50f0baa 100644 --- a/testgres/operations/helpers.py +++ b/testgres/operations/helpers.py @@ -1,11 +1,48 @@ +import locale + + class Helpers: + def _make_get_default_encoding_func(): + # locale.getencoding is added in Python 3.11 + if hasattr(locale, 'getencoding'): + return locale.getencoding + + # It must exist + return locale.getpreferredencoding + + # Prepared pointer on function to get a name of system codepage + _get_default_encoding_func = _make_get_default_encoding_func() + + def GetDefaultEncoding(): + # + # Original idea/source was: + # + # def os_ops.get_default_encoding(): + # if not hasattr(locale, 'getencoding'): + # locale.getencoding = locale.getpreferredencoding + # return locale.getencoding() or 'UTF-8' + # + + assert __class__._get_default_encoding_func is not None + + r = __class__._get_default_encoding_func() + + if r: + assert r is not None + assert type(r) == str # noqa: E721 + assert r != "" + return r + + # Is it an unexpected situation? + return 'UTF-8' + def PrepareProcessInput(input, encoding): if not input: return None if type(input) == str: # noqa: E721 if encoding is None: - return input.encode() + return input.encode(__class__.GetDefaultEncoding()) assert type(encoding) == str # noqa: E721 return input.encode(encoding) diff --git a/testgres/operations/raise_error.py b/testgres/operations/raise_error.py index c67833dd..0e760e74 100644 --- a/testgres/operations/raise_error.py +++ b/testgres/operations/raise_error.py @@ -1,4 +1,5 @@ from ..exceptions import ExecUtilException +from .helpers import Helpers class RaiseError: @@ -29,7 +30,7 @@ def _TranslateDataIntoString__FromBinary(data): assert type(data) == bytes # noqa: E721 try: - return data.decode('utf-8') + return data.decode(Helpers.GetDefaultEncoding()) except UnicodeDecodeError: pass From cd0b5f8671d61214afb2985adb83cd3efb9b852a Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 10 Dec 2024 17:01:57 +0300 Subject: [PATCH 13/13] Code normalization - New debug checks - Normalization --- testgres/operations/local_ops.py | 15 +++++++++++---- testgres/operations/remote_ops.py | 3 +++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index d6daaa3b..3e8ab8ca 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -86,6 +86,8 @@ def _run_command__generic(self, cmd, shell, input, stdin, stdout, stderr, get_pr if not get_process: input_prepared = Helpers.PrepareProcessInput(input, encoding) # throw + assert input_prepared is None or (type(input_prepared) == bytes) # noqa: E721 + process = subprocess.Popen( cmd, shell=shell, @@ -93,18 +95,23 @@ def _run_command__generic(self, cmd, shell, input, stdin, stdout, stderr, get_pr stdout=stdout or subprocess.PIPE, stderr=stderr or subprocess.PIPE, ) + assert not (process is None) if get_process: return process, None, None try: output, error = process.communicate(input=input_prepared, timeout=timeout) - if encoding: - output = output.decode(encoding) - error = error.decode(encoding) - return process, output, error except subprocess.TimeoutExpired: process.kill() raise ExecUtilException("Command timed out after {} seconds.".format(timeout)) + assert type(output) == bytes # noqa: E721 + assert type(error) == bytes # noqa: E721 + + if encoding: + output = output.decode(encoding) + error = error.decode(encoding) + return process, output, error + def _run_command(self, cmd, shell, input, stdin, stdout, stderr, get_process, timeout, encoding): """Execute a command and return the process and its output.""" if os.name == 'nt' and stdout is None: # Windows diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index c48f867b..00c50d93 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -75,12 +75,15 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, if not get_process: input_prepared = Helpers.PrepareProcessInput(input, encoding) # throw + assert input_prepared is None or (type(input_prepared) == bytes) # noqa: E721 + ssh_cmd = [] if isinstance(cmd, str): ssh_cmd = ['ssh', self.ssh_dest] + self.ssh_args + [cmd] elif isinstance(cmd, list): ssh_cmd = ['ssh', self.ssh_dest] + self.ssh_args + cmd process = subprocess.Popen(ssh_cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + assert not (process is None) if get_process: return process