Content-Length: 23158 | pFad | http://github.com/postgrespro/testgres/pull/203.patch

thub.com From 1309442d4be3bf3821b706892231fe36995d7c2a Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sat, 1 Mar 2025 12:01:12 +0300 Subject: [PATCH 1/2] Total refactoring of os_ops::execute_command Main - We check only an exit code to detect an error. - If someone utility returns a result through an exit code, a caller side should set ignore_errors=true and process this case itself. - If expect_error is true and no errors occurred, we raise an InvalidOperationException. --- testgres/operations/local_ops.py | 35 +++++----- testgres/operations/raise_error.py | 47 ++++--------- testgres/operations/remote_ops.py | 102 +++++++++++++++++++---------- testgres/utils.py | 13 ++-- tests/test_local.py | 5 +- tests/test_remote.py | 10 +-- tests/test_simple_remote.py | 43 ++++++------ 7 files changed, 134 insertions(+), 121 deletions(-) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 91070fe7..9828a45e 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -23,20 +23,6 @@ from distutils import rmtree CMD_TIMEOUT_SEC = 60 -error_markers = [b'error', b'Permission denied', b'fatal'] -err_out_markers = [b'Failure'] - - -def has_errors(output=None, error=None): - if output: - if isinstance(output, str): - output = output.encode(get_default_encoding()) - return any(marker in output for marker in err_out_markers) - if error: - if isinstance(error, str): - error = error.encode(get_default_encoding()) - return any(marker in error for marker in error_markers) - return False class LocalOperations(OsOperations): @@ -134,19 +120,28 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, 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): + + if expect_error: + if process.returncode == 0: + raise InvalidOperationException("We expected an execution error.") + elif ignore_errors: + pass + elif process.returncode == 0: + pass + else: + assert not expect_error + assert not ignore_errors + assert process.returncode != 0 RaiseError.UtilityExitedWithNonZeroCode( cmd=cmd, exit_code=process.returncode, - msg_arg=error or output, error=error, - out=output - ) + out=output) if verbose: return process.returncode, output, error - else: - return output + + return output # Environment setup def environ(self, var_name): diff --git a/testgres/operations/raise_error.py b/testgres/operations/raise_error.py index 6031b238..bb2945e6 100644 --- a/testgres/operations/raise_error.py +++ b/testgres/operations/raise_error.py @@ -1,50 +1,27 @@ from ..exceptions import ExecUtilException -from .helpers import Helpers class RaiseError: @staticmethod - def UtilityExitedWithNonZeroCode(cmd, exit_code, msg_arg, error, out): + def UtilityExitedWithNonZeroCode(cmd, exit_code, 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, + message="Utility exited with non-zero code.", command=cmd, exit_code=exit_code, out=out, error=error) @staticmethod - def _TranslateDataIntoString(data): - if type(data) == bytes: # noqa: E721 - return __class__._TranslateDataIntoString__FromBinary(data) - - return str(data) - - @staticmethod - def _TranslateDataIntoString__FromBinary(data): - assert type(data) == bytes # noqa: E721 - - try: - return data.decode(Helpers.GetDefaultEncoding()) - except UnicodeDecodeError: - pass - - return "#cannot_decode_text" - - @staticmethod - def _BinaryIsASCII(data): - assert type(data) == bytes # noqa: E721 - - for b in data: - if not (b >= 0 and b <= 127): - return False + def CommandExecutionError(cmd, exit_code, message, error, out): + assert type(exit_code) == int # noqa: E721 + assert type(message) == str # noqa: E721 + assert message != "" - return True + raise ExecUtilException( + message=message, + command=cmd, + exit_code=exit_code, + out=out, + error=error) diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 2a4e5c78..a827eac4 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -100,41 +100,39 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, return process try: - result, error = process.communicate(input=input_prepared, timeout=timeout) + output, error = process.communicate(input=input_prepared, timeout=timeout) except subprocess.TimeoutExpired: process.kill() raise ExecUtilException("Command timed out after {} seconds.".format(timeout)) - exit_status = process.returncode - - assert type(result) == bytes # noqa: E721 + assert type(output) == bytes # noqa: E721 assert type(error) == bytes # noqa: E721 - if not error: - error_found = False - else: - error_found = exit_status != 0 or any( - marker in error for marker in [b'error', b'Permission denied', b'fatal', b'No such file or directory'] - ) - - assert type(error_found) == bool # noqa: E721 - if encoding: - result = result.decode(encoding) + output = output.decode(encoding) error = error.decode(encoding) - if not ignore_errors and error_found and not expect_error: + if expect_error: + if process.returncode == 0: + raise InvalidOperationException("We expected an execution error.") + elif ignore_errors: + pass + elif process.returncode == 0: + pass + else: + assert not expect_error + assert not ignore_errors + assert process.returncode != 0 RaiseError.UtilityExitedWithNonZeroCode( cmd=cmd, - exit_code=exit_status, - msg_arg=error, + exit_code=process.returncode, error=error, - out=result) + out=output) if verbose: - return exit_status, result, error - else: - return result + return process.returncode, output, error + + return output # Environment setup def environ(self, var_name: str) -> str: @@ -165,8 +163,30 @@ def find_executable(self, executable): def is_executable(self, file): # Check if the file is executable - is_exec = self.exec_command("test -x {} && echo OK".format(file)) - return is_exec == b"OK\n" + command = ["test", "-x", file] + + exit_status, output, error = self.exec_command(cmd=command, encoding=get_default_encoding(), ignore_errors=True, verbose=True) + + assert type(output) == str # noqa: E721 + assert type(error) == str # noqa: E721 + + if exit_status == 0: + return True + + if exit_status == 1: + return False + + errMsg = "Test operation returns an unknown result code: {0}. File name is [{1}].".format( + exit_status, + file) + + RaiseError.UtilityExitedWithNonZeroCode( + cmd=command, + exit_code=exit_status, + msg_arg=errMsg, + error=error, + out=output + ) def set_env(self, var_name: str, var_val: str): """ @@ -251,15 +271,21 @@ def mkdtemp(self, prefix=None): else: command = ["mktemp", "-d"] - exit_status, result, error = self.exec_command(command, verbose=True, encoding=get_default_encoding(), ignore_errors=True) + exec_exitcode, exec_output, exec_error = self.exec_command(command, verbose=True, encoding=get_default_encoding(), ignore_errors=True) - assert type(result) == str # noqa: E721 - assert type(error) == str # noqa: E721 + assert type(exec_exitcode) == int # noqa: E721 + assert type(exec_output) == str # noqa: E721 + assert type(exec_error) == str # noqa: E721 - if exit_status != 0: - raise ExecUtilException("Could not create temporary directory. Error code: {0}. Error message: {1}".format(exit_status, error)) + if exec_exitcode != 0: + RaiseError.CommandExecutionError( + cmd=command, + exit_code=exec_exitcode, + message="Could not create temporary directory.", + error=exec_error, + out=exec_output) - temp_dir = result.strip() + temp_dir = exec_output.strip() return temp_dir def mkstemp(self, prefix=None): @@ -273,15 +299,21 @@ def mkstemp(self, prefix=None): else: command = ["mktemp"] - exit_status, result, error = self.exec_command(command, verbose=True, encoding=get_default_encoding(), ignore_errors=True) + exec_exitcode, exec_output, exec_error = self.exec_command(command, verbose=True, encoding=get_default_encoding(), ignore_errors=True) - assert type(result) == str # noqa: E721 - assert type(error) == str # noqa: E721 + assert type(exec_exitcode) == int # noqa: E721 + assert type(exec_output) == str # noqa: E721 + assert type(exec_error) == str # noqa: E721 - if exit_status != 0: - raise ExecUtilException("Could not create temporary file. Error code: {0}. Error message: {1}".format(exit_status, error)) + if exec_exitcode != 0: + RaiseError.CommandExecutionError( + cmd=command, + exit_code=exec_exitcode, + message="Could not create temporary file.", + error=exec_error, + out=exec_output) - temp_file = result.strip() + temp_file = exec_output.strip() return temp_file def copytree(self, src, dst): diff --git a/testgres/utils.py b/testgres/utils.py index 76d42b02..093eaff6 100644 --- a/testgres/utils.py +++ b/testgres/utils.py @@ -18,6 +18,7 @@ from .config import testgres_config as tconf from .operations.os_ops import OsOperations from .operations.remote_ops import RemoteOperations +from .operations.helpers import Helpers as OsHelpers # rows returned by PG_CONFIG _pg_config_data = {} @@ -79,13 +80,13 @@ def execute_utility2(os_ops: OsOperations, args, logfile=None, verbose=False, ig assert type(verbose) == bool # noqa: E721 assert type(ignore_errors) == bool # noqa: E721 - exit_status, out, error = os_ops.exec_command(args, verbose=True, ignore_errors=ignore_errors) - # decode result + exit_status, out, error = os_ops.exec_command( + args, + verbose=True, + ignore_errors=ignore_errors, + encoding=OsHelpers.GetDefaultEncoding()) + out = '' if not out else out - if isinstance(out, bytes): - out = out.decode('utf-8') - if isinstance(error, bytes): - error = error.decode('utf-8') # write new log entry if possible if logfile: diff --git a/tests/test_local.py b/tests/test_local.py index 60a96c18..a16a8e6b 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -40,10 +40,11 @@ def test_exec_command_failure(self): try: self.operations.exec_command(cmd, wait_exit=True, shell=True) except ExecUtilException as e: - error = e.message + assert e.message == "Utility exited with non-zero code." + assert type(e.error) == bytes # noqa: E721 + assert e.error.strip() == b"/bin/sh: 1: nonexistent_command: not found" break raise Exception("We wait an exception!") - 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 8b167e9f..5af72958 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -38,10 +38,11 @@ def test_exec_command_failure(self): try: self.operations.exec_command(cmd, verbose=True, wait_exit=True) except ExecUtilException as e: - error = e.message + assert e.message == "Utility exited with non-zero code." + assert type(e.error) == bytes # noqa: E721 + assert e.error.strip() == b"bash: line 1: nonexistent_command: command not found" break raise Exception("We wait an exception!") - 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): """ @@ -108,10 +109,11 @@ def test_makedirs_and_rmdirs_failure(self): try: self.operations.rmdirs(path, verbose=True) except ExecUtilException as e: - error = e.message + assert e.message == "Utility exited with non-zero code." + assert type(e.error) == bytes # noqa: E721 + assert e.error.strip() == b"rm: cannot remove '/root/test_dir': Permission denied" 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): """ diff --git a/tests/test_simple_remote.py b/tests/test_simple_remote.py index d4a28a2b..74b10635 100755 --- a/tests/test_simple_remote.py +++ b/tests/test_simple_remote.py @@ -178,27 +178,32 @@ def test_init__unk_LANG_and_LC_CTYPE(self): assert os.environ.get("LC_CTYPE") == unkData[1] assert not ("LC_COLLATE" in os.environ.keys()) - while True: + assert os.getenv('LANG') == unkData[0] + assert os.getenv('LANGUAGE') is None + assert os.getenv('LC_CTYPE') == unkData[1] + assert os.getenv('LC_COLLATE') is None + + exc: ExecUtilException = None + with __class__.helper__get_node() as node: try: - with __class__.helper__get_node(): - pass - except ExecUtilException as e: - # - # Example of an error message: - # - # warning: setlocale: LC_CTYPE: cannot change locale (UNKNOWN_CTYPE): No such file or directory - # postgres (PostgreSQL) 14.12 - # - errMsg = str(e) - - logging.info("Error message is: {0}".format(errMsg)) - - assert "LC_CTYPE" in errMsg - assert unkData[1] in errMsg - assert "warning: setlocale: LC_CTYPE: cannot change locale (" + unkData[1] + "): No such file or directory" in errMsg - assert ("postgres" in errMsg) or ("PostgreSQL" in errMsg) - break + node.init() # IT RAISES! + except InitNodeException as e: + exc = e.__cause__ + assert exc is not None + assert isinstance(exc, ExecUtilException) + + if exc is None: raise Exception("We expected an error!") + + assert isinstance(exc, ExecUtilException) + + errMsg = str(exc) + logging.info("Error message is {0}: {1}".format(type(exc).__name__, errMsg)) + + assert "warning: setlocale: LC_CTYPE: cannot change locale (" + unkData[1] + ")" in errMsg + assert "initdb: error: invalid locale settings; check LANG and LC_* environment variables" in errMsg + continue + finally: __class__.helper__restore_envvar("LANG", prev_LANG) __class__.helper__restore_envvar("LANGUAGE", prev_LANGUAGE) From d843df62f1d038905dd6a12d420edc9be670d4bf Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sat, 1 Mar 2025 21:53:19 +0300 Subject: [PATCH 2/2] The old behaviour of RaiseError.UtilityExitedWithNonZeroCode is restored Let's rollback the new code to avoid problems with probackup2' tests. --- testgres/operations/local_ops.py | 1 + testgres/operations/raise_error.py | 34 ++++++++++++++++++++++++++++-- testgres/operations/remote_ops.py | 3 ++- tests/test_local.py | 2 +- tests/test_remote.py | 4 ++-- 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 9828a45e..51003174 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -135,6 +135,7 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, RaiseError.UtilityExitedWithNonZeroCode( cmd=cmd, exit_code=process.returncode, + msg_arg=error or output, error=error, out=output) diff --git a/testgres/operations/raise_error.py b/testgres/operations/raise_error.py index bb2945e6..0d14be5a 100644 --- a/testgres/operations/raise_error.py +++ b/testgres/operations/raise_error.py @@ -1,13 +1,22 @@ from ..exceptions import ExecUtilException +from .helpers import Helpers class RaiseError: @staticmethod - def UtilityExitedWithNonZeroCode(cmd, exit_code, error, out): + def UtilityExitedWithNonZeroCode(cmd, exit_code, msg_arg, error, out): assert type(exit_code) == int # noqa: E721 + msg_arg_s = __class__._TranslateDataIntoString(msg_arg) + assert type(msg_arg_s) == str # noqa: E721 + + msg_arg_s = msg_arg_s.strip() + if msg_arg_s == "": + msg_arg_s = "#no_error_message" + + message = "Utility exited with non-zero code (" + str(exit_code) + "). Error: `" + msg_arg_s + "`" raise ExecUtilException( - message="Utility exited with non-zero code.", + message=message, command=cmd, exit_code=exit_code, out=out, @@ -25,3 +34,24 @@ def CommandExecutionError(cmd, exit_code, message, error, out): exit_code=exit_code, out=out, error=error) + + @staticmethod + def _TranslateDataIntoString(data): + if data is None: + return "" + + if type(data) == bytes: # noqa: E721 + return __class__._TranslateDataIntoString__FromBinary(data) + + return str(data) + + @staticmethod + def _TranslateDataIntoString__FromBinary(data): + assert type(data) == bytes # noqa: E721 + + try: + return data.decode(Helpers.GetDefaultEncoding()) + except UnicodeDecodeError: + pass + + return "#cannot_decode_text" diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index a827eac4..dc392bee 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -126,6 +126,7 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, RaiseError.UtilityExitedWithNonZeroCode( cmd=cmd, exit_code=process.returncode, + msg_arg=error, error=error, out=output) @@ -180,7 +181,7 @@ def is_executable(self, file): exit_status, file) - RaiseError.UtilityExitedWithNonZeroCode( + RaiseError.CommandExecutionError( cmd=command, exit_code=exit_status, msg_arg=errMsg, diff --git a/tests/test_local.py b/tests/test_local.py index a16a8e6b..ee5e19a0 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -40,7 +40,7 @@ def test_exec_command_failure(self): try: self.operations.exec_command(cmd, wait_exit=True, shell=True) except ExecUtilException as e: - assert e.message == "Utility exited with non-zero code." + assert e.message == "Utility exited with non-zero code (127). Error: `/bin/sh: 1: nonexistent_command: not found`" assert type(e.error) == bytes # noqa: E721 assert e.error.strip() == b"/bin/sh: 1: nonexistent_command: not found" break diff --git a/tests/test_remote.py b/tests/test_remote.py index 5af72958..61d98768 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -38,7 +38,7 @@ def test_exec_command_failure(self): try: self.operations.exec_command(cmd, verbose=True, wait_exit=True) except ExecUtilException as e: - assert e.message == "Utility exited with non-zero code." + assert e.message == "Utility exited with non-zero code (127). Error: `bash: line 1: nonexistent_command: command not found`" assert type(e.error) == bytes # noqa: E721 assert e.error.strip() == b"bash: line 1: nonexistent_command: command not found" break @@ -109,7 +109,7 @@ def test_makedirs_and_rmdirs_failure(self): try: self.operations.rmdirs(path, verbose=True) except ExecUtilException as e: - assert e.message == "Utility exited with non-zero code." + assert e.message == "Utility exited with non-zero code (1). Error: `rm: cannot remove '/root/test_dir': Permission denied`" assert type(e.error) == bytes # noqa: E721 assert e.error.strip() == b"rm: cannot remove '/root/test_dir': Permission denied" break

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!