From 2c26debe967920e0f2a74d430fe6410a8a9a0513 Mon Sep 17 00:00:00 2001 From: vshepard Date: Tue, 3 Dec 2024 01:30:03 +0100 Subject: [PATCH 01/59] Fix set_auto_conf with single quotes --- testgres/node.py | 13 +++++++++---- tests/test_simple.py | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 4ae30908..be5019a4 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1627,17 +1627,22 @@ def set_auto_conf(self, options, config='postgresql.auto.conf', rm_options={}): name, var = line.partition('=')[::2] name = name.strip() var = var.strip() - var = var.strip('"') - var = var.strip("'") - # remove options specified in rm_options list + # Handle quoted values and remove escaping + if var.startswith("'") and var.endswith("'"): + var = var[1:-1].replace("''", "'") + + # Remove options specified in rm_options list if name in rm_options: continue current_options[name] = var for option in options: - current_options[option] = options[option] + value = options[option] + if isinstance(value, str): + value = value.replace("'", "\\'") + current_options[option] = value auto_conf = '' for option in current_options: diff --git a/tests/test_simple.py b/tests/test_simple.py index 41203a65..a11d7932 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1061,6 +1061,33 @@ def test_simple_with_bin_dir(self): except FileNotFoundError: pass # Expected error + def test_set_auto_conf(self): + with get_new_node() as node: + node.init().start() + + options = { + "archive_command": "cp '%p' \"/mnt/server/archivedir/%f\"", + 'restore_command': 'cp "/mnt/server/archivedir/%f" \'%p\'', + } + + node.set_auto_conf(options) + node.stop() + node.slow_start() + + auto_conf_path = f"{node.data_dir}/postgresql.auto.conf" + with open(auto_conf_path, "r") as f: + content = f.read() + self.assertIn( + "archive_command = 'cp \\'%p\\' \"/mnt/server/archivedir/%f\"", + content, + "archive_command stored wrong" + ) + self.assertIn( + "restore_command = 'cp \"/mnt/server/archivedir/%f\" \\'%p\\''", + content, + "restore_command stored wrong" + ) + if __name__ == '__main__': if os.environ.get('ALT_CONFIG'): From a4092af44fae5a1a92e9c9fbfec449fd99c8e520 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 3 Dec 2024 16:50:47 +0300 Subject: [PATCH 02/59] Node.set_auto_conf is improved - we do not touch existing values - escaping of '\n', '\r', '\t', '\b' and '\\' is added - translation of bool into 'on|off' is added test_set_auto_conf is updated. --- testgres/node.py | 39 +++++++++++++++++++++++++++--------- tests/test_simple.py | 47 +++++++++++++++++++++++++++++++------------- 2 files changed, 63 insertions(+), 23 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index be5019a4..7469b0d6 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1626,11 +1626,6 @@ def set_auto_conf(self, options, config='postgresql.auto.conf', rm_options={}): name, var = line.partition('=')[::2] name = name.strip() - var = var.strip() - - # Handle quoted values and remove escaping - if var.startswith("'") and var.endswith("'"): - var = var[1:-1].replace("''", "'") # Remove options specified in rm_options list if name in rm_options: @@ -1640,14 +1635,18 @@ def set_auto_conf(self, options, config='postgresql.auto.conf', rm_options={}): for option in options: value = options[option] - if isinstance(value, str): - value = value.replace("'", "\\'") + valueType = type(value) + + if valueType == str: + value = __class__._escape_config_value(value) + elif valueType == bool: + value = "on" if value else "off" + current_options[option] = value auto_conf = '' for option in current_options: - auto_conf += "{0} = '{1}'\n".format( - option, current_options[option]) + auto_conf += option + " = " + str(current_options[option]) + "\n" for directive in current_directives: auto_conf += directive + "\n" @@ -1695,6 +1694,28 @@ def _get_bin_path(self, filename): bin_path = get_bin_path(filename) return bin_path + def _escape_config_value(value): + result = "'" + + for ch in value: + if (ch == "'"): + result += "\\'" + elif (ch == "\n"): + result += "\\n" + elif (ch == "\r"): + result += "\\r" + elif (ch == "\t"): + result += "\\t" + elif (ch == "\b"): + result += "\\b" + elif (ch == "\\"): + result += "\\\\" + else: + result += ch + + result += "'" + return result + class NodeApp: diff --git a/tests/test_simple.py b/tests/test_simple.py index a11d7932..ffefda6c 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1062,13 +1062,35 @@ def test_simple_with_bin_dir(self): pass # Expected error def test_set_auto_conf(self): + # elements contain [property id, value, storage value] + testData = [ + ["archive_command", + "cp '%p' \"/mnt/server/archivedir/%f\"", + "'cp \\'%p\\' \"/mnt/server/archivedir/%f\""], + ["restore_command", + 'cp "/mnt/server/archivedir/%f" \'%p\'', + "'cp \"/mnt/server/archivedir/%f\" \\'%p\\''"], + ["log_line_prefix", + "'\n\r\t\b\\\"", + "'\\\'\\n\\r\\t\\b\\\\\""], + ["log_connections", + True, + "on"], + ["log_disconnections", + False, + "off"], + ["autovacuum_max_workers", + 3, + "3"] + ] + with get_new_node() as node: node.init().start() - options = { - "archive_command": "cp '%p' \"/mnt/server/archivedir/%f\"", - 'restore_command': 'cp "/mnt/server/archivedir/%f" \'%p\'', - } + options = {} + + for x in testData: + options[x[0]] = x[1] node.set_auto_conf(options) node.stop() @@ -1077,16 +1099,13 @@ def test_set_auto_conf(self): auto_conf_path = f"{node.data_dir}/postgresql.auto.conf" with open(auto_conf_path, "r") as f: content = f.read() - self.assertIn( - "archive_command = 'cp \\'%p\\' \"/mnt/server/archivedir/%f\"", - content, - "archive_command stored wrong" - ) - self.assertIn( - "restore_command = 'cp \"/mnt/server/archivedir/%f\" \\'%p\\''", - content, - "restore_command stored wrong" - ) + + for x in testData: + self.assertIn( + x[0] + " = " + x[2], + content, + x[0] + " stored wrong" + ) if __name__ == '__main__': From ee7dc91f2f56d80ca975f091c4dd60a004770177 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 3 Dec 2024 19:01:12 +0300 Subject: [PATCH 03/59] Asserts in PostgresNode.set_auto_conf are added Let's control a correct usage of this function. Plus one assert was added in _escape_config_value. --- testgres/node.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/testgres/node.py b/testgres/node.py index 7469b0d6..74a18cdf 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1634,6 +1634,10 @@ def set_auto_conf(self, options, config='postgresql.auto.conf', rm_options={}): current_options[name] = var for option in options: + assert type(option) == str + assert option != "" + assert option.strip() == option + value = options[option] valueType = type(value) @@ -1695,6 +1699,8 @@ def _get_bin_path(self, filename): return bin_path def _escape_config_value(value): + assert type(value) == str + result = "'" for ch in value: From 9dd8dfec44ef56f732761d31c94cf98764a820a8 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 3 Dec 2024 21:11:00 +0300 Subject: [PATCH 04/59] Flake8 E721 is suppressed --- testgres/node.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 74a18cdf..d9fb0604 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1634,7 +1634,7 @@ def set_auto_conf(self, options, config='postgresql.auto.conf', rm_options={}): current_options[name] = var for option in options: - assert type(option) == str + assert type(option) == str # noqa: E721 assert option != "" assert option.strip() == option @@ -1699,7 +1699,7 @@ def _get_bin_path(self, filename): return bin_path def _escape_config_value(value): - assert type(value) == str + assert type(value) == str # noqa: E721 result = "'" From 1335e517e6d8bc16f2d628446c02a03078c5957e Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 4 Dec 2024 10:00:58 +0300 Subject: [PATCH 05/59] PostgresNode::_escape_config_value is updated (code style) --- testgres/node.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index d9fb0604..1706de11 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1704,17 +1704,17 @@ def _escape_config_value(value): result = "'" for ch in value: - if (ch == "'"): + if ch == "'": result += "\\'" - elif (ch == "\n"): + elif ch == "\n": result += "\\n" - elif (ch == "\r"): + elif ch == "\r": result += "\\r" - elif (ch == "\t"): + elif ch == "\t": result += "\\t" - elif (ch == "\b"): + elif ch == "\b": result += "\\b" - elif (ch == "\\"): + elif ch == "\\": result += "\\\\" else: result += ch From 6acbeb6517b6b4aac4d23f0a137821ddb6b7928f Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Fri, 6 Dec 2024 12:32:55 +0300 Subject: [PATCH 06/59] NodeApp::make_simple is refactored (tempfile.gettempdir) - [BUG FIX] Windows does not have "/tmp" directory. Let's use tempfile.gettempdir() - Aggregation of standard and custom options to avoid two calls of node.set_auto_conf --- testgres/node.py | 69 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 18 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 1706de11..0e5bb866 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -5,6 +5,7 @@ import signal import subprocess import threading +import tempfile from queue import Queue import time @@ -1761,6 +1762,8 @@ def make_simple( pg_options={}, checksum=True, bin_dir=None): + assert type(pg_options) == dict # noqa: E721 + if checksum and '--data-checksums' not in initdb_params: initdb_params.append('--data-checksums') node = self.make_empty(base_dir, port, bin_dir=bin_dir) @@ -1773,20 +1776,22 @@ def make_simple( node.major_version = float(node.major_version_str) # Set default parameters - options = {'max_connections': 100, - 'shared_buffers': '10MB', - 'fsync': 'off', - 'wal_level': 'logical', - 'hot_standby': 'off', - 'log_line_prefix': '%t [%p]: [%l-1] ', - 'log_statement': 'none', - 'log_duration': 'on', - 'log_min_duration_statement': 0, - 'log_connections': 'on', - 'log_disconnections': 'on', - 'restart_after_crash': 'off', - 'autovacuum': 'off', - 'unix_socket_directories': '/tmp'} + options = { + 'max_connections': 100, + 'shared_buffers': '10MB', + 'fsync': 'off', + 'wal_level': 'logical', + 'hot_standby': 'off', + 'log_line_prefix': '%t [%p]: [%l-1] ', + 'log_statement': 'none', + 'log_duration': 'on', + 'log_min_duration_statement': 0, + 'log_connections': 'on', + 'log_disconnections': 'on', + 'restart_after_crash': 'off', + 'autovacuum': 'off', + # 'unix_socket_directories': '/tmp', + } # Allow replication in pg_hba.conf if set_replication: @@ -1801,11 +1806,16 @@ def make_simple( else: options['wal_keep_segments'] = '12' - # set default values - node.set_auto_conf(options) - # Apply given parameters - node.set_auto_conf(pg_options) + for x in pg_options: + options[x] = pg_options[x] + + # Define delayed propertyes + if not ("unix_socket_directories" in options.keys()): + options["unix_socket_directories"] = __class__._gettempdir() + + # Set config values + node.set_auto_conf(options) # kludge for testgres # https://github.com/postgrespro/testgres/issues/54 @@ -1814,3 +1824,26 @@ def make_simple( node.set_auto_conf({}, 'postgresql.conf', ['wal_keep_segments']) return node + + def _gettempdir(): + v = tempfile.gettempdir() + + # + # Paranoid checks + # + if type(v) != str: # noqa: E721 + __class__._raise_bugcheck("tempfile.gettempdir returned a value with type {0}.".format(type(v).__name__)) + + if v == "": + __class__._raise_bugcheck("tempfile.gettempdir returned an empty string.") + + if not os.path.exists(v): + __class__._raise_bugcheck("tempfile.gettempdir returned a not exist path [{0}].".format(v)) + + # OK + return v + + def _raise_bugcheck(msg): + assert type(msg) == str # noqa: E721 + assert msg != "" + raise Exception("[BUG CHECK] " + msg) From 9d6f61472817c47477561911c7361d713a1fb1ca Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Fri, 6 Dec 2024 15:22:16 +0300 Subject: [PATCH 07/59] NodeApp::make_simple is updated [comment] --- testgres/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testgres/node.py b/testgres/node.py index 0e5bb866..62509790 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1790,7 +1790,7 @@ def make_simple( 'log_disconnections': 'on', 'restart_after_crash': 'off', 'autovacuum': 'off', - # 'unix_socket_directories': '/tmp', + # unix_socket_directories will be defined later } # Allow replication in pg_hba.conf From 1d44628c65ad9266f303ecbb21080ed790ff01d0 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Fri, 6 Dec 2024 15:47:20 +0300 Subject: [PATCH 08/59] NodeApp::make_simple uses iteritems(pg_options) --- testgres/node.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 62509790..48a100a9 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1807,8 +1807,8 @@ def make_simple( options['wal_keep_segments'] = '12' # Apply given parameters - for x in pg_options: - options[x] = pg_options[x] + for option_name, option_value in iteritems(pg_options): + options[option_name] = option_value # Define delayed propertyes if not ("unix_socket_directories" in options.keys()): From d07104f8885f0d0c9332a608a32fc915017ff891 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Fri, 6 Dec 2024 17:43:41 +0300 Subject: [PATCH 09/59] LocalOperations::_run_command is refactored LocalOperations::_run_command delegates its work into two new methods: - _run_command__nt - _run_command__generic --- testgres/operations/local_ops.py | 76 ++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index a0a9926d..5b7972ae 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -64,47 +64,55 @@ def _process_output(encoding, temp_file_path): output = output.decode(encoding) return output, None # In Windows stderr writing in stdout - 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 - with tempfile.NamedTemporaryFile(mode='w+b', delete=False) as temp_file: - stdout = temp_file - stderr = subprocess.STDOUT - process = subprocess.Popen( - cmd, - shell=shell, - stdin=stdin or subprocess.PIPE if input is not None else None, - stdout=stdout, - stderr=stderr, - ) - if get_process: - return process, None, None - temp_file_path = temp_file.name - - # Wait process finished - process.wait() - - output, error = self._process_output(encoding, temp_file_path) - return process, output, error - else: # Other OS + def _run_command__nt(self, cmd, shell, input, stdin, stdout, stderr, get_process, timeout, encoding): + with tempfile.NamedTemporaryFile(mode='w+b', delete=False) as temp_file: + stdout = temp_file + stderr = subprocess.STDOUT process = subprocess.Popen( cmd, shell=shell, stdin=stdin or subprocess.PIPE if input is not None else None, - stdout=stdout or subprocess.PIPE, - stderr=stderr or subprocess.PIPE, + stdout=stdout, + stderr=stderr, ) if get_process: return process, None, None - try: - output, error = process.communicate(input=input.encode(encoding) if input else None, 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)) + temp_file_path = temp_file.name + + # Wait process finished + process.wait() + + output, error = self._process_output(encoding, temp_file_path) + return process, output, error + + def _run_command__generic(self, cmd, shell, input, stdin, stdout, stderr, get_process, timeout, encoding): + process = subprocess.Popen( + cmd, + shell=shell, + stdin=stdin or subprocess.PIPE if input is not None else None, + stdout=stdout or subprocess.PIPE, + stderr=stderr or subprocess.PIPE, + ) + if get_process: + return process, None, None + try: + output, error = process.communicate(input=input.encode(encoding) if input else None, 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)) + + 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 + method = __class__._run_command__nt + else: # Other OS + method = __class__._run_command__generic + + return method(self, cmd, shell, input, stdin, stdout, stderr, get_process, timeout, encoding) def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, encoding=None, shell=False, text=False, input=None, stdin=None, stdout=None, stderr=None, get_process=False, timeout=None, From cf1d227d7fee878247a26f7f1b8636464d52c576 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sat, 7 Dec 2024 21:40:41 +0300 Subject: [PATCH 10/59] Local and remote test code formatting has been synchronized test_simple.py and test_simple_remote.py have been compared and synchronized. --- tests/test_simple.py | 7 ------- tests/test_simple_remote.py | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/test_simple.py b/tests/test_simple.py index ffefda6c..51cdc896 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -153,7 +153,6 @@ def test_init_unique_system_id(self): with scoped_config(cache_initdb=True, cached_initdb_unique=True) as config: - self.assertTrue(config.cache_initdb) self.assertTrue(config.cached_initdb_unique) @@ -376,13 +375,11 @@ def test_backup_multiple(self): with node.backup(xlog_method='fetch') as backup1, \ node.backup(xlog_method='fetch') as backup2: - self.assertNotEqual(backup1.base_dir, backup2.base_dir) with node.backup(xlog_method='fetch') as backup: with backup.spawn_primary('node1', destroy=False) as node1, \ backup.spawn_primary('node2', destroy=False) as node2: - self.assertNotEqual(node1.base_dir, node2.base_dir) def test_backup_exhaust(self): @@ -390,7 +387,6 @@ def test_backup_exhaust(self): node.init(allow_streaming=True).start() with node.backup(xlog_method='fetch') as backup: - # exhaust backup by creating new node with backup.spawn_primary(): pass @@ -778,7 +774,6 @@ def test_pg_config(self): # modify setting for this scope with scoped_config(cache_pg_config=False) as config: - # sanity check for value self.assertFalse(config.cache_pg_config) @@ -810,7 +805,6 @@ def test_config_stack(self): self.assertEqual(c1.cached_initdb_dir, d1) with scoped_config(cached_initdb_dir=d2) as c2: - stack_size = len(testgres.config.config_stack) # try to break a stack @@ -840,7 +834,6 @@ def test_unix_sockets(self): def test_auto_name(self): with get_new_node().init(allow_streaming=True).start() as m: with m.replicate().start() as r: - # check that nodes are running self.assertTrue(m.status()) self.assertTrue(r.status()) diff --git a/tests/test_simple_remote.py b/tests/test_simple_remote.py index 79bdb74c..936c31f2 100755 --- a/tests/test_simple_remote.py +++ b/tests/test_simple_remote.py @@ -94,7 +94,6 @@ def removing(f): class TestgresRemoteTests(unittest.TestCase): - def test_node_repr(self): with get_remote_node(conn_params=conn_params) as node: pattern = r"PostgresNode\(name='.+', port=.+, base_dir='.+'\)" @@ -748,6 +747,7 @@ def test_pg_config(self): # save right before config change c1 = get_pg_config() + # modify setting for this scope with scoped_config(cache_pg_config=False) as config: # sanity check for value From 22c649dcaa8129bc240d49b24f2e363832e70d9a Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sat, 7 Dec 2024 23:14:00 +0300 Subject: [PATCH 11/59] test_local.py and test_exec_command_failure__expect_error are added test_local.py contains set of test for LocalOperations - test_exec_command_success - test_exec_command_failure - test_exec_command_failure__expect_error Changes in TestRemoteOperations: - test_exec_command_failure exptects an exception - new test test_exec_command_failure__expect_error was added TestRemoteOperations::test_exec_command_failure__expect_error will fail because RemoteOperations::exec_command does not handle the 'expect_error' parameter correctly. --- tests/test_local.py | 46 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_remote.py | 23 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 tests/test_local.py diff --git a/tests/test_local.py b/tests/test_local.py new file mode 100644 index 00000000..1caba74b --- /dev/null +++ b/tests/test_local.py @@ -0,0 +1,46 @@ +import pytest + +from testgres import ExecUtilException +from testgres import LocalOperations + + +class TestLocalOperations: + + @pytest.fixture(scope="function", autouse=True) + def setup(self): + self.operations = LocalOperations() + + def test_exec_command_success(self): + """ + Test exec_command for successful command execution. + """ + cmd = "python3 --version" + response = self.operations.exec_command(cmd, wait_exit=True, shell=True) + + assert b'Python 3.' in response + + def test_exec_command_failure(self): + """ + Test exec_command for command execution failure. + """ + cmd = "nonexistent_command" + while True: + try: + self.operations.exec_command(cmd, wait_exit=True, shell=True) + except ExecUtilException as e: + 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'`" + + def test_exec_command_failure__expect_error(self): + """ + Test exec_command for command execution failure. + """ + cmd = "nonexistent_command" + + exit_status, result, error = self.operations.exec_command(cmd, verbose=True, wait_exit=True, shell=True, expect_error=True) + + assert error == b'/bin/sh: 1: nonexistent_command: not found\n' + assert exit_status == 127 + assert result == b'' diff --git a/tests/test_remote.py b/tests/test_remote.py index e0e4a555..565163f7 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -30,12 +30,27 @@ def test_exec_command_failure(self): Test exec_command for command execution failure. """ cmd = "nonexistent_command" - try: - exit_status, result, error = self.operations.exec_command(cmd, verbose=True, wait_exit=True) - except ExecUtilException as e: - error = e.message + while True: + try: + self.operations.exec_command(cmd, verbose=True, wait_exit=True) + except ExecUtilException as e: + 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' + def test_exec_command_failure__expect_error(self): + """ + Test exec_command for command execution failure. + """ + cmd = "nonexistent_command" + + exit_status, result, error = self.operations.exec_command(cmd, verbose=True, wait_exit=True, shell=True, expect_error=True) + + assert error == b'bash: line 1: nonexistent_command: command not found\n' + assert exit_status == 127 + assert result == b'' + def test_is_executable_true(self): """ Test is_executable for an existing executable. From 45cfbf738e1f5d7f46f8ab053f4a6a481abf53e7 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sat, 7 Dec 2024 23:26:04 +0300 Subject: [PATCH 12/59] RemoteOperations::exec_command must not raise an exception when 'expect_error' is True (#159) This commit fixes an issue #159. --- testgres/operations/remote_ops.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 20095051..88394eb7 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -83,26 +83,26 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, exit_status = process.returncode - if encoding: - result = result.decode(encoding) - error = error.decode(encoding) - - if expect_error: - raise Exception(result, error) + assert type(result) == bytes # noqa: E721 + assert type(error) == bytes # noqa: E721 if not error: - error_found = 0 + error_found = False else: - error = normalize_error(error) error_found = exit_status != 0 or any( - marker in error for marker in ['error', 'Permission denied', 'fatal', 'No such file or directory'] + marker in error for marker in [b'error', b'Permission denied', b'fatal', b'No such file or directory'] ) - if not ignore_errors and error_found: - if isinstance(error, bytes): - message = b"Utility exited with non-zero code. Error: " + error - else: - message = f"Utility exited with non-zero code. Error: {error}" + assert type(error_found) == bool # noqa: E721 + + if encoding: + result = result.decode(encoding) + 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) if verbose: From 1c64337682288201f4c163ad4f12afdca82e33cb Mon Sep 17 00:00:00 2001 From: Dmitry Kovalenko Date: Sun, 8 Dec 2024 00:12:41 +0300 Subject: [PATCH 13/59] TestLocalOperations tests skip Windows --- tests/test_local.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_local.py b/tests/test_local.py index 1caba74b..3493810f 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -1,4 +1,5 @@ import pytest +import platform from testgres import ExecUtilException from testgres import LocalOperations @@ -10,10 +11,16 @@ class TestLocalOperations: def setup(self): self.operations = LocalOperations() + def skip_if_windows(): + if platform.system().lower() == "windows": + pytest.skip("This test does not support Windows.") + def test_exec_command_success(self): """ Test exec_command for successful command execution. """ + __class__.skip_if_windows() + cmd = "python3 --version" response = self.operations.exec_command(cmd, wait_exit=True, shell=True) @@ -23,6 +30,8 @@ def test_exec_command_failure(self): """ Test exec_command for command execution failure. """ + __class__.skip_if_windows() + cmd = "nonexistent_command" while True: try: @@ -37,6 +46,8 @@ def test_exec_command_failure__expect_error(self): """ Test exec_command for command execution failure. """ + __class__.skip_if_windows() + cmd = "nonexistent_command" exit_status, result, error = self.operations.exec_command(cmd, verbose=True, wait_exit=True, shell=True, expect_error=True) From cb87d0a7b1c87a10c85787591c3d9ba4e6687e76 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sun, 8 Dec 2024 15:26:58 +0300 Subject: [PATCH 14/59] tests.helpers.RunConditions is added RunConditions contains the code to check the execution condition of tests. It is used in TestLocalOperations. --- tests/__init__.py | 0 tests/helpers/__init__.py | 0 tests/helpers/run_conditions.py | 11 +++++++++++ tests/test_local.py | 13 +++++-------- 4 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 tests/__init__.py create mode 100644 tests/helpers/__init__.py create mode 100644 tests/helpers/run_conditions.py diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/helpers/__init__.py b/tests/helpers/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/helpers/run_conditions.py b/tests/helpers/run_conditions.py new file mode 100644 index 00000000..8d57f753 --- /dev/null +++ b/tests/helpers/run_conditions.py @@ -0,0 +1,11 @@ +import pytest +import platform + + +class RunConditions: + # It is not a test kit! + __test__ = False + + def skip_if_windows(): + if platform.system().lower() == "windows": + pytest.skip("This test does not support Windows.") diff --git a/tests/test_local.py b/tests/test_local.py index 3493810f..da26468b 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -1,9 +1,10 @@ import pytest -import platform from testgres import ExecUtilException from testgres import LocalOperations +from .helpers.run_conditions import RunConditions + class TestLocalOperations: @@ -11,15 +12,11 @@ class TestLocalOperations: def setup(self): self.operations = LocalOperations() - def skip_if_windows(): - if platform.system().lower() == "windows": - pytest.skip("This test does not support Windows.") - def test_exec_command_success(self): """ Test exec_command for successful command execution. """ - __class__.skip_if_windows() + RunConditions.skip_if_windows() cmd = "python3 --version" response = self.operations.exec_command(cmd, wait_exit=True, shell=True) @@ -30,7 +27,7 @@ def test_exec_command_failure(self): """ Test exec_command for command execution failure. """ - __class__.skip_if_windows() + RunConditions.skip_if_windows() cmd = "nonexistent_command" while True: @@ -46,7 +43,7 @@ def test_exec_command_failure__expect_error(self): """ Test exec_command for command execution failure. """ - __class__.skip_if_windows() + RunConditions.skip_if_windows() cmd = "nonexistent_command" From f9ddd043aceb3ac86f6b63cd7fba00575e0d44b1 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sun, 8 Dec 2024 22:06:29 +0300 Subject: [PATCH 15/59] 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 16/59] [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 17/59] [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 18/59] 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 19/59] 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 20/59] 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 21/59] [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 22/59] [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 23/59] 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 24/59] 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 25/59] 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 26/59] 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 27/59] 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 From 1b7bba4479a98c9d207e0e89b16e8a8269e826a8 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 10 Dec 2024 20:37:56 +0300 Subject: [PATCH 28/59] Fix for TestgresRemoteTests::test_child_pids [thanks to Victoria Shepard] This change was extracted from #149. --- tests/test_simple_remote.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_simple_remote.py b/tests/test_simple_remote.py index 26ac7c61..c8dd2964 100755 --- a/tests/test_simple_remote.py +++ b/tests/test_simple_remote.py @@ -940,6 +940,9 @@ def test_child_pids(self): if pg_version_ge('10'): master_processes.append(ProcessType.LogicalReplicationLauncher) + if pg_version_ge('14'): + master_processes.remove(ProcessType.StatsCollector) + repl_processes = [ ProcessType.Startup, ProcessType.WalReceiver, From 0b1b3de1d7ca370e6b4bcdfec0030e4776065714 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 10 Dec 2024 21:42:25 +0300 Subject: [PATCH 29/59] Formatting It is part of PR #149. --- testgres/node.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index b5cbab27..0faf904b 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -739,13 +739,11 @@ def start(self, params=[], wait=True): if self.is_started: return self - _params = [ - self._get_bin_path("pg_ctl"), - "-D", self.data_dir, - "-l", self.pg_log_file, - "-w" if wait else '-W', # --wait or --no-wait - "start" - ] + params # yapf: disable + _params = [self._get_bin_path("pg_ctl"), + "-D", self.data_dir, + "-l", self.pg_log_file, + "-w" if wait else '-W', # --wait or --no-wait + "start"] + params # yapf: disable startup_retries = 5 while True: From e4c2e07b3ff0c4aa16188a60bb53f108e2d5c0ca Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 14:22:11 +0300 Subject: [PATCH 30/59] reserve_port and release_port are "pointers" to functions for a port numbers management. This need to replace port numbers management in unit tests. --- testgres/utils.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/testgres/utils.py b/testgres/utils.py index a4ee7877..4bd232b1 100644 --- a/testgres/utils.py +++ b/testgres/utils.py @@ -34,7 +34,7 @@ def __init__(self, version: str) -> None: super().__init__(version) -def reserve_port(): +def internal__reserve_port(): """ Generate a new port and add it to 'bound_ports'. """ @@ -45,7 +45,7 @@ def reserve_port(): return port -def release_port(port): +def internal__release_port(port): """ Free port provided by reserve_port(). """ @@ -53,6 +53,10 @@ def release_port(port): bound_ports.discard(port) +reserve_port = internal__reserve_port +release_port = internal__release_port + + def execute_utility(args, logfile=None, verbose=False): """ Execute utility (pg_ctl, pg_dump etc). From 85d2aa3917f8210dce06531b563ec34c42e6b544 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 14:38:39 +0300 Subject: [PATCH 31/59] TestgresTests::test_the_same_port is corrected --- tests/test_simple.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/test_simple.py b/tests/test_simple.py index fade468c..6b04f8bd 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1051,10 +1051,18 @@ def test_parse_pg_version(self): def test_the_same_port(self): with get_new_node() as node: node.init().start() + self.assertTrue(node._should_free_port) + self.assertEqual(type(node.port), int) - with get_new_node() as node2: - node2.port = node.port - node2.init().start() + with get_new_node(port=node.port) as node2: + self.assertEqual(type(node2.port), int) + self.assertEqual(node2.port, node.port) + self.assertFalse(node2._should_free_port) + + with self.assertRaises(StartNodeException) as ctx: + node2.init().start() + + self.assertIn("Cannot start node", str(ctx.exception)) def test_simple_with_bin_dir(self): with get_new_node() as node: From 88371d1a610c62591d3b3ec2d5e88b7a2437ffb9 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 14:46:23 +0300 Subject: [PATCH 32/59] OsOperations::read_binary(self, filename, start_pos) is added It is a specialized function to read binary data from files. --- testgres/operations/local_ops.py | 11 ++++++++ testgres/operations/os_ops.py | 6 +++++ testgres/operations/remote_ops.py | 19 +++++++++++++ tests/test_local.py | 45 +++++++++++++++++++++++++++++++ tests/test_remote.py | 43 +++++++++++++++++++++++++++++ 5 files changed, 124 insertions(+) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 3e8ab8ca..65dc0965 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -308,6 +308,17 @@ def readlines(self, filename, num_lines=0, binary=False, encoding=None): buffers * max(2, int(num_lines / max(cur_lines, 1))) ) # Adjust buffer size + def read_binary(self, filename, start_pos): + assert type(filename) == str # noqa: E721 + assert type(start_pos) == int # noqa: E721 + assert start_pos >= 0 + + with open(filename, 'rb') as file: # open in a binary mode + file.seek(start_pos, os.SEEK_SET) + r = file.read() + assert type(r) == bytes # noqa: E721 + return r + def isfile(self, remote_file): return os.path.isfile(remote_file) diff --git a/testgres/operations/os_ops.py b/testgres/operations/os_ops.py index 34242040..82d44a4e 100644 --- a/testgres/operations/os_ops.py +++ b/testgres/operations/os_ops.py @@ -98,6 +98,12 @@ def read(self, filename, encoding, binary): def readlines(self, filename): raise NotImplementedError() + def read_binary(self, filename, start_pos): + assert type(filename) == str # noqa: E721 + assert type(start_pos) == int # noqa: E721 + assert start_pos >= 0 + raise NotImplementedError() + def isfile(self, remote_file): raise NotImplementedError() diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 00c50d93..9d72731d 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -340,6 +340,16 @@ def readlines(self, filename, num_lines=0, binary=False, encoding=None): return lines + def read_binary(self, filename, start_pos): + assert type(filename) == str # noqa: E721 + assert type(start_pos) == int # noqa: E721 + assert start_pos >= 0 + + cmd = "tail -c +{} {}".format(start_pos + 1, __class__._escape_path(filename)) + r = self.exec_command(cmd) + assert type(r) == bytes # noqa: E721 + return r + def isfile(self, remote_file): stdout = self.exec_command("test -f {}; echo $?".format(remote_file)) result = int(stdout.strip()) @@ -386,6 +396,15 @@ def db_connect(self, dbname, user, password=None, host="localhost", port=5432): ) return conn + def _escape_path(path): + assert type(path) == str # noqa: E721 + assert path != "" # Ok? + + r = "'" + r += path + r += "'" + return r + def normalize_error(error): if isinstance(error, bytes): diff --git a/tests/test_local.py b/tests/test_local.py index cb96a3bc..812b4030 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -1,4 +1,7 @@ +import os + import pytest +import re from testgres import ExecUtilException from testgres import LocalOperations @@ -52,3 +55,45 @@ def test_exec_command_failure__expect_error(self): assert error == b'/bin/sh: 1: nonexistent_command: not found\n' assert exit_status == 127 assert result == b'' + + def test_read_binary__spec(self): + """ + Test LocalOperations::read_binary. + """ + filename = __file__ # current file + + with open(filename, 'rb') as file: # open in a binary mode + response0 = file.read() + + assert type(response0) == bytes # noqa: E721 + + response1 = self.operations.read_binary(filename, 0) + assert type(response1) == bytes # noqa: E721 + assert response1 == response0 + + response2 = self.operations.read_binary(filename, 1) + assert type(response2) == bytes # noqa: E721 + assert len(response2) < len(response1) + assert len(response2) + 1 == len(response1) + assert response2 == response1[1:] + + response3 = self.operations.read_binary(filename, len(response1)) + assert type(response3) == bytes # noqa: E721 + assert len(response3) == 0 + + response4 = self.operations.read_binary(filename, len(response2)) + assert type(response4) == bytes # noqa: E721 + assert len(response4) == 1 + assert response4[0] == response1[len(response1) - 1] + + response5 = self.operations.read_binary(filename, len(response1) + 1) + assert type(response5) == bytes # noqa: E721 + assert len(response5) == 0 + + def test_read_binary__spec__unk_file(self): + """ + Test LocalOperations::read_binary with unknown file. + """ + + with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): + self.operations.read_binary("/dummy", 0) diff --git a/tests/test_remote.py b/tests/test_remote.py index c1a91bc6..c775f72d 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -1,6 +1,7 @@ import os import pytest +import re from testgres import ExecUtilException from testgres import RemoteOperations @@ -181,6 +182,48 @@ def test_read_binary_file(self): assert isinstance(response, bytes) + def test_read_binary__spec(self): + """ + Test RemoteOperations::read_binary. + """ + filename = __file__ # currnt file + + with open(filename, 'rb') as file: # open in a binary mode + response0 = file.read() + + assert type(response0) == bytes # noqa: E721 + + response1 = self.operations.read_binary(filename, 0) + assert type(response1) == bytes # noqa: E721 + assert response1 == response0 + + response2 = self.operations.read_binary(filename, 1) + assert type(response2) == bytes # noqa: E721 + assert len(response2) < len(response1) + assert len(response2) + 1 == len(response1) + assert response2 == response1[1:] + + response3 = self.operations.read_binary(filename, len(response1)) + assert type(response3) == bytes # noqa: E721 + assert len(response3) == 0 + + response4 = self.operations.read_binary(filename, len(response2)) + assert type(response4) == bytes # noqa: E721 + assert len(response4) == 1 + assert response4[0] == response1[len(response1) - 1] + + response5 = self.operations.read_binary(filename, len(response1) + 1) + assert type(response5) == bytes # noqa: E721 + assert len(response5) == 0 + + def test_read_binary__spec__unk_file(self): + """ + Test RemoteOperations::read_binary with unknown file. + """ + + with pytest.raises(ExecUtilException, match=re.escape("tail: cannot open '/dummy' for reading: No such file or directory")): + self.operations.read_binary("/dummy", 0) + def test_touch(self): """ Test touch for creating a new file or updating access and modification times of an existing file. From 4fe189445b0351692b57fc908d366e4dd82cb9e6 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 15:16:48 +0300 Subject: [PATCH 33/59] OsOperations::get_file_size(self, filename) is added It is a function to get a size of file. --- testgres/operations/local_ops.py | 5 +++ testgres/operations/os_ops.py | 3 ++ testgres/operations/remote_ops.py | 64 +++++++++++++++++++++++++++++++ tests/test_local.py | 21 ++++++++++ tests/test_remote.py | 21 ++++++++++ 5 files changed, 114 insertions(+) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 65dc0965..82d1711d 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -325,6 +325,11 @@ def isfile(self, remote_file): def isdir(self, dirname): return os.path.isdir(dirname) + def get_file_size(self, filename): + assert filename is not None + assert type(filename) == str # noqa: E721 + return os.path.getsize(filename) + def remove_file(self, filename): return os.remove(filename) diff --git a/testgres/operations/os_ops.py b/testgres/operations/os_ops.py index 82d44a4e..2ab41246 100644 --- a/testgres/operations/os_ops.py +++ b/testgres/operations/os_ops.py @@ -107,6 +107,9 @@ def read_binary(self, filename, start_pos): def isfile(self, remote_file): raise NotImplementedError() + def get_file_size(self, filename): + raise NotImplementedError() + # Processes control def kill(self, pid, signal): # Kill the process diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 9d72731d..9f88140c 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -360,6 +360,70 @@ def isdir(self, dirname): response = self.exec_command(cmd) return response.strip() == b"True" + def get_file_size(self, filename): + C_ERR_SRC = "RemoteOpertions::get_file_size" + + assert filename is not None + assert type(filename) == str # noqa: E721 + cmd = "du -b " + __class__._escape_path(filename) + + s = self.exec_command(cmd, encoding=get_default_encoding()) + assert type(s) == str # noqa: E721 + + if len(s) == 0: + raise Exception( + "[BUG CHECK] Can't get size of file [{2}]. Remote operation returned an empty string. Check point [{0}][{1}].".format( + C_ERR_SRC, + "#001", + filename + ) + ) + + i = 0 + + while i < len(s) and s[i].isdigit(): + assert s[i] >= '0' + assert s[i] <= '9' + i += 1 + + if i == 0: + raise Exception( + "[BUG CHECK] Can't get size of file [{2}]. Remote operation returned a bad formatted string. Check point [{0}][{1}].".format( + C_ERR_SRC, + "#002", + filename + ) + ) + + if i == len(s): + raise Exception( + "[BUG CHECK] Can't get size of file [{2}]. Remote operation returned a bad formatted string. Check point [{0}][{1}].".format( + C_ERR_SRC, + "#003", + filename + ) + ) + + if not s[i].isspace(): + raise Exception( + "[BUG CHECK] Can't get size of file [{2}]. Remote operation returned a bad formatted string. Check point [{0}][{1}].".format( + C_ERR_SRC, + "#004", + filename + ) + ) + + r = 0 + + for i2 in range(0, i): + ch = s[i2] + assert ch >= '0' + assert ch <= '9' + # Here is needed to check overflow or that it is a human-valid result? + r = (r * 10) + ord(ch) - ord('0') + + return r + def remove_file(self, filename): cmd = "rm {}".format(filename) return self.exec_command(cmd) diff --git a/tests/test_local.py b/tests/test_local.py index 812b4030..a8a0bde0 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -97,3 +97,24 @@ def test_read_binary__spec__unk_file(self): with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): self.operations.read_binary("/dummy", 0) + + def test_get_file_size(self): + """ + Test LocalOperations::get_file_size. + """ + filename = __file__ # current file + + sz0 = os.path.getsize(filename) + assert type(sz0) == int # noqa: E721 + + sz1 = self.operations.get_file_size(filename) + assert type(sz1) == int # noqa: E721 + assert sz1 == sz0 + + def test_get_file_size__unk_file(self): + """ + Test LocalOperations::get_file_size. + """ + + with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): + self.operations.get_file_size("/dummy") diff --git a/tests/test_remote.py b/tests/test_remote.py index c775f72d..be1a56bb 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -224,6 +224,27 @@ def test_read_binary__spec__unk_file(self): with pytest.raises(ExecUtilException, match=re.escape("tail: cannot open '/dummy' for reading: No such file or directory")): self.operations.read_binary("/dummy", 0) + def test_get_file_size(self): + """ + Test LocalOperations::get_file_size. + """ + filename = __file__ # current file + + sz0 = os.path.getsize(filename) + assert type(sz0) == int # noqa: E721 + + sz1 = self.operations.get_file_size(filename) + assert type(sz1) == int # noqa: E721 + assert sz1 == sz0 + + def test_get_file_size__unk_file(self): + """ + Test LocalOperations::get_file_size. + """ + + with pytest.raises(ExecUtilException, match=re.escape("du: cannot access '/dummy': No such file or directory")): + self.operations.get_file_size("/dummy") + def test_touch(self): """ Test touch for creating a new file or updating access and modification times of an existing file. From 28ac4252e5761394b74bb782e29b986f1ffd31d9 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Thu, 12 Dec 2024 15:28:39 +0300 Subject: [PATCH 34/59] Port numbers management is improved (#164) - We don't release a port number that was defined by client - We only check log files to detect port number conflicts - We use slightly smarter log file checking A test is added. --- testgres/node.py | 89 +++++++++++++++++++++++++------- tests/test_simple.py | 117 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+), 17 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 0faf904b..7f5aa648 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -83,13 +83,13 @@ from .standby import First +from . import utils + from .utils import \ PgVer, \ eprint, \ get_bin_path, \ get_pg_version, \ - reserve_port, \ - release_port, \ execute_utility, \ options_string, \ clean_on_error @@ -158,7 +158,7 @@ def __init__(self, name=None, base_dir=None, port=None, conn_params: ConnectionP self.os_ops = LocalOperations(conn_params) self.host = self.os_ops.host - self.port = port or reserve_port() + self.port = port or utils.reserve_port() self.ssh_key = self.os_ops.ssh_key @@ -471,6 +471,28 @@ def _collect_special_files(self): return result + def _collect_log_files(self): + # dictionary of log files + size in bytes + + files = [ + self.pg_log_file + ] # yapf: disable + + result = {} + + for f in files: + # skip missing files + if not self.os_ops.path_exists(f): + continue + + file_size = self.os_ops.get_file_size(f) + assert type(file_size) == int # noqa: E721 + assert file_size >= 0 + + result[f] = file_size + + return result + def init(self, initdb_params=None, cached=True, **kwargs): """ Perform initdb for this node. @@ -722,6 +744,22 @@ def slow_start(self, replica=False, dbname='template1', username=None, max_attem OperationalError}, max_attempts=max_attempts) + def _detect_port_conflict(self, log_files0, log_files1): + assert type(log_files0) == dict # noqa: E721 + assert type(log_files1) == dict # noqa: E721 + + for file in log_files1.keys(): + read_pos = 0 + + if file in log_files0.keys(): + read_pos = log_files0[file] # the previous size + + file_content = self.os_ops.read_binary(file, read_pos) + file_content_s = file_content.decode() + if 'Is another postmaster already running on port' in file_content_s: + return True + return False + def start(self, params=[], wait=True): """ Starts the PostgreSQL node using pg_ctl if node has not been started. @@ -745,27 +783,42 @@ def start(self, params=[], wait=True): "-w" if wait else '-W', # --wait or --no-wait "start"] + params # yapf: disable - startup_retries = 5 + log_files0 = self._collect_log_files() + assert type(log_files0) == dict # noqa: E721 + + nAttempt = 0 + timeout = 1 while True: + nAttempt += 1 try: exit_status, out, error = execute_utility(_params, self.utils_log_file, verbose=True) if error and 'does not exist' in error: raise Exception except Exception as e: - files = self._collect_special_files() - if any(len(file) > 1 and 'Is another postmaster already ' - 'running on port' in file[1].decode() for - file in files): - logging.warning("Detected an issue with connecting to port {0}. " - "Trying another port after a 5-second sleep...".format(self.port)) - self.port = reserve_port() - options = {'port': str(self.port)} - self.set_auto_conf(options) - startup_retries -= 1 - time.sleep(5) - continue + if self._should_free_port and nAttempt < 5: + log_files1 = self._collect_log_files() + if self._detect_port_conflict(log_files0, log_files1): + log_files0 = log_files1 + logging.warning( + "Detected an issue with connecting to port {0}. " + "Trying another port after a {1}-second sleep...".format(self.port, timeout) + ) + time.sleep(timeout) + timeout = min(2 * timeout, 5) + cur_port = self.port + new_port = utils.reserve_port() # throw + try: + options = {'port': str(new_port)} + self.set_auto_conf(options) + except: # noqa: E722 + utils.release_port(new_port) + raise + self.port = new_port + utils.release_port(cur_port) + continue msg = 'Cannot start node' + files = self._collect_special_files() raise_from(StartNodeException(msg, files), e) break self._maybe_start_logger() @@ -930,8 +983,10 @@ def free_port(self): """ if self._should_free_port: + port = self.port self._should_free_port = False - release_port(self.port) + self.port = None + utils.release_port(port) def cleanup(self, max_attempts=3, full=False): """ diff --git a/tests/test_simple.py b/tests/test_simple.py index 6b04f8bd..0a09135c 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1064,6 +1064,123 @@ def test_the_same_port(self): self.assertIn("Cannot start node", str(ctx.exception)) + class tagPortManagerProxy: + sm_prev_testgres_reserve_port = None + sm_prev_testgres_release_port = None + + sm_DummyPortNumber = None + sm_DummyPortMaxUsage = None + + sm_DummyPortCurrentUsage = None + sm_DummyPortTotalUsage = None + + def __init__(self, dummyPortNumber, dummyPortMaxUsage): + assert type(dummyPortNumber) == int # noqa: E721 + assert type(dummyPortMaxUsage) == int # noqa: E721 + assert dummyPortNumber >= 0 + assert dummyPortMaxUsage >= 0 + + assert __class__.sm_prev_testgres_reserve_port is None + assert __class__.sm_prev_testgres_release_port is None + assert testgres.utils.reserve_port == testgres.utils.internal__reserve_port + assert testgres.utils.release_port == testgres.utils.internal__release_port + + __class__.sm_prev_testgres_reserve_port = testgres.utils.reserve_port + __class__.sm_prev_testgres_release_port = testgres.utils.release_port + + testgres.utils.reserve_port = __class__._proxy__reserve_port + testgres.utils.release_port = __class__._proxy__release_port + + assert testgres.utils.reserve_port == __class__._proxy__reserve_port + assert testgres.utils.release_port == __class__._proxy__release_port + + __class__.sm_DummyPortNumber = dummyPortNumber + __class__.sm_DummyPortMaxUsage = dummyPortMaxUsage + + __class__.sm_DummyPortCurrentUsage = 0 + __class__.sm_DummyPortTotalUsage = 0 + + def __enter__(self): + return self + + def __exit__(self, type, value, traceback): + assert __class__.sm_DummyPortCurrentUsage == 0 + + assert __class__.sm_prev_testgres_reserve_port is not None + assert __class__.sm_prev_testgres_release_port is not None + + assert testgres.utils.reserve_port == __class__._proxy__reserve_port + assert testgres.utils.release_port == __class__._proxy__release_port + + testgres.utils.reserve_port = __class__.sm_prev_testgres_reserve_port + testgres.utils.release_port = __class__.sm_prev_testgres_release_port + + __class__.sm_prev_testgres_reserve_port = None + __class__.sm_prev_testgres_release_port = None + + def _proxy__reserve_port(): + assert type(__class__.sm_DummyPortMaxUsage) == int # noqa: E721 + assert type(__class__.sm_DummyPortTotalUsage) == int # noqa: E721 + assert type(__class__.sm_DummyPortCurrentUsage) == int # noqa: E721 + assert __class__.sm_DummyPortTotalUsage >= 0 + assert __class__.sm_DummyPortCurrentUsage >= 0 + + assert __class__.sm_DummyPortTotalUsage <= __class__.sm_DummyPortMaxUsage + assert __class__.sm_DummyPortCurrentUsage <= __class__.sm_DummyPortTotalUsage + + assert __class__.sm_prev_testgres_reserve_port is not None + + if __class__.sm_DummyPortTotalUsage == __class__.sm_DummyPortMaxUsage: + return __class__.sm_prev_testgres_reserve_port() + + __class__.sm_DummyPortTotalUsage += 1 + __class__.sm_DummyPortCurrentUsage += 1 + return __class__.sm_DummyPortNumber + + def _proxy__release_port(dummyPortNumber): + assert type(dummyPortNumber) == int # noqa: E721 + + assert type(__class__.sm_DummyPortMaxUsage) == int # noqa: E721 + assert type(__class__.sm_DummyPortTotalUsage) == int # noqa: E721 + assert type(__class__.sm_DummyPortCurrentUsage) == int # noqa: E721 + assert __class__.sm_DummyPortTotalUsage >= 0 + assert __class__.sm_DummyPortCurrentUsage >= 0 + + assert __class__.sm_DummyPortTotalUsage <= __class__.sm_DummyPortMaxUsage + assert __class__.sm_DummyPortCurrentUsage <= __class__.sm_DummyPortTotalUsage + + assert __class__.sm_prev_testgres_release_port is not None + + if __class__.sm_DummyPortCurrentUsage > 0 and dummyPortNumber == __class__.sm_DummyPortNumber: + assert __class__.sm_DummyPortTotalUsage > 0 + __class__.sm_DummyPortCurrentUsage -= 1 + return + + return __class__.sm_prev_testgres_release_port(dummyPortNumber) + + def test_port_rereserve_during_node_start(self): + C_COUNT_OF_BAD_PORT_USAGE = 3 + + with get_new_node() as node1: + node1.init().start() + self.assertTrue(node1._should_free_port) + self.assertEqual(type(node1.port), int) # noqa: E721 + node1.safe_psql("SELECT 1;") + + with __class__.tagPortManagerProxy(node1.port, C_COUNT_OF_BAD_PORT_USAGE): + assert __class__.tagPortManagerProxy.sm_DummyPortNumber == node1.port + with get_new_node() as node2: + self.assertTrue(node2._should_free_port) + self.assertEqual(node2.port, node1.port) + + node2.init().start() + + self.assertNotEqual(node2.port, node1.port) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortCurrentUsage, 0) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortTotalUsage, C_COUNT_OF_BAD_PORT_USAGE) + + node2.safe_psql("SELECT 1;") + def test_simple_with_bin_dir(self): with get_new_node() as node: node.init().start() From 663612cb870fbeade43db3a0c6f771127fb650a2 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Sat, 14 Dec 2024 11:07:35 +0300 Subject: [PATCH 35/59] PostgresNode._C_MAX_START_ATEMPTS=5 is added (+ 1 new test) Also - TestgresTests.test_the_same_port is updated - TestgresTests.test_port_rereserve_during_node_start is updated - TestgresTests.test_port_conflict is added --- testgres/node.py | 14 +++++++++-- tests/test_simple.py | 58 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 7f5aa648..554c226d 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -128,6 +128,9 @@ def __repr__(self): class PostgresNode(object): + # a max number of node start attempts + _C_MAX_START_ATEMPTS = 5 + def __init__(self, name=None, base_dir=None, port=None, conn_params: ConnectionParams = ConnectionParams(), bin_dir=None, prefix=None): """ PostgresNode constructor. @@ -774,6 +777,9 @@ def start(self, params=[], wait=True): Returns: This instance of :class:`.PostgresNode`. """ + + assert __class__._C_MAX_START_ATEMPTS > 1 + if self.is_started: return self @@ -789,13 +795,17 @@ def start(self, params=[], wait=True): nAttempt = 0 timeout = 1 while True: + assert nAttempt >= 0 + assert nAttempt < __class__._C_MAX_START_ATEMPTS nAttempt += 1 try: exit_status, out, error = execute_utility(_params, self.utils_log_file, verbose=True) if error and 'does not exist' in error: raise Exception except Exception as e: - if self._should_free_port and nAttempt < 5: + assert nAttempt > 0 + assert nAttempt <= __class__._C_MAX_START_ATEMPTS + if self._should_free_port and nAttempt < __class__._C_MAX_START_ATEMPTS: log_files1 = self._collect_log_files() if self._detect_port_conflict(log_files0, log_files1): log_files0 = log_files1 @@ -806,7 +816,7 @@ def start(self, params=[], wait=True): time.sleep(timeout) timeout = min(2 * timeout, 5) cur_port = self.port - new_port = utils.reserve_port() # throw + new_port = utils.reserve_port() # can raise try: options = {'port': str(new_port)} self.set_auto_conf(options) diff --git a/tests/test_simple.py b/tests/test_simple.py index 0a09135c..93968466 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1053,6 +1053,8 @@ def test_the_same_port(self): node.init().start() self.assertTrue(node._should_free_port) self.assertEqual(type(node.port), int) + node_port_copy = node.port + self.assertEqual(node.safe_psql("SELECT 1;"), b'1\n') with get_new_node(port=node.port) as node2: self.assertEqual(type(node2.port), int) @@ -1064,6 +1066,11 @@ def test_the_same_port(self): self.assertIn("Cannot start node", str(ctx.exception)) + # node is still working + self.assertEqual(node.port, node_port_copy) + self.assertTrue(node._should_free_port) + self.assertEqual(node.safe_psql("SELECT 3;"), b'3\n') + class tagPortManagerProxy: sm_prev_testgres_reserve_port = None sm_prev_testgres_release_port = None @@ -1159,13 +1166,16 @@ def _proxy__release_port(dummyPortNumber): return __class__.sm_prev_testgres_release_port(dummyPortNumber) def test_port_rereserve_during_node_start(self): + assert testgres.PostgresNode._C_MAX_START_ATEMPTS == 5 + C_COUNT_OF_BAD_PORT_USAGE = 3 with get_new_node() as node1: node1.init().start() self.assertTrue(node1._should_free_port) self.assertEqual(type(node1.port), int) # noqa: E721 - node1.safe_psql("SELECT 1;") + node1_port_copy = node1.port + self.assertEqual(node1.safe_psql("SELECT 1;"), b'1\n') with __class__.tagPortManagerProxy(node1.port, C_COUNT_OF_BAD_PORT_USAGE): assert __class__.tagPortManagerProxy.sm_DummyPortNumber == node1.port @@ -1176,10 +1186,54 @@ def test_port_rereserve_during_node_start(self): node2.init().start() self.assertNotEqual(node2.port, node1.port) + self.assertTrue(node2._should_free_port) self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortCurrentUsage, 0) self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortTotalUsage, C_COUNT_OF_BAD_PORT_USAGE) + self.assertTrue(node2.is_started) + + self.assertEqual(node2.safe_psql("SELECT 2;"), b'2\n') + + # node1 is still working + self.assertEqual(node1.port, node1_port_copy) + self.assertTrue(node1._should_free_port) + self.assertEqual(node1.safe_psql("SELECT 3;"), b'3\n') + + def test_port_conflict(self): + assert testgres.PostgresNode._C_MAX_START_ATEMPTS > 1 + + C_COUNT_OF_BAD_PORT_USAGE = testgres.PostgresNode._C_MAX_START_ATEMPTS + + with get_new_node() as node1: + node1.init().start() + self.assertTrue(node1._should_free_port) + self.assertEqual(type(node1.port), int) # noqa: E721 + node1_port_copy = node1.port + self.assertEqual(node1.safe_psql("SELECT 1;"), b'1\n') + + with __class__.tagPortManagerProxy(node1.port, C_COUNT_OF_BAD_PORT_USAGE): + assert __class__.tagPortManagerProxy.sm_DummyPortNumber == node1.port + with get_new_node() as node2: + self.assertTrue(node2._should_free_port) + self.assertEqual(node2.port, node1.port) + + with self.assertRaises(StartNodeException) as ctx: + node2.init().start() + + self.assertIn("Cannot start node", str(ctx.exception)) + + self.assertEqual(node2.port, node1.port) + self.assertTrue(node2._should_free_port) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortCurrentUsage, 1) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortTotalUsage, C_COUNT_OF_BAD_PORT_USAGE) + self.assertFalse(node2.is_started) + + # node2 must release our dummyPort (node1.port) + self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortCurrentUsage, 0) - node2.safe_psql("SELECT 1;") + # node1 is still working + self.assertEqual(node1.port, node1_port_copy) + self.assertTrue(node1._should_free_port) + self.assertEqual(node1.safe_psql("SELECT 3;"), b'3\n') def test_simple_with_bin_dir(self): with get_new_node() as node: From 2f3fc40d39562ffa62e77863df8316becd353dee Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 18 Dec 2024 21:54:32 +0300 Subject: [PATCH 36/59] OsOperations.isdir is added It a correction of base interface - OsOperations. It seems we forgot to add a declaration of the "abstract" method "isdir". --- testgres/operations/os_ops.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testgres/operations/os_ops.py b/testgres/operations/os_ops.py index 2ab41246..d644509a 100644 --- a/testgres/operations/os_ops.py +++ b/testgres/operations/os_ops.py @@ -107,6 +107,9 @@ def read_binary(self, filename, start_pos): def isfile(self, remote_file): raise NotImplementedError() + def isdir(self, dirname): + raise NotImplementedError() + def get_file_size(self, filename): raise NotImplementedError() From 9f527435b28b3c9655d66ef7a2b5a120d767786d Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 18 Dec 2024 21:57:37 +0300 Subject: [PATCH 37/59] Tests for LocalOperations methods isdir and isfile are added --- tests/test_local.py | 64 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/test_local.py b/tests/test_local.py index a8a0bde0..e223b090 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -118,3 +118,67 @@ def test_get_file_size__unk_file(self): with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): self.operations.get_file_size("/dummy") + + def test_isfile_true(self): + """ + Test isfile for an existing file. + """ + filename = __file__ + + response = self.operations.isfile(filename) + + assert response is True + + def test_isfile_false__not_exist(self): + """ + Test isfile for a non-existing file. + """ + filename = os.path.join(os.path.dirname(__file__), "nonexistent_file.txt") + + response = self.operations.isfile(filename) + + assert response is False + + def test_isfile_false__directory(self): + """ + Test isfile for a firectory. + """ + name = os.path.dirname(__file__) + + assert self.operations.isdir(name) + + response = self.operations.isfile(name) + + assert response is False + + def test_isdir_true(self): + """ + Test isdir for an existing directory. + """ + name = os.path.dirname(__file__) + + response = self.operations.isdir(name) + + assert response is True + + def test_isdir_false__not_exist(self): + """ + Test isdir for a non-existing directory. + """ + name = os.path.join(os.path.dirname(__file__), "it_is_nonexistent_directory") + + response = self.operations.isdir(name) + + assert response is False + + def test_isdir_false__file(self): + """ + Test isdir for a file. + """ + name = __file__ + + assert self.operations.isfile(name) + + response = self.operations.isdir(name) + + assert response is False From 38ce127ec63189d4a35d1f73232435538329ef93 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 18 Dec 2024 21:58:30 +0300 Subject: [PATCH 38/59] Tests for RemoteOperations methods isdir and isfile are updated --- tests/test_remote.py | 50 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/tests/test_remote.py b/tests/test_remote.py index be1a56bb..67d66549 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -259,18 +259,62 @@ def test_isfile_true(self): """ Test isfile for an existing file. """ - filename = "/etc/hosts" + filename = __file__ response = self.operations.isfile(filename) assert response is True - def test_isfile_false(self): + def test_isfile_false__not_exist(self): """ Test isfile for a non-existing file. """ - filename = "/nonexistent_file.txt" + filename = os.path.join(os.path.dirname(__file__), "nonexistent_file.txt") response = self.operations.isfile(filename) assert response is False + + def test_isfile_false__directory(self): + """ + Test isfile for a firectory. + """ + name = os.path.dirname(__file__) + + assert self.operations.isdir(name) + + response = self.operations.isfile(name) + + assert response is False + + def test_isdir_true(self): + """ + Test isdir for an existing directory. + """ + name = os.path.dirname(__file__) + + response = self.operations.isdir(name) + + assert response is True + + def test_isdir_false__not_exist(self): + """ + Test isdir for a non-existing directory. + """ + name = os.path.join(os.path.dirname(__file__), "it_is_nonexistent_directory") + + response = self.operations.isdir(name) + + assert response is False + + def test_isdir_false__file(self): + """ + Test isdir for a file. + """ + name = __file__ + + assert self.operations.isfile(name) + + response = self.operations.isdir(name) + + assert response is False From 93d1122d0f59575a5b7f7b316f18d2824ecacc62 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 24 Dec 2024 11:51:52 +0300 Subject: [PATCH 39/59] Formatting --- testgres/node.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/testgres/node.py b/testgres/node.py index 554c226d..dff47cf6 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -131,7 +131,8 @@ class PostgresNode(object): # a max number of node start attempts _C_MAX_START_ATEMPTS = 5 - def __init__(self, name=None, base_dir=None, port=None, conn_params: ConnectionParams = ConnectionParams(), bin_dir=None, prefix=None): + def __init__(self, name=None, base_dir=None, port=None, conn_params: ConnectionParams = ConnectionParams(), + bin_dir=None, prefix=None): """ PostgresNode constructor. From 00fd9257158cd9b0aa7450b66762ad9c2c31f08b Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 24 Dec 2024 14:14:49 +0300 Subject: [PATCH 40/59] Node.start is refactored [Victoria Shepard' ideas are used, #149] - Save an orignal text of 'does not exist' error - When we reach maximum retry attempt of restarts - We log an error message - We raise exception "Cannot start node after multiple attempts" - A new port number is not tranlating into string - Reorganization TestgresTests.test_port_conflict is updated. --- testgres/node.py | 100 +++++++++++++++++++++++++++---------------- tests/test_simple.py | 2 +- 2 files changed, 63 insertions(+), 39 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index dff47cf6..7121339f 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -790,48 +790,72 @@ def start(self, params=[], wait=True): "-w" if wait else '-W', # --wait or --no-wait "start"] + params # yapf: disable - log_files0 = self._collect_log_files() - assert type(log_files0) == dict # noqa: E721 + def LOCAL__start_node(): + _, _, error = execute_utility(_params, self.utils_log_file, verbose=True) + assert type(error) == str # noqa: E721 + if error and 'does not exist' in error: + raise Exception(error) - nAttempt = 0 - timeout = 1 - while True: - assert nAttempt >= 0 - assert nAttempt < __class__._C_MAX_START_ATEMPTS - nAttempt += 1 + def LOCAL__raise_cannot_start_node(from_exception, msg): + assert isinstance(from_exception, Exception) + assert type(msg) == str # noqa: E721 + files = self._collect_special_files() + raise_from(StartNodeException(msg, files), from_exception) + + def LOCAL__raise_cannot_start_node__std(from_exception): + assert isinstance(from_exception, Exception) + LOCAL__raise_cannot_start_node(from_exception, 'Cannot start node') + + if not self._should_free_port: try: - exit_status, out, error = execute_utility(_params, self.utils_log_file, verbose=True) - if error and 'does not exist' in error: - raise Exception + LOCAL__start_node() except Exception as e: - assert nAttempt > 0 - assert nAttempt <= __class__._C_MAX_START_ATEMPTS - if self._should_free_port and nAttempt < __class__._C_MAX_START_ATEMPTS: + LOCAL__raise_cannot_start_node__std(e) + else: + assert self._should_free_port + assert __class__._C_MAX_START_ATEMPTS > 1 + + log_files0 = self._collect_log_files() + assert type(log_files0) == dict # noqa: E721 + + nAttempt = 0 + timeout = 1 + while True: + assert nAttempt >= 0 + assert nAttempt < __class__._C_MAX_START_ATEMPTS + nAttempt += 1 + try: + LOCAL__start_node() + except Exception as e: + assert nAttempt > 0 + assert nAttempt <= __class__._C_MAX_START_ATEMPTS + if nAttempt == __class__._C_MAX_START_ATEMPTS: + logging.error("Reached maximum retry attempts. Unable to start node.") + LOCAL__raise_cannot_start_node(e, "Cannot start node after multiple attempts") + log_files1 = self._collect_log_files() - if self._detect_port_conflict(log_files0, log_files1): - log_files0 = log_files1 - logging.warning( - "Detected an issue with connecting to port {0}. " - "Trying another port after a {1}-second sleep...".format(self.port, timeout) - ) - time.sleep(timeout) - timeout = min(2 * timeout, 5) - cur_port = self.port - new_port = utils.reserve_port() # can raise - try: - options = {'port': str(new_port)} - self.set_auto_conf(options) - except: # noqa: E722 - utils.release_port(new_port) - raise - self.port = new_port - utils.release_port(cur_port) - continue - - msg = 'Cannot start node' - files = self._collect_special_files() - raise_from(StartNodeException(msg, files), e) - break + if not self._detect_port_conflict(log_files0, log_files1): + LOCAL__raise_cannot_start_node__std(e) + + log_files0 = log_files1 + logging.warning( + "Detected a conflict with using the port {0}. " + "Trying another port after a {1}-second sleep...".format(self.port, timeout) + ) + time.sleep(timeout) + timeout = min(2 * timeout, 5) + cur_port = self.port + new_port = utils.reserve_port() # can raise + try: + options = {'port': str(new_port)} + self.set_auto_conf(options) + except: # noqa: E722 + utils.release_port(new_port) + raise + self.port = new_port + utils.release_port(cur_port) + continue + break self._maybe_start_logger() self.is_started = True return self diff --git a/tests/test_simple.py b/tests/test_simple.py index 93968466..9cf29c64 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1219,7 +1219,7 @@ def test_port_conflict(self): with self.assertRaises(StartNodeException) as ctx: node2.init().start() - self.assertIn("Cannot start node", str(ctx.exception)) + self.assertIn("Cannot start node after multiple attempts", str(ctx.exception)) self.assertEqual(node2.port, node1.port) self.assertTrue(node2._should_free_port) From 2fa4426095acda3caadbdd47f6b029d699c0a0aa Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 24 Dec 2024 17:19:04 +0300 Subject: [PATCH 41/59] Formatting --- testgres/operations/remote_ops.py | 1 - 1 file changed, 1 deletion(-) diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 9f88140c..3aa2d8c4 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -39,7 +39,6 @@ def cmdline(self): class RemoteOperations(OsOperations): def __init__(self, conn_params: ConnectionParams): - if not platform.system().lower() == "linux": raise EnvironmentError("Remote operations are supported only on Linux!") From a33860d0cb625516535cf5814cac2e89562a5f70 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 24 Dec 2024 20:47:20 +0300 Subject: [PATCH 42/59] PSQL passes a database name through the explicit '-d ' parameter --- testgres/node.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index 7121339f..f7d6e839 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1107,13 +1107,12 @@ def _psql( else: raise Exception("Input data must be None or bytes.") - dbname = dbname or default_dbname() - psql_params = [ self._get_bin_path("psql"), "-p", str(self.port), "-h", self.host, "-U", username or self.os_ops.username, + "-d", dbname or default_dbname(), "-X", # no .psqlrc "-A", # unaligned output "-t", # print rows only @@ -1135,9 +1134,6 @@ def _psql( else: raise QueryException('Query or filename must be provided') - # should be the last one - psql_params.append(dbname) - return self.os_ops.exec_command( psql_params, verbose=True, From 3ca3ff7eef2687c717c3cbcedc04f37b98ec7ed9 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Tue, 24 Dec 2024 22:43:39 +0300 Subject: [PATCH 43/59] Ssh command line in RemoteOperations::execute is corrected We use subprocess.list2cmdline(cmd) to pack a user command line. It allows PostgresNode::_psql to build PSQL command line without a "special case" for remote-host. Also RemoteOperations::execute raises exception if 'cmd' parameter has an unknown type. --- testgres/node.py | 5 +---- testgres/operations/remote_ops.py | 5 ++++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/testgres/node.py b/testgres/node.py index f7d6e839..3d023399 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -1125,10 +1125,7 @@ def _psql( # select query source if query: - if self.os_ops.remote: - psql_params.extend(("-c", '"{}"'.format(query))) - else: - psql_params.extend(("-c", query)) + psql_params.extend(("-c", query)) elif filename: psql_params.extend(("-f", filename)) else: diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 3aa2d8c4..fb5dd4b2 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -80,7 +80,10 @@ def exec_command(self, cmd, wait_exit=False, verbose=False, expect_error=False, 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 + ssh_cmd = ['ssh', self.ssh_dest] + self.ssh_args + [subprocess.list2cmdline(cmd)] + else: + raise ValueError("Invalid 'cmd' argument type - {0}".format(type(cmd).__name__)) + process = subprocess.Popen(ssh_cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) assert not (process is None) if get_process: From 7c0f1846205c89acc96139fca50b24b76dad4a28 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 25 Dec 2024 12:33:38 +0300 Subject: [PATCH 44/59] RemoteOperations is updated (read_binary, get_file_size) get_file_size and get_file_size use a list for command list arguments. It allows to use standard way to escape a filename. Our bicycle "_escape_path" is deleted. --- testgres/operations/remote_ops.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index fb5dd4b2..128a2a21 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -347,7 +347,7 @@ def read_binary(self, filename, start_pos): assert type(start_pos) == int # noqa: E721 assert start_pos >= 0 - cmd = "tail -c +{} {}".format(start_pos + 1, __class__._escape_path(filename)) + cmd = ["tail", "-c", "+{}".format(start_pos + 1), filename] r = self.exec_command(cmd) assert type(r) == bytes # noqa: E721 return r @@ -367,7 +367,7 @@ def get_file_size(self, filename): assert filename is not None assert type(filename) == str # noqa: E721 - cmd = "du -b " + __class__._escape_path(filename) + cmd = ["du", "-b", filename] s = self.exec_command(cmd, encoding=get_default_encoding()) assert type(s) == str # noqa: E721 @@ -462,15 +462,6 @@ def db_connect(self, dbname, user, password=None, host="localhost", port=5432): ) return conn - def _escape_path(path): - assert type(path) == str # noqa: E721 - assert path != "" # Ok? - - r = "'" - r += path - r += "'" - return r - def normalize_error(error): if isinstance(error, bytes): From 5b263f3c6231a98de7a72c1bf9445989e00a7f62 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 25 Dec 2024 13:43:25 +0300 Subject: [PATCH 45/59] Comments are corrected --- tests/test_remote.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_remote.py b/tests/test_remote.py index 67d66549..780ad46e 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -226,7 +226,7 @@ def test_read_binary__spec__unk_file(self): def test_get_file_size(self): """ - Test LocalOperations::get_file_size. + Test RemoteOperations::get_file_size. """ filename = __file__ # current file @@ -239,7 +239,7 @@ def test_get_file_size(self): def test_get_file_size__unk_file(self): """ - Test LocalOperations::get_file_size. + Test RemoteOperations::get_file_size. """ with pytest.raises(ExecUtilException, match=re.escape("du: cannot access '/dummy': No such file or directory")): From 4e23e030cfb8b2531cae5abfd4f471809e7f7371 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 25 Dec 2024 13:50:05 +0300 Subject: [PATCH 46/59] OsOps::read methods were corrected Now they always read a file as binary. When 'binary' parameter is False we will use 'encoding' parameter to decode bytes into string. Binary read does not allow an usage of 'encoding' parameter (InvalidOperationException is raised). New tests are added. --- testgres/operations/local_ops.py | 36 ++++++++++++++--- testgres/operations/remote_ops.py | 35 +++++++++++++--- tests/test_local.py | 66 ++++++++++++++++++++++++++++++- tests/test_remote.py | 64 ++++++++++++++++++++++++++++++ 4 files changed, 189 insertions(+), 12 deletions(-) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index 82d1711d..d6013ab5 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -10,6 +10,7 @@ import psutil from ..exceptions import ExecUtilException +from ..exceptions import InvalidOperationException from .os_ops import ConnectionParams, OsOperations, pglib, get_default_encoding from .raise_error import RaiseError from .helpers import Helpers @@ -266,13 +267,36 @@ def touch(self, filename): os.utime(filename, None) def read(self, filename, encoding=None, binary=False): - mode = "rb" if binary else "r" - with open(filename, mode) as file: + assert type(filename) == str # noqa: E721 + assert encoding is None or type(encoding) == str # noqa: E721 + assert type(binary) == bool # noqa: E721 + + if binary: + if encoding is not None: + raise InvalidOperationException("Enconding is not allowed for read binary operation") + + return self._read__binary(filename) + + # python behavior + assert None or "abc" == "abc" + assert "" or "abc" == "abc" + + return self._read__text_with_encoding(filename, encoding or get_default_encoding()) + + def _read__text_with_encoding(self, filename, encoding): + assert type(filename) == str # noqa: E721 + assert type(encoding) == str # noqa: E721 + content = self._read__binary(filename) + assert type(content) == bytes # noqa: E721 + content_s = content.decode(encoding) + assert type(content_s) == str # noqa: E721 + return content_s + + def _read__binary(self, filename): + assert type(filename) == str # noqa: E721 + with open(filename, 'rb') as file: # open in a binary mode content = file.read() - if binary: - return content - if isinstance(content, bytes): - return content.decode(encoding or get_default_encoding()) + assert type(content) == bytes # noqa: E721 return content def readlines(self, filename, num_lines=0, binary=False, encoding=None): diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index 128a2a21..abcb8fe1 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 ..exceptions import InvalidOperationException from .os_ops import OsOperations, ConnectionParams, get_default_encoding from .raise_error import RaiseError from .helpers import Helpers @@ -319,13 +320,37 @@ def touch(self, filename): self.exec_command("touch {}".format(filename)) def read(self, filename, binary=False, encoding=None): - cmd = "cat {}".format(filename) - result = self.exec_command(cmd, encoding=encoding) + assert type(filename) == str # noqa: E721 + assert encoding is None or type(encoding) == str # noqa: E721 + assert type(binary) == bool # noqa: E721 - if not binary and result: - result = result.decode(encoding or get_default_encoding()) + if binary: + if encoding is not None: + raise InvalidOperationException("Enconding is not allowed for read binary operation") - return result + return self._read__binary(filename) + + # python behavior + assert None or "abc" == "abc" + assert "" or "abc" == "abc" + + return self._read__text_with_encoding(filename, encoding or get_default_encoding()) + + def _read__text_with_encoding(self, filename, encoding): + assert type(filename) == str # noqa: E721 + assert type(encoding) == str # noqa: E721 + content = self._read__binary(filename) + assert type(content) == bytes # noqa: E721 + content_s = content.decode(encoding) + assert type(content_s) == str # noqa: E721 + return content_s + + def _read__binary(self, filename): + assert type(filename) == str # noqa: E721 + cmd = ["cat", filename] + content = self.exec_command(cmd) + assert type(content) == bytes # noqa: E721 + return content def readlines(self, filename, num_lines=0, binary=False, encoding=None): if num_lines > 0: diff --git a/tests/test_local.py b/tests/test_local.py index e223b090..47a63994 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -4,6 +4,7 @@ import re from testgres import ExecUtilException +from testgres import InvalidOperationException from testgres import LocalOperations from .helpers.run_conditions import RunConditions @@ -56,6 +57,67 @@ def test_exec_command_failure__expect_error(self): assert exit_status == 127 assert result == b'' + def test_read__text(self): + """ + Test LocalOperations::read for text data. + """ + filename = __file__ # current file + + with open(filename, 'r') as file: # open in a text mode + response0 = file.read() + + assert type(response0) == str # noqa: E721 + + response1 = self.operations.read(filename) + assert type(response1) == str # noqa: E721 + assert response1 == response0 + + response2 = self.operations.read(filename, encoding=None, binary=False) + assert type(response2) == str # noqa: E721 + assert response2 == response0 + + response3 = self.operations.read(filename, encoding="") + assert type(response3) == str # noqa: E721 + assert response3 == response0 + + response4 = self.operations.read(filename, encoding="UTF-8") + assert type(response4) == str # noqa: E721 + assert response4 == response0 + + def test_read__binary(self): + """ + Test LocalOperations::read for binary data. + """ + filename = __file__ # current file + + with open(filename, 'rb') as file: # open in a binary mode + response0 = file.read() + + assert type(response0) == bytes # noqa: E721 + + response1 = self.operations.read(filename, binary=True) + assert type(response1) == bytes # noqa: E721 + assert response1 == response0 + + def test_read__binary_and_encoding(self): + """ + Test LocalOperations::read for binary data and encoding. + """ + filename = __file__ # current file + + with pytest.raises( + InvalidOperationException, + match=re.escape("Enconding is not allowed for read binary operation")): + self.operations.read(filename, encoding="", binary=True) + + def test_read__unknown_file(self): + """ + Test LocalOperations::read with unknown file. + """ + + with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): + self.operations.read("/dummy") + def test_read_binary__spec(self): """ Test LocalOperations::read_binary. @@ -95,7 +157,9 @@ def test_read_binary__spec__unk_file(self): Test LocalOperations::read_binary with unknown file. """ - with pytest.raises(FileNotFoundError, match=re.escape("[Errno 2] No such file or directory: '/dummy'")): + with pytest.raises( + FileNotFoundError, + match=re.escape("[Errno 2] No such file or directory: '/dummy'")): self.operations.read_binary("/dummy", 0) def test_get_file_size(self): diff --git a/tests/test_remote.py b/tests/test_remote.py index 780ad46e..7421ca3a 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -4,6 +4,7 @@ import re from testgres import ExecUtilException +from testgres import InvalidOperationException from testgres import RemoteOperations from testgres import ConnectionParams @@ -182,6 +183,69 @@ def test_read_binary_file(self): assert isinstance(response, bytes) + def test_read__text(self): + """ + Test RemoteOperations::read for text data. + """ + filename = __file__ # current file + + with open(filename, 'r') as file: # open in a text mode + response0 = file.read() + + assert type(response0) == str # noqa: E721 + + response1 = self.operations.read(filename) + assert type(response1) == str # noqa: E721 + assert response1 == response0 + + response2 = self.operations.read(filename, encoding=None, binary=False) + assert type(response2) == str # noqa: E721 + assert response2 == response0 + + response3 = self.operations.read(filename, encoding="") + assert type(response3) == str # noqa: E721 + assert response3 == response0 + + response4 = self.operations.read(filename, encoding="UTF-8") + assert type(response4) == str # noqa: E721 + assert response4 == response0 + + def test_read__binary(self): + """ + Test RemoteOperations::read for binary data. + """ + filename = __file__ # current file + + with open(filename, 'rb') as file: # open in a binary mode + response0 = file.read() + + assert type(response0) == bytes # noqa: E721 + + response1 = self.operations.read(filename, binary=True) + assert type(response1) == bytes # noqa: E721 + assert response1 == response0 + + def test_read__binary_and_encoding(self): + """ + Test RemoteOperations::read for binary data and encoding. + """ + filename = __file__ # current file + + with pytest.raises( + InvalidOperationException, + match=re.escape("Enconding is not allowed for read binary operation")): + self.operations.read(filename, encoding="", binary=True) + + def test_read__unknown_file(self): + """ + Test RemoteOperations::read with unknown file. + """ + + with pytest.raises( + ExecUtilException, + match=re.escape("cat: /dummy: No such file or directory")): + self.operations.read("/dummy") + def test_read_binary__spec(self): """ Test RemoteOperations::read_binary. From 28c91c2e2670f74bae5beaa18a6f3a30f0f0dfa6 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 25 Dec 2024 15:11:07 +0300 Subject: [PATCH 47/59] [BUG FIX] xxxOperations::_read__text_with_encoding opens a file as text LocalOps uses "open(filename, mode='r', encoding=encoding)" RemoteOps uses "io.TextIOWrapper(io.BytesIO(binaryData), encoding=encoding)" It solves a problem on Windows. --- testgres/operations/local_ops.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index d6013ab5..e1c3b9fd 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -286,11 +286,10 @@ def read(self, filename, encoding=None, binary=False): def _read__text_with_encoding(self, filename, encoding): assert type(filename) == str # noqa: E721 assert type(encoding) == str # noqa: E721 - content = self._read__binary(filename) - assert type(content) == bytes # noqa: E721 - content_s = content.decode(encoding) - assert type(content_s) == str # noqa: E721 - return content_s + with open(filename, mode='r', encoding=encoding) as file: # open in a text mode + content = file.read() + assert type(content) == str # noqa: E721 + return content def _read__binary(self, filename): assert type(filename) == str # noqa: E721 From 2679646fd6f12038887db3d123fc6bdc8d7fb6b8 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 25 Dec 2024 16:15:00 +0300 Subject: [PATCH 48/59] [BUG FIX] Part for RemoteOps changes... --- testgres/operations/remote_ops.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index abcb8fe1..bb00cfaf 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -3,6 +3,7 @@ import platform import subprocess import tempfile +import io # we support both pg8000 and psycopg2 try: @@ -341,7 +342,9 @@ def _read__text_with_encoding(self, filename, encoding): assert type(encoding) == str # noqa: E721 content = self._read__binary(filename) assert type(content) == bytes # noqa: E721 - content_s = content.decode(encoding) + buf0 = io.BytesIO(content) + buf1 = io.TextIOWrapper(buf0, encoding=encoding) + content_s = buf1.read() assert type(content_s) == str # noqa: E721 return content_s From 6c514bfe0308e912556105db0029541449af44b3 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 25 Dec 2024 16:15:57 +0300 Subject: [PATCH 49/59] Code normalization --- testgres/operations/local_ops.py | 4 ++-- testgres/operations/remote_ops.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index e1c3b9fd..c88c16ca 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -278,8 +278,8 @@ def read(self, filename, encoding=None, binary=False): return self._read__binary(filename) # python behavior - assert None or "abc" == "abc" - assert "" or "abc" == "abc" + assert (None or "abc") == "abc" + assert ("" or "abc") == "abc" return self._read__text_with_encoding(filename, encoding or get_default_encoding()) diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index bb00cfaf..c0307195 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -332,8 +332,8 @@ def read(self, filename, binary=False, encoding=None): return self._read__binary(filename) # python behavior - assert None or "abc" == "abc" - assert "" or "abc" == "abc" + assert (None or "abc") == "abc" + assert ("" or "abc") == "abc" return self._read__text_with_encoding(filename, encoding or get_default_encoding()) From 0139bbd858d1fe4bf6566ff6c3e633c32b592063 Mon Sep 17 00:00:00 2001 From: Dmitry Kovalenko Date: Wed, 25 Dec 2024 16:57:42 +0300 Subject: [PATCH 50/59] [windows] TestgresTests is updated - test_the_same_port - test_port_rereserve_during_node_start - test_port_conflict - use rm_carriage_returns --- tests/test_simple.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_simple.py b/tests/test_simple.py index 9cf29c64..8148d05d 100644 --- a/tests/test_simple.py +++ b/tests/test_simple.py @@ -1054,7 +1054,7 @@ def test_the_same_port(self): self.assertTrue(node._should_free_port) self.assertEqual(type(node.port), int) node_port_copy = node.port - self.assertEqual(node.safe_psql("SELECT 1;"), b'1\n') + self.assertEqual(rm_carriage_returns(node.safe_psql("SELECT 1;")), b'1\n') with get_new_node(port=node.port) as node2: self.assertEqual(type(node2.port), int) @@ -1069,7 +1069,7 @@ def test_the_same_port(self): # node is still working self.assertEqual(node.port, node_port_copy) self.assertTrue(node._should_free_port) - self.assertEqual(node.safe_psql("SELECT 3;"), b'3\n') + self.assertEqual(rm_carriage_returns(node.safe_psql("SELECT 3;")), b'3\n') class tagPortManagerProxy: sm_prev_testgres_reserve_port = None @@ -1175,7 +1175,7 @@ def test_port_rereserve_during_node_start(self): self.assertTrue(node1._should_free_port) self.assertEqual(type(node1.port), int) # noqa: E721 node1_port_copy = node1.port - self.assertEqual(node1.safe_psql("SELECT 1;"), b'1\n') + self.assertEqual(rm_carriage_returns(node1.safe_psql("SELECT 1;")), b'1\n') with __class__.tagPortManagerProxy(node1.port, C_COUNT_OF_BAD_PORT_USAGE): assert __class__.tagPortManagerProxy.sm_DummyPortNumber == node1.port @@ -1191,12 +1191,12 @@ def test_port_rereserve_during_node_start(self): self.assertEqual(__class__.tagPortManagerProxy.sm_DummyPortTotalUsage, C_COUNT_OF_BAD_PORT_USAGE) self.assertTrue(node2.is_started) - self.assertEqual(node2.safe_psql("SELECT 2;"), b'2\n') + self.assertEqual(rm_carriage_returns(node2.safe_psql("SELECT 2;")), b'2\n') # node1 is still working self.assertEqual(node1.port, node1_port_copy) self.assertTrue(node1._should_free_port) - self.assertEqual(node1.safe_psql("SELECT 3;"), b'3\n') + self.assertEqual(rm_carriage_returns(node1.safe_psql("SELECT 3;")), b'3\n') def test_port_conflict(self): assert testgres.PostgresNode._C_MAX_START_ATEMPTS > 1 @@ -1208,7 +1208,7 @@ def test_port_conflict(self): self.assertTrue(node1._should_free_port) self.assertEqual(type(node1.port), int) # noqa: E721 node1_port_copy = node1.port - self.assertEqual(node1.safe_psql("SELECT 1;"), b'1\n') + self.assertEqual(rm_carriage_returns(node1.safe_psql("SELECT 1;")), b'1\n') with __class__.tagPortManagerProxy(node1.port, C_COUNT_OF_BAD_PORT_USAGE): assert __class__.tagPortManagerProxy.sm_DummyPortNumber == node1.port @@ -1233,7 +1233,7 @@ def test_port_conflict(self): # node1 is still working self.assertEqual(node1.port, node1_port_copy) self.assertTrue(node1._should_free_port) - self.assertEqual(node1.safe_psql("SELECT 3;"), b'3\n') + self.assertEqual(rm_carriage_returns(node1.safe_psql("SELECT 3;")), b'3\n') def test_simple_with_bin_dir(self): with get_new_node() as node: From 660ab62d96cda35d19ac69db556a986e4a241297 Mon Sep 17 00:00:00 2001 From: Dmitry Kovalenko Date: Wed, 25 Dec 2024 16:59:44 +0300 Subject: [PATCH 51/59] [windows] PostgresNode.start (LOCAL__start_node) is corrected [BUG FIX] execute_utility may return None in error. --- testgres/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testgres/node.py b/testgres/node.py index 3d023399..baf532de 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -792,7 +792,7 @@ def start(self, params=[], wait=True): def LOCAL__start_node(): _, _, error = execute_utility(_params, self.utils_log_file, verbose=True) - assert type(error) == str # noqa: E721 + assert error is None or type(error) == str # noqa: E721 if error and 'does not exist' in error: raise Exception(error) From 6465b4183f63d82edbbb24ab9a41fa36da551185 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 25 Dec 2024 17:34:35 +0300 Subject: [PATCH 52/59] A comment is added --- testgres/node.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testgres/node.py b/testgres/node.py index baf532de..e203bb7d 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -791,6 +791,7 @@ def start(self, params=[], wait=True): "start"] + params # yapf: disable def LOCAL__start_node(): + # 'error' will be None on Windows _, _, error = execute_utility(_params, self.utils_log_file, verbose=True) assert error is None or type(error) == str # noqa: E721 if error and 'does not exist' in error: From 2c1dd97fac0053e0b61ab917838e5f88ecaead6b Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Wed, 25 Dec 2024 18:11:09 +0300 Subject: [PATCH 53/59] OsOps::read_binary is updated (offset) - the parameter 'start_pos' is renamed with 'offset' - we raise a runtime-error when 'offset' is negative Tests are added. --- testgres/operations/local_ops.py | 10 ++++++---- testgres/operations/os_ops.py | 6 +++--- testgres/operations/remote_ops.py | 10 ++++++---- tests/test_local.py | 10 ++++++++++ tests/test_remote.py | 10 ++++++++++ 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/testgres/operations/local_ops.py b/testgres/operations/local_ops.py index c88c16ca..8bdb22cd 100644 --- a/testgres/operations/local_ops.py +++ b/testgres/operations/local_ops.py @@ -331,13 +331,15 @@ def readlines(self, filename, num_lines=0, binary=False, encoding=None): buffers * max(2, int(num_lines / max(cur_lines, 1))) ) # Adjust buffer size - def read_binary(self, filename, start_pos): + def read_binary(self, filename, offset): assert type(filename) == str # noqa: E721 - assert type(start_pos) == int # noqa: E721 - assert start_pos >= 0 + assert type(offset) == int # noqa: E721 + + if offset < 0: + raise ValueError("Negative 'offset' is not supported.") with open(filename, 'rb') as file: # open in a binary mode - file.seek(start_pos, os.SEEK_SET) + file.seek(offset, os.SEEK_SET) r = file.read() assert type(r) == bytes # noqa: E721 return r diff --git a/testgres/operations/os_ops.py b/testgres/operations/os_ops.py index d644509a..35525b3c 100644 --- a/testgres/operations/os_ops.py +++ b/testgres/operations/os_ops.py @@ -98,10 +98,10 @@ def read(self, filename, encoding, binary): def readlines(self, filename): raise NotImplementedError() - def read_binary(self, filename, start_pos): + def read_binary(self, filename, offset): assert type(filename) == str # noqa: E721 - assert type(start_pos) == int # noqa: E721 - assert start_pos >= 0 + assert type(offset) == int # noqa: E721 + assert offset >= 0 raise NotImplementedError() def isfile(self, remote_file): diff --git a/testgres/operations/remote_ops.py b/testgres/operations/remote_ops.py index c0307195..2f34ecec 100644 --- a/testgres/operations/remote_ops.py +++ b/testgres/operations/remote_ops.py @@ -370,12 +370,14 @@ def readlines(self, filename, num_lines=0, binary=False, encoding=None): return lines - def read_binary(self, filename, start_pos): + def read_binary(self, filename, offset): assert type(filename) == str # noqa: E721 - assert type(start_pos) == int # noqa: E721 - assert start_pos >= 0 + assert type(offset) == int # noqa: E721 - cmd = ["tail", "-c", "+{}".format(start_pos + 1), filename] + if offset < 0: + raise ValueError("Negative 'offset' is not supported.") + + cmd = ["tail", "-c", "+{}".format(offset + 1), filename] r = self.exec_command(cmd) assert type(r) == bytes # noqa: E721 return r diff --git a/tests/test_local.py b/tests/test_local.py index 47a63994..d7adce17 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -162,6 +162,16 @@ def test_read_binary__spec__unk_file(self): match=re.escape("[Errno 2] No such file or directory: '/dummy'")): self.operations.read_binary("/dummy", 0) + def test_read_binary__spec__negative_offset(self): + """ + Test LocalOperations::read_binary with negative offset. + """ + + with pytest.raises( + ValueError, + match=re.escape("Negative 'offset' is not supported.")): + self.operations.read_binary(__file__, -1) + def test_get_file_size(self): """ Test LocalOperations::get_file_size. diff --git a/tests/test_remote.py b/tests/test_remote.py index 7421ca3a..7071a9d9 100755 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -288,6 +288,16 @@ def test_read_binary__spec__unk_file(self): with pytest.raises(ExecUtilException, match=re.escape("tail: cannot open '/dummy' for reading: No such file or directory")): self.operations.read_binary("/dummy", 0) + def test_read_binary__spec__negative_offset(self): + """ + Test RemoteOperations::read_binary with negative offset. + """ + + with pytest.raises( + ValueError, + match=re.escape("Negative 'offset' is not supported.")): + self.operations.read_binary(__file__, -1) + def test_get_file_size(self): """ Test RemoteOperations::get_file_size. From c6e6f1095c072b1828056a54e73054d2d9788f50 Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Fri, 27 Dec 2024 15:49:36 +0300 Subject: [PATCH 54/59] PostgresNode::start is refactored We do not translate a new node port into string when we pack it in new option dictionary. --- testgres/node.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testgres/node.py b/testgres/node.py index e203bb7d..6f466ec9 100644 --- a/testgres/node.py +++ b/testgres/node.py @@ -848,7 +848,7 @@ def LOCAL__raise_cannot_start_node__std(from_exception): cur_port = self.port new_port = utils.reserve_port() # can raise try: - options = {'port': str(new_port)} + options = {'port': new_port} self.set_auto_conf(options) except: # noqa: E722 utils.release_port(new_port) From 9e2928ea4ef6a17e5cbb1401164194d631b44f1a Mon Sep 17 00:00:00 2001 From: "e.garbuz" Date: Mon, 20 Jan 2025 05:48:22 +0300 Subject: [PATCH 55/59] Add check backup-source for testing pg_probackup3 --- testgres/plugins/pg_probackup2/pg_probackup2/init_helpers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/testgres/plugins/pg_probackup2/pg_probackup2/init_helpers.py b/testgres/plugins/pg_probackup2/pg_probackup2/init_helpers.py index b7174a7c..078fdbab 100644 --- a/testgres/plugins/pg_probackup2/pg_probackup2/init_helpers.py +++ b/testgres/plugins/pg_probackup2/pg_probackup2/init_helpers.py @@ -172,6 +172,10 @@ def __init__(self): self.ptrack = test_env.get('PG_PROBACKUP_PTRACK', None) == 'ON' and self.pg_config_version >= 110000 self.wal_tree_enabled = test_env.get('PG_PROBACKUP_WAL_TREE_ENABLED', None) == 'ON' + self.bckp_source = test_env.get('PG_PROBACKUP_SOURCE', 'pro').lower() + if self.bckp_source not in ('base', 'direct', 'pro'): + raise Exception("Wrong PG_PROBACKUP_SOURCE value. Available options: base|direct|pro") + self.paranoia = test_env.get('PG_PROBACKUP_PARANOIA', None) == 'ON' env_compress = test_env.get('ARCHIVE_COMPRESSION', None) if env_compress: From ab7de699e9c90be4a83170e412547cb6849a366e Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Fri, 31 Jan 2025 11:09:17 +0300 Subject: [PATCH 56/59] PortManager::find_free_port is updated We will use exclude_ports only when it is not none. Creation of empty exclude_ports is removed. --- testgres/helpers/port_manager.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/testgres/helpers/port_manager.py b/testgres/helpers/port_manager.py index 6afdf8a9..a7e2a85f 100644 --- a/testgres/helpers/port_manager.py +++ b/testgres/helpers/port_manager.py @@ -26,10 +26,8 @@ def find_free_port(self, ports: Optional[Set[int]] = None, exclude_ports: Option if ports is None: ports = set(range(1024, 65535)) - if exclude_ports is None: - exclude_ports = set() - - ports.difference_update(set(exclude_ports)) + if exclude_ports is not None: + ports.difference_update(set(exclude_ports)) sampled_ports = random.sample(tuple(ports), min(len(ports), 100)) From 27c40d28645c677c5c05f2bd9409b3dc21b8c26d Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Fri, 31 Jan 2025 11:25:46 +0300 Subject: [PATCH 57/59] PortManager::find_free_port is updated Asserts are added: - ports must be the "set" - exclude_ports must be iterable Do not convert exclude_ports into "set" [optimization?] --- testgres/helpers/port_manager.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testgres/helpers/port_manager.py b/testgres/helpers/port_manager.py index a7e2a85f..f59df259 100644 --- a/testgres/helpers/port_manager.py +++ b/testgres/helpers/port_manager.py @@ -26,8 +26,11 @@ def find_free_port(self, ports: Optional[Set[int]] = None, exclude_ports: Option if ports is None: ports = set(range(1024, 65535)) + assert type(ports) == set + if exclude_ports is not None: - ports.difference_update(set(exclude_ports)) + assert isinstance(exclude_ports, Iterable) + ports.difference_update(exclude_ports) sampled_ports = random.sample(tuple(ports), min(len(ports), 100)) From bc893d8b1d36f2073a3629c102c615e21240245a Mon Sep 17 00:00:00 2001 From: "d.kovalenko" Date: Fri, 31 Jan 2025 11:33:01 +0300 Subject: [PATCH 58/59] noqa: E721 --- testgres/helpers/port_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testgres/helpers/port_manager.py b/testgres/helpers/port_manager.py index f59df259..cfc5c096 100644 --- a/testgres/helpers/port_manager.py +++ b/testgres/helpers/port_manager.py @@ -26,7 +26,7 @@ def find_free_port(self, ports: Optional[Set[int]] = None, exclude_ports: Option if ports is None: ports = set(range(1024, 65535)) - assert type(ports) == set + assert type(ports) == set # noqa: E721 if exclude_ports is not None: assert isinstance(exclude_ports, Iterable) From 67beb95ce02bd824a0f0cc75fd52ebe426de666e Mon Sep 17 00:00:00 2001 From: asavchkov Date: Tue, 11 Feb 2025 18:52:41 +0700 Subject: [PATCH 59/59] Up version --- setup.py | 2 +- testgres/plugins/pg_probackup2/setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index a41094d6..16586b88 100755 --- a/setup.py +++ b/setup.py @@ -27,7 +27,7 @@ readme = f.read() setup( - version='1.10.3', + version='1.10.4', name='testgres', packages=['testgres', 'testgres.operations', 'testgres.helpers'], description='Testing utility for PostgreSQL and its extensions', diff --git a/testgres/plugins/pg_probackup2/setup.py b/testgres/plugins/pg_probackup2/setup.py index ade2d85d..619b8d39 100644 --- a/testgres/plugins/pg_probackup2/setup.py +++ b/testgres/plugins/pg_probackup2/setup.py @@ -4,7 +4,7 @@ from distutils.core import setup setup( - version='0.0.4', + version='0.0.5', name='testgres_pg_probackup2', packages=['pg_probackup2', 'pg_probackup2.storage'], description='Plugin for testgres that manages pg_probackup2',