diff --git a/src/backup.py b/src/backup.py index d91db5a0..9c54be17 100644 --- a/src/backup.py +++ b/src/backup.py @@ -15,9 +15,10 @@ from testgres.operations.os_ops import OsOperations +from . import utils + from .utils import \ get_bin_path2, \ - execute_utility2, \ clean_on_error @@ -86,7 +87,13 @@ def __init__(self, "-X", xlog_method.value ] # yapf: disable _params += options - execute_utility2(self.os_ops, _params, self.log_file) + + utils.execute_utility3( + self.os_ops, + _params, + self.log_file, + ) + return def __enter__(self): return self diff --git a/src/cache.py b/src/cache.py index 72ec5698..e205a510 100644 --- a/src/cache.py +++ b/src/cache.py @@ -8,14 +8,12 @@ from .defaults import generate_system_id +from . import utils + from .exceptions import \ InitNodeException, \ ExecUtilException -from .utils import \ - get_bin_path2, \ - execute_utility2 - from testgres.operations.local_ops import LocalOperations from testgres.operations.os_ops import OsOperations @@ -39,13 +37,17 @@ def make_utility_path(name): if bin_path: return os_ops.build_path(bin_path, name) - return get_bin_path2(os_ops, name) + return utils.get_bin_path2(os_ops, name) def call_initdb(initdb_dir, log=logfile): try: initdb_path = make_utility_path("initdb") _params = [initdb_path, "-D", initdb_dir, "-N"] - execute_utility2(os_ops, _params + (params or []), log) + utils.execute_utility3( + os_ops, + _params + (params or []), + log, + ) except ExecUtilException as e: raise_from(InitNodeException("Failed to run initdb"), e) @@ -78,7 +80,12 @@ def call_initdb(initdb_dir, log=logfile): # XXX: build new WAL segment with our system id _params = [make_utility_path("pg_resetwal"), "-D", data_dir, "-f"] - execute_utility2(os_ops, _params, logfile) + + utils.execute_utility3( + os_ops, + _params, + logfile, + ) except ExecUtilException as e: msg = "Failed to reset WAL for system id" diff --git a/src/node.py b/src/node.py index b369c73a..c18cbae6 100644 --- a/src/node.py +++ b/src/node.py @@ -92,7 +92,6 @@ PgVer, \ eprint, \ get_pg_version2, \ - execute_utility2, \ options_string, \ clean_on_error @@ -1093,11 +1092,16 @@ def get_control_data(self): _params += ["-D"] if self._pg_version >= PgVer('9.5') else [] _params += [self.data_dir] - data = execute_utility2(self._os_ops, _params, self.utils_log_file) + exec_r = utils.execute_utility3( + self._os_ops, + _params, + self.utils_log_file, + ) + assert type(exec_r.stdout) is str out_dict = {} - for line in data.splitlines(): + for line in exec_r.stdout.splitlines(): key, _, value = line.partition(':') out_dict[key.strip()] = value.strip() @@ -1248,10 +1252,17 @@ def _start( def LOCAL__start_node(): # 'error' will be None on Windows - _, _, error = execute_utility2(self._os_ops, _params, self.utils_log_file, verbose=True, exec_env=exec_env) - assert error is None or type(error) is str - if error and 'does not exist' in error: - raise Exception(error) + exec_r = utils.execute_utility3( + self._os_ops, + _params, + self.utils_log_file, + exec_env=exec_env, + ) + # TODO: WTF? Remove it! + assert exec_r.returncode == 0 + assert exec_r.stderr is None or type(exec_r.stderr) is str + if exec_r.stderr is not None and 'does not exist' in exec_r.stderr: + raise ExecUtilException(exec_r.stderr) def LOCAL__raise_cannot_start_node__std(from_exception): assert isinstance(from_exception, Exception) @@ -1338,7 +1349,11 @@ def stop(self, params=[], wait=True): ] + params # yapf: disable try: - execute_utility2(self._os_ops, _params, self.utils_log_file) + utils.execute_utility3( + self._os_ops, + _params, + logfile=self.utils_log_file, + ) self._manually_started_pm_pid = None finally: # always stop the reader thread, even if pg_ctl stop failed, @@ -1399,9 +1414,16 @@ def restart(self, params=[]): ] + params # yapf: disable try: - error_code, out, error = execute_utility2(self._os_ops, _params, self.utils_log_file, verbose=True) - if error and 'could not start server' in error: - raise ExecUtilException + exec_r = utils.execute_utility3( + self._os_ops, + _params, + logfile=self.utils_log_file, + ) + # TODO: WTF? Remove it! + assert exec_r.returncode == 0 + assert exec_r.stderr is None or type(exec_r.stderr) is str + if exec_r.stderr is not None and 'could not start server' in exec_r.stderr: + raise ExecUtilException(exec_r.stderr) except ExecUtilException as e: msg = 'Cannot restart node' files = self._collect_special_files() @@ -1428,7 +1450,11 @@ def reload(self, params=[]): "reload" ] + params # yapf: disable - execute_utility2(self._os_ops, _params, self.utils_log_file) + utils.execute_utility3( + self._os_ops, + _params, + logfile=self.utils_log_file, + ) return self @@ -1450,7 +1476,11 @@ def promote(self, dbname=None, username=None): "promote" ] # yapf: disable - execute_utility2(self._os_ops, _params, self.utils_log_file) + utils.execute_utility3( + self._os_ops, + _params, + self.utils_log_file, + ) # for versions below 10 `promote` is asynchronous so we need to wait # until it actually becomes writable @@ -1485,7 +1515,11 @@ def pg_ctl(self, params): "-w" # wait ] + params # yapf: disable - return execute_utility2(self._os_ops, _params, self.utils_log_file) + return utils.execute_utility3( + self._os_ops, + _params, + self.utils_log_file, + ).stdout def release_resources(self): """ @@ -1740,7 +1774,11 @@ def tmpfile(): if options: _params.extend(options) - execute_utility2(self._os_ops, _params, self.utils_log_file) + utils.execute_utility3( + self._os_ops, + _params, + self.utils_log_file, + ) return filename @@ -1769,7 +1807,11 @@ def restore(self, filename, dbname=None, username=None): # try pg_restore if dump is binary format, and psql if not try: - execute_utility2(self._os_ops, _params, self.utils_log_file) + utils.execute_utility3( + self._os_ops, + _params, + self.utils_log_file, + ) except ExecUtilException: self.psql(filename=filename, dbname=dbname, username=username) @@ -2130,7 +2172,11 @@ def pgbench_run(self, dbname=None, username=None, options=[], **kwargs): # should be the last one _params.append(dbname) - return execute_utility2(self._os_ops, _params, self.utils_log_file) + return utils.execute_utility3( + self._os_ops, + _params, + self.utils_log_file, + ).stdout def connect(self, dbname=None, diff --git a/src/utils.py b/src/utils.py index 6cbaa0d5..622ec9e4 100644 --- a/src/utils.py +++ b/src/utils.py @@ -1,19 +1,9 @@ # coding: utf-8 +from __future__ import annotations from __future__ import division from __future__ import print_function -import os -import sys -import time - -from contextlib import contextmanager -from packaging.version import Version, InvalidVersion -import re -import typing - -from six import iteritems - from .exceptions import ExecUtilException, InvalidOperationException from .config import testgres_config as tconf from .raise_error import RaiseError @@ -21,7 +11,10 @@ from .consts import PG_CTL__STATUS__OK from .consts import PG_CTL__STATUS__NODE_IS_STOPPED from .consts import PG_CTL__STATUS__BAD_DATADIR +from testgres.operations.types import T_OS_CMD +from testgres.operations.types import T_OS_EXEC_ENV from testgres.operations.os_ops import OsOperations +from testgres.operations.os_ops import OsCommandResult from testgres.operations.remote_ops import RemoteOperations from testgres.operations.local_ops import LocalOperations from testgres.operations.helpers import Helpers as OsHelpers @@ -31,6 +24,17 @@ from .impl.platforms import internal_platform_utils_factory from .impl import internal_utils +import os +import sys +import time +import re +import typing + +from six import iteritems +from contextlib import contextmanager +from packaging.version import Version, InvalidVersion + + # rows returned by PG_CONFIG _pg_config_data = {} @@ -81,16 +85,21 @@ def execute_utility(args, logfile=None, verbose=False): Returns: stdout of executed utility. """ - return execute_utility2(tconf.os_ops, args, logfile, verbose) + return execute_utility2( + tconf.os_ops, + args, + logfile, + verbose, + ) def execute_utility2( - os_ops: OsOperations, - args, - logfile=None, - verbose=False, - ignore_errors=False, - exec_env=None, + os_ops: OsOperations, + args, + logfile=None, + verbose=False, + ignore_errors=False, + exec_env=None, ): assert os_ops is not None assert isinstance(os_ops, OsOperations) @@ -98,45 +107,76 @@ def execute_utility2( assert type(ignore_errors) is bool assert exec_env is None or type(exec_env) is dict - exec_r = os_ops.exec_command( + exec_r = execute_utility3( + os_ops, args, - verbose=True, - ignore_errors=ignore_errors, - encoding=OsHelpers.GetDefaultEncoding(), + logfile, + check=not ignore_errors, exec_env=exec_env, ) - assert type(exec_r) is tuple - assert len(exec_r) == 3 + assert type(exec_r) is OsCommandResult + + assert type(exec_r.returncode) is int + assert exec_r.stdout is None or type(exec_r.stdout) is str + assert exec_r.stderr is None or type(exec_r.stderr) is str + + if not verbose: + return exec_r.stdout + + return exec_r.returncode, exec_r.stdout, exec_r.stderr + + +def execute_utility3( + os_ops: OsOperations, + args: T_OS_CMD, + logfile: typing.Optional[str] = None, + check: bool = True, + exec_env: typing.Optional[T_OS_EXEC_ENV] = None, +) -> OsCommandResult: + assert os_ops is not None + assert isinstance(os_ops, OsOperations) + assert type(check) is bool + assert exec_env is None or type(exec_env) is dict - exit_status, out, _ = exec_r + exec_r = os_ops.run( + args, + check=check, + encoding=OsHelpers.GetDefaultEncoding(), + exec_env=exec_env, + ) - assert type(exit_status) is int - assert type(out) is str + assert type(exec_r) is OsCommandResult # write new log entry if possible if logfile: try: + log_lines = [ + os_ops.join_command_arguments(args), + ] + + if exec_r.stdout is None: + log_lines.append("# #NONE#") + else: + # comment-out lines + assert type(exec_r.stdout) is str + log_lines += ['# ' + line for line in exec_r.stdout.splitlines()] + + log_lines.append("") + os_ops.write( filename=logfile, - data=os_ops.join_command_arguments(args), + data="\n".join(log_lines), truncate=False, ) - if out: - # comment-out lines - lines = [u'\n'] + ['# ' + line for line in out.splitlines()] + [u'\n'] - os_ops.write( - filename=logfile, - data=lines, - truncate=False, - ) except IOError: raise ExecUtilException( - "Problem with writing to logfile `{}` during run command `{}`".format(logfile, args)) - if verbose: - return exec_r + "Problem with writing to logfile `{}` during run command `{}`".format( + logfile, + args, + )) - return out + return exec_r def get_bin_path(filename): @@ -449,14 +489,17 @@ def get(self) -> T_PLATFORM_UTILS: time.sleep(sleep_time) sleep_time = sleep_time * C_SLEEP_TIME_MULT - status_code, out, error = execute_utility2( + exec_r = execute_utility3( os_ops, _params, utils_log_file, - verbose=True, - ignore_errors=True, + check=False, ) + status_code = exec_r.returncode + out = exec_r.stdout + error = exec_r.stderr + assert type(status_code) is int assert type(out) is str assert type(error) is str diff --git a/tests/test_utils.py b/tests/test_utils.py index d46dc442..c5b7990e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,10 +1,9 @@ -from .helpers.global_data import OsOpsDescr -from .helpers.global_data import OsOpsDescrs -from .helpers.global_data import OsOperations +from tests.helpers.global_data import OsOpsDescr +from tests.helpers.global_data import OsOpsDescrs +from tests.helpers.global_data import OsOperations from src.utils import parse_pg_version from src.utils import get_pg_config2 -from src.utils import execute_utility2 from src import scoped_config import pytest @@ -65,55 +64,3 @@ def test_get_pg_config2(self, os_ops: OsOperations): a = get_pg_config2(os_ops, None) b = get_pg_config2(os_ops, None) assert (id(a) != id(b)) - - def test_execute_utility2__log(self, os_ops: OsOperations): - assert isinstance(os_ops, OsOperations) - - log_file: typing.Optional[str] = None - - try: - C_OUT_DATA = "AAAA" - - log_file = os_ops.mkstemp("testgres--") - assert os_ops.path_exists(log_file) - - os_ops.write( - log_file, - C_OUT_DATA + "\n", - truncate=False, - binary=False, - ) - - cmd = ["sh", "-c", "echo BBBB"] - - execute_utility2( - os_ops, - cmd, - logfile=log_file, - ) - - assert os_ops.path_exists(log_file) - - log_content = os_ops.read( - log_file, - binary=False, - ) - - expected_content_lines = [ - C_OUT_DATA, - "sh -c 'echo BBBB'", - "# BBBB", - "", - ] - - expected_content_s = "\n".join(expected_content_lines) - - assert log_content == expected_content_s - finally: - if log_file is not None: - assert type(log_file) is str - os_ops.remove_file(log_file) - - assert type(log_file) is str - assert not os_ops.path_exists(log_file) - return diff --git a/tests/units/utils/__init__.py b/tests/units/utils/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/units/utils/test_command_execute2.py b/tests/units/utils/test_command_execute2.py new file mode 100644 index 00000000..d08d2e25 --- /dev/null +++ b/tests/units/utils/test_command_execute2.py @@ -0,0 +1,132 @@ +from tests.helpers.global_data import OsOpsDescr +from tests.helpers.global_data import OsOpsDescrs +from tests.helpers.global_data import OsOperations + +from testgres.operations.exceptions import ExecUtilException + +from src.utils import execute_utility2 + +import pytest +import typing + + +class TestUtils__command_execute2: + sm_os_ops_descrs: typing.List[OsOpsDescr] = [ + OsOpsDescrs.sm_local_os_ops_descr, + OsOpsDescrs.sm_remote_os_ops_descr + ] + + @pytest.fixture( + params=[descr.os_ops for descr in sm_os_ops_descrs], + ids=[descr.sign for descr in sm_os_ops_descrs] + ) + def os_ops(self, request: pytest.FixtureRequest) -> OsOperations: + assert isinstance(request, pytest.FixtureRequest) + assert isinstance(request.param, OsOperations) + return request.param + + def test_execute_utility2__log(self, os_ops: OsOperations): + assert isinstance(os_ops, OsOperations) + + log_file: typing.Optional[str] = None + + try: + C_OUT_DATA = "AAAA" + + log_file = os_ops.mkstemp("testgres--") + assert os_ops.path_exists(log_file) + + os_ops.write( + log_file, + C_OUT_DATA + "\n", + truncate=False, + binary=False, + ) + + cmd = ["sh", "-c", "echo BBBB"] + + r = execute_utility2( + os_ops, + cmd, + logfile=log_file, + ) + + assert type(r) is str + assert r == "BBBB\n" + + assert os_ops.path_exists(log_file) + + log_content = os_ops.read( + log_file, + binary=False, + ) + + expected_content_lines = [ + C_OUT_DATA, + "sh -c 'echo BBBB'", + "# BBBB", + "", + ] + + expected_content_s = "\n".join(expected_content_lines) + + assert log_content == expected_content_s + finally: + if log_file is not None: + assert type(log_file) is str + os_ops.remove_file(log_file) + + assert type(log_file) is str + assert not os_ops.path_exists(log_file) + return + + def test_execute_utility3__error__check_false(self, os_ops: OsOperations): + assert isinstance(os_ops, OsOperations) + + cmd = ["sh", "-c", "nonexistent_command"] + + r = execute_utility2( + os_ops, + cmd, + verbose=True, + ignore_errors=True, + ) + + assert type(r) is tuple + assert len(r) == 3 + assert type(r[0]) is int + assert type(r[1]) is str + assert type(r[2]) is str + + assert r[0] == 127 + assert r[1] == "" + assert "nonexistent_command" in r[2] + assert "not found" in r[2] + return + + def test_execute_utility3__error__check_true(self, os_ops: OsOperations): + assert isinstance(os_ops, OsOperations) + + cmd = ["sh", "-c", "nonexistent_command"] + + with pytest.raises(expected_exception=ExecUtilException) as x: + execute_utility2( + os_ops, + cmd, + ignore_errors=False, + ) + + assert type(x.value) is ExecUtilException + assert type(x.value.exit_code) is int + assert x.value.exit_code == 127 + + assert type(x.value.message) is str + assert type(x.value.out) is str + assert type(x.value.error) is str + + assert x.value.message.startswith("Utility exited with non-zero code (127). Error:") + assert "nonexistent_command" in x.value.message + assert "not found" in x.value.message + assert "nonexistent_command" in x.value.error + assert "not found" in x.value.error + return diff --git a/tests/units/utils/test_command_execute3.py b/tests/units/utils/test_command_execute3.py new file mode 100644 index 00000000..6c41df5f --- /dev/null +++ b/tests/units/utils/test_command_execute3.py @@ -0,0 +1,133 @@ +from tests.helpers.global_data import OsOpsDescr +from tests.helpers.global_data import OsOpsDescrs +from tests.helpers.global_data import OsOperations + +from src.utils import execute_utility3 + +from testgres.operations.os_ops import OsCommandResult +from testgres.operations.exceptions import ExecUtilException + +import pytest +import typing + + +class TestUtils__command_execute3: + sm_os_ops_descrs: typing.List[OsOpsDescr] = [ + OsOpsDescrs.sm_local_os_ops_descr, + OsOpsDescrs.sm_remote_os_ops_descr + ] + + @pytest.fixture( + params=[descr.os_ops for descr in sm_os_ops_descrs], + ids=[descr.sign for descr in sm_os_ops_descrs] + ) + def os_ops(self, request: pytest.FixtureRequest) -> OsOperations: + assert isinstance(request, pytest.FixtureRequest) + assert isinstance(request.param, OsOperations) + return request.param + + def test_execute_utility3__log(self, os_ops: OsOperations): + assert isinstance(os_ops, OsOperations) + + log_file: typing.Optional[str] = None + + try: + C_OUT_DATA = "AAAA" + + log_file = os_ops.mkstemp("testgres--") + assert os_ops.path_exists(log_file) + + os_ops.write( + log_file, + C_OUT_DATA + "\n", + truncate=False, + binary=False, + ) + + cmd = ["sh", "-c", "echo BBBB"] + + r = execute_utility3( + os_ops, + cmd, + logfile=log_file, + ) + + assert type(r) is OsCommandResult + assert r.returncode == 0 + assert r.stdout == "BBBB\n" + assert r.stderr == "" + + assert os_ops.path_exists(log_file) + + log_content = os_ops.read( + log_file, + binary=False, + ) + + expected_content_lines = [ + C_OUT_DATA, + "sh -c 'echo BBBB'", + "# BBBB", + "", + ] + + expected_content_s = "\n".join(expected_content_lines) + + assert log_content == expected_content_s + finally: + if log_file is not None: + assert type(log_file) is str + os_ops.remove_file(log_file) + + assert type(log_file) is str + assert not os_ops.path_exists(log_file) + return + + def test_execute_utility3__error__check_false(self, os_ops: OsOperations): + assert isinstance(os_ops, OsOperations) + + cmd = ["sh", "-c", "nonexistent_command"] + + r = execute_utility3( + os_ops, + cmd, + check=False, + ) + + assert type(r) is OsCommandResult + assert type(r.returncode) is int + assert type(r.stdout) is str + assert type(r.stderr) is str + + assert r.returncode == 127 + assert r.stdout == "" + assert "nonexistent_command" in r.stderr + assert "not found" in r.stderr + return + + def test_execute_utility3__error__check_true(self, os_ops: OsOperations): + assert isinstance(os_ops, OsOperations) + + cmd = ["sh", "-c", "nonexistent_command"] + + with pytest.raises(expected_exception=ExecUtilException) as x: + execute_utility3( + os_ops, + cmd, + check=True, + ) + + assert type(x.value) is ExecUtilException + assert type(x.value.exit_code) is int + assert x.value.exit_code == 127 + + assert type(x.value.message) is str + assert type(x.value.out) is str + assert type(x.value.error) is str + + assert x.value.message.startswith("Utility exited with non-zero code (127). Error:") + assert "nonexistent_command" in x.value.message + assert "not found" in x.value.message + assert "nonexistent_command" in x.value.error + assert "not found" in x.value.error + return