<HTML><BODY><div>Hi, Igor!</div><div>LGTM, except for a few nits below.</div><div> </div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16763078100681782174_BODY">Unfortunately, <utils.selfrun> is too complex to be maintained, so the<br>corresponding tests are split into two files: the test itself and the<br>script to be run by the test. As a result of the patch <utils.makecmd><br>helper is introduced: it inherits some approaches from <utils.selfrun>,<br>but it's considered for more general use.<br><br>Relates to tarantool/tarantool#8252<br><br>Signed-off-by: Igor Munkin <<a href="/compose?To=imun@tarantool.org">imun@tarantool.org</a>><br>---<br> .../gh-4427-ffi-sandwich.test.lua | 69 ++++++++-----------<br> .../gh-4427-ffi-sandwich/script.lua | 25 +++++++<br> .../lj-351-print-tostring-number.test.lua | 34 +++------<br> .../lj-351-print-tostring-number/script.lua | 9 +++<br> .../lj-586-debug-non-string-error.test.lua | 2 +-<br> .../lj-flush-on-trace.test.lua | 68 ++++++++----------<br> .../lj-flush-on-trace/script.lua | 23 +++++++<br> test/tarantool-tests/utils.lua | 66 +++++-------------<br> 8 files changed, 148 insertions(+), 148 deletions(-)<br> create mode 100644 test/tarantool-tests/gh-4427-ffi-sandwich/script.lua<br> create mode 100644 test/tarantool-tests/lj-351-print-tostring-number/script.lua<br> create mode 100644 test/tarantool-tests/lj-flush-on-trace/script.lua<br><br>diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua<br>index dd02130c..f4795db0 100644<br>--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua<br>+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua<br>@@ -3,52 +3,43 @@ local utils = require('utils')</div></div></div></div></blockquote><div><snipped></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>+ -- TODO: Leave another toxic comment regarding SIP on macOS.</div></div></div></div></blockquote><div>That comment is unnecessary. Here and below.</div><div> </div><div><snipped></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div><br>diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua<br>index eb11d40d..41a7c22a 100644<br>--- a/test/tarantool-tests/utils.lua<br>+++ b/test/tarantool-tests/utils.lua<br>@@ -1,7 +1,6 @@<br> local M = {}<br> <br> local ffi = require('ffi')<br>-local tap = require('tap')<br> local bc = require('jit.bc')<br> local bit = require('bit')<br> <br>@@ -44,55 +43,28 @@ function M.luacmd(args)<br> return table.concat(args, ' ', idx + 1, -1)<br> end<br> <br>-local function unshiftenv(variable, value, sep)<br>- local envvar = os.getenv(variable)<br>- return ('%s="%s%s"'):format(variable, value,<br>- envvar and ('%s%s'):format(sep, envvar) or '')<br>+local function makeenv(tabenv)<br>+ if tabenv == nil then return '' end<br>+ local flatenv = {}<br>+ for var, value in pairs(tabenv) do<br>+ table.insert(flatenv, ('%s=%s'):format(var, value))<br>+ end<br>+ return table.concat(flatenv, ' ')<br> end<br> <br>-function M.selfrun(arg, checks)<br>- -- If TEST_SELFRUN is set, it means the test has been run via<br>- -- <io.popen>, so just return from this routine and proceed<br>- -- the execution to the test payload, ...<br>- if os.getenv('TEST_SELFRUN') then return end<br>-<br>- -- ... otherwise initialize <tap>, setup testing environment<br>- -- and run this chunk via <io.popen> for each case in <checks>.<br>- -- XXX: The function doesn't return back from this moment. It<br>- -- checks whether all assertions are fine and exits.<br>-<br>- local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))<br>-<br>- test:plan(#checks)<br>-<br>- local libext = package.cpath:match('?.(%a+);')<br>- local vars = {<br>+function M.makecmd(arg, opts)<br>+ return setmetatable({<br> LUABIN = M.luacmd(arg),<br>- SCRIPT = arg[0],<br>- PATH = arg[0]:gsub('%.test%.lua', ''),<br>- SUFFIX = libext,<br>- ENV = table.concat({<br>- unshiftenv('LUA_PATH', '<PATH>/?.lua', ';'),<br>- unshiftenv('LUA_CPATH', '<PATH>/?.<SUFFIX>', ';'),<br>- unshiftenv((libext == 'dylib' and 'DYLD' or 'LD') .. '_LIBRARY_PATH',<br>- '<PATH>', ':'),<br>- 'TEST_SELFRUN=1',<br>- }, ' '),<br>- }<br>-<br>- local cmd = string.gsub('<ENV> <LUABIN> 2>&1 <SCRIPT>', '%<(%w+)>', vars)<br>-<br>- for _, ch in pairs(checks) do<br>- local testf = test[ch.test]<br>- assert(testf, ("tap doesn't provide test.%s function"):format(ch.test))<br>- local proc = io.popen((cmd .. (' %s'):rep(#ch.arg)):format(unpack(ch.arg)))<br>- local res = proc:read('*all'):gsub('^%s+', ''):gsub('%s+$', '')<br>- -- XXX: explicitly pass <test> as an argument to <testf><br>- -- to emulate test:is(...), test:like(...), etc.<br>- testf(test, res, ch.res, ch.msg)<br>- end<br>-<br>- os.exit(test:check() and 0 or 1)<br>+ SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$', '/script.lua'),<br>+ ENV = opts and makeenv(opts.env) or '',<br>+ REDIRECT = opts and opts.redirect or '',<br>+ }, {<br>+ __call = function(self, ...)</div></div></div></div></blockquote><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div>+ local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)<br>+ .. (' %s'):rep(select('#', ...)):format(...)<br>+ return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')<br>+ end<br>+ })</div></div></div></div></blockquote><div>That part is barely readable, so I think you should either drop</div><div>a comment with explanation, or rewrite it in a more readable way.</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div> end<br> <br> function M.skipcond(condition, message)<br>--<br>2.30.2</div></div></div></div></blockquote><div><div>--<br>Best regards,</div><div>Maxim Kokryashkin</div></div><div> </div></div></blockquote></BODY></HTML>