On 6 Apr 2021, at 21:32, Igor Munkin <imun@tarantool.org> wrote:

Sergos,

Thanks for your review!

On 06.04.21, Sergey Ostanevich wrote:

Hi!

LGTM with some ignorable nits below.

Added your tag:
| Reviewed-by: Sergey Ostanevich <sergos@tarantool.org>

Sergos

On 5 Apr 2021, at 20:11, Igor Munkin <imun@tarantool.org> wrote:

Previous implementation of <utils.selfrun> implicitly obliges the
developer to pass the arguments to the test script for the second run.
This has been unnoticed, since the existing tests are invoked for two
different cases, so the script arguments were kinda obligatory, but they
are not required in a general case.

As a result the function signals the testing system that the script is
being run the second time via TEST_SELFRUN environment variable.

To me it looks like it was an infinite recursion, that was interrupted
only by chance in case a test does not tolerate the reentrance. 

Precisely. Honestly, it was a "soft" requirement for such tests (as I
have written above), but considering your attempts with implementing
tests for JIT machinery, I've finally realized this problem.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
.../gh-4427-ffi-sandwich.test.lua             | 51 +++++++++----------
.../lj-flush-on-trace.test.lua                | 51 +++++++++----------
test/tarantool-tests/utils.lua                |  7 +++
3 files changed, 57 insertions(+), 52 deletions(-)

<snipped>

diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index aebbf6ac..d2dd71b0 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -14,6 +14,12 @@ local function luacmd(args)
end

function M.selfrun(arg, checks)
+  -- If TEST_SELFRUN is set, just execute the test payload below

To make a symmetry to the next comment: "we’re running the io.popen version
of the test, proceed with test payload execution past "

+  -- <selfrun> call, ...
+  if os.getenv('TEST_SELFRUN') then return end
+
+  -- ... otherwise run this chunk via <io.popen>.

I would add that the test payload won’t be run and the io.popen results
will be reported instead.

I've adjusted the comments the following way:

================================================================================

diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index 9bdb71ec..c0403cf1 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -25,11 +25,15 @@ local function unshiftenv(variable, value, sep)
end

function M.selfrun(arg, checks)
-  -- If TEST_SELFRUN is set, just execute the test payload below
-  -- <selfrun> call, ...
+  -- If TEST_SELFRUN is set, it means the test has been run via
+  -- <io.popen>, so just return from this routine and proceed
+  -- the execution to the test payload, ...
  if os.getenv('TEST_SELFRUN') then return end

-  -- ... otherwise run this chunk via <io.popen>.
+  -- ... otherwise initialize <tap>, setup testing environment
+  -- and run this chunk via <io.popen> for each case in <checks>.
+  -- XXX: The function doesn't return back from this moment. It
+  -- checks whether all assertions are fine and exits.

  local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))


================================================================================

+
 local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))

 test:plan(#checks)
@@ -28,6 +34,7 @@ function M.selfrun(arg, checks)
 local cmd = string.gsub('LUA_PATH="<PATH>/?.lua;$LUA_PATH" ' ..
                         'LUA_CPATH="<PATH>/?.<SUFFIX>;$LUA_CPATH" ' ..
                         'LD_LIBRARY_PATH=<PATH>:$LD_LIBRARY_PATH ' ..
+                          'TEST_SELFRUN=1' ..
                         '<LUABIN> 2>&1 <SCRIPT>', '%<(%w+)>', vars)

 for _, ch in pairs(checks) do
-- 
2.25.0

-- 
Best regards,
IM