Re: [Tarantool-patches] [PATCH 4/4] test: enable luajit-tap:lj-flush-on-trace tests

[Igor Munkin <imun@tarantool.org>]

Vlad,

Thanks for your comments!

On 10.04.20, Vladislav Shpilevoy wrote:
> Hi!
> 
> >>>  if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
> >>> diff --git a/test/app-tap/lj-flush-on-trace.test.lua b/test/app-tap/lj-flush-on-trace.test.lua
> >>> new file mode 100755
> >>> index 000000000..70b7bd9a2
> >>> --- /dev/null
> >>> +++ b/test/app-tap/lj-flush-on-trace.test.lua
> >>> @@ -0,0 +1,30 @@
> >>> +#!/usr/bin/env tarantool
> >>> +
> >>> +local tap = require('tap')
> >>> +
> >>> +local test = tap.test('lj-flush-on-trace')
> >>> +
> >>> +local cmd = string.gsub(
> >>> +  'LUA_CPATH=$/?.so LD_LIBRARY_PATH=$ tarantool 2>&1 $/test.lua %d %d',
> >>> +  '%$', os.getenv('BUILDDIR') .. '/test/luajit-tap/lj-flush-on-trace')
> >>> +
> >>> +local checks = {
> >>> +  { hotloop = 1, trigger = 1, success = true  },
> >>> +  { hotloop = 1, trigger = 2, success = false },
> >>> +}
> >>> +
> >>> +test:plan(#checks)
> >>> +
> >>> +for _, ch in pairs(checks) do
> >>> +  local res
> >>> +  local proc = io.popen(cmd:format(ch.hotloop, ch.trigger))
> >>> +  for s in proc:lines('*l') do res = s end
> >>> +  assert(res, 'proc:lines failed')
> >>
> >> This file is exactly the same as the other file for running a
> >> luajit test in the previous commit. I propose you to move this to
> >> a separate file, which would provide API to run arbitrary test via
> >> io.popen. Unless this won't be dropped if you find a way to make
> >> the new luajit tests runable via test-run as is.
> > 
> > Sounds rational. They are not the same but quite similar (the difference
> > is only in the result values).
> 
> At least first 24 lines of them match almost completely (except file
> name to start with io.popen), that is more than half of each. I think
> that part either should be simplified so as there is nothing to extract
> already (my proposal about io.popen() of self may help, or may not), or
> just extracted into a common file like
> 
>     test/app-tap/utils/utils.lua

utils.lua is introduced here[1].

> 
> You can extract even more if you pass expected output with 'checks'
> array elements.
> 
> If you want to do that in scope of luajit tests rework, then add it to
> the ticket description, please, so as not to forget.

As we discussed, I reworked the tests and dropped all excess files in
app-tap directory. Since the changes also affects luajit repo, here are
diffs for both tarantool and luajit repos:

## tarantool/luajit:

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

diff --git a/test/lj-flush-on-trace.skipcond b/test/lj-flush-on-trace.skipcond
new file mode 100644
index 0000000..2a2ec4d
--- /dev/null
+++ b/test/lj-flush-on-trace.skipcond
@@ -0,0 +1,7 @@
+import platform
+
+# Disabled on FreeBSD due to #4819.
+if platform.system() == 'FreeBSD':
+    self.skip = 1
+
+# vim: set ft=python:
diff --git a/test/lj-flush-on-trace/test.lua b/test/lj-flush-on-trace.test.lua
old mode 100644
new mode 100755
similarity index 53%
rename from test/lj-flush-on-trace/test.lua
rename to test/lj-flush-on-trace.test.lua
index ff6e0b6..0b3ccf4
--- a/test/lj-flush-on-trace/test.lua
+++ b/test/lj-flush-on-trace.test.lua
@@ -1,3 +1,26 @@
+#!/usr/bin/env tarantool
+
+if #arg == 0 then
+  require('utils').selfrun(arg, {
+    {
+      arg = {
+        1, -- hotloop (arg[1])
+        1, -- trigger (arg[2])
+      },
+      res = 'OK',
+      msg = 'Trace is aborted',
+    },
+    {
+      arg = {
+        1, -- hotloop (arg[1])
+        2, -- trigger (arg[2])
+      },
+      res = 'JIT mode change is detected while executing the trace',
+      msg = 'Trace is recorded',
+    },
+  })
+end
+
 local cfg = {
   hotloop = arg[1] or 1,
   trigger = arg[2] or 1,
@@ -7,8 +30,8 @@ local ffi = require('ffi')
 local ffiflush = ffi.load('libflush')
 ffi.cdef('void flush(struct flush *state, int i)')
 
--- Save the current coroutine and set the value to trigger ipp
--- call the Lua routine instead of pure C implementation.
+-- Save the current coroutine and set the value to trigger
+-- <flush> call the Lua routine instead of C implementation.
 local flush = require('libflush')(cfg.trigger)
 
 -- Depending on trigger and hotloop values the following contexts

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

## tarantool/tarantool:

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

diff --git a/test/app-tap/lj-flush-on-trace.skipcond b/test/app-tap/lj-flush-on-trace.skipcond
deleted file mode 100644
index 2a2ec4d97..000000000
--- a/test/app-tap/lj-flush-on-trace.skipcond
+++ /dev/null
@@ -1,7 +0,0 @@
-import platform
-
-# Disabled on FreeBSD due to #4819.
-if platform.system() == 'FreeBSD':
-    self.skip = 1
-
-# vim: set ft=python:
diff --git a/test/app-tap/lj-flush-on-trace.test.lua b/test/app-tap/lj-flush-on-trace.test.lua
deleted file mode 100755
index ad6a484ed..000000000
--- a/test/app-tap/lj-flush-on-trace.test.lua
+++ /dev/null
@@ -1,34 +0,0 @@
-#!/usr/bin/env tarantool
-
-local tap = require('tap')
-
-local test = tap.test('lj-flush-on-trace')
-
-local vars = {
-  PATH   = os.getenv('BUILDDIR') .. '/test/luajit-tap/lj-flush-on-trace',
-  SUFFIX = package.cpath:match('?.(%a+);'),
-}
-
-local cmd = string.gsub('LUA_CPATH=<PATH>/?.<SUFFIX> LD_LIBRARY_PATH=<PATH> '
-  .. 'tarantool 2>&1 <PATH>/test.lua %d %d', '%<(%w+)>', vars)
-
-local checks = {
-  { hotloop = 1, trigger = 1, success = true  },
-  { hotloop = 1, trigger = 2, success = false },
-}
-
-test:plan(#checks)
-
-for _, ch in pairs(checks) do
-  local res
-  local proc = io.popen(cmd:format(ch.hotloop, ch.trigger))
-  for s in proc:lines('*l') do res = s end
-  assert(res, 'proc:lines failed')
-  if ch.success then
-    test:is(res, 'OK')
-  else
-    test:is(res, 'JIT mode change is detected while executing the trace')
-  end
-end
-
-os.exit(test:check() and 0 or 1)

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

I updated the upstream branches and also sent the v2 for both series:
* tarantool/luajit[2].
* tarantool/tarantool[3].

> 
> > I guess I can do it in scope of #4862[1]
> > if you're OK with it.
> > 
> >>
> >>> +  if ch.success then
> >>> +    test:is(res, 'OK')
> >>> +  else
> >>> +    test:is(res, 'JIT mode change is detected while executing the trace')
> >>> +  end
> >>> +end
> >>> +
> >>> +os.exit(test:check() and 0 or 1)
> >>>
> > 
> > [1]: https://github.com/tarantool/tarantool/issues/4862
> > 

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/015936.html
[2]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/015935.html
[3]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/015939.html

-- 
Best regards,
IM