From 122998689846fdb0b7bbbf995f787d35eabc6374 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 20 Nov 2020 20:18:42 +0000 Subject: [PATCH 01/22] Explicit imports of needed objects There are two import approaches in Python: import the whole module and import only needed objects (class, variable etc). We don't have code a style guide for test-run project and I personally like second approach. Moreover it seems that 'from foo import bar' is the preferred way within the test-run's code. This patch has been made by Alexander Turenko in scope of task to make code compatible with Python 3, but actually it is not related to it. Part of #20 Co-authored-by: Alexander Turenko --- dispatcher.py | 38 ++++++++++++++++++++------------------ lib/test.py | 10 +++++----- lib/test_suite.py | 6 +++--- lib/worker.py | 12 ++++++------ listeners.py | 16 +++++++++------- test-run.py | 43 ++++++++++++++++++++++--------------------- 6 files changed, 65 insertions(+), 60 deletions(-) diff --git a/dispatcher.py b/dispatcher.py index 9f9c0443..da910d19 100644 --- a/dispatcher.py +++ b/dispatcher.py @@ -9,11 +9,16 @@ import multiprocessing from multiprocessing.queues import SimpleQueue -import listeners -import lib +from lib import Options from lib.utils import set_fd_cloexec from lib.worker import WorkerTaskResult, WorkerDone from lib.colorer import color_stdout +from listeners import ArtifactsWatcher +from listeners import FailWatcher +from listeners import HangWatcher +from listeners import LogOutputWatcher +from listeners import OutputWatcher +from listeners import StatisticsWatcher class TcpPortDispatcher: @@ -122,30 +127,27 @@ def kill_all_workers(self): pass def init_listeners(self): - args = lib.Options().args + args = Options().args watch_hang = args.no_output_timeout >= 0 and \ not args.gdb and \ not args.gdbserver and \ not args.lldb and \ not args.valgrind - watch_fail = not lib.Options().args.is_force - - log_output_watcher = listeners.LogOutputWatcher() - self.statistics = listeners.StatisticsWatcher( - log_output_watcher.get_logfile) - self.artifacts = listeners.ArtifactsWatcher( - log_output_watcher.get_logfile) - output_watcher = listeners.OutputWatcher() + watch_fail = not Options().args.is_force + + log_output_watcher = LogOutputWatcher() + self.statistics = StatisticsWatcher(log_output_watcher.get_logfile) + self.artifacts = ArtifactsWatcher(log_output_watcher.get_logfile) + output_watcher = OutputWatcher() self.listeners = [self.statistics, log_output_watcher, output_watcher, self.artifacts] if watch_fail: - self.fail_watcher = listeners.FailWatcher( - self.terminate_all_workers) + self.fail_watcher = FailWatcher(self.terminate_all_workers) self.listeners.append(self.fail_watcher) if watch_hang: warn_timeout = 60.0 if args.long else 10.0 - hang_watcher = listeners.HangWatcher( - output_watcher.not_done_worker_ids, self.kill_all_workers, - warn_timeout, float(args.no_output_timeout)) + hang_watcher = HangWatcher(output_watcher.not_done_worker_ids, + self.kill_all_workers, warn_timeout, + float(args.no_output_timeout)) self.listeners.append(hang_watcher) def run_max_workers(self): @@ -315,8 +317,8 @@ def flush_ready(self, inputs): # leave only output listeners in self.listeners new_listeners = [] for listener in self.listeners: - if isinstance(listener, (listeners.LogOutputWatcher, - listeners.OutputWatcher)): + if isinstance(listener, (LogOutputWatcher, + OutputWatcher)): listener.report_at_timeout = False new_listeners.append(listener) self.listeners = new_listeners diff --git a/lib/test.py b/lib/test.py index ad22243d..c3b2763c 100644 --- a/lib/test.py +++ b/lib/test.py @@ -16,7 +16,7 @@ except ImportError: from StringIO import StringIO -import lib +from lib import Options from lib.colorer import color_stdout from lib.utils import non_empty_valgrind_logs from lib.utils import print_tail_n @@ -208,7 +208,7 @@ def run(self, server): self.is_equal_result = filecmp.cmp(self.result, self.tmp_result) elif self.is_executed_ok: - if lib.Options().args.is_verbose: + if Options().args.is_verbose: color_stdout('\n') with open(self.tmp_result, 'r') as f: color_stdout(f.read(), schema='log') @@ -241,7 +241,7 @@ def run(self, server): not self.is_equal_result and not os.path.isfile(self.result) and not is_tap and - lib.Options().args.update_result): + Options().args.update_result): shutil.copy(self.tmp_result, self.result) short_status = 'new' color_stdout("[ new ]\n", schema='test_new') @@ -249,7 +249,7 @@ def run(self, server): not self.is_equal_result and os.path.isfile(self.result) and not is_tap and - lib.Options().args.update_result): + Options().args.update_result): shutil.copy(self.tmp_result, self.result) short_status = 'updated' color_stdout("[ updated ]\n", schema='test_new') @@ -343,7 +343,7 @@ def check_tap_output(self): schema='error') color_stdout('\nNo result file (%s) found.\n' % self.result, schema='error') - if not lib.Options().args.update_result: + if not Options().args.update_result: msg = 'Run the test with --update-result option to write the new result file.\n' color_stdout(msg, schema='error') self.is_crash_reported = True diff --git a/lib/test_suite.py b/lib/test_suite.py index 91f8088b..e141c83c 100644 --- a/lib/test_suite.py +++ b/lib/test_suite.py @@ -3,7 +3,7 @@ import os import re -import lib +from lib import Options from lib.app_server import AppServer from lib.colorer import color_stdout from lib.inspector import TarantoolInspector @@ -153,7 +153,7 @@ def collect_tests(self): else: raise ValueError('Cannot collect tests of unknown type') - if not lib.Options().args.reproduce: + if not Options().args.reproduce: color_stdout("Collecting tests in ", schema='ts_text') color_stdout( '%s (Found %s tests)' % ( @@ -265,7 +265,7 @@ def run_test(self, test, server, inspector): result_checksum = None # cleanup only if test passed or if --force mode enabled - if lib.Options().args.is_force or short_status == 'pass': + if Options().args.is_force or short_status == 'pass': inspector.cleanup_nondefault() return short_status, result_checksum diff --git a/lib/worker.py b/lib/worker.py index a8643fe8..9ed9b812 100644 --- a/lib/worker.py +++ b/lib/worker.py @@ -7,7 +7,7 @@ import yaml from datetime import datetime -import lib +from lib import Options from lib.colorer import color_log from lib.colorer import color_stdout from lib.tarantool_server import TarantoolServer @@ -20,13 +20,13 @@ def find_suites(): - suite_names = lib.Options().args.suites + suite_names = Options().args.suites if suite_names == []: for root, dirs, names in os.walk(os.getcwd(), followlinks=True): if "suite.ini" in names: suite_names.append(os.path.basename(root)) - suites = [TestSuite(suite_name, lib.Options().args) + suites = [TestSuite(suite_name, Options().args) for suite_name in sorted(suite_names)] return suites @@ -48,7 +48,7 @@ def parse_reproduce_file(filepath): def get_reproduce_file(worker_name): - main_vardir = os.path.realpath(lib.Options().args.vardir) + main_vardir = os.path.realpath(Options().args.vardir) reproduce_dir = os.path.join(main_vardir, 'reproduce') return os.path.join(reproduce_dir, '%s.list.yaml' % worker_name) @@ -100,7 +100,7 @@ def reproduce_task_groups(task_groups): this group as in the reproduce file. """ found_keys = [] - reproduce = parse_reproduce_file(lib.Options().args.reproduce) + reproduce = parse_reproduce_file(Options().args.reproduce) if not reproduce: raise ValueError('[reproduce] Tests list cannot be empty') for i, task_id in enumerate(reproduce): @@ -362,7 +362,7 @@ def run_loop(self, task_queue, result_queue): result_queue.put(self.wrap_result(task_id, short_status, result_checksum)) if short_status == 'fail': - if lib.Options().args.is_force: + if Options().args.is_force: self.restart_server() color_stdout( 'Worker "%s" got failed test; restarted the server\n' diff --git a/listeners.py b/listeners.py index 54f8dddc..f8301386 100644 --- a/listeners.py +++ b/listeners.py @@ -3,7 +3,7 @@ import yaml import shutil -import lib +from lib import Options from lib.colorer import color_stdout from lib.colorer import decolor from lib.worker import WorkerCurrentTask @@ -13,6 +13,8 @@ from lib.worker import get_reproduce_file from lib.utils import prefix_each_line from lib.utils import safe_makedirs +from lib.utils import print_tail_n +from lib.utils import print_unidiff class BaseWatcher(object): @@ -69,7 +71,7 @@ def print_statistics(self): color_stdout('# reproduce file: %s\n' % reproduce_file_path) if show_reproduce_content: color_stdout("---\n", schema='separator') - lib.utils.print_tail_n(reproduce_file_path) + print_tail_n(reproduce_file_path) color_stdout("...\n", schema='separator') return True @@ -97,7 +99,7 @@ def save_artifacts(self): if not self.failed_workers: return - vardir = lib.Options().args.vardir + vardir = Options().args.vardir artifacts_dir = os.path.join(vardir, 'artifacts') artifacts_log_dir = os.path.join(artifacts_dir, 'log') artifacts_reproduce_dir = os.path.join(artifacts_dir, 'reproduce') @@ -124,7 +126,7 @@ def save_artifacts(self): class LogOutputWatcher(BaseWatcher): def __init__(self): self.fds = dict() - self.logdir = os.path.join(lib.Options().args.vardir, 'log') + self.logdir = os.path.join(Options().args.vardir, 'log') try: os.makedirs(self.logdir) except OSError: @@ -205,7 +207,7 @@ def process_result(self, obj): return # Skip color_log() events if --debug is not passed. - if obj.log_only and not lib.Options().args.debug: + if obj.log_only and not Options().args.debug: return # Prepend color_log() messages with a timestamp. @@ -267,7 +269,7 @@ def process_result(self, obj): return # Skip color_log() events if --debug is not passed. - if obj.log_only and not lib.Options().args.debug: + if obj.log_only and not Options().args.debug: return self.warned_seconds_ago = 0.0 @@ -313,7 +315,7 @@ def process_timeout(self, delta_seconds): for task in hung_tasks: color_stdout("Test hung! Result content mismatch:\n", schema='error') - lib.utils.print_unidiff(task.task_result, task.task_tmp_result) + print_unidiff(task.task_result, task.task_tmp_result) color_stdout('\n[Main process] No output from workers. ' 'It seems that we hang. Send SIGKILL to workers; ' 'exiting...\n', schema='error') diff --git a/test-run.py b/test-run.py index d63f5ed8..5c205054 100755 --- a/test-run.py +++ b/test-run.py @@ -1,7 +1,6 @@ #!/usr/bin/env python2 """Tarantool regression test suite front-end.""" - # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions # are met: @@ -54,14 +53,16 @@ import sys import time -import lib -import lib.worker - -from dispatcher import Dispatcher +from lib import Options from lib.colorer import color_stdout +from lib.utils import print_tail_n +from lib.worker import get_task_groups +from lib.worker import get_reproduce_file +from lib.worker import reproduce_task_groups +from lib.worker import print_greetings +from dispatcher import Dispatcher from listeners import HangError - EXIT_SUCCESS = 0 EXIT_HANG = 1 EXIT_INTERRUPTED = 2 @@ -73,7 +74,7 @@ def main_loop_parallel(): color_stdout("Started {0}\n".format(" ".join(sys.argv)), schema='tr_text') - args = lib.Options().args + args = Options().args jobs = args.jobs if jobs < 1: # faster result I got was with 2 * cpu_count @@ -94,16 +95,16 @@ def main_loop_parallel(): format(args.no_output_timeout), schema='tr_text') color_stdout("\n", schema='tr_text') - task_groups = lib.worker.get_task_groups() - if lib.Options().args.reproduce: - task_groups = lib.worker.reproduce_task_groups(task_groups) + task_groups = get_task_groups() + if Options().args.reproduce: + task_groups = reproduce_task_groups(task_groups) jobs = 1 randomize = False dispatcher = Dispatcher(task_groups, jobs, randomize) dispatcher.start() - lib.worker.print_greetings() + print_greetings() color_stdout("\n", '=' * 86, "\n", schema='separator') color_stdout("WORKR".ljust(6), schema='t_name') @@ -113,7 +114,7 @@ def main_loop_parallel(): color_stdout('-' * 81, "\n", schema='separator') try: - is_force = lib.Options().args.is_force + is_force = Options().args.is_force dispatcher.wait() dispatcher.wait_processes() color_stdout('-' * 81, "\n", schema='separator') @@ -152,8 +153,8 @@ def main_parallel(): def main_loop_consistent(failed_test_ids): # find and prepare all tasks/groups, print information - task_groups = lib.worker.get_task_groups().items() - lib.worker.print_greetings() + task_groups = get_task_groups().items() + print_greetings() for name, task_group in task_groups: # print information about current test suite @@ -173,15 +174,15 @@ def main_loop_consistent(failed_test_ids): short_status = worker.run_task(task_id) if short_status == 'fail': reproduce_file_path = \ - lib.worker.get_reproduce_file(worker.name) + get_reproduce_file(worker.name) color_stdout('Reproduce file %s\n' % reproduce_file_path, schema='error') if show_reproduce_content: color_stdout("---\n", schema='separator') - lib.utils.print_tail_n(reproduce_file_path) + print_tail_n(reproduce_file_path) color_stdout("...\n", schema='separator') failed_test_ids.append(task_id) - if not lib.Options().args.is_force: + if not Options().args.is_force: worker.stop_server(cleanup=False) return @@ -203,11 +204,11 @@ def main_consistent(): except RuntimeError as e: color_stdout("\nFatal error: %s. Execution aborted.\n" % e, schema='error') - if lib.Options().args.gdb: + if Options().args.gdb: time.sleep(100) return -1 - if failed_test_ids and lib.Options().args.is_force: + if failed_test_ids and Options().args.is_force: color_stdout("\n===== %d tests failed:\n" % len(failed_test_ids), schema='error') for test_id in failed_test_ids: @@ -223,8 +224,8 @@ def main_consistent(): status = 0 - force_parallel = bool(lib.Options().args.reproduce) - if not force_parallel and lib.Options().args.jobs == -1: + force_parallel = bool(Options().args.reproduce) + if not force_parallel and Options().args.jobs == -1: status = main_consistent() else: status = main_parallel() From 8fbb67330eee3d3506419738baf7ce3938a413e0 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Thu, 19 Nov 2020 15:09:01 +0000 Subject: [PATCH 02/22] Replace Travis CI with GH Actions Patch introduces a GH Actions workflow [1] and get rid of travis integration. Note that Python 3.4 is unused because it's support has been deprecated: DEPRECATION: Python 3.4 support has been deprecated. pip 19.1 will be the last one supporting it. Please upgrade your Python as Python 3.4 won't be maintained after March 2019 (cf PEP 429). So it has been removed from a matrix. 1. https://docs.github.com/en/actions/guides/building-and-testing-python Part of #20 --- .github/workflows/test.yml | 37 +++++++++++++++++++++++++++++++++++++ .travis.yml | 14 -------------- Makefile | 2 +- 3 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/test.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 00000000..62bca231 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,37 @@ +name: test-run + +on: [push, pull_request] + +jobs: + build: + if: ( github.event_name == 'push' || + github.event.pull_request.head.repo.full_name != github.repository ) && + ( github.repository == 'tarantool/test-run' ) + + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + python-version: [2.7, 3.5, 3.6, 3.7, 3.8, 3.8-dev] + + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: display python version + run: python -c "import sys; print(sys.version)" + - name: setup dependencies + run: | + sudo apt update -y + sudo apt-get -y install lua5.1 luarocks + sudo luarocks install luacheck + - name: setup python dependencies + run: | + pip install -r requirements.txt + pip install -r requirements-test.txt + - name: run static analysis + run: | + make lint diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index d5c4344d..00000000 --- a/.travis.yml +++ /dev/null @@ -1,14 +0,0 @@ -language: python -python: 2.7 - -before_install: - - sudo apt update -y - - sudo apt-get -y install lua5.1 luarocks - - sudo luarocks install luacheck - -install: - - pip install -r requirements.txt - - pip install -r requirements-test.txt - -script: - - make lint diff --git a/Makefile b/Makefile index a2143d37..9e8b6f5c 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ default: lint: flake8 luacheck flake8: - python2 -m flake8 *.py lib/*.py + python -m flake8 *.py lib/*.py luacheck: luacheck --config .luacheckrc . From 7a5de97f78e5227e96459956d6be6535e6df8cb8 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Thu, 19 Nov 2020 21:54:09 +0000 Subject: [PATCH 03/22] python3: remove unused variable flake8 complains regarding this line when called from Python 3, but does not complain when called from Python 2. Part of #20 Co-authored-by: Alexander Turenko --- lib/preprocessor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/preprocessor.py b/lib/preprocessor.py index 29d6c517..c67dc65b 100644 --- a/lib/preprocessor.py +++ b/lib/preprocessor.py @@ -216,7 +216,7 @@ def server_start(self, ctype, sname, opts): self.connections[sname] = self.servers[sname].admin try: self.connections[sname]('return true', silent=True) - except socket.error as e: + except socket.error: LuaPreprocessorException( 'Can\'t start server {0}'.format(repr(sname))) return not crash_occured From 4f11d83b0bfac8ba652f2416b347a009645532f8 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Thu, 19 Nov 2020 16:33:36 +0000 Subject: [PATCH 04/22] python3: replace xrange() with range() Part of #20 Co-authored-by: Alexander Turenko --- lib/connpool.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/connpool.py b/lib/connpool.py index b54dfadf..1c296983 100644 --- a/lib/connpool.py +++ b/lib/connpool.py @@ -35,9 +35,9 @@ def __init__(self, size, exc_classes=DEFAULT_EXC_CLASSES, keepalive=None): self.keepalive = keepalive # Exceptions list must be in tuple form to be caught properly self.exc_classes = tuple(exc_classes) - for i in xrange(size): + for i in range(size): self.lock.acquire() - for i in xrange(size): + for i in range(size): greenlet = TestRunGreenlet(self._addOne) greenlet.start_later(self.SPAWN_FREQUENCY * i) if self.keepalive: From 0b635d1e26e90867abeff6eab50ad23e4ad2a975 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Thu, 19 Nov 2020 15:03:52 +0000 Subject: [PATCH 05/22] python3: make print calls compatible with Python 3.x In a Python 3 'print' becomes a function, see [1]. Patch makes print calls compatible with Python 3. 1. https://docs.python.org/3/whatsnew/3.0.html#print-is-a-function Part of #20 Co-authored-by: Alexander Turenko --- lib/admin_connection.py | 2 +- lib/box_connection.py | 8 ++++---- lib/tarantool_server.py | 4 ++-- lib/test_suite.py | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/admin_connection.py b/lib/admin_connection.py index 1af31275..3f05546c 100644 --- a/lib/admin_connection.py +++ b/lib/admin_connection.py @@ -54,7 +54,7 @@ def _new_connection(self): # tarantool/gh-1163 # 1. raise only if handshake is not full # 2. be silent on crashes or if it's server.stop() operation - print 'Handshake error {\n', handshake, '\n}' + print('Handshake error {\n', handshake, '\n}') raise RuntimeError('Broken tarantool console handshake') return s diff --git a/lib/box_connection.py b/lib/box_connection.py index 6eb5121c..dad84a20 100644 --- a/lib/box_connection.py +++ b/lib/box_connection.py @@ -75,11 +75,11 @@ def execute_no_reconnect(self, command, silent=True): if not command: return if not silent: - print command + print(command) cmd = command.replace(SEPARATOR, ' ') + SEPARATOR response = self.py_con.call(cmd) if not silent: - print response + print(response) return response def execute(self, command, silent=True): @@ -88,8 +88,8 @@ def execute(self, command, silent=True): def call(self, command, *args): if not command: return - print 'call ', command, args + print('call ', command, args) response = self.py_con.call(command, *args) result = str(response) - print result + print(result) return result diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py index 4dc640a6..e36fe8a3 100644 --- a/lib/tarantool_server.py +++ b/lib/tarantool_server.py @@ -1153,7 +1153,7 @@ def read_pidfile(self): def test_option_get(self, option_list_str, silent=False): args = [self.binary] + shlex.split(option_list_str) if not silent: - print " ".join([os.path.basename(self.binary)] + args[1:]) + print(" ".join([os.path.basename(self.binary)] + args[1:])) output = subprocess.Popen(args, cwd=self.vardir, stdout=subprocess.PIPE, @@ -1161,7 +1161,7 @@ def test_option_get(self, option_list_str, silent=False): return output def test_option(self, option_list_str): - print self.test_option_get(option_list_str) + print(self.test_option_get(option_list_str)) @staticmethod def find_tests(test_suite, suite_path): diff --git a/lib/test_suite.py b/lib/test_suite.py index e141c83c..b0aa42ee 100644 --- a/lib/test_suite.py +++ b/lib/test_suite.py @@ -196,7 +196,7 @@ def gen_server(self): try: return Server(self.ini, test_suite=self) except Exception as e: - print e + print(e) raise RuntimeError("Unknown server: core = {0}".format( self.ini["core"])) From a379942c867d12539d2835e71524779a17622d27 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 20 Nov 2020 20:53:07 +0000 Subject: [PATCH 06/22] python3: fix mode format in chmod() call flake8 warn about SyntaxError E999: "leading zeros in decimal integer literals are not permitted; use an 0o prefix for octal integers". Patch fixes an error. Part of #20 Co-authored-by: Alexander Turenko --- lib/tarantool_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py index e36fe8a3..2a996edf 100644 --- a/lib/tarantool_server.py +++ b/lib/tarantool_server.py @@ -759,7 +759,7 @@ def deploy(self, silent=True, **kwargs): def copy_files(self): if self.script: shutil.copy(self.script, self.script_dst) - os.chmod(self.script_dst, 0777) + os.chmod(self.script_dst, 0o777) if self.lua_libs: for i in self.lua_libs: source = os.path.join(self.testdir, i) From d8f6183c846ebad786656edba0e02101000c8a28 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Thu, 4 Feb 2021 18:32:21 +0300 Subject: [PATCH 07/22] python3: integer and string types Introduced string and integer types in lib/utils.py like we do in tarantool-python and use these types to check object types in a code. Part of #20 --- lib/preprocessor.py | 7 ++++--- lib/pytap13.py | 3 ++- lib/utils.py | 21 +++++++++++++++------ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/lib/preprocessor.py b/lib/preprocessor.py index c67dc65b..b96795b0 100644 --- a/lib/preprocessor.py +++ b/lib/preprocessor.py @@ -6,12 +6,13 @@ import yaml from gevent import socket -import six from lib.admin_connection import AdminAsyncConnection from lib.colorer import color_log from lib.utils import signum from lib.utils import signame +from lib.utils import integer_types +from lib.utils import string_types class Namespace(object): @@ -412,9 +413,9 @@ def variable(self, ctype, ref, ret): if ctype == 'set': self.curcon[0].reconnect() result = eval(ret[1:-1], {}, self.environ.__dict__) - if isinstance(result, six.integer_types): + if isinstance(result, integer_types): cmd = '{0}={1}'.format(ref, result) - elif isinstance(result, six.string_types): + elif isinstance(result, string_types): cmd = '{0}="{1}"'.format(ref, result) else: raise LuaPreprocessorException( diff --git a/lib/pytap13.py b/lib/pytap13.py index 295a0d15..1cbf6f3f 100644 --- a/lib/pytap13.py +++ b/lib/pytap13.py @@ -23,6 +23,7 @@ from StringIO import StringIO import yaml +from lib.utils import string_types RE_VERSION = re.compile(r"^\s*TAP version 13\s*$") @@ -173,7 +174,7 @@ def _parse(self, source): self.tests.append(Test('not ok', len(self.tests), comment=comment)) def parse(self, source): - if isinstance(source, (str, unicode)): + if isinstance(source, string_types): self._parse(StringIO(source)) elif hasattr(source, "__iter__"): self._parse(source) diff --git a/lib/utils.py b/lib/utils.py index 2fb12ed9..21834e5f 100644 --- a/lib/utils.py +++ b/lib/utils.py @@ -1,6 +1,5 @@ import os import sys -import six import collections import signal import random @@ -21,6 +20,16 @@ UNIX_SOCKET_LEN_LIMIT = 107 +# Useful for very coarse version differentiation. +PY3 = sys.version_info[0] == 3 + +if PY3: + string_types = str, + integer_types = int, +else: + string_types = basestring, # noqa: F821 + integer_types = (int, long) # noqa: F821 + def check_libs(): deps = [ @@ -68,7 +77,7 @@ def check_port(port, rais=True, ipv4=True, ipv6=True): connections (UNIX Sockets in case of file path). False -- otherwise. """ try: - if isinstance(port, (int, long)): + if isinstance(port, integer_types): if ipv4: sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) sock.bind(('127.0.0.1', port)) @@ -139,22 +148,22 @@ def find_in_path(name): def signame(signal): - if isinstance(signal, six.integer_types): + if isinstance(signal, integer_types): return SIGNAMES[signal] if Signals and isinstance(signal, Signals): return SIGNAMES[int(signal)] - if isinstance(signal, six.string_types): + if isinstance(signal, string_types): return signal raise TypeError('signame(): signal argument of unexpected type: {}'.format( str(type(signal)))) def signum(signal): - if isinstance(signal, six.integer_types): + if isinstance(signal, integer_types): return signal if Signals and isinstance(signal, Signals): return int(signal) - if isinstance(signal, six.string_types): + if isinstance(signal, string_types): if not signal.startswith('SIG'): signal = 'SIG' + signal return SIGNUMS[signal] From bd78adf2e5bbf95bac7d5d3a0ceec3bde9af8e38 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 20 Nov 2020 09:41:51 +0000 Subject: [PATCH 08/22] python3: replace execfile() with exec() and compile() In Python 3 execfile() function has been removed [1]. Instead of execfile(fn) exec() suggested. 1. https://docs.python.org/3.3/whatsnew/3.0.html?highlight=execfile#builtins Part of #20 Co-authored-by: Alexander Turenko --- lib/tarantool_server.py | 6 ++++-- lib/test.py | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py index 2a996edf..33c19c51 100644 --- a/lib/tarantool_server.py +++ b/lib/tarantool_server.py @@ -396,8 +396,10 @@ class PythonTest(Test): def execute(self, server): super(PythonTest, self).execute(server) - execfile(self.name, dict(locals(), test_run_current_test=self, - **server.__dict__)) + new_globals = dict(locals(), test_run_current_test=self, **server.__dict__) + with open(self.name) as f: + code = compile(f.read(), self.name, 'exec') + exec(code, new_globals) # crash was detected (possibly on non-default server) if server.current_test.is_crash_reported: raise TestExecutionError diff --git a/lib/test.py b/lib/test.py index c3b2763c..496eea19 100644 --- a/lib/test.py +++ b/lib/test.py @@ -173,7 +173,10 @@ def run(self, server): if os.path.exists(self.skip_cond): sys.stdout = FilteredStream(self.tmp_result) stdout_fileno = sys.stdout.stream.fileno() - execfile(self.skip_cond, dict(locals(), **server.__dict__)) + new_globals = dict(locals(), **server.__dict__) + with open(self.skip_cond, 'r') as f: + code = compile(f.read(), self.skip_cond, 'exec') + exec(code, new_globals) sys.stdout.close() sys.stdout = save_stdout if not self.skip: From 067553916941d796b9491bac6f793ecf3b7d145f Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 20 Nov 2020 20:43:01 +0000 Subject: [PATCH 09/22] python3: fix import of StringIO module From What's New In Python 3.0 [1]: The StringIO and cStringIO modules are gone. Instead, import the io module and use io.StringIO or io.BytesIO for text and data respectively. 1. http://docs.python.org/3.0/whatsnew/3.0.html Part of #20 Co-authored-by: Alexander Turenko --- lib/pytap13.py | 7 +++++-- lib/tarantool_server.py | 6 ++++-- lib/test.py | 6 ++++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/pytap13.py b/lib/pytap13.py index 1cbf6f3f..5aef4313 100644 --- a/lib/pytap13.py +++ b/lib/pytap13.py @@ -17,10 +17,13 @@ # Author: Josef Skladanka import re + try: - from CStringIO import StringIO -except ImportError: + # Python 2 from StringIO import StringIO +except ImportError: + # Python 3 + from io import StringIO import yaml from lib.utils import string_types diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py index 33c19c51..b26d2625 100644 --- a/lib/tarantool_server.py +++ b/lib/tarantool_server.py @@ -19,9 +19,11 @@ from threading import Timer try: - from cStringIO import StringIO -except ImportError: + # Python 2 from StringIO import StringIO +except ImportError: + # Python 3 + from io import StringIO from lib.admin_connection import AdminConnection, AdminAsyncConnection from lib.box_connection import BoxConnection diff --git a/lib/test.py b/lib/test.py index 496eea19..f7670063 100644 --- a/lib/test.py +++ b/lib/test.py @@ -12,9 +12,11 @@ from utils import safe_makedirs try: - from cStringIO import StringIO -except ImportError: + # Python 2 from StringIO import StringIO +except ImportError: + # Python 3 + from io import StringIO from lib import Options from lib.colorer import color_stdout From 549dd474c147bc681a5ece4099eb9f3ae05772de Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 20 Nov 2020 20:17:54 +0000 Subject: [PATCH 10/22] python3: fix import of SimpleQueue Part of #20 Co-authored-by: Alexander Turenko --- dispatcher.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/dispatcher.py b/dispatcher.py index da910d19..f878e9a4 100644 --- a/dispatcher.py +++ b/dispatcher.py @@ -7,7 +7,31 @@ import yaml import multiprocessing -from multiprocessing.queues import SimpleQueue + +# SimpleQueue is available from multiprocessing.queues on +# all Python versions known at the moment of writting the code +# (up to 3.9). +# +# It was additionally exposed directly from the multiprocessing +# module since Python 3.3 ([1]). +# +# However the mandatory argument 'ctx' +# (see multiprocessing.get_context()) was added to the constructor +# of SimpleQueue from multiprocessing.queues since Python 3.4 +# ([2]). +# +# So we should import SimpleQueue from multiprocessing on +# Python 3.3+ (and must to do so on Python 3.4+) to uniformly +# instantiate it (without constructor arguments). +# +# [1]: https://bugs.python.org/issue11836 +# [2]: https://bugs.python.org/issue18999 +try: + # Python 3.3+ + from multiprocessing import SimpleQueue +except ImportError: + # Python 2 + from multiprocessing.queues import SimpleQueue from lib import Options from lib.utils import set_fd_cloexec From 4c6bf7762137e974c36d98d868f2488be7da176b Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Sat, 21 Nov 2020 22:10:53 +0000 Subject: [PATCH 11/22] python3: fix configparser module import Since Python 3.2 inline comments (colon, may be in a middle of a line) are disabled by default (inline_comment_prefixes=None). Since Python 3.2 sections and options duplicates are forbidded by default (strict=True). We should enable inline comments back. Now it 'works' only because we have inline comments only within the disabled option and because parsing of the option value disregards extra values (unknown test names). It'll not work for commenting other options. That would be undesirable degradation. Note: ConfigParser (and SafeConfigParser) constructors have no inline_comment_prefixes option in Python 2, so we should pass it only on Python 3.2+. Regarding the strict mode: I think it is good to keep it enabled (I mean, don't disable explicitly) to protect ourself from mistakes. So both options are enabled by explicitly under Python 3. Part of #20 Co-authored-by: Alexander Turenko --- lib/test_suite.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/test_suite.py b/lib/test_suite.py index b0aa42ee..67cdadff 100644 --- a/lib/test_suite.py +++ b/lib/test_suite.py @@ -1,7 +1,14 @@ -import ConfigParser +try: + # Python 2 + import ConfigParser as configparser +except ImportError: + # Python 3 + import configparser + import json import os import re +import sys from lib import Options from lib.app_server import AppServer @@ -90,7 +97,11 @@ def __init__(self, suite_path, args): raise RuntimeError("Suite %s doesn't exist" % repr(suite_path)) # read the suite config - config = ConfigParser.ConfigParser() + parser_kwargs = dict() + if sys.version_info[0] == 3: + parser_kwargs['inline_comment_prefixes'] = (';',) + parser_kwargs['strict'] = True + config = configparser.ConfigParser(**parser_kwargs) config.read(os.path.join(suite_path, "suite.ini")) self.ini.update(dict(config.items("default"))) self.ini.update(self.args.__dict__) From fcaabc6aa6f677dfeb8259f7852932fae269c6f9 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Thu, 4 Feb 2021 18:30:20 +0300 Subject: [PATCH 12/22] python3: conversion iteritems() -> items() Part of #20 --- lib/app_server.py | 2 +- lib/preprocessor.py | 4 ++-- listeners.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/app_server.py b/lib/app_server.py index 00a82b8f..c33e5fd2 100644 --- a/lib/app_server.py +++ b/lib/app_server.py @@ -252,7 +252,7 @@ def is_correct(run): test_suite.ini, params=params, conf_name=conf_name - ) for conf_name, params in runs.iteritems() + ) for conf_name, params in runs.items() if is_correct(conf_name)]) else: tests.append(AppTest(test_name, diff --git a/lib/preprocessor.py b/lib/preprocessor.py index b96795b0..a2c1232b 100644 --- a/lib/preprocessor.py +++ b/lib/preprocessor.py @@ -436,7 +436,7 @@ def stop_nondefault(self, signal=signal.SIGTERM): names), schema='info') if sys.stdout.__class__.__name__ == 'FilteredStream': sys.stdout.clear_all_filters() - for k, v in self.servers.iteritems(): + for k, v in self.servers.items(): # don't stop the default server if k == 'default': continue @@ -449,7 +449,7 @@ def cleanup_nondefault(self): names = [k for k in self.servers.keys() if k != 'default'] color_log('DEBUG: Cleanup non-default servers: {}\n'.format(names), schema='info') - for k, v in self.servers.iteritems(): + for k, v in self.servers.items(): # don't cleanup the default server if k == 'default': continue diff --git a/listeners.py b/listeners.py index f8301386..b6a9b79b 100644 --- a/listeners.py +++ b/listeners.py @@ -293,7 +293,7 @@ def process_timeout(self, delta_seconds): schema=color_schema) hung_tasks = [task for worker_id, task - in self.worker_current_task.iteritems() + in self.worker_current_task.items() if worker_id in worker_ids] for task in hung_tasks: result_file = task.task_tmp_result From 2de7d39c0aa79ca954c5ce10ad13e9394ba83da3 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Mon, 23 Nov 2020 11:40:13 +0000 Subject: [PATCH 13/22] python3: get rid of package level imports Unlike Python 2, Python 3 does not treat `import bar` from a module in a package `foo` as importing of `foo.bar`. Absolute import that is relative to a test-run root (like `import lib.utils`) still work, because `sys.path` contains a script directory (after symlinks resolution) on both Python versions. https://www.python.org/dev/peps/pep-0328/ Part of #20 Co-authored-by: Alexander Turenko --- lib/__init__.py | 2 +- lib/admin_connection.py | 6 +++--- lib/app_server.py | 2 +- lib/box_connection.py | 2 +- lib/connpool.py | 2 +- lib/server.py | 1 + lib/server_mixins.py | 6 ++++-- lib/tarantool_connection.py | 8 ++++---- lib/tarantool_server.py | 2 +- lib/test.py | 4 ++-- 10 files changed, 19 insertions(+), 16 deletions(-) diff --git a/lib/__init__.py b/lib/__init__.py index c03a6d86..0dffc40f 100644 --- a/lib/__init__.py +++ b/lib/__init__.py @@ -5,7 +5,7 @@ from lib.options import Options from lib.tarantool_server import TarantoolServer from lib.unittest_server import UnittestServer -from utils import warn_unix_sockets_at_start +from lib.utils import warn_unix_sockets_at_start __all__ = ['Options'] diff --git a/lib/admin_connection.py b/lib/admin_connection.py index 3f05546c..f3a7d97c 100644 --- a/lib/admin_connection.py +++ b/lib/admin_connection.py @@ -24,9 +24,9 @@ import re import sys -from tarantool_connection import TarantoolConnection -from tarantool_connection import TarantoolPool -from tarantool_connection import TarantoolAsyncConnection +from lib.tarantool_connection import TarantoolConnection +from lib.tarantool_connection import TarantoolPool +from lib.tarantool_connection import TarantoolAsyncConnection ADMIN_SEPARATOR = '\n' diff --git a/lib/app_server.py b/lib/app_server.py index c33e5fd2..e95c36d0 100644 --- a/lib/app_server.py +++ b/lib/app_server.py @@ -21,8 +21,8 @@ from lib.utils import format_process from lib.utils import signame from lib.utils import warn_unix_socket -from test import TestRunGreenlet, TestExecutionError from threading import Timer +from lib.test import TestRunGreenlet, TestExecutionError def timeout_handler(server_process, test_timeout): diff --git a/lib/box_connection.py b/lib/box_connection.py index dad84a20..9dd212d2 100644 --- a/lib/box_connection.py +++ b/lib/box_connection.py @@ -25,7 +25,7 @@ import ctypes import socket -from tarantool_connection import TarantoolConnection +from lib.tarantool_connection import TarantoolConnection # monkey patch tarantool and msgpack from lib.utils import check_libs diff --git a/lib/connpool.py b/lib/connpool.py index 1c296983..f2f633a3 100644 --- a/lib/connpool.py +++ b/lib/connpool.py @@ -10,7 +10,7 @@ from contextlib import contextmanager from functools import wraps -from test import TestRunGreenlet +from lib.test import TestRunGreenlet __all__ = ["ConnectionPool", "retry"] diff --git a/lib/server.py b/lib/server.py index c699997a..d13aa024 100644 --- a/lib/server.py +++ b/lib/server.py @@ -2,6 +2,7 @@ import os import shutil from itertools import product + from lib.server_mixins import ValgrindMixin from lib.server_mixins import GdbMixin from lib.server_mixins import GdbServerMixin diff --git a/lib/server_mixins.py b/lib/server_mixins.py index 44da01fe..b2ba8a69 100644 --- a/lib/server_mixins.py +++ b/lib/server_mixins.py @@ -1,11 +1,13 @@ import os import glob import shlex +from six.moves import shlex_quote + from lib.utils import find_in_path from lib.utils import print_tail_n from lib.utils import non_empty_valgrind_logs -from lib.colorer import color_stdout, color_log -from six.moves import shlex_quote +from lib.colorer import color_log +from lib.colorer import color_stdout def shlex_join(strings): diff --git a/lib/tarantool_connection.py b/lib/tarantool_connection.py index 019ed990..1669fe5c 100644 --- a/lib/tarantool_connection.py +++ b/lib/tarantool_connection.py @@ -30,10 +30,10 @@ import gevent from gevent import socket as gsocket -from connpool import ConnectionPool -from test import TestRunGreenlet -from utils import warn_unix_socket -from utils import set_fd_cloexec +from lib.connpool import ConnectionPool +from lib.test import TestRunGreenlet +from lib.utils import warn_unix_socket +from lib.utils import set_fd_cloexec class TarantoolPool(ConnectionPool): diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py index b26d2625..f759b45a 100644 --- a/lib/tarantool_server.py +++ b/lib/tarantool_server.py @@ -42,7 +42,7 @@ from lib.utils import signame from lib.utils import warn_unix_socket from lib.utils import prefix_each_line -from test import TestRunGreenlet, TestExecutionError +from lib.test import TestRunGreenlet, TestExecutionError def save_join(green_obj, timeout=None): diff --git a/lib/test.py b/lib/test.py index f7670063..1e863a4a 100644 --- a/lib/test.py +++ b/lib/test.py @@ -2,14 +2,12 @@ import gevent import os import pprint -import pytap13 import re import shutil import sys import traceback from functools import partial from hashlib import md5 -from utils import safe_makedirs try: # Python 2 @@ -23,6 +21,8 @@ from lib.utils import non_empty_valgrind_logs from lib.utils import print_tail_n from lib.utils import print_unidiff as utils_print_unidiff +from lib.utils import safe_makedirs +from lib import pytap13 class TestExecutionError(OSError): From f737c805531525802864043ecbadba0f1ba61eda Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Tue, 29 Dec 2020 14:21:16 +0000 Subject: [PATCH 14/22] python3: rename func_name to __name__ What's New In Python 3.0 [1]: The function attributes named func_X have been renamed to use the __X__ form, freeing up these names in the function attribute namespace for user-defined attributes. To wit, func_closure, func_code, func_defaults, func_dict, func_doc, func_globals, func_name were renamed to __closure__, __code__, __defaults__, __dict__, __doc__, __globals__, __name__, respectively. 1. https://docs.python.org/3/whatsnew/3.0.html#operators-and-special-methods Part of #20 --- lib/connpool.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/connpool.py b/lib/connpool.py index f2f633a3..b80ba584 100644 --- a/lib/connpool.py +++ b/lib/connpool.py @@ -134,7 +134,7 @@ def deco(*args, **kwargs): except exc_classes as e: if logger is not None: logger.log(retry_log_level, - retry_log_message.format(f=f.func_name, e=e)) + retry_log_message.format(f=f.__name__, e=e)) gevent.sleep(interval) failures += 1 if max_failures is not None \ @@ -142,6 +142,6 @@ def deco(*args, **kwargs): if logger is not None: logger.log(max_failure_log_level, max_failure_log_message.format( - f=f.func_name, e=e)) + f=f.__name__, e=e)) raise return deco From 7b092143bee8d7bcf34be019502c1e6f63af6f8c Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Tue, 12 Jan 2021 14:05:46 +0000 Subject: [PATCH 15/22] python3: use string byte literal Fix Tarantool connection liveness and box-py/call.test.py test. Part of #20 Co-authored-by: Alexander Turenko --- lib/tarantool_connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tarantool_connection.py b/lib/tarantool_connection.py index 1669fe5c..b246d4f4 100644 --- a/lib/tarantool_connection.py +++ b/lib/tarantool_connection.py @@ -146,7 +146,7 @@ def opt_reconnect(self): """ try: if not self.is_connected or self.socket.recv( - 1, socket.MSG_DONTWAIT | socket.MSG_PEEK) == '': + 1, socket.MSG_DONTWAIT | socket.MSG_PEEK) == b'': self.reconnect() except socket.error as e: if e.errno == errno.EAGAIN: From 6f8e6ac7829cdce24b277c47efbaeac6f7ec94f4 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Tue, 19 Jan 2021 11:48:48 +0000 Subject: [PATCH 16/22] python3: bump gevent version gevent 1.2 and below does not support Python 3.7 and above, see [1]. 1. https://github.com/gevent/gevent/issues/1297 Part of #20 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index a1c72dcb..365d1b1f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ PyYAML==5.1 argparse==1.1 msgpack-python==0.4.6 -gevent==1.1b5 +gevent==21.1.2 six>=1.8.0 From 260c10730f5ea9ddc7358778de88fd05b93c45cf Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 22 Jan 2021 12:44:40 +0000 Subject: [PATCH 17/22] python3: set 'fork' start method In Python 3 start method 'spawn' in multiprocessing module becomes default on Mac OS [1]. The 'spawn' method causes re-execution of some code, which is already executed in the main process. At least it is seen on the lib/__init__.py code, which removes the 'var' directory. Some other code may have side effects too, it requires investigation. The method also requires object serialization that doesn't work when objects use lambdas, whose for example used in class TestSuite (lib/test_suite.py). The latter problem is easy to fix, but the former looks more fundamental. So we stick to the 'fork' method now. The new start method is available on Python 3 only: Traceback (most recent call last): File "../../test/test-run.py", line 227, in multiprocessing.set_start_method('fork') AttributeError: 'module' object has no attribute 'set_start_method' 1. https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods Fixes #265 Part of #20 Co-authored-by: Alexander Turenko --- test-run.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test-run.py b/test-run.py index 5c205054..f8202725 100755 --- a/test-run.py +++ b/test-run.py @@ -56,6 +56,7 @@ from lib import Options from lib.colorer import color_stdout from lib.utils import print_tail_n +from lib.utils import PY3 from lib.worker import get_task_groups from lib.worker import get_reproduce_file from lib.worker import reproduce_task_groups @@ -218,6 +219,23 @@ def main_consistent(): if __name__ == "__main__": + # In Python 3 start method 'spawn' in multiprocessing module becomes + # default on Mac OS. + # + # The 'spawn' method causes re-execution of some code, which is already + # executed in the main process. At least it is seen on the + # lib/__init__.py code, which removes the 'var' directory. Some other + # code may have side effects too, it requires investigation. + # + # The method also requires object serialization that doesn't work when + # objects use lambdas, whose for example used in class TestSuite + # (lib/test_suite.py). + # + # The latter problem is easy to fix, but the former looks more + # fundamental. So we stick to the 'fork' method now. + if PY3: + multiprocessing.set_start_method('fork') + # don't sure why, but it values 1 or 2 gives 1.5x speedup for parallel # test-run (and almost doesn't affect consistent test-run) os.environ['OMP_NUM_THREADS'] = '2' From 277076380747ac6d874bd77dc6d08fd2c0c21e66 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Wed, 24 Feb 2021 15:37:53 +0300 Subject: [PATCH 18/22] python3: decouple bytes and strings In Python 2 default string type (``) is a binary string, non-unicode. We receive data from a socket, from a Popen stream, from a file as a string and operate on those strings without any conversions. Python 3 draws a line here. We usually operate on unicode strings in the code (because this is the default string type, ``), but receive bytes from a socket and a Popen stream. We can use unicode or binary streams for files (unicode by default[^1]). This commit decouples bytes and strings. In most cases it means that we convert data from bytes to a string after receiving from a socket / Popen stream and convert it back from a string to bytes before writting to a socket. Those operations are no-op on Python 2. So, the general rule for our APIs is to accept and return `` disregarding Python version. Not ``, not ``. The only non-trivial change is around `FilteredStream` and writes into `sys.stdout`. The `FilteredStream` instance replaces `sys.stdout` during execution of a test, so it should follow the usual convention and accept `` in the `write()` method. This is both intuitive and necessary, because `*.py` tests rely on `print('bla bla')` to write into a result file. However the stream should also accept ``, because we have a unit test (`unit/json.test`), which produces a binary output, which does not conform UTF-8 encoding. The separate `write_bytes()` method was introduced for this sake. UnittestServer and AppServer write tests output as bytes directly, TarantoolServer rely on the usual string output. We also use bytes directly, when write from one stream to another one: in `app_server.py` for stderr (writting to a log file), in `tarantool_server.py` for log destination property (because it is the destination for Popen). [^1]: Technically it depends on a Python version and a system locale. There are situations, when default text file streams encoding is not 'utf-8'. They will be handled in the next commit. Part of #20 Co-authored-by: Sergey Bronnikov --- lib/admin_connection.py | 12 +++++++----- lib/app_server.py | 4 ++-- lib/inspector.py | 6 ++++-- lib/tarantool_server.py | 7 ++++--- lib/test.py | 41 ++++++++++++++++++++++------------------- lib/unittest_server.py | 2 +- lib/utils.py | 39 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 79 insertions(+), 32 deletions(-) diff --git a/lib/admin_connection.py b/lib/admin_connection.py index f3a7d97c..49ac1079 100644 --- a/lib/admin_connection.py +++ b/lib/admin_connection.py @@ -28,6 +28,8 @@ from lib.tarantool_connection import TarantoolPool from lib.tarantool_connection import TarantoolAsyncConnection +from lib.utils import bytes_to_str +from lib.utils import str_to_bytes ADMIN_SEPARATOR = '\n' @@ -36,13 +38,13 @@ def get_handshake(sock, length=128, max_try=100): """ Correct way to get tarantool handshake """ - result = "" + result = b"" i = 0 while len(result) != length and i < max_try: - result = "%s%s" % (result, sock.recv(length-len(result))) + result = b"%s%s" % (result, sock.recv(length-len(result))) # max_try counter for tarantool/gh-1362 i += 1 - return result + return bytes_to_str(result) class AdminPool(TarantoolPool): @@ -61,12 +63,12 @@ def _new_connection(self): class ExecMixIn(object): def cmd(self, socket, cmd, silent): - socket.sendall(cmd) + socket.sendall(str_to_bytes(cmd)) bufsiz = 4096 res = "" while True: - buf = socket.recv(bufsiz) + buf = bytes_to_str(socket.recv(bufsiz)) if not buf: break res = res + buf diff --git a/lib/app_server.py b/lib/app_server.py index e95c36d0..d4675c8a 100644 --- a/lib/app_server.py +++ b/lib/app_server.py @@ -38,8 +38,8 @@ def run_server(execs, cwd, server, logfile, retval): timer.start() stdout, stderr = server.process.communicate() timer.cancel() - sys.stdout.write(stdout) - with open(logfile, 'a') as f: + sys.stdout.write_bytes(stdout) + with open(logfile, 'ab') as f: f.write(stderr) retval['returncode'] = server.process.wait() server.process = None diff --git a/lib/inspector.py b/lib/inspector.py index 1d68deaa..9733cc1f 100644 --- a/lib/inspector.py +++ b/lib/inspector.py @@ -6,8 +6,10 @@ from gevent.lock import Semaphore from gevent.server import StreamServer +from lib.utils import bytes_to_str from lib.utils import find_port from lib.utils import prefix_each_line +from lib.utils import str_to_bytes from lib.colorer import color_stdout from lib.colorer import color_log from lib.colorer import qa_notice @@ -77,7 +79,7 @@ def readline(socket, delimiter='\n', size=4096): while data: try: - data = socket.recv(size) + data = bytes_to_str(socket.recv(size)) except IOError: # catch instance halt connection refused errors data = '' @@ -119,7 +121,7 @@ def handle(self, socket, addr): color_log("DEBUG: test-run's response for [{}]\n{}\n".format( line, prefix_each_line(' | ', result)), schema='test-run command') - socket.sendall(result) + socket.sendall(str_to_bytes(result)) self.sem.release() diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py index f759b45a..f611dbaf 100644 --- a/lib/tarantool_server.py +++ b/lib/tarantool_server.py @@ -35,6 +35,7 @@ from lib.server import Server from lib.server import DEFAULT_SNAPSHOT_NAME from lib.test import Test +from lib.utils import bytes_to_str from lib.utils import find_port from lib.utils import extract_schema_from_snapshot from lib.utils import format_process @@ -577,7 +578,7 @@ def _iproto(self, port): @property def log_des(self): if not hasattr(self, '_log_des'): - self._log_des = open(self.logfile, 'a') + self._log_des = open(self.logfile, 'ab') return self._log_des @log_des.deleter @@ -663,7 +664,7 @@ def __del__(self): @classmethod def version(cls): p = subprocess.Popen([cls.binary, "--version"], stdout=subprocess.PIPE) - version = p.stdout.read().rstrip() + version = bytes_to_str(p.stdout.read()).rstrip() p.wait() return version @@ -1162,7 +1163,7 @@ def test_option_get(self, option_list_str, silent=False): cwd=self.vardir, stdout=subprocess.PIPE, stderr=subprocess.STDOUT).stdout.read() - return output + return bytes_to_str(output) def test_option(self, option_list_str): print(self.test_option_get(option_list_str)) diff --git a/lib/test.py b/lib/test.py index 1e863a4a..0f002802 100644 --- a/lib/test.py +++ b/lib/test.py @@ -9,19 +9,14 @@ from functools import partial from hashlib import md5 -try: - # Python 2 - from StringIO import StringIO -except ImportError: - # Python 3 - from io import StringIO - from lib import Options from lib.colorer import color_stdout +from lib.utils import assert_bytes from lib.utils import non_empty_valgrind_logs from lib.utils import print_tail_n from lib.utils import print_unidiff as utils_print_unidiff from lib.utils import safe_makedirs +from lib.utils import str_to_bytes from lib import pytap13 @@ -48,21 +43,17 @@ def __repr__(self): class FilteredStream: """Helper class to filter .result file output""" def __init__(self, filename): - # - # always open the output stream in line-buffered mode, - # to see partial results of a failed test - # - self.stream = open(filename, "w+", 1) + self.stream = open(filename, "wb+") self.filters = [] self.inspector = None - def write(self, fragment): - """Apply all filters, then write result to the undelrying stream. - Do line-oriented filtering: the fragment doesn't have to represent - just one line.""" - fragment_stream = StringIO(fragment) + def write_bytes(self, fragment): + """ The same as ``write()``, but accepts ```` as + input. + """ + assert_bytes(fragment) skipped = False - for line in fragment_stream: + for line in fragment.splitlines(True): original_len = len(line.strip()) for pattern, replacement in self.filters: line = re.sub(pattern, replacement, line) @@ -73,8 +64,20 @@ def write(self, fragment): if not skipped: self.stream.write(line) + def write(self, fragment): + """ Apply all filters, then write result to the underlying + stream. + + Do line-oriented filtering: the fragment doesn't have + to represent just one line. + + Accepts ```` as input, just like the standard + ``sys.stdout.write()``. + """ + self.write_bytes(str_to_bytes(fragment)) + def push_filter(self, pattern, replacement): - self.filters.append([pattern, replacement]) + self.filters.append([str_to_bytes(pattern), str_to_bytes(replacement)]) def pop_filter(self): self.filters.pop() diff --git a/lib/unittest_server.py b/lib/unittest_server.py index a7cc20e5..d57de52c 100644 --- a/lib/unittest_server.py +++ b/lib/unittest_server.py @@ -16,7 +16,7 @@ def execute(self, server): server.current_test = self execs = server.prepare_args() proc = Popen(execs, cwd=server.vardir, stdout=PIPE, stderr=STDOUT) - sys.stdout.write(proc.communicate()[0]) + sys.stdout.write_bytes(proc.communicate()[0]) class UnittestServer(Server): diff --git a/lib/utils.py b/lib/utils.py index 21834e5f..a92db07c 100644 --- a/lib/utils.py +++ b/lib/utils.py @@ -22,6 +22,7 @@ # Useful for very coarse version differentiation. PY3 = sys.version_info[0] == 3 +PY2 = sys.version_info[0] == 2 if PY3: string_types = str, @@ -312,3 +313,41 @@ def extract_schema_from_snapshot(snapshot_path): if res[0] == 'version': return res return None + + +def assert_bytes(b): + """ Ensure given value is . + """ + if type(b) != bytes: + raise ValueError('Internal error: expected {}, got {}: {}'.format( + str(bytes), str(type(b)), repr(b))) + + +def assert_str(s): + """ Ensure given value is . + """ + if type(s) != str: + raise ValueError('Internal error: expected {}, got {}: {}'.format( + str(str), str(type(s)), repr(s))) + + +def bytes_to_str(b): + """ Convert to . + + No-op on Python 2. + """ + assert_bytes(b) + if PY2: + return b + return b.decode('utf-8') + + +def str_to_bytes(s): + """ Convert to . + + No-op on Python 2. + """ + assert_str(s) + if PY2: + return s + return s.encode('utf-8') From e7c61875a7bb71dee19303d6ef483ce325c96760 Mon Sep 17 00:00:00 2001 From: Alexander Turenko Date: Fri, 12 Mar 2021 23:37:39 +0300 Subject: [PATCH 19/22] python3: set text file streams encoding to utf-8 The problem: test files, result files contain UTF-8 symbols, which are out of ASCII range. Just `open(file, 'r').read()` without the encoding='utf-8' argument fails to decode them, when the default encoding for file text streams is not 'utf-8'. We meet this situation on Python 3.6.8 (provided by CentOS 7 and CentOS 8), when the POSIX locale is set (`LC_ALL=C`). The solution is described in the code comment: replace `open()` built-in function and always set `encoding='utf-8'`. That's hacky way, but it looks better than change every `open()` call across the code and don't forget to do that in all future code (and keep Python 2 compatibility in the mind). But maybe we'll revisit the approach later. There is another way to hack the `open()` behaviour that works for me on Python 3.6.8: | import _bootlocale | _bootlocale.getpreferredencoding = (lambda *args: 'utf8') However it leans on Python internals and looks less reliable than the implemented one. Part of #20 --- test-run.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test-run.py b/test-run.py index f8202725..e778adf7 100755 --- a/test-run.py +++ b/test-run.py @@ -236,6 +236,36 @@ def main_consistent(): if PY3: multiprocessing.set_start_method('fork') + # test-run assumes that text file streams are UTF-8 (as + # contrary to ASCII) on Python 3. It is necessary to process + # non ASCII symbols in test files, result files and so on. + # + # Default text file stream encoding depends on a system + # locale with exception for the POSIX locale (C locale): in + # this case UTF-8 is used (see PEP-0540). Sadly, this + # behaviour is in effect since Python 3.7. + # + # We want to achieve the same behaviour on lower Python + # versions, at least on 3.6.8, which is provided by CentOS 7 + # and CentOS 8. + # + # So we hack the open() builtin. + # + # https://stackoverflow.com/a/53347548/1598057 + if PY3 and sys.version_info[0:2] < (3, 7): + std_open = __builtins__.open + + def open_as_utf8(*args, **kwargs): + if len(args) >= 2: + mode = args[1] + else: + mode = kwargs.get('mode', '') + if 'b' not in mode: + kwargs.setdefault('encoding', 'utf-8') + return std_open(*args, **kwargs) + + __builtins__.open = open_as_utf8 + # don't sure why, but it values 1 or 2 gives 1.5x speedup for parallel # test-run (and almost doesn't affect consistent test-run) os.environ['OMP_NUM_THREADS'] = '2' From 59c976ffbc665914c0e77a17231b823ebd3acb68 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Wed, 10 Feb 2021 18:13:49 +0300 Subject: [PATCH 20/22] python3: make path to unit test absolute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Tarantool unit tests runs using test-run error with output like below may happen: [027] small/small_class.test [027] Test.run() received the following error: [027] Traceback (most recent call last): [027] File "/__w/tarantool/tarantool/test-run/lib/test.py", line 192, in run [027] self.execute(server) [027] File "/__w/tarantool/tarantool/test-run/lib/unittest_server.py", line 20, in execute [027] proc = Popen(execs, cwd=server.vardir, stdout=PIPE, stderr=STDOUT) [027] File "/usr/lib/python3.5/subprocess.py", line 676, in __init__ [027] restore_signals, start_new_session) [027] File "/usr/lib/python3.5/subprocess.py", line 1282, in _execute_child [027] raise child_exception_type(errno_num, err_msg) [027] FileNotFoundError: [Errno 2] No such file or directory: '../test/small/small_class.test' [027] [ fail ] The root cause of error is changed behaviour of Popen in Python 3 in comparison to Python 2. One should explicitly set path to a current work dir: Python 2 [1]: "If cwd is not None, the child’s current directory will be changed to cwd before it is executed. Note that this directory is not considered when searching the executable, so you can’t specify the program’s path relative to cwd." Python 3 [2]: "If cwd is not None, the function changes the working directory to cwd before executing the child. <...> In particular, the function looks for executable (or for the first item in args) relative to cwd if the executable path is a relative path." 1. https://docs.python.org/2/library/subprocess.html#subprocess.Popen 2. https://docs.python.org/3/library/subprocess.html#subprocess.Popen Part of #20 --- lib/unittest_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/unittest_server.py b/lib/unittest_server.py index d57de52c..4df8f0b0 100644 --- a/lib/unittest_server.py +++ b/lib/unittest_server.py @@ -48,7 +48,7 @@ def binary(self): def prepare_args(self, args=[]): executable_path = os.path.join(self.builddir, "test", self.current_test.name) - return [executable_path] + args + return [os.path.abspath(executable_path)] + args def deploy(self, vardir=None, silent=True, wait=True): self.vardir = vardir From 22c3e24846812b047038dd46867d57d24d77afc0 Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Fri, 12 Mar 2021 14:33:10 +0300 Subject: [PATCH 21/22] python3: fix printing of called box command To make test box-py/call.test.py compatible with Python 2 and Python 3 it must have the same output when running with both versions of interpeter. With running on Python 2.7 output of called commands concluded to round brackets while on Python 3 it is not. [001] @@ -20,7 +20,7 @@ [001] - true [001] - null [001] ... [001] -call f1 () [001] +('call ', 'f1', ((),)) [001] - 'testing' [001] - 1 [001] - False To fix it print should use formatted string in print() call. Part of #20 --- lib/box_connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/box_connection.py b/lib/box_connection.py index 9dd212d2..6a3617a7 100644 --- a/lib/box_connection.py +++ b/lib/box_connection.py @@ -88,7 +88,7 @@ def execute(self, command, silent=True): def call(self, command, *args): if not command: return - print('call ', command, args) + print('call {} {}'.format(command, args)) response = self.py_con.call(command, *args) result = str(response) print(result) From e93d067763b6ced843d29947ac95d83c290309ec Mon Sep 17 00:00:00 2001 From: Sergey Bronnikov Date: Mon, 18 Jan 2021 15:09:45 +0000 Subject: [PATCH 22/22] python3: use python 3 in test-run.py's shebang Related issues and changes: - Support Python 3 in Tarantool test suite https://github.com/tarantool/tarantool/issues/5538 - Python 3 in tarantool-python https://github.com/tarantool/tarantool-python/pull/181 https://github.com/tarantool/tarantool-python/pull/186 - Use Python 3 in a test infrastructure https://github.com/tarantool/tarantool/issues/5652 Closes #20 --- test-run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-run.py b/test-run.py index e778adf7..0b90494e 100755 --- a/test-run.py +++ b/test-run.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python2 +#!/usr/bin/env python3 """Tarantool regression test suite front-end.""" # Redistribution and use in source and binary forms, with or without