From: Leonid Vasiliev <lvasiliev@tarantool.org> To: Artem Starshov <artemreyt@tarantool.org>, Alexander Turenko <alexander.turenko@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test Date: Tue, 29 Dec 2020 20:07:55 +0300 [thread overview] Message-ID: <d305f135-15d1-dc44-1b46-a668ea198565@tarantool.org> (raw) In-Reply-To: <54be7a8eac0b6f9291e03b7f9ef26bbcc367e7b0.1609259010.git.artemreyt@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 >
next prev parent reply other threads:[~2020-12-29 17:08 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-29 16:25 [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem Starshov 2020-12-29 16:25 ` [Tarantool-patches] [PATCH 1/3] test: tarantool -e test add more details and avoid busy loop Artem Starshov 2020-12-29 16:53 ` Leonid Vasiliev 2020-12-29 20:11 ` Alexander Turenko 2020-12-29 16:25 ` [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers Artem Starshov 2020-12-29 16:57 ` Leonid Vasiliev 2020-12-29 18:32 ` Alexander V. Tikhonov 2020-12-29 19:25 ` Alexander Turenko 2020-12-29 16:25 ` [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test Artem Starshov 2020-12-29 17:07 ` Leonid Vasiliev [this message] 2020-12-29 20:16 ` Alexander Turenko 2020-12-29 16:45 ` [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem 2020-12-29 17:11 ` Leonid Vasiliev
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=d305f135-15d1-dc44-1b46-a668ea198565@tarantool.org \ --to=lvasiliev@tarantool.org \ --cc=alexander.turenko@tarantool.org \ --cc=artemreyt@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox