From 17bee727326855f3558c8b6b46cc4f80ad859f65 Mon Sep 17 00:00:00 2001 From: "Alexander V. Tikhonov" Date: Wed, 9 Jun 2021 10:56:13 +0000 Subject: [PATCH 1/3] Add default server restart to each test run Found that the root cause of the issues happened with vinyl tests were backside effects of the not correct test 'vinyl/gh.test.lua' which leaved Tarantool worker process in inconsistent state. After it any other next test on the same Tarantool worker process could fail on running testings with snapshots calls, like tarantool/tarantool-qa#126: error: Snapshot is already in progress Either restarting Tarantool worker process could fail on stopping it, like #261 and tarantool/tarantool#5141: E> failed to process vylog record: delete_slice{slice_id=115, } E> ER_INVALID_VYLOG_FILE: Invalid VYLOG file: Slice 115 deleted but not registered Decided to avoid of such situations for now and in the future all tests must be run after Tarantool worker default server process was restarted. This patch moves this server restart call from check if the test failed to the common part of the tests run loop. Fixes #260 Fixes #261 Needed for tarantool/tarantool#5089 Closes tarantool/tarantool-qa#126 --- lib/worker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/worker.py b/lib/worker.py index adfe3c72..a73bbcc1 100644 --- a/lib/worker.py +++ b/lib/worker.py @@ -350,13 +350,13 @@ def run_loop(self, task_queue, result_queue): retries_left = self.suite.fragile_retries() # let's run till short_status became 'pass' while short_status != 'pass' and retries_left >= 0: + self.restart_server() # print message only after some fails occurred if short_status == 'fail': - self.restart_server() color_stdout( 'Test "%s", conf: "%s"\n' '\tfrom "fragile" list failed with results' - ' file checksum: "%s", rerunning with server restart ...\n' + ' file checksum: "%s", rerunning ...\n' % (task_id[0], task_id[1], result_checksum), schema='error') # run task and save the result to short_status short_status, result_checksum, duration = self.run_task(task_id) From b5adf2eca5afa7fed5b46dc9fe060f8a700c5b52 Mon Sep 17 00:00:00 2001 From: "Alexander V. Tikhonov" Date: Fri, 11 Jun 2021 13:49:34 +0000 Subject: [PATCH 2/3] Remove unused pretest_clean.lua script After the previous commit in the current patchset we don't use the same tarantool server process from test to test and we don't need to use any scripts like 'pretest_clean.lua' to cleanup it between tests. So this patch removes this script. --- README.md | 39 +---- lib/tarantool_server.py | 29 +--- pretest_clean.lua | 311 ---------------------------------------- 3 files changed, 7 insertions(+), 372 deletions(-) delete mode 100644 pretest_clean.lua diff --git a/README.md b/README.md index cf77b6f6..1326c516 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,6 @@ and a number of fields: * `long_run` - mark tests as long, enabled only with `--long` option (delimited with the space, e.g. `long_run=t1.test.lua t2.test.lua`) * `config` - test configuration file name -* `pretest_clean` - see [pretest_clean](#pretest_clean) Field `core` must be one of: @@ -338,39 +337,13 @@ test_run:cmd('setopt delimiter ""'); join(test_run, 30) ``` -### pretest_clean +### pretest_clean() -Add the following line to `suite.ini` to enable this option for a test suite: +Nothing will be done before a Python test and for `core = unittest` +test suites. -``` -pretest_clean = True -``` - -The behaviour of this option varies across test suite types (`core = tarantool / -app / unittest`). - -For a `core = tarantool` test suite enabling of this option will lead to -execution of a special clean function before each Lua test. The -`pretest_clean.lua` file is copied into a test directory (`var/ddd-suite-name`) -and `require('pretest_clean').clean()` is invoked before each test. - -This function performs the following steps: - -* drop all non-system spaces; -* delete all users except 'guest' and 'admin'; -* delete all roles except 'public', 'replicaiton' and 'super'; -* delete all `box.space._func` records except `box.schema.user.info`; -* delete all `box.space._cluster` records except one for a current instance; -* remove all global variables except ones that tarantool had at start (it uses a - predefined list, see the source); -* unload all packages except built-in ones (see the source for the list). - -Set _G.protected_globals to list of names to protect custom globals. - -Nothing will be done before a Python test. - -For a `core = app` test suite enabling of this option will lead to removing -tarantool WAL and snapshot files before each test. +For a `core = [app|tarantool]` test suites this function removes tarantool WAL +and snapshot files before each test. The following files will be removed: @@ -380,8 +353,6 @@ The following files will be removed: * `*.inprogress` * `[0-9]*/` -For a `core = unittest` test suite this option does not change any behaviour. - ### Used By - [Tarantool](https://github.com/tarantool/tarantool) - in-memory database and application server diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py index a2bc97ac..f7370e7b 100644 --- a/lib/tarantool_server.py +++ b/lib/tarantool_server.py @@ -122,25 +122,6 @@ def write_result_file_version_line(self): sys.stdout.write(self.RESULT_FILE_VERSION_TEMPLATE.format( self.result_file_version) + '\n') - def execute_pretest_clean(self, ts): - """ Clean globals, loaded packages, spaces, users, roles - and so on before each test if the option is set. - - Return True as success (or if this feature is disabled - in suite.ini) and False in case of an error. - """ - if not self.suite_ini['pretest_clean']: - return True - - command = "require('pretest_clean').clean()" - result = self.send_command(command, ts, 'lua') - result = result.replace('\r\n', '\n') - if result != '---\n...\n': - sys.stdout.write(result) - return False - - return True - def execute_pragma_sql_default_engine(self, ts): """ Set default engine for an SQL test if it is provided in a configuration. @@ -239,8 +220,6 @@ def flush(self, ts, command_log, command_exe): def exec_loop(self, ts): self.write_result_file_version_line() - if not self.execute_pretest_clean(ts): - return if not self.execute_pragma_sql_default_engine(ts): return @@ -789,10 +768,6 @@ def copy_files(self): shutil.copy(tntctl_file, self.vardir) shutil.copy(os.path.join(self.TEST_RUN_DIR, 'test_run.lua'), self.vardir) - # Need to use get here because of nondefault servers doesn't have ini. - if self.ini.get('pretest_clean', False): - shutil.copy(os.path.join(self.TEST_RUN_DIR, 'pretest_clean.lua'), - self.vardir) if self.snapshot_path: # Copy snapshot to the workdir. @@ -819,8 +794,8 @@ def prepare_args(self, args=[]): return cli_args def pretest_clean(self): - # Don't delete snap and logs for 'default' tarantool server - # because it works during worker lifetime. + # Tarantool servers restarts before each test on the worker. + # Snap and logs are removed within it. pass def cleanup(self, *args, **kwargs): diff --git a/pretest_clean.lua b/pretest_clean.lua deleted file mode 100644 index ec0dc204..00000000 --- a/pretest_clean.lua +++ /dev/null @@ -1,311 +0,0 @@ --- Copy of cleanup_cluster() from test_run.lua. -local function cleanup_cluster() - local cluster = box.space._cluster:select() - for _, tuple in pairs(cluster) do - if tuple[1] ~= box.info.id then - box.space._cluster:delete(tuple[1]) - end - end -end - -local function cleanup_list(list, allowed) - for k, _ in pairs(list) do - if not allowed[k] then - list[k] = nil - end - end -end - -local function clean() - local _SPACE_NAME = 3 - - box.space._space:pairs():map(function(tuple) - local name = tuple[_SPACE_NAME] - return name - end):filter(function(name) - -- skip internal spaces - local first_char = string.sub(name, 1, 1) - return first_char ~= '_' - end):each(function(name) - box.space[name]:drop() - end) - - local _USER_TYPE = 4 - local _USER_NAME = 3 - - local allowed_users = { - guest = true, - admin = true, - } - box.space._user:pairs():filter(function(tuple) - local tuple_type = tuple[_USER_TYPE] - return tuple_type == 'user' - end):map(function(tuple) - local name = tuple[_USER_NAME] - return name - end):filter(function(name) - return not allowed_users[name] - end):each(function(name) - box.schema.user.drop(name) - end) - - local allowed_roles = { - public = true, - replication = true, - super = true, - } - box.space._user:pairs():filter(function(tuple) - local tuple_type = tuple[_USER_TYPE] - return tuple_type == 'role' - end):map(function(tuple) - local name = tuple[_USER_NAME] - return name - end):filter(function(name) - return not allowed_roles[name] - end):each(function(name) - box.schema.role.drop(name) - end) - - local _FUNC_NAME = 3 - local allowed_funcs = { - ['box.schema.user.info'] = true, - ['TRIM'] = true, - ['TYPEOF'] = true, - ['PRINTF'] = true, - ['UNICODE'] = true, - ['CHAR'] = true, - ['HEX'] = true, - ['VERSION'] = true, - ['QUOTE'] = true, - ['REPLACE'] = true, - ['SUBSTR'] = true, - ['GROUP_CONCAT'] = true, - ['JULIANDAY'] = true, - ['DATE'] = true, - ['TIME'] = true, - ['DATETIME'] = true, - ['STRFTIME'] = true, - ['CURRENT_TIME'] = true, - ['CURRENT_TIMESTAMP'] = true, - ['CURRENT_DATE'] = true, - ['LENGTH'] = true, - ['POSITION'] = true, - ['ROUND'] = true, - ['UPPER'] = true, - ['LOWER'] = true, - ['IFNULL'] = true, - ['RANDOM'] = true, - ['CEIL'] = true, - ['CEILING'] = true, - ['CHARACTER_LENGTH'] = true, - ['CHAR_LENGTH'] = true, - ['FLOOR'] = true, - ['MOD'] = true, - ['OCTET_LENGTH'] = true, - ['ROW_COUNT'] = true, - ['COUNT'] = true, - ['LIKE'] = true, - ['ABS'] = true, - ['EXP'] = true, - ['LN'] = true, - ['POWER'] = true, - ['SQRT'] = true, - ['SUM'] = true, - ['TOTAL'] = true, - ['AVG'] = true, - ['RANDOMBLOB'] = true, - ['NULLIF'] = true, - ['ZEROBLOB'] = true, - ['MIN'] = true, - ['MAX'] = true, - ['COALESCE'] = true, - ['EVERY'] = true, - ['EXISTS'] = true, - ['EXTRACT'] = true, - ['SOME'] = true, - ['GREATER'] = true, - ['LESSER'] = true, - ['SOUNDEX'] = true, - ['LIKELIHOOD'] = true, - ['LIKELY'] = true, - ['UNLIKELY'] = true, - ['GREATEST'] = true, - ['LEAST'] = true, - ['_sql_stat_get'] = true, - ['_sql_stat_push'] = true, - ['_sql_stat_init'] = true, - ['LUA'] = true, - ['UUID'] = true, - } - box.space._func:pairs():map(function(tuple) - local name = tuple[_FUNC_NAME] - return name - end):filter(function(name) - return not allowed_funcs[name] - end):each(function(name) - box.schema.func.drop(name) - end) - - cleanup_cluster() - - local allowed_globals = { - -- modules - bit = true, - coroutine = true, - debug = true, - io = true, - jit = true, - math = true, - misc = true, - os = true, - package = true, - string = true, - table = true, - utf8 = true, - -- variables - _G = true, - _VERSION = true, - arg = true, - -- functions - assert = true, - collectgarbage = true, - dofile = true, - error = true, - gcinfo = true, - getfenv = true, - getmetatable = true, - ipairs = true, - load = true, - loadfile = true, - loadstring = true, - module = true, - next = true, - pairs = true, - pcall = true, - print = true, - rawequal = true, - rawget = true, - rawset = true, - require = true, - select = true, - setfenv = true, - setmetatable = true, - tonumber = true, - tonumber64 = true, - tostring = true, - type = true, - unpack = true, - xpcall = true, - -- tarantool - _TARANTOOL = true, - box = true, - dostring = true, - help = true, - newproxy = true, - role_check_grant_revoke_of_sys_priv = true, - tutorial = true, - update_format = true, - protected_globals = true, - } - for _, name in ipairs(rawget(_G, 'protected_globals') or {}) do - allowed_globals[name] = true - end - cleanup_list(_G, allowed_globals) - - -- Strict module tracks declared global variables when the - -- strict mode is enabled, so we need to flush its internal - -- state. - local mt = getmetatable(_G) - if mt ~= nil and type(mt.__declared) == 'table' then - cleanup_list(mt.__declared, allowed_globals) - end - - local allowed_packages = { - ['_G'] = true, - bit = true, - box = true, - ['box.backup'] = true, - ['box.internal'] = true, - ['box.internal.sequence'] = true, - ['box.internal.session'] = true, - ['box.internal.space'] = true, - buffer = true, - clock = true, - console = true, - coroutine = true, - crypto = true, - csv = true, - debug = true, - decimal = true, - digest = true, - errno = true, - ffi = true, - fiber = true, - fio = true, - fun = true, - help = true, - ['help.en_US'] = true, - ['http.client'] = true, - iconv = true, - ['internal.argparse'] = true, - ['internal.trigger'] = true, - io = true, - jit = true, - ['jit.bc'] = true, - ['jit.bcsave'] = true, - ['jit.dis_arm64'] = true, - ['jit.dis_x64'] = true, - ['jit.dis_x86'] = true, - ['jit.dump'] = true, - ['jit.opt'] = true, - ['jit.p'] = true, - ['jit.profile'] = true, - ['jit.util'] = true, - ['jit.v'] = true, - ['jit.vmdef'] = true, - ['jit.zone'] = true, - json = true, - log = true, - math = true, - misc = true, - msgpack = true, - msgpackffi = true, - ['net.box'] = true, - ['net.box.lib'] = true, - os = true, - package = true, - pickle = true, - popen = true, - pwd = true, - socket = true, - strict = true, - string = true, - table = true, - ['table.clear'] = true, - ['table.new'] = true, - tap = true, - tarantool = true, - title = true, - uri = true, - utf8 = true, - uuid = true, - xlog = true, - yaml = true, - } - cleanup_list(package.loaded, allowed_packages) - - local user_count = box.space._user:count() - assert(user_count == 4 or user_count == 5, - 'box.space._user:count() should be 4 (1.10) or 5 (2.0)') - assert(box.space._cluster:count() == 1, - 'box.space._cluster:count() should be only one') - - -- Ensure all traces of a previous test are gone: open - -- iterators and so on. They can affect statistics counters - -- that may be important for a test. - collectgarbage() -end - -return { - clean = clean; -} From 5c6417f537924b97c0ce456e9ae549e846f07e07 Mon Sep 17 00:00:00 2001 From: "Alexander V. Tikhonov" Date: Fri, 18 Jun 2021 16:23:05 +0000 Subject: [PATCH 3/3] Remove unneeded suite.ini 'pretest_clean' option Found that all currently used suite.ini files use this option enabled. Also there is no need to have any suite of tests where snap either log files whould be used by the next test from the previous, due to such test schema can be easily implemented within single test file. --- lib/test.py | 4 ++-- lib/test_suite.py | 1 - test/test-app/suite.ini | 1 - test/test-tarantool/suite.ini | 1 - 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/test.py b/lib/test.py index 58929192..d29896ae 100644 --- a/lib/test.py +++ b/lib/test.py @@ -151,8 +151,8 @@ def execute(self, server): # Note: don't forget to set 'server.current_test = self' in # inherited classes. Crash reporting relying on that. server.current_test = self - if self.suite_ini['pretest_clean']: - server.pretest_clean() + # All the test runs must be isolated between each other on each worker. + server.pretest_clean() def run(self, server): """ Execute the test assuming it's a python program. If the test diff --git a/lib/test_suite.py b/lib/test_suite.py index fd8ad008..c71e8351 100644 --- a/lib/test_suite.py +++ b/lib/test_suite.py @@ -136,7 +136,6 @@ def __init__(self, suite_path, args): # use old format dictionary self.fragile['tests'] = self.ini['fragile'] - self.parse_bool_opt('pretest_clean', False) self.parse_bool_opt('use_unix_sockets', False) self.parse_bool_opt('use_unix_sockets_iproto', False) self.parse_bool_opt('is_parallel', False) diff --git a/test/test-app/suite.ini b/test/test-app/suite.ini index 5b18ac67..7f0e64fd 100644 --- a/test/test-app/suite.ini +++ b/test/test-app/suite.ini @@ -2,5 +2,4 @@ core = app description = application tests is_parallel = True -pretest_clean = True use_unix_sockets_iproto = True diff --git a/test/test-tarantool/suite.ini b/test/test-tarantool/suite.ini index 1bfddc35..a477019b 100644 --- a/test/test-tarantool/suite.ini +++ b/test/test-tarantool/suite.ini @@ -3,5 +3,4 @@ core = tarantool description = tarantool tests script = box.lua use_unix_sockets = True -pretest_clean = True config = engine.cfg