From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9CD0A4765E0 for ; Tue, 29 Dec 2020 20:08:04 +0300 (MSK) References: <54be7a8eac0b6f9291e03b7f9ef26bbcc367e7b0.1609259010.git.artemreyt@tarantool.org> From: Leonid Vasiliev Message-ID: Date: Tue, 29 Dec 2020 20:07:55 +0300 MIME-Version: 1.0 In-Reply-To: <54be7a8eac0b6f9291e03b7f9ef26bbcc367e7b0.1609259010.git.artemreyt@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Artem Starshov , Alexander Turenko Cc: tarantool-patches@dev.tarantool.org Hi! Thank you for the patch. See some comments below: - I think you don't need to introduce a "new version". I propose to split this patch into 2 separate patches. First - increase the timeout, second - move common code. On 29.12.2020 19:25, Artem Starshov wrote: > Functions with timeout transferred to module > test/app-tap/lua/process_timeout.lua for futher > using in other tests. > > Before this version of test was already done, > but by mistake older version was rebased on > master. > --- > .../gh-4983-tnt-e-assert-false-hangs.test.lua | 70 ++++--------------- > test/app-tap/lua/process_timeout.lua | 57 +++++++++++++++ > test/app-tap/suite.ini | 2 +- > 3 files changed, 70 insertions(+), 59 deletions(-) > create mode 100644 test/app-tap/lua/process_timeout.lua > > diff --git a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua > index ff3281e0f..9d0f0e281 100755 > --- a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua > +++ b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua > @@ -1,70 +1,24 @@ > #!/usr/bin/env tarantool > > -local fiber = require('fiber') > -local clock = require('clock') > -local ffi = require('ffi') > -local fio = require('fio') > -local errno = require('errno') > +local common = require('process_timeout') > local tap = require('tap') > +local fio = require('fio') > +local ffi = require('ffi') > > -- > -- gh-4983: tarantool -e 'assert(false)' hangs > -- > > --- For process_is_alive. > -ffi.cdef([[ > - int kill(pid_t pid, int sig); > -]]) > - > ---- Verify whether a process is alive. > -local function process_is_alive(pid) > - local rc = ffi.C.kill(pid, 0) > - return rc == 0 or errno() ~= errno.ESRCH > -end > - > ---- Returns true if process completed before timeout, false otherwise. > -local function wait_process_completion(pid, timeout) > - local start_time = clock.monotonic() > - local process_completed = false > - while clock.monotonic() - start_time < timeout do > - if not process_is_alive(pid) then > - process_completed = true > - break > - end > - fiber.sleep(0.01) > - end > - return process_completed > -end > - > ---- Open file on reading with timeout. > -local function open_with_timeout(filename, timeout) > - local fh > - local start_time = clock.monotonic() > - while not fh and clock.monotonic() - start_time < timeout do > - fh = fio.open(filename, {'O_RDONLY'}) > - end > - return fh > -end > - > ---- Try to read from file with timeout with interval. > -local function read_with_timeout(fh, timeout, interval) > - local data = '' > - local start_time = clock.monotonic() > - while #data == 0 and clock.monotonic() - start_time < timeout do > - data = fh:read() > - if #data == 0 then fiber.sleep(interval) end > - end > - return data > -end > - > local TARANTOOL_PATH = arg[-1] > local output_file = fio.abspath('out.txt') > local line = ('%s -e "assert(false)" > %s 2>&1 & echo $!'): > format(TARANTOOL_PATH, output_file) > -local process_waiting_timeout = 30.0 > -local file_read_timeout = 30.0 > -local file_read_interval = 0.2 > -local file_open_timeout = 30.0 > + > +-- Like a default timeout for `cond_wait` in test-run > +local process_waiting_timeout = 60.0 > +local file_read_timeout = 60.0 > +local file_open_timeout = 60.0 > +local file_read_interval = 0.01 > > local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test) > test:plan(2) > @@ -72,7 +26,7 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test) > local pid = tonumber(io.popen(line):read('*line')) > assert(pid, 'pid of proccess can\'t be recieved') > > - local process_completed = wait_process_completion(pid, > + local process_completed = common.wait_process_completion(pid, > process_waiting_timeout) > > local details > @@ -91,10 +45,10 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test) > -- Kill process if hangs. > if not process_completed then ffi.C.kill(pid, 9) end > > - local fh = open_with_timeout(output_file, file_open_timeout) > + local fh = common.open_with_timeout(output_file, file_open_timeout) > assert(fh, 'error while opening ' .. output_file) > > - local data = read_with_timeout(fh, file_read_timeout, file_read_interval) > + local data = common.read_with_timeout(fh, file_read_timeout, file_read_interval) > test:like(data, 'assertion failed', 'assertion failure is displayed') > > fh:close() > diff --git a/test/app-tap/lua/process_timeout.lua b/test/app-tap/lua/process_timeout.lua > new file mode 100644 > index 000000000..141fb0e43 > --- /dev/null > +++ b/test/app-tap/lua/process_timeout.lua > @@ -0,0 +1,57 @@ > +local fiber = require('fiber') > +local clock = require('clock') > +local ffi = require('ffi') > +local fio = require('fio') > +local errno = require('errno') > + > +-- For process_is_alive. > +ffi.cdef([[ > + int kill(pid_t pid, int sig); > +]]) > + > +--- Verify whether a process is alive. > +local function process_is_alive(pid) > + local rc = ffi.C.kill(pid, 0) > + return rc == 0 or errno() ~= errno.ESRCH > +end > + > +--- Returns true if process completed before timeout, false otherwise. > +local function wait_process_completion(pid, timeout) > + local start_time = clock.monotonic() > + local process_completed = false > + while clock.monotonic() - start_time < timeout do > + if not process_is_alive(pid) then > + process_completed = true > + break > + end > + end > + return process_completed > +end > + > +--- Open file on reading with timeout. > +local function open_with_timeout(filename, timeout) > + local fh > + local start_time = clock.monotonic() > + while not fh and clock.monotonic() - start_time < timeout do > + fh = fio.open(filename, {'O_RDONLY'}) > + end > + return fh > +end > + > +--- Try to read from file with timeout with interval. > +local function read_with_timeout(fh, timeout, interval) > + local data = '' > + local start_time = clock.monotonic() > + while #data == 0 and clock.monotonic() - start_time < timeout do > + data = fh:read() > + if #data == 0 then fiber.sleep(interval) end > + end > + return data > +end > + > +return { > + process_is_alive = process_is_alive, No external function currently uses this function. I suggest removing it from the interface > + wait_process_completion = wait_process_completion, > + open_with_timeout = open_with_timeout, > + read_with_timeout = read_with_timeout > +} > diff --git a/test/app-tap/suite.ini b/test/app-tap/suite.ini > index 2f16128f9..6e27f88d6 100644 > --- a/test/app-tap/suite.ini > +++ b/test/app-tap/suite.ini > @@ -1,7 +1,7 @@ > [default] > core = app > description = application server tests (TAP) > -lua_libs = lua/require_mod.lua lua/serializer_test.lua > +lua_libs = lua/require_mod.lua lua/serializer_test.lua lua/process_timeout.lua > is_parallel = True > pretest_clean = True > use_unix_sockets_iproto = True >