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] [PATCHv2 1/1] test: move helper functions to sep module in -e assert(false) test Date: Wed, 30 Dec 2020 12:42:07 +0300 [thread overview] Message-ID: <408e4d7e-985b-b2a6-d152-f07d04d1fa87@tarantool.org> (raw) In-Reply-To: <1896088fbc94a5a22af0278081387f17fdbff2ec.1609316681.git.artemreyt@tarantool.org> Hi! Thank you for the patch. See several comments below: - "test: move helper functions to sep module in -e assert(false) test". "sep" -> "separated"? - I have the same questions as before (https://lists.tarantool.org/pipermail/tarantool-patches/2020-December/021871.html) 1) 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 30.12.2020 11:31, Artem Starshov wrote: > Functions with timeout transferred to module > test/app-tap/lua/process_timeout.lua for further > using in other tests. > > Before this version of test was already done, > but by mistake older version was rebased on > master. > > Follows up #4983 > --- > .../gh-4983-tnt-e-assert-false-hangs.test.lua | 73 ++++--------------- > test/app-tap/lua/process_timeout.lua | 57 +++++++++++++++ > test/app-tap/suite.ini | 2 +- > 3 files changed, 73 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 61eeaa8c1..2978a6203 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,27 @@ > #!/usr/bin/env tarantool > > -local fiber = require('fiber') > -local clock = require('clock') > -local ffi = require('ffi') > -local fio = require('fio') > -local errno = require('errno') > +-- Using io.popen and self written module process_timeout > +-- in presence of the 'popen' built-in module because the > +-- last one isn't available in 1.10 > +local process_timeout = 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 +29,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 = process_timeout.wait_process_completion(pid, > process_waiting_timeout) > > test:ok(process_completed, > @@ -81,10 +38,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 = process_timeout.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 = process_timeout.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, 2) 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-30 9:42 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-30 8:31 [Tarantool-patches] [PATCHv2 0/1] test: -e assert(false) test fixup Artem Starshov 2020-12-30 8:31 ` [Tarantool-patches] [PATCHv2 1/1] test: move helper functions to sep module in -e assert(false) test Artem Starshov 2020-12-30 9:42 ` Leonid Vasiliev [this message] 2020-12-30 9:55 [Tarantool-patches] [PATCHv2 0/1] test: -e assert(false) test fixup Artem Starshov 2020-12-30 9:55 ` [Tarantool-patches] [PATCHv2 1/1] test: move helper functions to sep module in -e assert(false) test Artem Starshov
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=408e4d7e-985b-b2a6-d152-f07d04d1fa87@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] [PATCHv2 1/1] test: move helper functions to sep module in -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