[Tarantool-patches] [PATCH luajit 0/7] Adjust tests to be run when JIT is disabled

[Tarantool-patches] [PATCH luajit 0/7] Adjust tests to be run when JIT is disabled

[Igor Munkin via Tarantool-patches]

This series make LuaJIT tests work when JIT is either disabled or turned
off. It looks large a bit, but it's not complex at all.

The first patch is a backport of a tiny change fixing header
dependencies for the build with -DLUAJIT_DISABLE_JIT=ON and whitespace
inconsistency (that is a bit irrelevant, but IMHO still OK).

The second patch is quite similar to the first one, but relates to the
changes made in our fork (sysprof, memprof, etc).

The third one is epic one: unfortunately, <utils.selfrun> is too complex
to be maintained, so the corresponding tests are split into two files:
the test itself and the script to be run by the test. There is a new
helper introduced within this patch, and three tests are refactored.

The fourth patch makes skipcond helper more convenient: it becomes
multi-conditional and it yields the test object, so we can organize more
readable chains of skip conditions.

The fifth patch disables all JIT-related tests in tarantool-tests suite
via skipcond introduced in the previous patch.

The sixth patch fixes <compiled_with_jit> condition according to its
usage in lua-Harness suite.

The last patch introduces a new flavor to exotic builds matrix to test
builds with JIT disabled.

Issue: https://github.com/tarantool/tarantool/issues/8252
Branch: https://github.com/tarantool/luajit/commits/imun/jit-off-ci
Tarantool PR: https://github.com/tarantool/tarantool/pull/8288
CI: https://github.com/tarantool/luajit/commit/7f01b6a

Igor Munkin (6):
  build: fix build with JIT disabled
  test: stop using utils.selfrun in tests
  test: make skipcond helper more convenient
  test: add skipcond for all JIT-related tests
  test: fix lua-Harness JIT-related tests
  ci: add nojit flavor for exotic builds

Mike Pall (1):
  Minor fixes.

 .github/workflows/exotic-builds-testing.yml   |  4 +-
 src/lib_base.c                                |  2 +-
 src/lj_gc.c                                   |  1 +
 src/lj_memprof.c                              |  9 +--
 src/lj_symtab.c                               | 13 +---
 src/lj_symtab.h                               |  2 +
 test/lua-Harness-tests/403-jit.t              |  2 +-
 test/lua-Harness-tests/411-luajit.t           |  2 +-
 .../bc-jit-unpatching.test.lua                |  6 +-
 .../fix-fold-simplify-conv-sext.test.lua      |  7 +-
 .../fix-slot-check-for-mm-record.test.lua     |  4 +-
 .../gh-4199-gc64-fuse.test.lua                | 12 +--
 .../gh-4427-ffi-sandwich.test.lua             | 75 ++++++++-----------
 .../gh-4427-ffi-sandwich/script.lua           | 25 +++++++
 ...gh-4476-fix-string-find-recording.test.lua |  4 +-
 .../gh-5813-resolving-of-c-symbols.test.lua   | 23 +++---
 .../gh-6065-jit-library-smoke-tests.test.lua  |  4 +-
 ...8-fix-side-exit-patching-on-arm64.test.lua |  5 +-
 test/tarantool-tests/gh-6189-cur_L.test.lua   |  7 +-
 ...ytecode-allocator-for-comparisons.test.lua |  5 +-
 .../gh-6371-string-char-no-arg.test.lua       |  5 +-
 ...6782-stitching-in-vmevent-handler.test.lua |  6 +-
 .../gh-6976-narrowing-of-unary-minus.test.lua |  5 +-
 ...4-add-proto-trace-sysprof-default.test.lua | 15 ++--
 .../lj-350-sload-typecheck.test.lua           |  8 +-
 .../lj-351-print-tostring-number.test.lua     | 34 +++------
 .../lj-351-print-tostring-number/script.lua   |  9 +++
 .../lj-356-ir-khash-non-string-obj.test.lua   | 11 ++-
 .../lj-357-arm64-hrefk.test.lua               |  6 +-
 .../lj-375-ir-bufput-signed-char.test.lua     |  4 +-
 .../lj-408-tonumber-cdata-record.test.lua     | 11 +--
 .../lj-416-xor-before-jcc.test.lua            |  6 +-
 .../lj-430-maxirconst.test.lua                | 11 +--
 ...lj-505-fold-no-strref-for-ptrdiff.test.lua |  4 +-
 .../lj-524-fold-conv-respect-src-irt.test.lua |  6 +-
 .../lj-556-fix-loop-realignment.test.lua      |  4 +-
 ...j-584-bad-renames-for-sunk-values.test.lua |  4 +-
 .../lj-586-debug-non-string-error.test.lua    |  2 +-
 .../lj-603-err-snap-restore.test.lua          | 19 +++--
 ...lj-672-cdata-allocation-recording.test.lua | 13 ++--
 .../lj-864-varg-rec-base-offset.test.lua      |  6 +-
 .../lj-906-fix-err-mem.test.lua               | 12 +--
 .../lj-flush-on-trace.test.lua                | 74 ++++++++----------
 .../lj-flush-on-trace/script.lua              | 23 ++++++
 .../misclib-getmetrics-capi.test.lua          | 18 ++---
 .../misclib-getmetrics-lapi.test.lua          | 14 ++--
 .../misclib-memprof-lapi.test.lua             | 42 +++++------
 .../misclib-sysprof-capi.test.lua             | 18 ++---
 .../misclib-sysprof-lapi.test.lua             | 24 +++---
 test/tarantool-tests/tap.lua                  | 12 +++
 test/tarantool-tests/utils.lua                | 74 +++++-------------
 51 files changed, 371 insertions(+), 341 deletions(-)
 create mode 100644 test/tarantool-tests/gh-4427-ffi-sandwich/script.lua
 create mode 100644 test/tarantool-tests/lj-351-print-tostring-number/script.lua
 create mode 100644 test/tarantool-tests/lj-flush-on-trace/script.lua

-- 
2.30.2

[Tarantool-patches] [PATCH luajit 1/7] Minor fixes.

[Igor Munkin via Tarantool-patches]

From: Mike Pall <mike>

(cherry picked from commit 54484e257f24c47d71b8233844e2ec8664db8071)

This is a tiny patch fixing header dependencies for the build with
-DLUAJIT_DISABLE_JIT=ON and whitespace inconsistency.

Igor Munkin:
* added the description

Part of tarantool/tarantool#8069

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 src/lib_base.c | 2 +-
 src/lj_gc.c    | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/lib_base.c b/src/lib_base.c
index 398c4f65..eb6da054 100644
--- a/src/lib_base.c
+++ b/src/lib_base.c
@@ -505,7 +505,7 @@ LJLIB_CF(print)
     tv = L->top-1;
   }
   shortcut = (tvisfunc(tv) && funcV(tv)->c.ffid == FF_tostring)
-              && !gcrefu(basemt_it(G(L), LJ_TNUMX));
+	      && !gcrefu(basemt_it(G(L), LJ_TNUMX));
   for (i = 0; i < nargs; i++) {
     cTValue *o = &L->base[i];
     const char *str;
diff --git a/src/lj_gc.c b/src/lj_gc.c
index 29387a48..c306047a 100644
--- a/src/lj_gc.c
+++ b/src/lj_gc.c
@@ -25,6 +25,7 @@
 #include "lj_cdata.h"
 #endif
 #include "lj_trace.h"
+#include "lj_dispatch.h"
 #include "lj_vm.h"
 
 #define GCSTEPSIZE	1024u
-- 
2.30.2

[Tarantool-patches] [PATCH luajit 2/7] build: fix build with JIT disabled

[Igor Munkin via Tarantool-patches]

struct GCtrace is defined only if LJ_HASJIT is set. Hence all spots
where GCtrace is used should be also moved under LJ_HASJIT define.

Relates to tarantool/tarantool#8252

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 src/lj_memprof.c |  9 ++++-----
 src/lj_symtab.c  | 13 +++----------
 src/lj_symtab.h  |  2 ++
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/lj_memprof.c b/src/lj_memprof.c
index d4a639fd..8cab8204 100644
--- a/src/lj_memprof.c
+++ b/src/lj_memprof.c
@@ -369,6 +369,8 @@ void lj_memprof_add_proto(const struct GCproto *pt)
   lj_symtab_dump_proto(&mp->out, pt);
 }
 
+#if LJ_HASJIT
+
 void lj_memprof_add_trace(const struct GCtrace *tr)
 {
   struct memprof *mp = &memprof;
@@ -380,6 +382,8 @@ void lj_memprof_add_trace(const struct GCtrace *tr)
   lj_symtab_dump_trace(&mp->out, tr);
 }
 
+#endif /* LJ_HASJIT */
+
 #else /* LJ_HASMEMPROF */
 
 int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt)
@@ -401,9 +405,4 @@ void lj_memprof_add_proto(const struct GCproto *pt)
   UNUSED(pt);
 }
 
-void lj_memprof_add_trace(const struct GCtrace *tr)
-{
-  UNUSED(tr);
-}
-
 #endif /* LJ_HASMEMPROF */
diff --git a/src/lj_symtab.c b/src/lj_symtab.c
index 91ee9a72..54984c05 100644
--- a/src/lj_symtab.c
+++ b/src/lj_symtab.c
@@ -53,16 +53,7 @@ void lj_symtab_dump_trace(struct lj_wbuf *out, const GCtrace *trace)
   lj_wbuf_addu64(out, (uint64_t)lineno);
 }
 
-#else
-
-static void lj_symtab_dump_trace(struct lj_wbuf *out, const GCtrace *trace)
-{
-  UNUSED(out);
-  UNUSED(trace);
-  lua_assert(0);
-}
-
-#endif
+#endif /* LJ_HASJIT */
 
 void lj_symtab_dump_proto(struct lj_wbuf *out, const GCproto *pt)
 {
@@ -491,11 +482,13 @@ void lj_symtab_dump(struct lj_wbuf *out, const struct global_State *g,
       lj_symtab_dump_proto(out, pt);
       break;
     }
+#if LJ_HASJIT
     case (~LJ_TTRACE): {
       lj_wbuf_addbyte(out, SYMTAB_TRACE);
       lj_symtab_dump_trace(out, gco2trace(o));
       break;
     }
+#endif /* LJ_HASJIT */
     default:
       break;
     }
diff --git a/src/lj_symtab.h b/src/lj_symtab.h
index 6faa5cb1..6ec0cd7c 100644
--- a/src/lj_symtab.h
+++ b/src/lj_symtab.h
@@ -60,10 +60,12 @@
 #define SYMTAB_TRACE ((uint8_t)2)
 #define SYMTAB_FINAL ((uint8_t)0x80)
 
+#if LJ_HASJIT
 /*
 ** Dumps traceinfo into the symbol table.
 */
 void lj_symtab_dump_trace(struct lj_wbuf *out, const GCtrace *trace);
+#endif /* LJ_HASJIT */
 
 /*
 ** Dumps function prototype.
-- 
2.30.2

[Tarantool-patches] [PATCH luajit 3/7] test: stop using utils.selfrun in tests

[Igor Munkin via Tarantool-patches]

Unfortunately, <utils.selfrun> is too complex to be maintained, so the
corresponding tests are split into two files: the test itself and the
script to be run by the test. As a result of the patch <utils.makecmd>
helper is introduced: it inherits some approaches from <utils.selfrun>,
but it's considered for more general use.

Relates to tarantool/tarantool#8252

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .../gh-4427-ffi-sandwich.test.lua             | 69 ++++++++-----------
 .../gh-4427-ffi-sandwich/script.lua           | 25 +++++++
 .../lj-351-print-tostring-number.test.lua     | 34 +++------
 .../lj-351-print-tostring-number/script.lua   |  9 +++
 .../lj-586-debug-non-string-error.test.lua    |  2 +-
 .../lj-flush-on-trace.test.lua                | 68 ++++++++----------
 .../lj-flush-on-trace/script.lua              | 23 +++++++
 test/tarantool-tests/utils.lua                | 66 +++++-------------
 8 files changed, 148 insertions(+), 148 deletions(-)
 create mode 100644 test/tarantool-tests/gh-4427-ffi-sandwich/script.lua
 create mode 100644 test/tarantool-tests/lj-351-print-tostring-number/script.lua
 create mode 100644 test/tarantool-tests/lj-flush-on-trace/script.lua

diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
index dd02130c..f4795db0 100644
--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
@@ -3,52 +3,43 @@ local utils = require('utils')
 -- Disabled on *BSD due to #4819.
 utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
 
-utils.selfrun(arg, {
-  {
-    arg = {
-      1, -- hotloop (arg[1])
-      1, -- trigger (arg[2])
-    },
-    msg = 'Trace is aborted',
-    res = tostring(3), -- hotloop + trigger + 1
-    test = 'is',
-  },
-  {
-    arg = {
-      1, -- hotloop (arg[1])
-      2, -- trigger (arg[2])
-    },
-    msg = 'Trace is recorded',
-    res = 'Lua VM re%-entrancy is detected while executing the trace',
-    test = 'like',
-  },
-})
-
------ Test payload. ----------------------------------------------
-
-local cfg = {
-  hotloop = arg[1] or 1,
-  trigger = arg[2] or 1,
-}
+local tap = require('tap')
 
-local ffi = require('ffi')
-local ffisandwich = ffi.load('libsandwich')
-ffi.cdef('int increment(struct sandwich *state, int i)')
+local test = tap.test('gh-4427-ffi-sandwich')
+test:plan(2)
 
--- Save the current coroutine and set the value to trigger
--- <increment> call the Lua routine instead of C implementation.
-local sandwich = require('libsandwich')(cfg.trigger)
+local script = utils.makecmd(arg, {
+  -- TODO: Leave another toxic comment regarding SIP on macOS.
+  env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
+  redirect = '2>&1',
+})
 
 -- Depending on trigger and hotloop values the following contexts
 -- are possible:
 -- * if trigger <= hotloop -> trace recording is aborted
 -- * if trigger >  hotloop -> trace is recorded but execution
 --   leads to panic
-jit.opt.start("3", string.format("hotloop=%d", cfg.hotloop))
+local hotloop = 1
+local cases = {
+  abort = {
+    trigger = hotloop,
+    expected = '#4427 still works',
+    test = 'is',
+    message = 'Trace is aborted',
+  },
+  panic = {
+    trigger = hotloop + 1,
+    expected = 'Lua VM re%-entrancy is detected while executing the trace',
+    test = 'like',
+    message = 'Trace is compiled',
+  },
+}
 
-local res
-for i = 0, cfg.trigger + cfg.hotloop do
-  res = ffisandwich.increment(sandwich, i)
+for _, subtest in pairs(cases) do
+  local output = script(hotloop, subtest.trigger)
+  -- XXX: explicitly pass <test> as an argument to <testf>
+  -- to emulate test:is(...), test:like(...), etc.
+  test[subtest.test](test, output, subtest.expected, subtest.message)
 end
--- Check the resulting value if panic didn't occur earlier.
-print(res)
+
+os.exit(test:check() and 0 or 1)
diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua b/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua
new file mode 100644
index 00000000..9ecd964e
--- /dev/null
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua
@@ -0,0 +1,25 @@
+local hotloop = assert(arg[1], 'hotloop argument is missing')
+local trigger = assert(arg[2], 'trigger argument is missing')
+
+local ffi = require('ffi')
+local ffisandwich = ffi.load('libsandwich')
+ffi.cdef('int increment(struct sandwich *state, int i)')
+
+-- Save the current coroutine and set the value to trigger
+-- <increment> call the Lua routine instead of C implementation.
+local sandwich = require('libsandwich')(trigger)
+
+-- Depending on trigger and hotloop values the following contexts
+-- are possible:
+-- * if trigger <= hotloop -> trace recording is aborted
+-- * if trigger >  hotloop -> trace is recorded but execution
+--   leads to panic
+jit.opt.start("3", string.format("hotloop=%d", hotloop))
+
+local res
+for i = 0, hotloop + trigger do
+  res = ffisandwich.increment(sandwich, i)
+end
+-- Check the resulting value if panic didn't occur earlier.
+assert(res == hotloop + trigger + 1, 'res is calculated correctly')
+io.write('#4427 still works')
diff --git a/test/tarantool-tests/lj-351-print-tostring-number.test.lua b/test/tarantool-tests/lj-351-print-tostring-number.test.lua
index da5b31be..72a9ec2b 100644
--- a/test/tarantool-tests/lj-351-print-tostring-number.test.lua
+++ b/test/tarantool-tests/lj-351-print-tostring-number.test.lua
@@ -1,4 +1,9 @@
-local utils = require('utils')
+local tap = require('tap')
+
+local test = tap.test('lj-351-print-tostring-number')
+test:plan(8)
+
+local script = require('utils').makecmd(arg)
 
 local cases = {
   {typename = 'nil', value = 'nil'},
@@ -15,27 +20,10 @@ local cases = {
   {typename = 'cdata', value = '1ULL'}
 }
 
-local checks = {}
-
-for i, case in pairs(cases) do
-  checks[i] = {
-    arg = {('"%s"'):format(case.value), case.typename},
-    msg = ('%s'):format(case.typename),
-    res = ('__tostring is reloaded for %s'):format(case.typename),
-    test = 'is',
-  }
+for _, subtest in pairs(cases) do
+  local output = script(('"%s"'):format(subtest.value), subtest.typename)
+  test:is(output, ('__tostring is reloaded for %s'):format(subtest.typename),
+          ('subtest is OK for %s type'):format(subtest.typename))
 end
 
-utils.selfrun(arg, checks)
-
------ Test payload. ----------------------------------------------
-
-local test = [[
-  local testvar = %s
-  debug.setmetatable(testvar, {__tostring = function(o)
-    return ('__tostring is reloaded for %s'):format(type(o))
-  end})
-  print(testvar)
-]]
-
-pcall(load(test:format(unpack(arg))))
+os.exit(test:check() and 0 or 1)
diff --git a/test/tarantool-tests/lj-351-print-tostring-number/script.lua b/test/tarantool-tests/lj-351-print-tostring-number/script.lua
new file mode 100644
index 00000000..c3066f49
--- /dev/null
+++ b/test/tarantool-tests/lj-351-print-tostring-number/script.lua
@@ -0,0 +1,9 @@
+local test = [[
+  local testvar = %s
+  debug.setmetatable(testvar, {__tostring = function(o)
+    return ('__tostring is reloaded for %s'):format(type(o))
+  end})
+  print(testvar)
+]]
+
+pcall(load(test:format(unpack(arg))))
diff --git a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
index f02353fe..dcb730a2 100644
--- a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
+++ b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
@@ -8,7 +8,7 @@ test:plan(1)
 -- that testing the debug interactive interface always ends with
 -- sending commands to another instance via stdin. However, the
 -- module with test helpers lacks the suitable routine.
--- `utils.selfrun()` doesn't fit for this, since `debug.debug()`
+-- `utils.makecmd()` doesn't fit for this, since `debug.debug()`
 -- captures `io.stdin` and waits at `fgets()` in debug busy loop.
 -- As it's already mentioned, such tests are not usual, so there
 -- is no need to introduce a new helper to utils module (at least
diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
index c46b93f0..cf92757c 100644
--- a/test/tarantool-tests/lj-flush-on-trace.test.lua
+++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
@@ -3,51 +3,43 @@ local utils = require('utils')
 -- Disabled on *BSD due to #4819.
 utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
 
-utils.selfrun(arg, {
-  {
-    arg = {
-      1, -- hotloop (arg[1])
-      1, -- trigger (arg[2])
-    },
-    msg = 'Trace is aborted',
-    res = 'OK',
-    test = 'is',
-  },
-  {
-    arg = {
-      1, -- hotloop (arg[1])
-      2, -- trigger (arg[2])
-    },
-    msg = 'Trace is recorded',
-    res = 'JIT mode change is detected while executing the trace',
-    test = 'like',
-  },
-})
-
------ Test payload. ----------------------------------------------
-
-local cfg = {
-  hotloop = arg[1] or 1,
-  trigger = arg[2] or 1,
-}
+local tap = require('tap')
 
-local ffi = require('ffi')
-local ffiflush = ffi.load('libflush')
-ffi.cdef('void flush(struct flush *state, int i)')
+local test = tap.test('lj-flush-on-trace')
+test:plan(2)
 
--- 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)
+local script = utils.makecmd(arg, {
+  -- TODO: Leave another toxic comment regarding SIP on macOS.
+  env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
+  redirect = '2>&1',
+})
 
 -- Depending on trigger and hotloop values the following contexts
 -- are possible:
 -- * if trigger <= hotloop -> trace recording is aborted
 -- * if trigger >  hotloop -> trace is recorded but execution
 --   leads to panic
-jit.opt.start("3", string.format("hotloop=%d", cfg.hotloop))
+local hotloop = 1
+local cases = {
+  abort = {
+    trigger = hotloop,
+    expected = 'LJ flush still works',
+    test = 'is',
+    message = 'Trace is aborted',
+  },
+  panic = {
+    trigger = hotloop + 1,
+    expected = 'JIT mode change is detected while executing the trace',
+    test = 'like',
+    message = 'Trace is compiled',
+  },
+}
 
-for i = 0, cfg.trigger + cfg.hotloop do
-  ffiflush.flush(flush, i)
+for _, subtest in pairs(cases) do
+  local output = script(hotloop, subtest.trigger)
+  -- XXX: explicitly pass <test> as an argument to <testf>
+  -- to emulate test:is(...), test:like(...), etc.
+  test[subtest.test](test, output, subtest.expected, subtest.message)
 end
--- Panic didn't occur earlier.
-print('OK')
+
+os.exit(test:check() and 0 or 1)
diff --git a/test/tarantool-tests/lj-flush-on-trace/script.lua b/test/tarantool-tests/lj-flush-on-trace/script.lua
new file mode 100644
index 00000000..d2c35534
--- /dev/null
+++ b/test/tarantool-tests/lj-flush-on-trace/script.lua
@@ -0,0 +1,23 @@
+local hotloop = assert(arg[1], 'hotloop argument is missing')
+local trigger = assert(arg[2], 'trigger argument is missing')
+
+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
+-- <flush> call the Lua routine instead of C implementation.
+local flush = require('libflush')(trigger)
+
+-- Depending on trigger and hotloop values the following contexts
+-- are possible:
+-- * if trigger <= hotloop -> trace recording is aborted
+-- * if trigger >  hotloop -> trace is recorded but execution
+--   leads to panic
+jit.opt.start("3", string.format("hotloop=%d", hotloop))
+
+for i = 0, trigger + hotloop do
+  ffiflush.flush(flush, i)
+end
+-- Panic didn't occur earlier.
+io.write('LJ flush still works')
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index eb11d40d..41a7c22a 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -1,7 +1,6 @@
 local M = {}
 
 local ffi = require('ffi')
-local tap = require('tap')
 local bc = require('jit.bc')
 local bit = require('bit')
 
@@ -44,55 +43,28 @@ function M.luacmd(args)
   return table.concat(args, ' ', idx + 1, -1)
 end
 
-local function unshiftenv(variable, value, sep)
-  local envvar = os.getenv(variable)
-  return ('%s="%s%s"'):format(variable, value,
-                              envvar and ('%s%s'):format(sep, envvar) or '')
+local function makeenv(tabenv)
+  if tabenv == nil then return '' end
+  local flatenv = {}
+  for var, value in pairs(tabenv) do
+    table.insert(flatenv, ('%s=%s'):format(var, value))
+  end
+  return table.concat(flatenv, ' ')
 end
 
-function M.selfrun(arg, checks)
-  -- 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 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'))
-
-  test:plan(#checks)
-
-  local libext = package.cpath:match('?.(%a+);')
-  local vars = {
+function M.makecmd(arg, opts)
+  return setmetatable({
     LUABIN = M.luacmd(arg),
-    SCRIPT = arg[0],
-    PATH   = arg[0]:gsub('%.test%.lua', ''),
-    SUFFIX = libext,
-    ENV = table.concat({
-      unshiftenv('LUA_PATH', '<PATH>/?.lua', ';'),
-      unshiftenv('LUA_CPATH', '<PATH>/?.<SUFFIX>', ';'),
-      unshiftenv((libext == 'dylib' and 'DYLD' or 'LD') .. '_LIBRARY_PATH',
-                 '<PATH>', ':'),
-      'TEST_SELFRUN=1',
-    }, ' '),
-  }
-
-  local cmd = string.gsub('<ENV> <LUABIN> 2>&1 <SCRIPT>', '%<(%w+)>', vars)
-
-  for _, ch in pairs(checks) do
-    local testf = test[ch.test]
-    assert(testf, ("tap doesn't provide test.%s function"):format(ch.test))
-    local proc = io.popen((cmd .. (' %s'):rep(#ch.arg)):format(unpack(ch.arg)))
-    local res = proc:read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
-    -- XXX: explicitly pass <test> as an argument to <testf>
-    -- to emulate test:is(...), test:like(...), etc.
-    testf(test, res, ch.res, ch.msg)
-  end
-
-  os.exit(test:check() and 0 or 1)
+    SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$', '/script.lua'),
+    ENV = opts and makeenv(opts.env) or '',
+    REDIRECT = opts and opts.redirect or '',
+  }, {
+    __call = function(self, ...)
+      local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
+                  .. (' %s'):rep(select('#', ...)):format(...)
+      return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
+    end
+  })
 end
 
 function M.skipcond(condition, message)
-- 
2.30.2

[Tarantool-patches] [PATCH luajit 4/7] test: make skipcond helper more convenient

[Igor Munkin via Tarantool-patches]

This patch provides two enhancements for <utils.skipcond>:
1. As a result of the patch, <utils.skipcond> becomes multi-conditional,
   so there is no need to concatenate one huge and complex condition
   with one huge unreadable reason. Now skipcond receives the table with
   conditions and skips the test if one of the given conditions is met.
2. <utils.skipcond> yields the test object, that allows to chain
   skipcond call right next to the test constructor.

Finally as a result of the aforementioned changes <utils.skipcond> is
moved to tap.lua.

Relates to tarantool/tarantool#8252

Co-authored-by: Sergey Kaplun <skaplun@tarantool.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .../gh-4199-gc64-fuse.test.lua                | 11 +++++----
 .../gh-4427-ffi-sandwich.test.lua             | 11 ++++-----
 .../gh-5813-resolving-of-c-symbols.test.lua   | 23 ++++++++-----------
 ...4-add-proto-trace-sysprof-default.test.lua | 14 ++++-------
 .../lj-430-maxirconst.test.lua                | 10 ++++----
 .../lj-603-err-snap-restore.test.lua          | 18 ++++++++-------
 ...lj-672-cdata-allocation-recording.test.lua | 12 +++++-----
 .../lj-906-fix-err-mem.test.lua               | 12 +++++-----
 .../lj-flush-on-trace.test.lua                | 11 ++++-----
 .../misclib-getmetrics-capi.test.lua          | 17 ++++++--------
 .../misclib-getmetrics-lapi.test.lua          | 13 ++++-------
 .../misclib-memprof-lapi.test.lua             | 22 +++++++++---------
 .../misclib-sysprof-capi.test.lua             | 18 ++++++---------
 .../misclib-sysprof-lapi.test.lua             | 17 +++++---------
 test/tarantool-tests/tap.lua                  | 12 ++++++++++
 test/tarantool-tests/utils.lua                |  8 -------
 16 files changed, 104 insertions(+), 125 deletions(-)

diff --git a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
index 4d69250f..65f9faac 100644
--- a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
+++ b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
@@ -1,11 +1,12 @@
--- The test is GC64 only.
-local ffi = require('ffi')
-require('utils').skipcond(not ffi.abi('gc64'), 'test is GC64 only')
-
 local tap = require('tap')
-local test = tap.test('gh-4199-gc64-fuse')
+local test = tap.test('gh-4199-gc64-fuse'):skipcond({
+  ['Test requires GC64 mode enabled'] = not require('ffi').abi('gc64'),
+})
+
 test:plan(1)
 
+local ffi = require('ffi')
+
 collectgarbage()
 -- Chomp memory in currently allocated GC space.
 collectgarbage('stop')
diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
index f4795db0..2d6c3b06 100644
--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
@@ -1,14 +1,11 @@
-local utils = require('utils')
-
--- Disabled on *BSD due to #4819.
-utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
-
 local tap = require('tap')
+local test = tap.test('gh-4427-ffi-sandwich'):skipcond({
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
 
-local test = tap.test('gh-4427-ffi-sandwich')
 test:plan(2)
 
-local script = utils.makecmd(arg, {
+local script = require('utils').makecmd(arg, {
   -- TODO: Leave another toxic comment regarding SIP on macOS.
   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
   redirect = '2>&1',
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
index e0b6d86d..019fed29 100644
--- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
+++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
@@ -1,18 +1,15 @@
--- Memprof is implemented for x86 and x64 architectures only.
-local utils = require("utils")
-
-utils.skipcond(
-  jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
-  jit.arch.." architecture or "..jit.os..
-  " OS is NIY for memprof c symbols resolving"
-)
-
 local tap = require("tap")
-local test = tap.test("gh-5813-resolving-of-c-symbols")
+local test = tap.test("gh-5813-resolving-of-c-symbols"):skipcond({
+  ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and jit.arch ~= "x64",
+  ["Memprof is implemented for Linux only"] = jit.os ~= "Linux",
+})
+
 test:plan(5)
 
-jit.off()
-jit.flush()
+-- XXX: Run JIT tuning functions in a safe frame to avoid errors
+-- thrown when LuaJIT is compiled with JIT engine disabled.
+pcall(jit.off)
+pcall(jit.flush)
 
 local bufread = require "utils.bufread"
 local symtab = require "utils.symtab"
@@ -20,7 +17,7 @@ local testboth = require "resboth"
 local testhash = require "reshash"
 local testgnuhash = require "resgnuhash"
 
-local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
+local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")
 
 local function tree_contains(node, name)
   if node == nil then
diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
index 15bd0a8b..f139b6b5 100644
--- a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
+++ b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
@@ -1,13 +1,9 @@
--- Sysprof is implemented for x86 and x64 architectures only.
-require('utils').skipcond(
-  jit.arch ~= 'x86' and jit.arch ~= 'x64' or jit.os ~= 'Linux'
-    or require('ffi').abi('gc64'),
-  jit.arch..' architecture or '..jit.os..
-  ' OS is NIY for sysprof'
-)
-
 local tap = require('tap')
-local test = tap.test('gh-7264-add-proto-trace-sysprof-default.test.lua')
+local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
+  ['Sysprof is implemented for x86_64 only'] = jit.arch ~= 'x86' and jit.arch ~= 'x64',
+  ['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux',
+})
+
 test:plan(2)
 
 local chunk = [[
diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
index 8bc523c2..633ab676 100644
--- a/test/tarantool-tests/lj-430-maxirconst.test.lua
+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
@@ -1,12 +1,12 @@
--- Disabled on *BSD due to #4819.
-require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
-
 local tap = require('tap')
-local traceinfo = require('jit.util').traceinfo
+local test = tap.test('lj-430-maxirconst'):skipcond({
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
 
-local test = tap.test('lj-430-maxirconst')
 test:plan(2)
 
+local traceinfo = require('jit.util').traceinfo
+
 -- This function has only 3 IR constant.
 local function irconst3()
 end
diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
index b5353e85..c67da00e 100644
--- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
+++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
@@ -1,9 +1,15 @@
 local tap = require('tap')
-
 -- Test to demonstrate the incorrect JIT behaviour when an error
 -- is raised on restoration from the snapshot.
 -- See also https://github.com/LuaJIT/LuaJIT/issues/603.
-local test = tap.test('lj-603-err-snap-restore.test.lua')
+local test = tap.test('lj-603-err-snap-restore'):skipcond({
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+  -- XXX: The different amount of stack slots is in-use for
+  -- Tarantool at start, so just skip test for it.
+  -- luacheck: no global
+  ['Disable test for Tarantool'] = _TARANTOOL,
+})
+
 test:plan(2)
 
 -- XXX: This is fragile. We need a specific amount of Lua stack
@@ -15,7 +21,7 @@ test:plan(2)
 -- error handling"), etc.).
 -- This amount is suited well for GC64 and non-GC64 mode.
 -- luacheck: no unused
-local _, _, _, _, _, _
+local _, _, _, _, _, _, _, _, _, _, _, _, _, _, _
 
 local handler_is_called = false
 local recursive_f
@@ -38,11 +44,7 @@ end
 recursive_f()
 
 test:ok(true)
--- Disabled on *BSD due to #4819.
--- XXX: The different amount of stack slots is in-use for
--- Tarantool at start, so just skip test for it.
--- luacheck: no global
-test:ok(jit.os == 'BSD' or _TARANTOOL or not handler_is_called)
+test:ok(not handler_is_called)
 
 -- XXX: Don't use `os.exit()` here by intention. When error on
 -- snap restoration is raised, `err_unwind()` doesn't stop on
diff --git a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
index 1dc741d8..2165afe3 100644
--- a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
+++ b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
@@ -1,13 +1,13 @@
--- Disabled on *BSD due to #4819.
-require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
+local tap = require('tap')
+local test = tap.test('lj-672-cdata-allocation-recording'):skipcond({
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
+
+test:plan(1)
 
 local ffi = require('ffi')
 local traceinfo = require('jit.util').traceinfo
 
-local tap = require('tap')
-local test = tap.test('lj-672-cdata-allocation-recording')
-test:plan(1)
-
 -- Structure with array.
 ffi.cdef('struct my_struct {int a; char d[8];}')
 
diff --git a/test/tarantool-tests/lj-906-fix-err-mem.test.lua b/test/tarantool-tests/lj-906-fix-err-mem.test.lua
index 450beeff..6c6df338 100644
--- a/test/tarantool-tests/lj-906-fix-err-mem.test.lua
+++ b/test/tarantool-tests/lj-906-fix-err-mem.test.lua
@@ -1,13 +1,13 @@
 local tap = require('tap')
-local ffi = require('ffi')
-local table_new = require('table.new')
-
--- Avoid test to be killed.
-require('utils').skipcond(ffi.abi('gc64'), 'test is not GC64 only')
+local test = tap.test('lj-906-fix-err-mem'):skipcond({
+  ['Test requires GC64 mode disabled'] = require('ffi').abi('gc64'),
+})
 
-local test = tap.test('lj-906-fix-err-mem')
 test:plan(1)
 
+local ffi = require('ffi')
+local table_new = require('table.new')
+
 local KB = 1024
 local MB = 1024 * KB
 local sizes = {8 * MB, 8 * KB, 8}
diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
index cf92757c..60a0ccbd 100644
--- a/test/tarantool-tests/lj-flush-on-trace.test.lua
+++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
@@ -1,14 +1,11 @@
-local utils = require('utils')
-
--- Disabled on *BSD due to #4819.
-utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
-
 local tap = require('tap')
+local test = tap.test('lj-flush-on-trace'):skipcond({
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
 
-local test = tap.test('lj-flush-on-trace')
 test:plan(2)
 
-local script = utils.makecmd(arg, {
+local script = require('utils').makecmd(arg, {
   -- TODO: Leave another toxic comment regarding SIP on macOS.
   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
   redirect = '2>&1',
diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
index 42a9adf9..a41a1c7a 100644
--- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
+++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
@@ -1,12 +1,15 @@
-local utils = require('utils')
+local tap = require('tap')
+local test = tap.test("clib-misc-getmetrics"):skipcond({
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
 
--- Disabled on *BSD due to #4819.
-utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
+test:plan(11)
 
 local path = arg[0]:gsub('%.test%.lua', '')
 local suffix = package.cpath:match('?.(%a+);')
 package.cpath = ('%s/?.%s;'):format(path, suffix)..package.cpath
 
+local maxnins = require('utils').const.maxnins
 local jit_opt_default = {
     3, -- level
     "hotloop=56",
@@ -14,11 +17,6 @@ local jit_opt_default = {
     "minstitch=0",
 }
 
-local tap = require('tap')
-
-local test = tap.test("clib-misc-getmetrics")
-test:plan(11)
-
 local testgetmetrics = require("testgetmetrics")
 
 test:ok(testgetmetrics.base())
@@ -96,8 +94,7 @@ end))
 
 -- Compiled loop with a side exit which does not get compiled.
 test:ok(testgetmetrics.snap_restores(function()
-    jit.opt.start(0, "hotloop=1", "hotexit=2",
-                  ("minstitch=%d"):format(utils.const.maxnins))
+    jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
 
     local function foo(i)
         -- math.fmod is not yet compiled!
diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
index 0c170d0c..e5ee7902 100644
--- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
+++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
@@ -2,16 +2,14 @@
 -- Major portions taken verbatim or adapted from the LuaVela testing suite.
 -- Copyright (C) 2015-2019 IPONWEB Ltd.
 
-local utils = require('utils')
-
--- Disabled on *BSD due to #4819.
-utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
-
 local tap = require('tap')
+local test = tap.test("lib-misc-getmetrics"):skipcond({
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+})
 
-local test = tap.test("lib-misc-getmetrics")
 test:plan(10)
 
+local maxnins = require('utils').const.maxnins
 local jit_opt_default = {
     3, -- level
     "hotloop=56",
@@ -281,8 +279,7 @@ test:test("snap-restores-loop-side-exit-non-compiled", function(subtest)
     -- Compiled loop with a side exit which does not get compiled.
     subtest:plan(1)
 
-    jit.opt.start(0, "hotloop=1", "hotexit=2",
-                  ("minstitch=%d"):format(utils.const.maxnins))
+    jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
 
     local function foo(i)
         -- math.fmod is not yet compiled!
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index bae0c27c..accb1ee1 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -1,14 +1,14 @@
--- Memprof is implemented for x86 and x64 architectures only.
-local utils = require("utils")
-
-utils.skipcond(
-  jit.arch ~= "x86" and jit.arch ~= "x64",
-  jit.arch.." architecture is NIY for memprof"
-)
-
+-- XXX: This comment is a reminder to reimplement memprof tests
+-- assertions to make them more indepentent to the changes made.
+-- Now I just leave this 5 lines comment to preserve line numbers.
+-- TODO: This line will be removed in the next patch.
+-- TODO: This line will be removed in the next patch.
 local tap = require("tap")
+local test = tap.test("misc-memprof-lapi"):skipcond({
+  ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
+                                               jit.arch ~= "x64",
+})
 
-local test = tap.test("misc-memprof-lapi")
 test:plan(5)
 
 local jit_opt_default = {
@@ -28,8 +28,8 @@ local memprof = require "memprof.parse"
 local process = require "memprof.process"
 local symtab = require "utils.symtab"
 
-local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
-local BAD_PATH = utils.profilename("memprofdata/tmp.bin")
+local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")
+local BAD_PATH = require("utils").profilename("memprofdata/tmp.bin")
 local SRC_PATH = "@"..arg[0]
 
 local function default_payload()
diff --git a/test/tarantool-tests/misclib-sysprof-capi.test.lua b/test/tarantool-tests/misclib-sysprof-capi.test.lua
index dad0fe4a..2b0e25ae 100644
--- a/test/tarantool-tests/misclib-sysprof-capi.test.lua
+++ b/test/tarantool-tests/misclib-sysprof-capi.test.lua
@@ -1,21 +1,17 @@
--- Sysprof is implemented for x86 and x64 architectures only.
-local utils = require("utils")
-utils.skipcond(
-  jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
-  jit.arch.." architecture or "..jit.os..
-  " OS is NIY for sysprof"
-)
+local tap = require("tap")
+local test = tap.test("clib-misc-sysprof"):skipcond({
+  ["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and jit.arch ~= "x64",
+  ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
+})
+
+test:plan(2)
 
 local testsysprof = require("testsysprof")
 
-local tap = require("tap")
 local jit = require('jit')
 
 jit.off()
 
-local test = tap.test("clib-misc-sysprof")
-test:plan(2)
-
 test:ok(testsysprof.base())
 test:ok(testsysprof.validation())
 
diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
index 4bf10e8d..8dc36592 100644
--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
@@ -1,14 +1,9 @@
--- Sysprof is implemented for x86 and x64 architectures only.
-local utils = require("utils")
-utils.skipcond(
-  jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
-  jit.arch.." architecture or "..jit.os..
-  " OS is NIY for sysprof"
-)
-
 local tap = require("tap")
+local test = tap.test("misc-sysprof-lapi"):skipcond({
+  ["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and jit.arch ~= "x64",
+  ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
+})
 
-local test = tap.test("misc-sysprof-lapi")
 test:plan(19)
 
 jit.off()
@@ -18,8 +13,8 @@ local bufread = require("utils.bufread")
 local symtab = require("utils.symtab")
 local sysprof = require("sysprof.parse")
 
-local TMP_BINFILE = utils.profilename("sysprofdata.tmp.bin")
-local BAD_PATH = utils.profilename("sysprofdata/tmp.bin")
+local TMP_BINFILE = require("utils").profilename("sysprofdata.tmp.bin")
+local BAD_PATH = require("utils").profilename("sysprofdata/tmp.bin")
 
 local function payload()
   local function fib(n)
diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
index a1f54d20..ac04c01d 100644
--- a/test/tarantool-tests/tap.lua
+++ b/test/tarantool-tests/tap.lua
@@ -304,6 +304,17 @@ local function check(test)
   return test.planned == test.total and test.failed == 0
 end
 
+local function skipcond(test, conditions)
+  for message, condition in pairs(conditions) do
+    if condition then
+      test:plan(1)
+      test:skip(message)
+      os.exit(test:check() and 0 or 1)
+    end
+  end
+  return test
+end
+
 test_mt = {
   __index = {
     test       = new,
@@ -313,6 +324,7 @@ test_mt = {
     ok         = ok,
     fail       = fail,
     skip       = skip,
+    skipcond   = skipcond,
     is         = is,
     isnt       = isnt,
     isnil      = isnil,
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index 41a7c22a..4fb66600 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -67,14 +67,6 @@ function M.makecmd(arg, opts)
   })
 end
 
-function M.skipcond(condition, message)
-  if not condition then return end
-  local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))
-  test:plan(1)
-  test:skip(message)
-  os.exit(test:check() and 0 or 1)
-end
-
 function M.hasbc(f, bytecode)
   assert(type(f) == 'function', 'argument #1 should be a function')
   assert(type(bytecode) == 'string', 'argument #2 should be a string')
-- 
2.30.2

[Tarantool-patches] [PATCH luajit 5/7] test: add skipcond for all JIT-related tests

[Igor Munkin via Tarantool-patches]

This patch adjusts all tests for JIT engine to avoid failures when JIT
is disabled, so skipcond with the result of <jit.status> as a condition
is added to handle this.

Part of tarantool/tarantool#8252

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .../bc-jit-unpatching.test.lua                |  6 +++--
 .../fix-fold-simplify-conv-sext.test.lua      |  7 ++---
 .../fix-slot-check-for-mm-record.test.lua     |  4 ++-
 .../gh-4199-gc64-fuse.test.lua                |  1 +
 .../gh-4427-ffi-sandwich.test.lua             |  1 +
 ...gh-4476-fix-string-find-recording.test.lua |  4 ++-
 .../gh-6065-jit-library-smoke-tests.test.lua  |  4 ++-
 ...8-fix-side-exit-patching-on-arm64.test.lua |  5 +++-
 test/tarantool-tests/gh-6189-cur_L.test.lua   |  7 +++--
 ...ytecode-allocator-for-comparisons.test.lua |  5 +++-
 .../gh-6371-string-char-no-arg.test.lua       |  5 ++--
 ...6782-stitching-in-vmevent-handler.test.lua |  6 +++--
 .../gh-6976-narrowing-of-unary-minus.test.lua |  5 +++-
 ...4-add-proto-trace-sysprof-default.test.lua |  1 +
 .../lj-350-sload-typecheck.test.lua           |  8 +++---
 .../lj-356-ir-khash-non-string-obj.test.lua   | 11 +++++---
 .../lj-357-arm64-hrefk.test.lua               |  6 +++--
 .../lj-375-ir-bufput-signed-char.test.lua     |  4 ++-
 .../lj-408-tonumber-cdata-record.test.lua     | 11 ++++----
 .../lj-416-xor-before-jcc.test.lua            |  6 +++--
 .../lj-430-maxirconst.test.lua                |  1 +
 ...lj-505-fold-no-strref-for-ptrdiff.test.lua |  4 ++-
 .../lj-524-fold-conv-respect-src-irt.test.lua |  6 +++--
 .../lj-556-fix-loop-realignment.test.lua      |  4 ++-
 ...j-584-bad-renames-for-sunk-values.test.lua |  4 ++-
 .../lj-603-err-snap-restore.test.lua          |  1 +
 ...lj-672-cdata-allocation-recording.test.lua |  1 +
 .../lj-864-varg-rec-base-offset.test.lua      |  6 +++--
 .../lj-flush-on-trace.test.lua                |  1 +
 .../misclib-getmetrics-capi.test.lua          |  1 +
 .../misclib-getmetrics-lapi.test.lua          |  1 +
 .../misclib-memprof-lapi.test.lua             | 26 +++++++++----------
 .../misclib-sysprof-lapi.test.lua             |  7 ++---
 33 files changed, 112 insertions(+), 58 deletions(-)

diff --git a/test/tarantool-tests/bc-jit-unpatching.test.lua b/test/tarantool-tests/bc-jit-unpatching.test.lua
index 71247f0c..2c3b7c9a 100644
--- a/test/tarantool-tests/bc-jit-unpatching.test.lua
+++ b/test/tarantool-tests/bc-jit-unpatching.test.lua
@@ -1,9 +1,11 @@
 local tap = require('tap')
-local utils = require('utils')
+local test = tap.test('bc-jit-unpatching'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('bc-jit-unpatching')
 test:plan(1)
 
+local utils = require('utils')
 -- Function with up-recursion.
 local function f(n)
   return n < 2 and n or f(n - 1) + f(n - 2)
diff --git a/test/tarantool-tests/fix-fold-simplify-conv-sext.test.lua b/test/tarantool-tests/fix-fold-simplify-conv-sext.test.lua
index 07e22c36..60eb3e7c 100644
--- a/test/tarantool-tests/fix-fold-simplify-conv-sext.test.lua
+++ b/test/tarantool-tests/fix-fold-simplify-conv-sext.test.lua
@@ -1,12 +1,13 @@
 local tap = require('tap')
-local ffi = require('ffi')
-
-local test = tap.test('fix-fold-simplify-conv-sext')
+local test = tap.test('fix-fold-simplify-conv-sext'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
 local NSAMPLES = 4
 local NTEST = NSAMPLES * 2 - 1
 test:plan(NTEST)
 
+local ffi = require('ffi')
 local samples = ffi.new('int [?]', NSAMPLES)
 
 -- Prepare data.
diff --git a/test/tarantool-tests/fix-slot-check-for-mm-record.test.lua b/test/tarantool-tests/fix-slot-check-for-mm-record.test.lua
index 8df72ec4..6161747f 100644
--- a/test/tarantool-tests/fix-slot-check-for-mm-record.test.lua
+++ b/test/tarantool-tests/fix-slot-check-for-mm-record.test.lua
@@ -1,7 +1,9 @@
 -- luacheck: globals a0 a1
 local tap = require('tap')
+local test = tap.test('fix-slot-check-for-mm-record'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('fix-slot-check-for-mm-record')
 test:plan(1)
 
 -- Before the patch, JIT compiler doesn't check slots overflow
diff --git a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
index 65f9faac..4513d43b 100644
--- a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
+++ b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
@@ -1,5 +1,6 @@
 local tap = require('tap')
 local test = tap.test('gh-4199-gc64-fuse'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Test requires GC64 mode enabled'] = not require('ffi').abi('gc64'),
 })
 
diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
index 2d6c3b06..ff3eaf01 100644
--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
@@ -1,5 +1,6 @@
 local tap = require('tap')
 local test = tap.test('gh-4427-ffi-sandwich'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
 })
 
diff --git a/test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua b/test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua
index f48af173..0758b38f 100644
--- a/test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua
+++ b/test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua
@@ -1,6 +1,8 @@
 local tap = require('tap')
+local test = tap.test("gh-4476-fix-string-find-recording"):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test("gh-4476-fix-string-find-recording")
 test:plan(1)
 
 local err = [[module 'kit.1.10.3-136' not found:
diff --git a/test/tarantool-tests/gh-6065-jit-library-smoke-tests.test.lua b/test/tarantool-tests/gh-6065-jit-library-smoke-tests.test.lua
index 7110e351..5d7fd7e2 100644
--- a/test/tarantool-tests/gh-6065-jit-library-smoke-tests.test.lua
+++ b/test/tarantool-tests/gh-6065-jit-library-smoke-tests.test.lua
@@ -1,6 +1,8 @@
 local tap = require('tap')
+local test = tap.test('gh-6065-jit-library-smoke-tests'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('gh-6065-jit-library-smoke-tests')
 test:plan(1)
 
 -- Just check whether LuaJIT is built with JIT support. Otherwise,
diff --git a/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua b/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
index 4dcf3e22..cfcc6adb 100644
--- a/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
+++ b/test/tarantool-tests/gh-6098-fix-side-exit-patching-on-arm64.test.lua
@@ -1,5 +1,8 @@
 local tap = require('tap')
-local test = tap.test('gh-6098-fix-side-exit-patching-on-arm64')
+local test = tap.test('gh-6098-fix-side-exit-patching-on-arm64'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
 test:plan(1)
 
 -- The function to be tested for side exit patching:
diff --git a/test/tarantool-tests/gh-6189-cur_L.test.lua b/test/tarantool-tests/gh-6189-cur_L.test.lua
index 7f2184ec..a5096a7c 100644
--- a/test/tarantool-tests/gh-6189-cur_L.test.lua
+++ b/test/tarantool-tests/gh-6189-cur_L.test.lua
@@ -1,9 +1,12 @@
-local libcur_L = require('libcur_L')
 local tap = require('tap')
+local test = tap.test('gh-6189-cur_L'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('gh-6189-cur_L')
 test:plan(1)
 
+local libcur_L = require('libcur_L')
+
 local function cbool(cond)
   if cond then
     return 1
diff --git a/test/tarantool-tests/gh-6227-bytecode-allocator-for-comparisons.test.lua b/test/tarantool-tests/gh-6227-bytecode-allocator-for-comparisons.test.lua
index 9788923a..da399bcf 100644
--- a/test/tarantool-tests/gh-6227-bytecode-allocator-for-comparisons.test.lua
+++ b/test/tarantool-tests/gh-6227-bytecode-allocator-for-comparisons.test.lua
@@ -1,5 +1,8 @@
 local tap = require('tap')
-local test = tap.test('gh-6227-bytecode-allocator-for-comparisons')
+local test = tap.test('gh-6227-bytecode-allocator-for-comparisons'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
 test:plan(1)
 
 -- Test file to demonstrate assertion failure during recording
diff --git a/test/tarantool-tests/gh-6371-string-char-no-arg.test.lua b/test/tarantool-tests/gh-6371-string-char-no-arg.test.lua
index ec871d19..90121860 100644
--- a/test/tarantool-tests/gh-6371-string-char-no-arg.test.lua
+++ b/test/tarantool-tests/gh-6371-string-char-no-arg.test.lua
@@ -1,10 +1,11 @@
 local tap = require('tap')
-
 -- Test file to demonstrate assertion after `string.char()`
 -- recording.
 -- See also, https://github.com/tarantool/tarantool/issues/6371.
+local test = tap.test('gh-6371-string-char-no-arg'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('gh-6371-string-char-no-arg')
 -- XXX: Number of loop iterations.
 -- * 1 -- instruction becomes hot.
 -- * 2 -- recording of the loop body.
diff --git a/test/tarantool-tests/gh-6782-stitching-in-vmevent-handler.test.lua b/test/tarantool-tests/gh-6782-stitching-in-vmevent-handler.test.lua
index 6087e5ae..385e7648 100644
--- a/test/tarantool-tests/gh-6782-stitching-in-vmevent-handler.test.lua
+++ b/test/tarantool-tests/gh-6782-stitching-in-vmevent-handler.test.lua
@@ -1,9 +1,11 @@
 local tap = require('tap')
-
 -- Test file to demonstrate incorrect stitching behaviour
 -- in vmevent handler.
 -- See also https://github.com/tarantool/tarantool/issues/6782.
-local test = tap.test('gh-6782-stitching-in-vmevent-handler')
+local test = tap.test('gh-6782-stitching-in-vmevent-handler'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
 test:plan(1)
 
 -- Just dump bytecodes is enough.
diff --git a/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua b/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua
index b4792f59..40387cca 100644
--- a/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua
+++ b/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua
@@ -1,5 +1,8 @@
 local tap = require('tap')
-local test = tap.test('gh-6976-narrowing-of-unary-minus')
+local test = tap.test('gh-6976-narrowing-of-unary-minus'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
 test:plan(2)
 
 jit.opt.start('hotloop=1')
diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
index f139b6b5..bc8f3d94 100644
--- a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
+++ b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
@@ -1,5 +1,6 @@
 local tap = require('tap')
 local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Sysprof is implemented for x86_64 only'] = jit.arch ~= 'x86' and jit.arch ~= 'x64',
   ['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux',
 })
diff --git a/test/tarantool-tests/lj-350-sload-typecheck.test.lua b/test/tarantool-tests/lj-350-sload-typecheck.test.lua
index 33794943..5b25864d 100644
--- a/test/tarantool-tests/lj-350-sload-typecheck.test.lua
+++ b/test/tarantool-tests/lj-350-sload-typecheck.test.lua
@@ -1,13 +1,15 @@
 local tap = require('tap')
-local traceinfo = require('jit.util').traceinfo
-
 -- Test file to demonstrate the incorrect GC64 JIT asembling
 -- `IR_SLOAD`.
 -- See also https://github.com/LuaJIT/LuaJIT/pull/350.
-local test = tap.test('lj-350-sload-typecheck')
+local test = tap.test('lj-350-sload-typecheck'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
 test:plan(1)
 
+local traceinfo = require('jit.util').traceinfo
+
 -- Contains only IR_SLOAD after recording.
 local function sload(arg)
   return arg
diff --git a/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua b/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua
index 7f304183..9977205d 100644
--- a/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua
+++ b/test/tarantool-tests/lj-356-ir-khash-non-string-obj.test.lua
@@ -1,17 +1,20 @@
 local tap = require('tap')
-local traceinfo = require('jit.util').traceinfo
-local table_new = require('table.new')
-
 -- Test file to demonstrate the incorrect GC64 JIT behaviour
 -- of an `IR_HREF` for the on-trace-constant key lookup.
 -- See also https://github.com/LuaJIT/LuaJIT/pull/356.
-local test = tap.test('lj-356-ir-khash-non-string-obj')
+local test = tap.test('lj-356-ir-khash-non-string-obj'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
 local N_ITERATIONS = 4
 
 -- Amount of iteration for trace compilation and execution and
 -- additional check, that there is no new trace compiled.
 test:plan(N_ITERATIONS + 1)
 
+local traceinfo = require('jit.util').traceinfo
+local table_new = require('table.new')
+
 -- To reproduce the issue we need to compile a trace with
 -- `IR_HREF`, with a lookup of constant hash key GC value. To
 -- prevent an `IR_HREFK` to be emitted instead, we need a table
diff --git a/test/tarantool-tests/lj-357-arm64-hrefk.test.lua b/test/tarantool-tests/lj-357-arm64-hrefk.test.lua
index 8af9143a..d7e9c85e 100644
--- a/test/tarantool-tests/lj-357-arm64-hrefk.test.lua
+++ b/test/tarantool-tests/lj-357-arm64-hrefk.test.lua
@@ -1,9 +1,11 @@
 local tap = require('tap')
-
 -- Test file to demonstrate the incorrect JIT behaviour for HREFK
 -- IR compilation on arm64.
 -- See also https://github.com/LuaJIT/LuaJIT/issues/357.
-local test = tap.test('lj-357-arm64-hrefk')
+local test = tap.test('lj-357-arm64-hrefk'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
 test:plan(2)
 
 jit.opt.start('hotloop=1', 'hotexit=1')
diff --git a/test/tarantool-tests/lj-375-ir-bufput-signed-char.test.lua b/test/tarantool-tests/lj-375-ir-bufput-signed-char.test.lua
index 7c8df948..f600d898 100644
--- a/test/tarantool-tests/lj-375-ir-bufput-signed-char.test.lua
+++ b/test/tarantool-tests/lj-375-ir-bufput-signed-char.test.lua
@@ -1,6 +1,8 @@
 local tap = require('tap')
+local test = tap.test('lj-375-ir-bufput-signed-char'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('lj-375-ir-bufput-signed-char')
 -- XXX: Number of loop iterations.
 -- 1 -- instruction becomes hot
 -- 2, 3 -- trace is recorded (considering loop recording
diff --git a/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua b/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua
index a8235e93..bdd0aaaa 100644
--- a/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua
+++ b/test/tarantool-tests/lj-408-tonumber-cdata-record.test.lua
@@ -1,18 +1,19 @@
-local ffi = require('ffi')
 local tap = require('tap')
-
 -- Test file to demonstrate the incorrect JIT recording for
 -- `tonumber()` function with cdata argument for failed
 -- conversions.
 -- See also https://github.com/LuaJIT/LuaJIT/issues/408,
 -- https://github.com/LuaJIT/LuaJIT/pull/412,
 -- https://github.com/tarantool/tarantool/issues/7655.
-local test = tap.test('lj-408-tonumber-cdata-record')
-
-local NULL = ffi.cast('void *', 0)
+local test = tap.test('lj-408-tonumber-cdata-record'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
 test:plan(4)
 
+local ffi = require('ffi')
+local NULL = ffi.cast('void *', 0)
+
 local function check(x)
   -- Don't use a tail call to avoid "leaving loop in root trace"
   -- error, so the trace will be compiled.
diff --git a/test/tarantool-tests/lj-416-xor-before-jcc.test.lua b/test/tarantool-tests/lj-416-xor-before-jcc.test.lua
index f9a2a869..861114e8 100644
--- a/test/tarantool-tests/lj-416-xor-before-jcc.test.lua
+++ b/test/tarantool-tests/lj-416-xor-before-jcc.test.lua
@@ -1,7 +1,8 @@
-local ffi = require('ffi')
 local tap = require('tap')
+local test = tap.test('lj-416-xor-before-jcc'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('lj-416-xor-before-jcc')
 test:plan(1)
 
 -- To reproduce this issue, we need:
@@ -30,6 +31,7 @@ test:plan(1)
 -- ucomisd and the jnb, thereby causing the jnb to do the wrong
 -- thing.
 
+local ffi = require('ffi')
 ffi.cdef[[
   int test_xor_func(int a, int b, int c, int d, int e, int f, void * g, int h);
 ]]
diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
index 633ab676..531acd7d 100644
--- a/test/tarantool-tests/lj-430-maxirconst.test.lua
+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
@@ -1,5 +1,6 @@
 local tap = require('tap')
 local test = tap.test('lj-430-maxirconst'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
 })
 
diff --git a/test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua b/test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua
index 2866fb12..fec08b30 100644
--- a/test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua
+++ b/test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua
@@ -1,6 +1,8 @@
 local tap = require('tap')
+local test = tap.test("lj-505-fold-icorrect-behavior"):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test("lj-505-fold-icorrect-behavior")
 test:plan(1)
 
 -- Test file to demonstrate Lua fold machinery icorrect behavior, details:
diff --git a/test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua b/test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua
index cd0f0f04..b2ccae63 100644
--- a/test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua
+++ b/test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua
@@ -1,9 +1,11 @@
 local tap = require('tap')
-local ffi = require('ffi')
+local test = tap.test("or-524-fold-icorrect-behavior"):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test("or-524-fold-icorrect-behavior")
 test:plan(1)
 
+local ffi = require('ffi')
 -- Test file to demonstrate LuaJIT folding machinery incorrect behaviour,
 -- details:
 --     https://github.com/LuaJIT/LuaJIT/issues/524
diff --git a/test/tarantool-tests/lj-556-fix-loop-realignment.test.lua b/test/tarantool-tests/lj-556-fix-loop-realignment.test.lua
index 6015f55f..b94bd3e9 100644
--- a/test/tarantool-tests/lj-556-fix-loop-realignment.test.lua
+++ b/test/tarantool-tests/lj-556-fix-loop-realignment.test.lua
@@ -1,6 +1,8 @@
 local tap = require('tap')
+local test = tap.test('lj-556-fix-loop-realignment'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('lj-556-fix-loop-realignment')
 test:plan(1)
 
 -- Test file to demonstrate JIT misbehaviour for loop realignment
diff --git a/test/tarantool-tests/lj-584-bad-renames-for-sunk-values.test.lua b/test/tarantool-tests/lj-584-bad-renames-for-sunk-values.test.lua
index f037c898..e0c3e577 100644
--- a/test/tarantool-tests/lj-584-bad-renames-for-sunk-values.test.lua
+++ b/test/tarantool-tests/lj-584-bad-renames-for-sunk-values.test.lua
@@ -1,6 +1,8 @@
 local tap = require('tap')
+local test = tap.test('lj-584-bad-renames-for-sunk-values'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
 
-local test = tap.test('lj-584-bad-renames-for-sunk-values')
 test:plan(1)
 
 -- Test file to demonstrate LuaJIT assembler misbehaviour.
diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
index c67da00e..28aaf7f5 100644
--- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
+++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
@@ -3,6 +3,7 @@ local tap = require('tap')
 -- is raised on restoration from the snapshot.
 -- See also https://github.com/LuaJIT/LuaJIT/issues/603.
 local test = tap.test('lj-603-err-snap-restore'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
   -- XXX: The different amount of stack slots is in-use for
   -- Tarantool at start, so just skip test for it.
diff --git a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
index 2165afe3..1d6b19ba 100644
--- a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
+++ b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
@@ -1,5 +1,6 @@
 local tap = require('tap')
 local test = tap.test('lj-672-cdata-allocation-recording'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
 })
 
diff --git a/test/tarantool-tests/lj-864-varg-rec-base-offset.test.lua b/test/tarantool-tests/lj-864-varg-rec-base-offset.test.lua
index d74c3c2b..d41e33ff 100644
--- a/test/tarantool-tests/lj-864-varg-rec-base-offset.test.lua
+++ b/test/tarantool-tests/lj-864-varg-rec-base-offset.test.lua
@@ -1,10 +1,12 @@
 local tap = require('tap')
-
 -- Test file to demonstrate LuaJIT misbehaviour during recording
 -- BC_VARG with nvarargs >= nresults in GC64 mode.
 -- See also https://github.com/LuaJIT/LuaJIT/issues/864,
 -- https://github.com/tarantool/tarantool/issues/7172.
-local test = tap.test('lj-864-varg-rec-base-offset')
+local test = tap.test('lj-864-varg-rec-base-offset'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
 test:plan(1)
 
 jit.opt.start('hotloop=1')
diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
index 60a0ccbd..e157622a 100644
--- a/test/tarantool-tests/lj-flush-on-trace.test.lua
+++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
@@ -1,5 +1,6 @@
 local tap = require('tap')
 local test = tap.test('lj-flush-on-trace'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
 })
 
diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
index a41a1c7a..c8879f57 100644
--- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
+++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
@@ -1,5 +1,6 @@
 local tap = require('tap')
 local test = tap.test("clib-misc-getmetrics"):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
 })
 
diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
index e5ee7902..b1f86cea 100644
--- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
+++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
@@ -4,6 +4,7 @@
 
 local tap = require('tap')
 local test = tap.test("lib-misc-getmetrics"):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
 })
 
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index accb1ee1..e2701c13 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -1,8 +1,6 @@
 -- XXX: This comment is a reminder to reimplement memprof tests
 -- assertions to make them more indepentent to the changes made.
--- Now I just leave this 5 lines comment to preserve line numbers.
--- TODO: This line will be removed in the next patch.
--- TODO: This line will be removed in the next patch.
+-- Now I just leave this 3 lines comment to preserve line numbers.
 local tap = require("tap")
 local test = tap.test("misc-memprof-lapi"):skipcond({
   ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
@@ -18,8 +16,10 @@ local jit_opt_default = {
     "minstitch=0",
 }
 
-jit.off()
-jit.flush()
+-- XXX: Run JIT tuning functions in a safe frame to avoid errors
+-- thrown when LuaJIT is compiled with JIT engine disabled.
+pcall(jit.off)
+pcall(jit.flush)
 
 local table_new = require "table.new"
 
@@ -263,16 +263,14 @@ test:test("symtab-enrich-str", function(subtest)
   )
 end)
 
--- Test profiler with enabled JIT.
-jit.on()
-
 test:test("jit-output", function(subtest)
-  -- Disabled on *BSD due to #4819.
-  if jit.os == 'BSD' then
-    subtest:plan(1)
-    subtest:skip('Disabled due to #4819')
-    return
-  end
+  subtest:skipcond({
+    ['Test requires JIT enabled'] = not jit.status(),
+    ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+  })
+
+  -- Test profiler with enabled JIT.
+  jit.on()
 
   subtest:plan(4)
 
diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
index 8dc36592..dcde81b9 100644
--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
@@ -6,8 +6,10 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
 
 test:plan(19)
 
-jit.off()
-jit.flush()
+-- XXX: Run JIT tuning functions in a safe frame to avoid errors
+-- thrown when LuaJIT is compiled with JIT engine disabled.
+pcall(jit.off)
+pcall(jit.flush)
 
 local bufread = require("utils.bufread")
 local symtab = require("utils.symtab")
@@ -125,5 +127,4 @@ check_mode("C", 100)
 
 os.remove(TMP_BINFILE)
 
-jit.on()
 os.exit(test:check() and 0 or 1)
-- 
2.30.2

[Tarantool-patches] [PATCH luajit 6/7] test: fix lua-Harness JIT-related tests

[Igor Munkin via Tarantool-patches]

lua-Harness consider whether JIT is enabled or not in scope of 403-jit.t
and 411-luajit.t tests. However, the original condition is wrong, since
<jit.status> yields false for both cases, when JIT is just turned off
and when LuaJIT is build without compiler support. So if <jit.status>
yields false, the latter case is considered. The condition is fixed to
differ both aforementioned cases the following way: when <jit.status>
yields only compiler status with no flags listing, LuaJIT is built
without compiler; if there is more than one value returned, JIT support
is on aboard.

Part of tarantool/tarantool#8252

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 test/lua-Harness-tests/403-jit.t    | 2 +-
 test/lua-Harness-tests/411-luajit.t | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/lua-Harness-tests/403-jit.t b/test/lua-Harness-tests/403-jit.t
index 0f986da9..0aa5d367 100755
--- a/test/lua-Harness-tests/403-jit.t
+++ b/test/lua-Harness-tests/403-jit.t
@@ -31,7 +31,7 @@ if not jit then
     skip_all("only with LuaJIT")
 end
 
-local compiled_with_jit = jit.status()
+local compiled_with_jit = select('#', jit.status()) > 1
 local luajit20 = jit.version_num < 20100 and not jit.version:match'RaptorJIT'
 local has_jit_opt = compiled_with_jit
 local has_jit_security = jit.security
diff --git a/test/lua-Harness-tests/411-luajit.t b/test/lua-Harness-tests/411-luajit.t
index 3a9a7b8f..e3b6c7a8 100755
--- a/test/lua-Harness-tests/411-luajit.t
+++ b/test/lua-Harness-tests/411-luajit.t
@@ -37,7 +37,7 @@ if not pcall(io.popen, lua .. [[ -e "a=1"]]) then
     skip_all("io.popen not supported")
 end
 
-local compiled_with_jit = jit.status()
+local compiled_with_jit = select('#', jit.status()) > 1
 local has_jutil = pcall(require, 'jit.util')
 local has_openresty_listing = profile.openresty or jit.version:match'moonjit'
 
-- 
2.30.2

[Tarantool-patches] [PATCH luajit 7/7] ci: add nojit flavor for exotic builds

[Igor Munkin via Tarantool-patches]

Since all testing machinery is adjusted for LuaJIT configuration with
disabled compiler support, the new flavor for exotic builds is
introduced in LuaJIT CI.

Part of tarantool/tarantool#8252

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/workflows/exotic-builds-testing.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/exotic-builds-testing.yml b/.github/workflows/exotic-builds-testing.yml
index 67ab9cc3..b28673c6 100644
--- a/.github/workflows/exotic-builds-testing.yml
+++ b/.github/workflows/exotic-builds-testing.yml
@@ -36,7 +36,7 @@ jobs:
         BUILDTYPE: [Debug, Release]
         ARCH: [ARM64, x86_64]
         GC64: [ON, OFF]
-        FLAVOR: [dualnum, checkhook]
+        FLAVOR: [dualnum, checkhook, nojit]
         include:
           - BUILDTYPE: Debug
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
@@ -46,6 +46,8 @@ jobs:
             FLAVORFLAGS: -DLUAJIT_NUMMODE=2
           - FLAVOR: checkhook
             FLAVORFLAGS: -DLUAJIT_ENABLE_CHECKHOOK=ON
+          - FLAVOR: nojit
+            FLAVORFLAGS: -DLUAJIT_DISABLE_JIT=ON
         exclude:
           # DUALNUM is default for ARM64, no need for additional testing.
           - FLAVOR: dualnum
-- 
2.30.2

Re: [Tarantool-patches] [PATCH luajit 1/7] Minor fixes.

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the patch!
LGTM!

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/7] build: fix build with JIT disabled

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the patch!
LGTM, except a single nit regarding the commit message.

On 13.02.23, Igor Munkin wrote:
> struct GCtrace is defined only if LJ_HASJIT is set. Hence all spots

Typo: s/Hence,/Hence/

> where GCtrace is used should be also moved under LJ_HASJIT define.
> 
> Relates to tarantool/tarantool#8252

Side note: I see "Relates" and "Related" in our commit logs, so I
suggest to use "Relates" for future commits.

> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>

<snipped>

> -- 
> 2.30.2
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 7/7] ci: add nojit flavor for exotic builds

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!

Thanks for the patch!
LGTM, except an ignorable suggestion.
You've mentioned before offline that dualnum, checkhook entries are not
sorted alphabetically. So, I suggest to sort these entries inside FLAVOR
definition and sort FLAVORFLAGS definitions as well.
Feel free to ignore.

On 13.02.23, Igor Munkin wrote:
> Since all testing machinery is adjusted for LuaJIT configuration with
> disabled compiler support, the new flavor for exotic builds is
> introduced in LuaJIT CI.
> 
> Part of tarantool/tarantool#8252
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  .github/workflows/exotic-builds-testing.yml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/.github/workflows/exotic-builds-testing.yml b/.github/workflows/exotic-builds-testing.yml
> index 67ab9cc3..b28673c6 100644
> --- a/.github/workflows/exotic-builds-testing.yml
> +++ b/.github/workflows/exotic-builds-testing.yml
> @@ -36,7 +36,7 @@ jobs:
>          BUILDTYPE: [Debug, Release]
>          ARCH: [ARM64, x86_64]
>          GC64: [ON, OFF]
> -        FLAVOR: [dualnum, checkhook]
> +        FLAVOR: [dualnum, checkhook, nojit]
>          include:
>            - BUILDTYPE: Debug
>              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> @@ -46,6 +46,8 @@ jobs:
>              FLAVORFLAGS: -DLUAJIT_NUMMODE=2
>            - FLAVOR: checkhook
>              FLAVORFLAGS: -DLUAJIT_ENABLE_CHECKHOOK=ON
> +          - FLAVOR: nojit
> +            FLAVORFLAGS: -DLUAJIT_DISABLE_JIT=ON
>          exclude:
>            # DUALNUM is default for ARM64, no need for additional testing.
>            - FLAVOR: dualnum
> -- 
> 2.30.2
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 6/7] test: fix lua-Harness JIT-related tests

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the patch!
Please, consider my comment below.

On 13.02.23, Igor Munkin wrote:
> lua-Harness consider whether JIT is enabled or not in scope of 403-jit.t
> and 411-luajit.t tests. However, the original condition is wrong, since
> <jit.status> yields false for both cases, when JIT is just turned off
> and when LuaJIT is build without compiler support. So if <jit.status>

Typo: s/So/So,/

> yields false, the latter case is considered. The condition is fixed to
> differ both aforementioned cases the following way: when <jit.status>

Typo? s/to differ ... cases the following way/to differ ... cases in the following way/

> yields only compiler status with no flags listing, LuaJIT is built
> without compiler; if there is more than one value returned, JIT support
> is on aboard.

I afraid that this check isn't fully correct either:
`jit.status()` yields CPU-specific flags and JIT optimization flags.
For ARM64 the subset of CPU-specific flags is empty. Hence, if someone
runs the tests with `jit.opt.start(0)` to disable all JIT optimization
this check will return false for LuaJIT compiled with JIT.

| $ src/luajit -e 'jit.opt.start(0) print(jit.arch, jit.status(), select("#", jit.status()) > 1)'
| arm64   true    false

Also, I found the easier way to check `LJ_HASJIT` is set or not.
We can simply check the existance of `jit.opt` module:

| $ src/luajit -e 'print(jit.opt)'
| nil

> 
> Part of tarantool/tarantool#8252
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  test/lua-Harness-tests/403-jit.t    | 2 +-
>  test/lua-Harness-tests/411-luajit.t | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/test/lua-Harness-tests/403-jit.t b/test/lua-Harness-tests/403-jit.t
> index 0f986da9..0aa5d367 100755
> --- a/test/lua-Harness-tests/403-jit.t
> +++ b/test/lua-Harness-tests/403-jit.t
> @@ -31,7 +31,7 @@ if not jit then
>      skip_all("only with LuaJIT")
>  end
>  
> -local compiled_with_jit = jit.status()
> +local compiled_with_jit = select('#', jit.status()) > 1
>  local luajit20 = jit.version_num < 20100 and not jit.version:match'RaptorJIT'
>  local has_jit_opt = compiled_with_jit
>  local has_jit_security = jit.security
> diff --git a/test/lua-Harness-tests/411-luajit.t b/test/lua-Harness-tests/411-luajit.t
> index 3a9a7b8f..e3b6c7a8 100755
> --- a/test/lua-Harness-tests/411-luajit.t
> +++ b/test/lua-Harness-tests/411-luajit.t
> @@ -37,7 +37,7 @@ if not pcall(io.popen, lua .. [[ -e "a=1"]]) then
>      skip_all("io.popen not supported")
>  end
>  
> -local compiled_with_jit = jit.status()
> +local compiled_with_jit = select('#', jit.status()) > 1
>  local has_jutil = pcall(require, 'jit.util')
>  local has_openresty_listing = profile.openresty or jit.version:match'moonjit'
>  
> -- 
> 2.30.2
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 5/7] test: add skipcond for all JIT-related tests

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the patch!
LGTM.

On 13.02.23, Igor Munkin wrote:

<snipped>

> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index accb1ee1..e2701c13 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -1,8 +1,6 @@
>  -- XXX: This comment is a reminder to reimplement memprof tests
>  -- assertions to make them more indepentent to the changes made.
> --- Now I just leave this 5 lines comment to preserve line numbers.
> --- TODO: This line will be removed in the next patch.
> --- TODO: This line will be removed in the next patch.
> +-- Now I just leave this 3 lines comment to preserve line numbers.

Side note: Still don't like this approach a lot, but, who cares.

>  local tap = require("tap")
>  local test = tap.test("misc-memprof-lapi"):skipcond({
>    ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
> @@ -18,8 +16,10 @@ local jit_opt_default = {
>      "minstitch=0",
>  }

<snipped>

> -- 
> 2.30.2
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 1/7] Minor fixes.

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1376 bytes --]


Hi, Igor!
Thanks for the patch!
LGTM.
--
Best regards,
Maxim Kokryashkin
 
  
>Понедельник, 13 февраля 2023, 20:03 +03:00 от Igor Munkin <imun@tarantool.org>:
> 
>From: Mike Pall <mike>
>
>(cherry picked from commit 54484e257f24c47d71b8233844e2ec8664db8071)
>
>This is a tiny patch fixing header dependencies for the build with
>-DLUAJIT_DISABLE_JIT=ON and whitespace inconsistency.
>
>Igor Munkin:
>* added the description
>
>Part of tarantool/tarantool#8069
>
>Signed-off-by: Igor Munkin < imun@tarantool.org >
>---
> src/lib_base.c | 2 +-
> src/lj_gc.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/src/lib_base.c b/src/lib_base.c
>index 398c4f65..eb6da054 100644
>--- a/src/lib_base.c
>+++ b/src/lib_base.c
>@@ -505,7 +505,7 @@ LJLIB_CF(print)
>     tv = L->top-1;
>   }
>   shortcut = (tvisfunc(tv) && funcV(tv)->c.ffid == FF_tostring)
>- && !gcrefu(basemt_it(G(L), LJ_TNUMX));
>+ && !gcrefu(basemt_it(G(L), LJ_TNUMX));
>   for (i = 0; i < nargs; i++) {
>     cTValue *o = &L->base[i];
>     const char *str;
>diff --git a/src/lj_gc.c b/src/lj_gc.c
>index 29387a48..c306047a 100644
>--- a/src/lj_gc.c
>+++ b/src/lj_gc.c
>@@ -25,6 +25,7 @@
> #include "lj_cdata.h"
> #endif
> #include "lj_trace.h"
>+#include "lj_dispatch.h"
> #include "lj_vm.h"
> 
> #define GCSTEPSIZE 1024u
>--
>2.30.2
 

[-- Attachment #2: Type: text/html, Size: 2126 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/7] build: fix build with JIT disabled

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 2903 bytes --]


Hi, Igor!
Thanks for the patch!
LGTM, except for the nits mentioned by Sergey.
--
Best regards,
Maxim Kokryashkin
 
  
>Понедельник, 13 февраля 2023, 20:03 +03:00 от Igor Munkin <imun@tarantool.org>:
> 
>struct GCtrace is defined only if LJ_HASJIT is set. Hence all spots
>where GCtrace is used should be also moved under LJ_HASJIT define.
>
>Relates to tarantool/tarantool#8252
>
>Signed-off-by: Igor Munkin < imun@tarantool.org >
>---
> src/lj_memprof.c | 9 ++++-----
> src/lj_symtab.c | 13 +++----------
> src/lj_symtab.h | 2 ++
> 3 files changed, 9 insertions(+), 15 deletions(-)
>
>diff --git a/src/lj_memprof.c b/src/lj_memprof.c
>index d4a639fd..8cab8204 100644
>--- a/src/lj_memprof.c
>+++ b/src/lj_memprof.c
>@@ -369,6 +369,8 @@ void lj_memprof_add_proto(const struct GCproto *pt)
>   lj_symtab_dump_proto(&mp->out, pt);
> }
> 
>+#if LJ_HASJIT
>+
> void lj_memprof_add_trace(const struct GCtrace *tr)
> {
>   struct memprof *mp = &memprof;
>@@ -380,6 +382,8 @@ void lj_memprof_add_trace(const struct GCtrace *tr)
>   lj_symtab_dump_trace(&mp->out, tr);
> }
> 
>+#endif /* LJ_HASJIT */
>+
> #else /* LJ_HASMEMPROF */
> 
> int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt)
>@@ -401,9 +405,4 @@ void lj_memprof_add_proto(const struct GCproto *pt)
>   UNUSED(pt);
> }
> 
>-void lj_memprof_add_trace(const struct GCtrace *tr)
>-{
>- UNUSED(tr);
>-}
>-
> #endif /* LJ_HASMEMPROF */
>diff --git a/src/lj_symtab.c b/src/lj_symtab.c
>index 91ee9a72..54984c05 100644
>--- a/src/lj_symtab.c
>+++ b/src/lj_symtab.c
>@@ -53,16 +53,7 @@ void lj_symtab_dump_trace(struct lj_wbuf *out, const GCtrace *trace)
>   lj_wbuf_addu64(out, (uint64_t)lineno);
> }
> 
>-#else
>-
>-static void lj_symtab_dump_trace(struct lj_wbuf *out, const GCtrace *trace)
>-{
>- UNUSED(out);
>- UNUSED(trace);
>- lua_assert(0);
>-}
>-
>-#endif
>+#endif /* LJ_HASJIT */
> 
> void lj_symtab_dump_proto(struct lj_wbuf *out, const GCproto *pt)
> {
>@@ -491,11 +482,13 @@ void lj_symtab_dump(struct lj_wbuf *out, const struct global_State *g,
>       lj_symtab_dump_proto(out, pt);
>       break;
>     }
>+#if LJ_HASJIT
>     case (~LJ_TTRACE): {
>       lj_wbuf_addbyte(out, SYMTAB_TRACE);
>       lj_symtab_dump_trace(out, gco2trace(o));
>       break;
>     }
>+#endif /* LJ_HASJIT */
>     default:
>       break;
>     }
>diff --git a/src/lj_symtab.h b/src/lj_symtab.h
>index 6faa5cb1..6ec0cd7c 100644
>--- a/src/lj_symtab.h
>+++ b/src/lj_symtab.h
>@@ -60,10 +60,12 @@
> #define SYMTAB_TRACE ((uint8_t)2)
> #define SYMTAB_FINAL ((uint8_t)0x80)
> 
>+#if LJ_HASJIT
> /*
> ** Dumps traceinfo into the symbol table.
> */
> void lj_symtab_dump_trace(struct lj_wbuf *out, const GCtrace *trace);
>+#endif /* LJ_HASJIT */
> 
> /*
> ** Dumps function prototype.
>--
>2.30.2
 

[-- Attachment #2: Type: text/html, Size: 4053 bytes --]

Re: [Tarantool-patches] [PATCH luajit 3/7] test: stop using utils.selfrun in tests

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the patch!
Generally LGTM, but please consider my entreaties about comments below.

On 13.02.23, Igor Munkin wrote:
> Unfortunately, <utils.selfrun> is too complex to be maintained, so the
> corresponding tests are split into two files: the test itself and the
> script to be run by the test. As a result of the patch <utils.makecmd>
> helper is introduced: it inherits some approaches from <utils.selfrun>,
> but it's considered for more general use.
> 
> Relates to tarantool/tarantool#8252
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

<snipped>

> diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> index dd02130c..f4795db0 100644
> --- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> @@ -3,52 +3,43 @@ local utils = require('utils')

<snipped>

> --- Save the current coroutine and set the value to trigger
> --- <increment> call the Lua routine instead of C implementation.
> -local sandwich = require('libsandwich')(cfg.trigger)
> +local script = utils.makecmd(arg, {

It will be nice to drop a comment here and below, that `makecmd()`
searches for <%testname%/script.lua>.

> +  -- TODO: Leave another toxic comment regarding SIP on macOS.

Minor: This TODO is unnecessary, so feel free to delete this line.

> +  env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
> +  redirect = '2>&1',
> +})
>  
>  -- Depending on trigger and hotloop values the following contexts
>  -- are possible:
>  -- * if trigger <= hotloop -> trace recording is aborted
>  -- * if trigger >  hotloop -> trace is recorded but execution
>  --   leads to panic
> -jit.opt.start("3", string.format("hotloop=%d", cfg.hotloop))
> +local hotloop = 1
> +local cases = {
> +  abort = {
> +    trigger = hotloop,
> +    expected = '#4427 still works',

Side note: We may provide this message from here to avoid it copy-pasting in
<gh-4427-ffi-sandwich/script.lua>, but this reduces the test
readability... At least, we need to call format twice in the test's
payload, so for better good is not to do it.

> +    test = 'is',
> +    message = 'Trace is aborted',
> +  },
> +  panic = {
> +    trigger = hotloop + 1,
> +    expected = 'Lua VM re%-entrancy is detected while executing the trace',
> +    test = 'like',
> +    message = 'Trace is compiled',
> +  },
> +}

<snipped>

> diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua b/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua
> new file mode 100644
> index 00000000..9ecd964e
> --- /dev/null
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-351-print-tostring-number.test.lua b/test/tarantool-tests/lj-351-print-tostring-number.test.lua
> index da5b31be..72a9ec2b 100644
> --- a/test/tarantool-tests/lj-351-print-tostring-number.test.lua
> +++ b/test/tarantool-tests/lj-351-print-tostring-number.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-351-print-tostring-number/script.lua b/test/tarantool-tests/lj-351-print-tostring-number/script.lua
> new file mode 100644
> index 00000000..c3066f49

<snipped>

> diff --git a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
> index f02353fe..dcb730a2 100644
> --- a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
> +++ b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
> index c46b93f0..cf92757c 100644
> --- a/test/tarantool-tests/lj-flush-on-trace.test.lua
> +++ b/test/tarantool-tests/lj-flush-on-trace.test.lua

<snipped>

> --- 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)
> +local script = utils.makecmd(arg, {
> +  -- TODO: Leave another toxic comment regarding SIP on macOS.

Minor: This TODO is unnecessary, so feel free to delete this line.

> +  env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
> +  redirect = '2>&1',
> +})

<snipped>

> diff --git a/test/tarantool-tests/lj-flush-on-trace/script.lua b/test/tarantool-tests/lj-flush-on-trace/script.lua
> new file mode 100644
> index 00000000..d2c35534
> --- /dev/null
> +++ b/test/tarantool-tests/lj-flush-on-trace/script.lua

<snipped>

> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index eb11d40d..41a7c22a 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua

<snipped>

> +function M.makecmd(arg, opts)

Please, add the comment about script location, it's good thing to
commment, IMHO.

> +  return setmetatable({
> +    LUABIN = M.luacmd(arg),
> +    SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$', '/script.lua'),
> +    ENV = opts and makeenv(opts.env) or '',

Is this option is used only for DYLD_LIBRARY_PATH (as I can see for now)?
Should we drop the comment about that here (that this is the main
reason)?

> +    REDIRECT = opts and opts.redirect or '',
> +  }, {
> +    __call = function(self, ...)
> +      local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
> +                  .. (' %s'):rep(select('#', ...)):format(...)

It's good to comment that we just format arguments to strings for the
script invocaiton.

> +      return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
> +    end
> +  })
>  end
>  
>  function M.skipcond(condition, message)
> -- 
> 2.30.2
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 4/7] test: make skipcond helper more convenient

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the patch!
`skipcond` improvement is delightful, I realy like it!

Please, consider my comments below.

On 13.02.23, Igor Munkin wrote:
> This patch provides two enhancements for <utils.skipcond>:
> 1. As a result of the patch, <utils.skipcond> becomes multi-conditional,
>    so there is no need to concatenate one huge and complex condition
>    with one huge unreadable reason. Now skipcond receives the table with
>    conditions and skips the test if one of the given conditions is met.
> 2. <utils.skipcond> yields the test object, that allows to chain
>    skipcond call right next to the test constructor.
> 
> Finally as a result of the aforementioned changes <utils.skipcond> is
> moved to tap.lua.
> 
> Relates to tarantool/tarantool#8252
> 
> Co-authored-by: Sergey Kaplun <skaplun@tarantool.org>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

<snipped>

>  16 files changed, 104 insertions(+), 125 deletions(-)
> 
> diff --git a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
> index 4d69250f..65f9faac 100644
> --- a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
> +++ b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua

<snipped>

> diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> index f4795db0..2d6c3b06 100644
> --- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua

<snipped>

> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> index e0b6d86d..019fed29 100644
> --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua

<snipped>

>  
> -jit.off()
> -jit.flush()
> +-- XXX: Run JIT tuning functions in a safe frame to avoid errors
> +-- thrown when LuaJIT is compiled with JIT engine disabled.
> +pcall(jit.off)
> +pcall(jit.flush)

Should this be the part of the next patch?

>  
>  local bufread = require "utils.bufread"
>  local symtab = require "utils.symtab"
> @@ -20,7 +17,7 @@ local testboth = require "resboth"

<snipped>

> diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> index 15bd0a8b..f139b6b5 100644
> --- a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> +++ b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
> index 8bc523c2..633ab676 100644
> --- a/test/tarantool-tests/lj-430-maxirconst.test.lua
> +++ b/test/tarantool-tests/lj-430-maxirconst.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> index b5353e85..c67da00e 100644
> --- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> +++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> @@ -1,9 +1,15 @@
>  local tap = require('tap')
> -
>  -- Test to demonstrate the incorrect JIT behaviour when an error
>  -- is raised on restoration from the snapshot.
>  -- See also https://github.com/LuaJIT/LuaJIT/issues/603.
> -local test = tap.test('lj-603-err-snap-restore.test.lua')
> +local test = tap.test('lj-603-err-snap-restore'):skipcond({
> +  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',

Side note: discussed offline that we can disable the whole test file,
even that the first test check the behaviour for the VM too.

> +  -- XXX: The different amount of stack slots is in-use for
> +  -- Tarantool at start, so just skip test for it.
> +  -- luacheck: no global
> +  ['Disable test for Tarantool'] = _TARANTOOL,
> +})
> +
>  test:plan(2)
>  
>  -- XXX: This is fragile. We need a specific amount of Lua stack
> @@ -15,7 +21,7 @@ test:plan(2)
>  -- error handling"), etc.).
>  -- This amount is suited well for GC64 and non-GC64 mode.
>  -- luacheck: no unused
> -local _, _, _, _, _, _
> +local _, _, _, _, _, _, _, _, _, _, _, _, _, _, _

Side note: Is necessary due to different allocations and memory mapping,
so we can hit `ERRERR` error during raising the `STKOV` error and
handler will be called.

>  
>  local handler_is_called = false
>  local recursive_f
> @@ -38,11 +44,7 @@ end

<snipped>

> diff --git a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
> index 1dc741d8..2165afe3 100644
> --- a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
> +++ b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-906-fix-err-mem.test.lua b/test/tarantool-tests/lj-906-fix-err-mem.test.lua
> index 450beeff..6c6df338 100644
> --- a/test/tarantool-tests/lj-906-fix-err-mem.test.lua
> +++ b/test/tarantool-tests/lj-906-fix-err-mem.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
> index cf92757c..60a0ccbd 100644
> --- a/test/tarantool-tests/lj-flush-on-trace.test.lua
> +++ b/test/tarantool-tests/lj-flush-on-trace.test.lua

<snipped>

> diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> index 42a9adf9..a41a1c7a 100644
> --- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> +++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> @@ -1,12 +1,15 @@

<snipped>

> +local maxnins = require('utils').const.maxnins

Minor: since this is the constant for the test should we name it in the
upper-case?

>  local jit_opt_default = {

<snipped>

> -    jit.opt.start(0, "hotloop=1", "hotexit=2",
> -                  ("minstitch=%d"):format(utils.const.maxnins))
> +    jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
>  
>      local function foo(i)
>          -- math.fmod is not yet compiled!
> diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> index 0c170d0c..e5ee7902 100644
> --- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua

<snipped>

>  
> +local maxnins = require('utils').const.maxnins

Ditto.

>  local jit_opt_default = {
>      3, -- level
>      "hotloop=56",

<snipped>

> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> index bae0c27c..accb1ee1 100644
> --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua

<snipped>

> @@ -28,8 +28,8 @@ local memprof = require "memprof.parse"
>  local process = require "memprof.process"
>  local symtab = require "utils.symtab"
>  
> -local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
> -local BAD_PATH = utils.profilename("memprofdata/tmp.bin")
> +local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")

Don't get the point.
Why do not require this module once, so there is no need to change these
lines?

> +local BAD_PATH = require("utils").profilename("memprofdata/tmp.bin")
>  local SRC_PATH = "@"..arg[0]
>  
>  local function default_payload()
> diff --git a/test/tarantool-tests/misclib-sysprof-capi.test.lua b/test/tarantool-tests/misclib-sysprof-capi.test.lua
> index dad0fe4a..2b0e25ae 100644
> --- a/test/tarantool-tests/misclib-sysprof-capi.test.lua
> +++ b/test/tarantool-tests/misclib-sysprof-capi.test.lua

<snipped>

> diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> index 4bf10e8d..8dc36592 100644
> --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua

<snipped>

> @@ -18,8 +13,8 @@ local bufread = require("utils.bufread")
>  local symtab = require("utils.symtab")
>  local sysprof = require("sysprof.parse")
>  
> -local TMP_BINFILE = utils.profilename("sysprofdata.tmp.bin")
> -local BAD_PATH = utils.profilename("sysprofdata/tmp.bin")
> +local TMP_BINFILE = require("utils").profilename("sysprofdata.tmp.bin")
> +local BAD_PATH = require("utils").profilename("sysprofdata/tmp.bin")

Ditto.

>  
>  local function payload()
>    local function fib(n)
> diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
> index a1f54d20..ac04c01d 100644
> --- a/test/tarantool-tests/tap.lua
> +++ b/test/tarantool-tests/tap.lua
> @@ -304,6 +304,17 @@ local function check(test)
>    return test.planned == test.total and test.failed == 0
>  end
>  
> +local function skipcond(test, conditions)
> +  for message, condition in pairs(conditions) do
> +    if condition then
> +      test:plan(1)
> +      test:skip(message)
> +      os.exit(test:check() and 0 or 1)

Does this mean that the subtest should be the last test to run in the
file? It's kinda not obvious. Also, I suggest the following approach:
Keep that condition search result, and skip every test (and subtest) if
the condition met. We can still skip full test, like you done, if it has
no parent test to run (has no test.parent).

> +    end
> +  end
> +  return test
> +end
> +
>  test_mt = {
>    __index = {
>      test       = new,
> @@ -313,6 +324,7 @@ test_mt = {
>      ok         = ok,
>      fail       = fail,
>      skip       = skip,
> +    skipcond   = skipcond,
>      is         = is,
>      isnt       = isnt,
>      isnil      = isnil,
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index 41a7c22a..4fb66600 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua

<snipped>

> -- 
> 2.30.2
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 7/7] ci: add nojit flavor for exotic builds

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]


Hi, Igor!
Thanks for the patch!
LGTM
--
Best regards,
Maxim Kokryashkin
 
 
> 
>>Since all testing machinery is adjusted for LuaJIT configuration with
>>disabled compiler support, the new flavor for exotic builds is
>>introduced in LuaJIT CI.
>>
>>Part of tarantool/tarantool#8252
>>
>>Signed-off-by: Igor Munkin < imun@tarantool.org >
>>---
>> .github/workflows/exotic-builds-testing.yml | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>>diff --git a/.github/workflows/exotic-builds-testing.yml b/.github/workflows/exotic-builds-testing.yml
>>index 67ab9cc3..b28673c6 100644
>>--- a/.github/workflows/exotic-builds-testing.yml
>>+++ b/.github/workflows/exotic-builds-testing.yml
>>@@ -36,7 +36,7 @@ jobs:
>>         BUILDTYPE: [Debug, Release]
>>         ARCH: [ARM64, x86_64]
>>         GC64: [ON, OFF]
>>- FLAVOR: [dualnum, checkhook]
>>+ FLAVOR: [dualnum, checkhook, nojit]
>>         include:
>>           - BUILDTYPE: Debug
>>             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>>@@ -46,6 +46,8 @@ jobs:
>>             FLAVORFLAGS: -DLUAJIT_NUMMODE=2
>>           - FLAVOR: checkhook
>>             FLAVORFLAGS: -DLUAJIT_ENABLE_CHECKHOOK=ON
>>+ - FLAVOR: nojit
>>+ FLAVORFLAGS: -DLUAJIT_DISABLE_JIT=ON
>>         exclude:
>>           # DUALNUM is default for ARM64, no need for additional testing.
>>           - FLAVOR: dualnum
>>--
>>2.30.2
> 

[-- Attachment #2: Type: text/html, Size: 2678 bytes --]

Re: [Tarantool-patches] [PATCH luajit 3/7] test: stop using utils.selfrun in tests

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 4996 bytes --]


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

[-- Attachment #2: Type: text/html, Size: 6640 bytes --]

Re: [Tarantool-patches] [PATCH luajit 4/7] test: make skipcond helper more convenient

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 20578 bytes --]


Hi, Igor!
Thanks for the patch!
Please consider my comments below.
 
> 
>>This patch provides two enhancements for <utils.skipcond>:
>>1. As a result of the patch, <utils.skipcond> becomes multi-conditional,
>>   so there is no need to concatenate one huge and complex condition
>>   with one huge unreadable reason. Now skipcond receives the table with
>>   conditions and skips the test if one of the given conditions is met.
>>2. <utils.skipcond> yields the test object, that allows to chain
>>   skipcond call right next to the test constructor.
>>
>>Finally as a result of the aforementioned changes <utils.skipcond> is
>>moved to tap.lua.
>>
>>Relates to tarantool/tarantool#8252
>>
>>Co-authored-by: Sergey Kaplun < skaplun@tarantool.org >
>>Signed-off-by: Igor Munkin < imun@tarantool.org >
>>---
>> .../gh-4199-gc64-fuse.test.lua | 11 +++++----
>> .../gh-4427-ffi-sandwich.test.lua | 11 ++++-----
>> .../gh-5813-resolving-of-c-symbols.test.lua | 23 ++++++++-----------
>> ...4-add-proto-trace-sysprof-default.test.lua | 14 ++++-------
>> .../lj-430-maxirconst.test.lua | 10 ++++----
>> .../lj-603-err-snap-restore.test.lua | 18 ++++++++-------
>> ...lj-672-cdata-allocation-recording.test.lua | 12 +++++-----
>> .../lj-906-fix-err-mem.test.lua | 12 +++++-----
>> .../lj-flush-on-trace.test.lua | 11 ++++-----
>> .../misclib-getmetrics-capi.test.lua | 17 ++++++--------
>> .../misclib-getmetrics-lapi.test.lua | 13 ++++-------
>> .../misclib-memprof-lapi.test.lua | 22 +++++++++---------
>> .../misclib-sysprof-capi.test.lua | 18 ++++++---------
>> .../misclib-sysprof-lapi.test.lua | 17 +++++---------
>> test/tarantool-tests/tap.lua | 12 ++++++++++
>> test/tarantool-tests/utils.lua | 8 -------
>> 16 files changed, 104 insertions(+), 125 deletions(-)
>>
>>diff --git a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
>>index 4d69250f..65f9faac 100644
>>--- a/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
>>+++ b/test/tarantool-tests/gh-4199-gc64-fuse.test.lua
>>@@ -1,11 +1,12 @@
>>--- The test is GC64 only.
>>-local ffi = require('ffi')
>>-require('utils').skipcond(not ffi.abi('gc64'), 'test is GC64 only')
>>-
>> local tap = require('tap')
>>-local test = tap.test('gh-4199-gc64-fuse')
>>+local test = tap.test('gh-4199-gc64-fuse'):skipcond({
>>+ ['Test requires GC64 mode enabled'] = not require('ffi').abi('gc64'),
>>+})
>>+
>> test:plan(1)
>> 
>>+local ffi = require('ffi')
>I don’t think that you needed to move that expression here, since
>now you require the FFI module twice.
>>+
>> collectgarbage()
>> -- Chomp memory in currently allocated GC space.
>> collectgarbage('stop')
>>diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>>index f4795db0..2d6c3b06 100644
>>--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>>+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>>@@ -1,14 +1,11 @@
>>-local utils = require('utils')
>>-
>>--- Disabled on *BSD due to #4819.
>>-utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
>>-
>> local tap = require('tap')
>>+local test = tap.test('gh-4427-ffi-sandwich'):skipcond({
>>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>+})
>> 
>>-local test = tap.test('gh-4427-ffi-sandwich')
>> test:plan(2)
>> 
>>-local script = utils.makecmd(arg, {
>>+local script = require('utils').makecmd(arg, {
>>   -- TODO: Leave another toxic comment regarding SIP on macOS.
>>   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
>>   redirect = '2>&1',
>>diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
>>index e0b6d86d..019fed29 100644
>>--- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
>>+++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
>>@@ -1,18 +1,15 @@
>>--- Memprof is implemented for x86 and x64 architectures only.
>>-local utils = require("utils")
>>-
>>-utils.skipcond(
>>- jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
>>- jit.arch.." architecture or "..jit.os..
>>- " OS is NIY for memprof c symbols resolving"
>>-)
>>-
>> local tap = require("tap")
>>-local test = tap.test("gh-5813-resolving-of-c-symbols")
>>+local test = tap.test("gh-5813-resolving-of-c-symbols"):skipcond({
>>+ ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and jit.arch ~= "x64",
>>+ ["Memprof is implemented for Linux only"] = jit.os ~= "Linux",
>>+})
>>+
>> test:plan(5)
>> 
>>-jit.off()
>>-jit.flush()
>>+-- XXX: Run JIT tuning functions in a safe frame to avoid errors
>>+-- thrown when LuaJIT is compiled with JIT engine disabled.
>>+pcall(jit.off)
>>+pcall(jit.flush)
>I think that change should be in commit «build: fix build with JIT disabled»
>(a835fb0f778db6e9f0109a66ea1d2ac78fe682e4)
>> 
>> local bufread = require "utils.bufread"
>> local symtab = require "utils.symtab"
>>@@ -20,7 +17,7 @@ local testboth = require "resboth"
>> local testhash = require "reshash"
>> local testgnuhash = require "resgnuhash"
>> 
>>-local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
>>+local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")
>> 
>> local function tree_contains(node, name)
>>   if node == nil then
>>diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
>>index 15bd0a8b..f139b6b5 100644
>>--- a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
>>+++ b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
>>@@ -1,13 +1,9 @@
>>--- Sysprof is implemented for x86 and x64 architectures only.
>>-require('utils').skipcond(
>>- jit.arch ~= 'x86' and jit.arch ~= 'x64' or jit.os ~= 'Linux'
>>- or require('ffi').abi('gc64'),
>>- jit.arch..' architecture or '..jit.os..
>>- ' OS is NIY for sysprof'
>>-)
>>-
>> local tap = require('tap')
>>-local test = tap.test('gh-7264-add-proto-trace-sysprof-default.test.lua')
>>+local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
>>+ ['Sysprof is implemented for x86_64 only'] = jit.arch ~= 'x86' and jit.arch ~= 'x64',
>>+ ['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux',
>>+})
>>+
>> test:plan(2)
>> 
>> local chunk = [[
>>diff --git a/test/tarantool-tests/lj-430-maxirconst.test.lua b/test/tarantool-tests/lj-430-maxirconst.test.lua
>>index 8bc523c2..633ab676 100644
>>--- a/test/tarantool-tests/lj-430-maxirconst.test.lua
>>+++ b/test/tarantool-tests/lj-430-maxirconst.test.lua
>>@@ -1,12 +1,12 @@
>>--- Disabled on *BSD due to #4819.
>>-require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
>>-
>> local tap = require('tap')
>>-local traceinfo = require('jit.util').traceinfo
>>+local test = tap.test('lj-430-maxirconst'):skipcond({
>>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>+})
>> 
>>-local test = tap.test('lj-430-maxirconst')
>> test:plan(2)
>> 
>>+local traceinfo = require('jit.util').traceinfo
>>+
>> -- This function has only 3 IR constant.
>> local function irconst3()
>> end
>>diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
>>index b5353e85..c67da00e 100644
>>--- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
>>+++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
>>@@ -1,9 +1,15 @@
>> local tap = require('tap')
>>-
>> -- Test to demonstrate the incorrect JIT behaviour when an error
>> -- is raised on restoration from the snapshot.
>> -- See also  https://github.com/LuaJIT/LuaJIT/issues/603 .
>>-local test = tap.test('lj-603-err-snap-restore.test.lua')
>>+local test = tap.test('lj-603-err-snap-restore'):skipcond({
>>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>+ -- XXX: The different amount of stack slots is in-use for
>>+ -- Tarantool at start, so just skip test for it.
>>+ -- luacheck: no global
>>+ ['Disable test for Tarantool'] = _TARANTOOL,
>>+})
>>+
>> test:plan(2)
>> 
>> -- XXX: This is fragile. We need a specific amount of Lua stack
>>@@ -15,7 +21,7 @@ test:plan(2)
>> -- error handling"), etc.).
>> -- This amount is suited well for GC64 and non-GC64 mode.
>> -- luacheck: no unused
>>-local _, _, _, _, _, _
>>+local _, _, _, _, _, _, _, _, _, _, _, _, _, _, _
>Side note: I’ve already mentioned it several times, but I’ll say this again:
>that test is insanely fragile, and something needs to stop this nonsense.
>> 
>> local handler_is_called = false
>> local recursive_f
>>@@ -38,11 +44,7 @@ end
>> recursive_f()
>> 
>> test:ok(true)
>>--- Disabled on *BSD due to #4819.
>>--- XXX: The different amount of stack slots is in-use for
>>--- Tarantool at start, so just skip test for it.
>>--- luacheck: no global
>>-test:ok(jit.os == 'BSD' or _TARANTOOL or not handler_is_called)
>>+test:ok(not handler_is_called)
>> 
>> -- XXX: Don't use `os.exit()` here by intention. When error on
>> -- snap restoration is raised, `err_unwind()` doesn't stop on
>>diff --git a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
>>index 1dc741d8..2165afe3 100644
>>--- a/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
>>+++ b/test/tarantool-tests/lj-672-cdata-allocation-recording.test.lua
>>@@ -1,13 +1,13 @@
>>--- Disabled on *BSD due to #4819.
>>-require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
>>+local tap = require('tap')
>>+local test = tap.test('lj-672-cdata-allocation-recording'):skipcond({
>>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>+})
>>+
>>+test:plan(1)
>> 
>> local ffi = require('ffi')
>> local traceinfo = require('jit.util').traceinfo
>> 
>>-local tap = require('tap')
>>-local test = tap.test('lj-672-cdata-allocation-recording')
>>-test:plan(1)
>>-
>> -- Structure with array.
>> ffi.cdef('struct my_struct {int a; char d[8];}')
>> 
>>diff --git a/test/tarantool-tests/lj-906-fix-err-mem.test.lua b/test/tarantool-tests/lj-906-fix-err-mem.test.lua
>>index 450beeff..6c6df338 100644
>>--- a/test/tarantool-tests/lj-906-fix-err-mem.test.lua
>>+++ b/test/tarantool-tests/lj-906-fix-err-mem.test.lua
>>@@ -1,13 +1,13 @@
>> local tap = require('tap')
>>-local ffi = require('ffi')
>>-local table_new = require('table.new')
>>-
>>--- Avoid test to be killed.
>>-require('utils').skipcond(ffi.abi('gc64'), 'test is not GC64 only')
>>+local test = tap.test('lj-906-fix-err-mem'):skipcond({
>>+ ['Test requires GC64 mode disabled'] = require('ffi').abi('gc64'),
>>+})
>> 
>>-local test = tap.test('lj-906-fix-err-mem')
>> test:plan(1)
>> 
>>+local ffi = require('ffi')
>>+local table_new = require('table.new')
>>+
>> local KB = 1024
>> local MB = 1024 * KB
>> local sizes = {8 * MB, 8 * KB, 8}
>>diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
>>index cf92757c..60a0ccbd 100644
>>--- a/test/tarantool-tests/lj-flush-on-trace.test.lua
>>+++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
>>@@ -1,14 +1,11 @@
>>-local utils = require('utils')
>>-
>>--- Disabled on *BSD due to #4819.
>>-utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
>>-
>> local tap = require('tap')
>>+local test = tap.test('lj-flush-on-trace'):skipcond({
>>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>+})
>> 
>>-local test = tap.test('lj-flush-on-trace')
>> test:plan(2)
>> 
>>-local script = utils.makecmd(arg, {
>>+local script = require('utils').makecmd(arg, {
>>   -- TODO: Leave another toxic comment regarding SIP on macOS.
>>   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
>>   redirect = '2>&1',
>>diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>index 42a9adf9..a41a1c7a 100644
>>--- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>+++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>@@ -1,12 +1,15 @@
>>-local utils = require('utils')
>>+local tap = require('tap')
>>+local test = tap.test("clib-misc-getmetrics"):skipcond({
>>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>+})
>> 
>>--- Disabled on *BSD due to #4819.
>>-utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
>>+test:plan(11)
>> 
>> local path = arg[0]:gsub('%.test%.lua', '')
>> local suffix = package.cpath:match('?.(%a+);')
>> package.cpath = ('%s/?.%s;'):format(path, suffix)..package.cpath
>> 
>>+local maxnins = require('utils').const.maxnins
>> local jit_opt_default = {
>>     3, -- level
>>     "hotloop=56",
>>@@ -14,11 +17,6 @@ local jit_opt_default = {
>>     "minstitch=0",
>> }
>> 
>>-local tap = require('tap')
>>-
>>-local test = tap.test("clib-misc-getmetrics")
>>-test:plan(11)
>>-
>> local testgetmetrics = require("testgetmetrics")
>> 
>> test:ok(testgetmetrics.base())
>>@@ -96,8 +94,7 @@ end))
>> 
>> -- Compiled loop with a side exit which does not get compiled.
>> test:ok(testgetmetrics.snap_restores(function()
>>- jit.opt.start(0, "hotloop=1", "hotexit=2",
>>- ("minstitch=%d"):format(utils.const.maxnins))
>>+ jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
>> 
>>     local function foo(i)
>>         -- math.fmod is not yet compiled!
>>diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>index 0c170d0c..e5ee7902 100644
>>--- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>+++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>@@ -2,16 +2,14 @@
>> -- Major portions taken verbatim or adapted from the LuaVela testing suite.
>> -- Copyright (C) 2015-2019 IPONWEB Ltd.
>> 
>>-local utils = require('utils')
>>-
>>--- Disabled on *BSD due to #4819.
>>-utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
>>-
>> local tap = require('tap')
>>+local test = tap.test("lib-misc-getmetrics"):skipcond({
>>+ ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>+})
>> 
>>-local test = tap.test("lib-misc-getmetrics")
>> test:plan(10)
>> 
>>+local maxnins = require('utils').const.maxnins
>> local jit_opt_default = {
>>     3, -- level
>>     "hotloop=56",
>>@@ -281,8 +279,7 @@ test:test("snap-restores-loop-side-exit-non-compiled", function(subtest)
>>     -- Compiled loop with a side exit which does not get compiled.
>>     subtest:plan(1)
>> 
>>- jit.opt.start(0, "hotloop=1", "hotexit=2",
>>- ("minstitch=%d"):format(utils.const.maxnins))
>>+ jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
>> 
>>     local function foo(i)
>>         -- math.fmod is not yet compiled!
>>diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>index bae0c27c..accb1ee1 100644
>>--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>@@ -1,14 +1,14 @@
>>--- Memprof is implemented for x86 and x64 architectures only.
>>-local utils = require("utils")
>>-
>>-utils.skipcond(
>>- jit.arch ~= "x86" and jit.arch ~= "x64",
>>- jit.arch.." architecture is NIY for memprof"
>>-)
>>-
>>+-- XXX: This comment is a reminder to reimplement memprof tests
>>+-- assertions to make them more indepentent to the changes made.
>>+-- Now I just leave this 5 lines comment to preserve line numbers.
>>+-- TODO: This line will be removed in the next patch.
>>+-- TODO: This line will be removed in the next patch.
>> local tap = require("tap")
>>+local test = tap.test("misc-memprof-lapi"):skipcond({
>>+ ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
>>+ jit.arch ~= "x64",
>>+})
>> 
>>-local test = tap.test("misc-memprof-lapi")
>> test:plan(5)
>> 
>> local jit_opt_default = {
>>@@ -28,8 +28,8 @@ local memprof = require "memprof.parse"
>> local process = require "memprof.process"
>> local symtab = require "utils.symtab"
>> 
>>-local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
>>-local BAD_PATH = utils.profilename("memprofdata/tmp.bin")
>>+local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")
>>+local BAD_PATH = require("utils").profilename("memprofdata/tmp.bin")
>IMO, it’s better to require the «utils» module only once.
>> local SRC_PATH = "@"..arg[0]
>> 
>> local function default_payload()
>>diff --git a/test/tarantool-tests/misclib-sysprof-capi.test.lua b/test/tarantool-tests/misclib-sysprof-capi.test.lua
>>index dad0fe4a..2b0e25ae 100644
>>--- a/test/tarantool-tests/misclib-sysprof-capi.test.lua
>>+++ b/test/tarantool-tests/misclib-sysprof-capi.test.lua
>>@@ -1,21 +1,17 @@
>>--- Sysprof is implemented for x86 and x64 architectures only.
>>-local utils = require("utils")
>>-utils.skipcond(
>>- jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
>>- jit.arch.." architecture or "..jit.os..
>>- " OS is NIY for sysprof"
>>-)
>>+local tap = require("tap")
>>+local test = tap.test("clib-misc-sysprof"):skipcond({
>>+ ["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and jit.arch ~= "x64",
>>+ ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
>>+})
>>+
>>+test:plan(2)
>> 
>> local testsysprof = require("testsysprof")
>> 
>>-local tap = require("tap")
>> local jit = require('jit')
>> 
>> jit.off()
>> 
>>-local test = tap.test("clib-misc-sysprof")
>>-test:plan(2)
>>-
>> test:ok(testsysprof.base())
>> test:ok(testsysprof.validation())
>> 
>>diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>index 4bf10e8d..8dc36592 100644
>>--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>@@ -1,14 +1,9 @@
>>--- Sysprof is implemented for x86 and x64 architectures only.
>>-local utils = require("utils")
>>-utils.skipcond(
>>- jit.arch ~= "x86" and jit.arch ~= "x64" or jit.os ~= "Linux",
>>- jit.arch.." architecture or "..jit.os..
>>- " OS is NIY for sysprof"
>>-)
>>-
>> local tap = require("tap")
>>+local test = tap.test("misc-sysprof-lapi"):skipcond({
>>+ ["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and jit.arch ~= "x64",
>>+ ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
>>+})
>> 
>>-local test = tap.test("misc-sysprof-lapi")
>> test:plan(19)
>> 
>> jit.off()
>>@@ -18,8 +13,8 @@ local bufread = require("utils.bufread")
>> local symtab = require("utils.symtab")
>> local sysprof = require("sysprof.parse")
>> 
>>-local TMP_BINFILE = utils.profilename("sysprofdata.tmp.bin")
>>-local BAD_PATH = utils.profilename("sysprofdata/tmp.bin")
>>+local TMP_BINFILE = require("utils").profilename("sysprofdata.tmp.bin")
>>+local BAD_PATH = require("utils").profilename("sysprofdata/tmp.bin")
>Same as for memprof.
>> 
>> local function payload()
>>   local function fib(n)
>>diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
>>index a1f54d20..ac04c01d 100644
>>--- a/test/tarantool-tests/tap.lua
>>+++ b/test/tarantool-tests/tap.lua
>>@@ -304,6 +304,17 @@ local function check(test)
>>   return test.planned == test.total and test.failed == 0
>> end
>> 
>>+local function skipcond(test, conditions)
>>+ for message, condition in pairs(conditions) do
>>+ if condition then
>>+ test:plan(1)
>>+ test:skip(message)
>>+ os.exit(test:check() and 0 or 1)
>>+ end
>>+ end
>>+ return test
>>+end
>>+
>> test_mt = {
>>   __index = {
>>     test = new,
>>@@ -313,6 +324,7 @@ test_mt = {
>>     ok = ok,
>>     fail = fail,
>>     skip = skip,
>>+ skipcond = skipcond,
>>     is = is,
>>     isnt = isnt,
>>     isnil = isnil,
>>diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
>>index 41a7c22a..4fb66600 100644
>>--- a/test/tarantool-tests/utils.lua
>>+++ b/test/tarantool-tests/utils.lua
>>@@ -67,14 +67,6 @@ function M.makecmd(arg, opts)
>>   })
>> end
>> 
>>-function M.skipcond(condition, message)
>>- if not condition then return end
>>- local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))
>>- test:plan(1)
>>- test:skip(message)
>>- os.exit(test:check() and 0 or 1)
>>-end
>>-
>> function M.hasbc(f, bytecode)
>>   assert(type(f) == 'function', 'argument #1 should be a function')
>>   assert(type(bytecode) == 'string', 'argument #2 should be a string')
>>--
>>2.30.2
>--
>Best regards,
>Maxim Kokryashkin
> 

[-- Attachment #2: Type: text/html, Size: 23859 bytes --]

Re: [Tarantool-patches] [PATCH luajit 5/7] test: add skipcond for all JIT-related tests

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]


Hi, Igor!
Thanks for the patch!
Please consider my comments below.
 
`lj-906-fix-err-mem.test.lua` has a `jit.off()`
call in the line 78, which should be wrapped in a pcall.
 
This change should be moved to commit
«build: fix build with JIT disabled»
(a835fb0f778db6e9f0109a66ea1d2ac78fe682e4)
along with the change that I pointed out below. 
> 
>diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>index 8dc36592..dcde81b9 100644
>--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>@@ -6,8 +6,10 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
> 
> test:plan(19)
> 
>-jit.off()
>-jit.flush()
>+-- XXX: Run JIT tuning functions in a safe frame to avoid errors
>+-- thrown when LuaJIT is compiled with JIT engine disabled.
>+pcall(jit.off)
>+pcall(jit.flush)
Same as for the change in `fix-err-mem`. 
> 
> local bufread = require("utils.bufread")
> local symtab = require("utils.symtab")
>@@ -125,5 +127,4 @@ check_mode("C", 100)
> 
> os.remove(TMP_BINFILE)
> 
>-jit.on()
> os.exit(test:check() and 0 or 1)
>--
>2.30.2
--
Best regards,
Maxim Kokryashkin
 

[-- Attachment #2: Type: text/html, Size: 1700 bytes --]

Re: [Tarantool-patches] [PATCH luajit 6/7] test: fix lua-Harness JIT-related tests

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 2276 bytes --]


Hi, Igor!
Thanks for the patch!
Except for the comments mentioned by Sergey, I have a few
other regarding the commit message.
 
> 
>>lua-Harness consider whether JIT is enabled or not in scope of 403-jit.t
>Typo: s/consider/considers
>Typo: s/in scope of/in the scope of the/
>>and 411-luajit.t tests. However, the original condition is wrong, since
>><jit.status> yields false for both cases, when JIT is just turned off
>>and when LuaJIT is build without compiler support. So if <jit.status>
>Typo: s/build/built
>>yields false, the latter case is considered. The condition is fixed to
>>differ both aforementioned cases the following way: when <jit.status>
>>yields only compiler status with no flags listing, LuaJIT is built
>>without compiler; if there is more than one value returned, JIT support
>>is on aboard.
>Typo: s/aboard/board
>>Part of tarantool/tarantool#8252
>>
>>Signed-off-by: Igor Munkin < imun@tarantool.org >
>>---
>> test/lua-Harness-tests/403-jit.t | 2 +-
>> test/lua-Harness-tests/411-luajit.t | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>>diff --git a/test/lua-Harness-tests/403-jit.t b/test/lua-Harness-tests/403-jit.t
>>index 0f986da9..0aa5d367 100755
>>--- a/test/lua-Harness-tests/403-jit.t
>>+++ b/test/lua-Harness-tests/403-jit.t
>>@@ -31,7 +31,7 @@ if not jit then
>>     skip_all("only with LuaJIT")
>> end
>> 
>>-local compiled_with_jit = jit.status()
>>+local compiled_with_jit = select('#', jit.status()) > 1
>> local luajit20 = jit.version_num < 20100 and not jit.version:match'RaptorJIT'
>> local has_jit_opt = compiled_with_jit
>> local has_jit_security = jit.security
>>diff --git a/test/lua-Harness-tests/411-luajit.t b/test/lua-Harness-tests/411-luajit.t
>>index 3a9a7b8f..e3b6c7a8 100755
>>--- a/test/lua-Harness-tests/411-luajit.t
>>+++ b/test/lua-Harness-tests/411-luajit.t
>>@@ -37,7 +37,7 @@ if not pcall(io.popen, lua .. [[ -e "a=1"]]) then
>>     skip_all("io.popen not supported")
>> end
>> 
>>-local compiled_with_jit = jit.status()
>>+local compiled_with_jit = select('#', jit.status()) > 1
>> local has_jutil = pcall(require, 'jit.util')
>> local has_openresty_listing = profile.openresty or jit.version:match'moonjit'
>> 
>>--
>>2.30.2
>--
>Best regards,
>Maxim Kokryashkin
> 

[-- Attachment #2: Type: text/html, Size: 3484 bytes --]

Re: [Tarantool-patches] [PATCH luajit 2/7] build: fix build with JIT disabled

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for your review!

On 13.02.23, Sergey Kaplun wrote:
> Hi, Igor!
> Thanks for the patch!
> LGTM, except a single nit regarding the commit message.
> 
> On 13.02.23, Igor Munkin wrote:
> > struct GCtrace is defined only if LJ_HASJIT is set. Hence all spots
> 
> Typo: s/Hence,/Hence/

Fixed, force-pushed.

> 
> > where GCtrace is used should be also moved under LJ_HASJIT define.
> > 
> > Relates to tarantool/tarantool#8252
> 
> Side note: I see "Relates" and "Related" in our commit logs, so I
> suggest to use "Relates" for future commits.

I always use "Relates" (except maybe some early commits).

> 
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> 
> <snipped>
> 
> > -- 
> > 2.30.2
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 3/7] test: stop using utils.selfrun in tests

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for your review! See my answers below.

On 15.02.23, Sergey Kaplun wrote:
> Hi, Igor!
> Thanks for the patch!
> Generally LGTM, but please consider my entreaties about comments below.
> 
> On 13.02.23, Igor Munkin wrote:
> > Unfortunately, <utils.selfrun> is too complex to be maintained, so the
> > corresponding tests are split into two files: the test itself and the
> > script to be run by the test. As a result of the patch <utils.makecmd>
> > helper is introduced: it inherits some approaches from <utils.selfrun>,
> > but it's considered for more general use.
> > 
> > Relates to tarantool/tarantool#8252
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> 
> <snipped>
> 
> > diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> > index dd02130c..f4795db0 100644
> > --- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> > +++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> > @@ -3,52 +3,43 @@ local utils = require('utils')
> 
> <snipped>
> 
> > --- Save the current coroutine and set the value to trigger
> > --- <increment> call the Lua routine instead of C implementation.
> > -local sandwich = require('libsandwich')(cfg.trigger)
> > +local script = utils.makecmd(arg, {
> 
> It will be nice to drop a comment here and below, that `makecmd()`
> searches for <%testname%/script.lua>.
> 
> > +  -- TODO: Leave another toxic comment regarding SIP on macOS.
> 
> Minor: This TODO is unnecessary, so feel free to delete this line.
> 
> > +  env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
> > +  redirect = '2>&1',
> > +})

Fixed both cases above. You can find the diff below:

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

diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
index ff3eaf01..ad06c329 100644
--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
@@ -6,8 +6,23 @@ local test = tap.test('gh-4427-ffi-sandwich'):skipcond({
 
 test:plan(2)
 
+-- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
+-- with the given environment, launch options and CLI arguments.
 local script = require('utils').makecmd(arg, {
-  -- TODO: Leave another toxic comment regarding SIP on macOS.
+  -- XXX: Apple tries their best to "protect their users from
+  -- malware". As a result SIP (see the link[1] below) has been
+  -- designed and released. Now, Apple developers are so
+  -- protected, that they can load nothing being not installed in
+  -- the system, since the environment is sanitized before the
+  -- child process is launched. In particular, environment
+  -- variables starting with DYLD_ and LD_ are unset for child
+  -- process. For more info, see the docs[2] below.
+  --
+  -- The environment variable below is used by FFI machinery to
+  -- find the proper shared library.
+  --
+  -- [1]: https://support.apple.com/en-us/HT204899
+  -- [2]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
   redirect = '2>&1',
 })

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

> >  
> >  -- Depending on trigger and hotloop values the following contexts
> >  -- are possible:
> >  -- * if trigger <= hotloop -> trace recording is aborted
> >  -- * if trigger >  hotloop -> trace is recorded but execution
> >  --   leads to panic
> > -jit.opt.start("3", string.format("hotloop=%d", cfg.hotloop))
> > +local hotloop = 1
> > +local cases = {
> > +  abort = {
> > +    trigger = hotloop,
> > +    expected = '#4427 still works',
> 
> Side note: We may provide this message from here to avoid it copy-pasting in
> <gh-4427-ffi-sandwich/script.lua>, but this reduces the test
> readability... At least, we need to call format twice in the test's
> payload, so for better good is not to do it.

Yeah, there is no silver bullet for this, unfortunately. This is lesser
evil in our case, IMHO.

> 
> > +    test = 'is',
> > +    message = 'Trace is aborted',
> > +  },
> > +  panic = {
> > +    trigger = hotloop + 1,
> > +    expected = 'Lua VM re%-entrancy is detected while executing the trace',
> > +    test = 'like',
> > +    message = 'Trace is compiled',
> > +  },
> > +}

<snipped>

> > diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
> > index c46b93f0..cf92757c 100644
> > --- a/test/tarantool-tests/lj-flush-on-trace.test.lua
> > +++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
> 
> <snipped>
> 
> > --- 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)
> > +local script = utils.makecmd(arg, {
> > +  -- TODO: Leave another toxic comment regarding SIP on macOS.
> 
> Minor: This TODO is unnecessary, so feel free to delete this line.

Added the same comments as for the case above. The diff is below:

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

diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
index e157622a..a355c688 100644
--- a/test/tarantool-tests/lj-flush-on-trace.test.lua
+++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
@@ -6,8 +6,23 @@ local test = tap.test('lj-flush-on-trace'):skipcond({

 test:plan(2)

+-- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
+-- with the given environment, launch options and CLI arguments.
 local script = require('utils').makecmd(arg, {
-  -- TODO: Leave another toxic comment regarding SIP on macOS.
+  -- XXX: Apple tries their best to "protect their users from
+  -- malware". As a result SIP (see the link[1] below) has been
+  -- designed and released. Now, Apple developers are so
+  -- protected, that they can load nothing being not installed in
+  -- the system, since the environment is sanitized before the
+  -- child process is launched. In particular, environment
+  -- variables starting with DYLD_ and LD_ are unset for child
+  -- process. For more info, see the docs[2] below.
+  --
+  -- The environment variable below is used by FFI machinery to
+  -- find the proper shared library.
+  --
+  -- [1]: https://support.apple.com/en-us/HT204899
+  -- [2]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
   redirect = '2>&1',
 })

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

> 
> > +  env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
> > +  redirect = '2>&1',
> > +})

<snipped>

> > diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> > index eb11d40d..41a7c22a 100644
> > --- a/test/tarantool-tests/utils.lua
> > +++ b/test/tarantool-tests/utils.lua
> 
> <snipped>
> 
> > +function M.makecmd(arg, opts)
> 
> Please, add the comment about script location, it's good thing to
> commment, IMHO.

Added the comment to clear the usage. The diff is below:

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

diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index 4fb66600..6a9645ca 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -52,6 +52,11 @@ local function makeenv(tabenv)
   return table.concat(flatenv, ' ')
 end
 
+-- <makecmd> creates a command that runs %testname%/script.lua by
+-- <LUAJIT_TEST_BINARY> with the given environment, launch options
+-- and CLI arguments. The function yields an object (i.e. table)
+-- with the aforementioned parameters. To launch the command just
+-- call the object.
 function M.makecmd(arg, opts)
   return setmetatable({
     LUABIN = M.luacmd(arg),

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

> 
> > +  return setmetatable({
> > +    LUABIN = M.luacmd(arg),
> > +    SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$', '/script.lua'),
> > +    ENV = opts and makeenv(opts.env) or '',
> 
> Is this option is used only for DYLD_LIBRARY_PATH (as I can see for now)?
> Should we drop the comment about that here (that this is the main
> reason)?

Not really. This is just a general approach to tweak the child process
environment. And yes, it's used only for DYLD_LIBRARY_PATH for now, but
I want to create a general interface to run commands in tests.

> 
> > +    REDIRECT = opts and opts.redirect or '',
> > +  }, {
> > +    __call = function(self, ...)
> > +      local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
> > +                  .. (' %s'):rep(select('#', ...)):format(...)
> 
> It's good to comment that we just format arguments to strings for the
> script invocaiton.

Well, yeah... my Perl background is coming out ;)

Anyway, here is the comment:

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

diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
index ac04c01d..f5e08043 100644
--- a/test/tarantool-tests/tap.lua
+++ b/test/tarantool-tests/tap.lua
@@ -60,8 +65,17 @@ function M.makecmd(arg, opts)
     REDIRECT = opts and opts.redirect or '',
   }, {
     __call = function(self, ...)
+      -- This line just makes the command for <io.popen> by the
+      -- following steps:
+      -- 1. Replace the placeholders with the corresponding values
+      --    given to the command constructor (e.g. script, env)
+      -- 2. Join all CLI arguments given to the __call metamethod
+      -- 3. Concatenate the results of step 1 and step 2 to obtain
+      --    the resulting command.
       local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
                   .. (' %s'):rep(select('#', ...)):format(...)
+      -- Trim both leading and trailing whitespace from the output
+      -- produced by the child process.
       return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
     end
   })

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

> 
> > +      return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
> > +    end
> > +  })
> >  end
> >  
> >  function M.skipcond(condition, message)
> > -- 
> > 2.30.2
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 3/7] test: stop using utils.selfrun in tests

[Igor Munkin via Tarantool-patches]

Max,

Thanks for your review! See my answers below.

On 16.02.23, Maxim Kokryashkin wrote:
> 
> Hi, Igor!
> LGTM, except for a few nits below.
>  

<snipped>

> >>diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> >>index dd02130c..f4795db0 100644
> >>--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> >>+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> >>@@ -3,52 +3,43 @@ local utils = require('utils')
> ><snipped>
> >>+ -- TODO: Leave another toxic comment regarding SIP on macOS.
> >That comment is unnecessary. Here and below.

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

diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
index ff3eaf01..ad06c329 100644
--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
@@ -6,8 +6,23 @@ local test = tap.test('gh-4427-ffi-sandwich'):skipcond({
 
 test:plan(2)
 
+-- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
+-- with the given environment, launch options and CLI arguments.
 local script = require('utils').makecmd(arg, {
-  -- TODO: Leave another toxic comment regarding SIP on macOS.
+  -- XXX: Apple tries their best to "protect their users from
+  -- malware". As a result SIP (see the link[1] below) has been
+  -- designed and released. Now, Apple developers are so
+  -- protected, that they can load nothing being not installed in
+  -- the system, since the environment is sanitized before the
+  -- child process is launched. In particular, environment
+  -- variables starting with DYLD_ and LD_ are unset for child
+  -- process. For more info, see the docs[2] below.
+  --
+  -- The environment variable below is used by FFI machinery to
+  -- find the proper shared library.
+  --
+  -- [1]: https://support.apple.com/en-us/HT204899
+  -- [2]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
   redirect = '2>&1',
 })

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

> > 
> ><snipped>
> >>
> >>diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> >>index eb11d40d..41a7c22a 100644
> >>--- a/test/tarantool-tests/utils.lua
> >>+++ b/test/tarantool-tests/utils.lua

<snipped>

> >>+ }, {
> >>+ __call = function(self, ...)
> >>+ local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
> >>+ .. (' %s'):rep(select('#', ...)):format(...)
> >>+ return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
> >>+ end
> >>+ })
> >That part is barely readable, so I think you should either drop
> >a comment with explanation, or rewrite it in a more readable way.

Well, yeah... my Perl background is coming out ;)

Anyway, here is the comment:

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

diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
index ac04c01d..f5e08043 100644
--- a/test/tarantool-tests/tap.lua
+++ b/test/tarantool-tests/tap.lua
@@ -60,8 +65,17 @@ function M.makecmd(arg, opts)
     REDIRECT = opts and opts.redirect or '',
   }, {
     __call = function(self, ...)
+      -- This line just makes the command for <io.popen> by the
+      -- following steps:
+      -- 1. Replace the placeholders with the corresponding values
+      --    given to the command constructor (e.g. script, env)
+      -- 2. Join all CLI arguments given to the __call metamethod
+      -- 3. Concatenate the results of step 1 and step 2 to obtain
+      --    the resulting command.
       local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
                   .. (' %s'):rep(select('#', ...)):format(...)
+      -- Trim both leading and trailing whitespace from the output
+      -- produced by the child process.
       return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
     end
   })

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

> >> end
> >> 
> >> function M.skipcond(condition, message)
> >>--
> >>2.30.2
> >--
> >Best regards,
> >Maxim Kokryashkin
> > 

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 4/7] test: make skipcond helper more convenient

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for the patch!

On 15.02.23, Sergey Kaplun wrote:
> Hi, Igor!
> Thanks for the patch!
> `skipcond` improvement is delightful, I realy like it!
> 
> Please, consider my comments below.
> 
> On 13.02.23, Igor Munkin wrote:
> > This patch provides two enhancements for <utils.skipcond>:
> > 1. As a result of the patch, <utils.skipcond> becomes multi-conditional,
> >    so there is no need to concatenate one huge and complex condition
> >    with one huge unreadable reason. Now skipcond receives the table with
> >    conditions and skips the test if one of the given conditions is met.
> > 2. <utils.skipcond> yields the test object, that allows to chain
> >    skipcond call right next to the test constructor.
> > 
> > Finally as a result of the aforementioned changes <utils.skipcond> is
> > moved to tap.lua.
> > 
> > Relates to tarantool/tarantool#8252
> > 
> > Co-authored-by: Sergey Kaplun <skaplun@tarantool.org>
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---

<snipped>

> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > index e0b6d86d..019fed29 100644
> > --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> 
> <snipped>
> 
> >  
> > -jit.off()
> > -jit.flush()
> > +-- XXX: Run JIT tuning functions in a safe frame to avoid errors
> > +-- thrown when LuaJIT is compiled with JIT engine disabled.
> > +pcall(jit.off)
> > +pcall(jit.flush)
> 
> Should this be the part of the next patch?

Yes, thanks for noticing. Moved as well as all other occurrences.

> 
> >  
> >  local bufread = require "utils.bufread"
> >  local symtab = require "utils.symtab"

<snipped>

> > diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> > index b5353e85..c67da00e 100644
> > --- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> > +++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
> > @@ -1,9 +1,15 @@
> >  local tap = require('tap')
> > -
> >  -- Test to demonstrate the incorrect JIT behaviour when an error
> >  -- is raised on restoration from the snapshot.
> >  -- See also https://github.com/LuaJIT/LuaJIT/issues/603.
> > -local test = tap.test('lj-603-err-snap-restore.test.lua')
> > +local test = tap.test('lj-603-err-snap-restore'):skipcond({
> > +  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
> 
> Side note: discussed offline that we can disable the whole test file,
> even that the first test check the behaviour for the VM too.

I guess we'll go back to your original approach here (see Max
suggestion in the next thread).

> 
> > +  -- XXX: The different amount of stack slots is in-use for
> > +  -- Tarantool at start, so just skip test for it.
> > +  -- luacheck: no global
> > +  ['Disable test for Tarantool'] = _TARANTOOL,
> > +})
> > +
> >  test:plan(2)
> >  
> >  -- XXX: This is fragile. We need a specific amount of Lua stack
> > @@ -15,7 +21,7 @@ test:plan(2)
> >  -- error handling"), etc.).
> >  -- This amount is suited well for GC64 and non-GC64 mode.
> >  -- luacheck: no unused
> > -local _, _, _, _, _, _
> > +local _, _, _, _, _, _, _, _, _, _, _, _, _, _, _
> 
> Side note: Is necessary due to different allocations and memory mapping,
> so we can hit `ERRERR` error during raising the `STKOV` error and
> handler will be called.

Well, I can't agree with Max here: I guess this should be stopped within
this patchset. See the next thread for the changes.

> 
> >  
> >  local handler_is_called = false
> >  local recursive_f

<snipped>

> > diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> > index 42a9adf9..a41a1c7a 100644
> > --- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> > +++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> > @@ -1,12 +1,15 @@
> 
> <snipped>
> 
> > +local maxnins = require('utils').const.maxnins
> 
> Minor: since this is the constant for the test should we name it in the
> upper-case?

Well, I don't have any arguments for or against this approach, so I just
do it, if you want. The diff is below.

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

diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
index c8879f57..654e5545 100644
--- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
+++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
@@ -10,7 +10,7 @@ local path = arg[0]:gsub('%.test%.lua', '')
 local suffix = package.cpath:match('?.(%a+);')
 package.cpath = ('%s/?.%s;'):format(path, suffix)..package.cpath
 
-local maxnins = require('utils').const.maxnins
+local MAXNINS = require('utils').const.maxnins
 local jit_opt_default = {
     3, -- level
     "hotloop=56",
@@ -95,7 +95,7 @@ end))
 
 -- Compiled loop with a side exit which does not get compiled.
 test:ok(testgetmetrics.snap_restores(function()
-    jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
+    jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(MAXNINS))
 
     local function foo(i)
         -- math.fmod is not yet compiled!

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

> 
> >  local jit_opt_default = {

<snipped>

> > diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> > index 0c170d0c..e5ee7902 100644
> > --- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> 
> <snipped>
> 
> >  
> > +local maxnins = require('utils').const.maxnins
> 
> Ditto.

Ditto, see the diff below:

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

diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
index b1f86cea..881e717b 100644
--- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
+++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
@@ -10,7 +10,7 @@ local test = tap.test("lib-misc-getmetrics"):skipcond({
 
 test:plan(10)
 
-local maxnins = require('utils').const.maxnins
+local MAXNINS = require('utils').const.maxnins
 local jit_opt_default = {
     3, -- level
     "hotloop=56",
@@ -280,7 +280,7 @@ test:test("snap-restores-loop-side-exit-non-compiled", function(subtest)
     -- Compiled loop with a side exit which does not get compiled.
     subtest:plan(1)
 
-    jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
+    jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(MAXNINS))
 
     local function foo(i)
         -- math.fmod is not yet compiled!

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

> 
> >  local jit_opt_default = {
> >      3, -- level
> >      "hotloop=56",
> 
> <snipped>
> 
> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > index bae0c27c..accb1ee1 100644
> > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> 
> <snipped>
> 
> > @@ -28,8 +28,8 @@ local memprof = require "memprof.parse"
> >  local process = require "memprof.process"
> >  local symtab = require "utils.symtab"
> >  
> > -local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
> > -local BAD_PATH = utils.profilename("memprofdata/tmp.bin")
> > +local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")
> 
> Don't get the point.
> Why do not require this module once, so there is no need to change these
> lines?

Hm. Ya prosto objebalsa tut. You and Max are right, I've fixed this the
following way (the diff is below):

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

diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
index e2701c13..1d3f734e 100644
--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
@@ -27,9 +27,10 @@ local bufread = require "utils.bufread"
 local memprof = require "memprof.parse"
 local process = require "memprof.process"
 local symtab = require "utils.symtab"
+local profilename = require("utils").profilename

-local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")
-local BAD_PATH = require("utils").profilename("memprofdata/tmp.bin")
+local TMP_BINFILE = profilename("memprofdata.tmp.bin")
+local BAD_PATH = profilename("memprofdata/tmp.bin")
 local SRC_PATH = "@"..arg[0]

 local function default_payload()

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

> 
> > +local BAD_PATH = require("utils").profilename("memprofdata/tmp.bin")
> >  local SRC_PATH = "@"..arg[0]
> >  
> >  local function default_payload()

<snipped>

> > diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> > index 4bf10e8d..8dc36592 100644
> > --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> > +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> 
> <snipped>
> 
> > @@ -18,8 +13,8 @@ local bufread = require("utils.bufread")
> >  local symtab = require("utils.symtab")
> >  local sysprof = require("sysprof.parse")
> >  
> > -local TMP_BINFILE = utils.profilename("sysprofdata.tmp.bin")
> > -local BAD_PATH = utils.profilename("sysprofdata/tmp.bin")
> > +local TMP_BINFILE = require("utils").profilename("sysprofdata.tmp.bin")
> > +local BAD_PATH = require("utils").profilename("sysprofdata/tmp.bin")
> 
> Ditto.

Ditto, see the diff below:

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

diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
index dcde81b9..3e7c0005 100644
--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
@@ -14,9 +14,10 @@ pcall(jit.flush)
 local bufread = require("utils.bufread")
 local symtab = require("utils.symtab")
 local sysprof = require("sysprof.parse")
+local profilename = require("utils").profilename

-local TMP_BINFILE = require("utils").profilename("sysprofdata.tmp.bin")
-local BAD_PATH = require("utils").profilename("sysprofdata/tmp.bin")
+local TMP_BINFILE = profilename("sysprofdata.tmp.bin")
+local BAD_PATH = profilename("sysprofdata/tmp.bin")

 local function payload()
   local function fib(n)

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

> 
> >  
> >  local function payload()
> >    local function fib(n)
> > diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
> > index a1f54d20..ac04c01d 100644
> > --- a/test/tarantool-tests/tap.lua
> > +++ b/test/tarantool-tests/tap.lua
> > @@ -304,6 +304,17 @@ local function check(test)
> >    return test.planned == test.total and test.failed == 0
> >  end
> >  
> > +local function skipcond(test, conditions)
> > +  for message, condition in pairs(conditions) do
> > +    if condition then
> > +      test:plan(1)
> > +      test:skip(message)
> > +      os.exit(test:check() and 0 or 1)
> 
> Does this mean that the subtest should be the last test to run in the
> file? It's kinda not obvious. Also, I suggest the following approach:
> Keep that condition search result, and skip every test (and subtest) if
> the condition met. We can still skip full test, like you done, if it has
> no parent test to run (has no test.parent).

It means that ya objebalsa one more time... Well, I have to declare that
this approach doesn't work for subtests. Hence, I suggest the following:
1. Proceed with this patchset for the root (i.e. parent) tests, since
   this is the most popular way to write tests in Tarantool suite.
2. Think a little how to solve the problem for subtests. For now it's
   too complex: plain Lua assertions, JIT engine tuning, etc --
   everything need to be considered and carefully handled.

Anyway, to prevent skipcond misuse I added the next assertion:

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

diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
index ac04c01d..f7be9c81 100644
--- a/test/tarantool-tests/tap.lua
+++ b/test/tarantool-tests/tap.lua
@@ -305,6 +305,7 @@ local function check(test)
 end
 
 local function skipcond(test, conditions)
+  assert(test.parent ~= nil, 'FIXME: test:skipcond works only for parent tests')
   for message, condition in pairs(conditions) do
     if condition then
       test:plan(1)

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

> 
> > +    end
> > +  end
> > +  return test
> > +end
> > +
> >  test_mt = {
> >    __index = {
> >      test       = new,

<snipped>

> > -- 
> > 2.30.2
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 3/7] test: stop using utils.selfrun in tests

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the fixes!

LGTM, just a minor nit below!

On 27.02.23, Igor Munkin wrote:
> Sergey,
> 
> Thanks for your review! See my answers below.
> 

<snipped>

> > 
> > > +    REDIRECT = opts and opts.redirect or '',
> > > +  }, {
> > > +    __call = function(self, ...)
> > > +      local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
> > > +                  .. (' %s'):rep(select('#', ...)):format(...)
> > 
> > It's good to comment that we just format arguments to strings for the
> > script invocaiton.
> 
> Well, yeah... my Perl background is coming out ;)
> 
> Anyway, here is the comment:
> 
> ================================================================================
> 
> diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
> index ac04c01d..f5e08043 100644
> --- a/test/tarantool-tests/tap.lua
> +++ b/test/tarantool-tests/tap.lua
> @@ -60,8 +65,17 @@ function M.makecmd(arg, opts)
>      REDIRECT = opts and opts.redirect or '',
>    }, {
>      __call = function(self, ...)
> +      -- This line just makes the command for <io.popen> by the
> +      -- following steps:
> +      -- 1. Replace the placeholders with the corresponding values
> +      --    given to the command constructor (e.g. script, env)
> +      -- 2. Join all CLI arguments given to the __call metamethod
> +      -- 3. Concatenate the results of step 1 and step 2 to obtain
> +      --    the resulting command.

Extra dot in the end of the list (or missing in the previous bullets).

>        local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
>                    .. (' %s'):rep(select('#', ...)):format(...)
> +      -- Trim both leading and trailing whitespace from the output
> +      -- produced by the child process.
>        return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
>      end
>    })
> 
> ================================================================================
> 
> > 
> > > +      return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
> > > +    end
> > > +  })
> > >  end
> > >  
> > >  function M.skipcond(condition, message)
> > > -- 
> > > 2.30.2
> > > 
> > 
> > -- 
> > Best regards,
> > Sergey Kaplun
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 0/7] Adjust tests to be run when JIT is disabled

[Igor Munkin via Tarantool-patches]

Hi folks,

I've finally decided to split this series into two patchsets:
1. One with TAP and test-related improvements[1].
2. Another one related to #8252[2] (I'll send, when the first one is
   merged into the trunk).

Anyway, I'll consider all the comments you've left for the patches in
this thread, and fix them in the upcoming series.

[1]: https://lists.tarantool.org/tarantool-patches/cover.1677236706.git.imun@tarantool.org/T/#t
[2]: https://github.com/tarantool/tarantool/issues/8252

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 4/7] test: make skipcond helper more convenient

[Sergey Kaplun via Tarantool-patches]

Hi, Igor!
Thanks for the fixes!

On 27.02.23, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch!

What can I say, except "You welcome!"? :)

See other comments in the next thread.

> 

<snipped>

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/7] build: fix build with JIT disabled

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]


Hi, Igor!
Thanks for the fixes!
LGTM
--
Best regards,
Maxim Kokryashkin
 
  
>Понедельник, 27 февраля 2023, 12:18 +03:00 от Igor Munkin <imun@tarantool.org>:
> 
>Sergey,
>
>Thanks for your review!
>
>On 13.02.23, Sergey Kaplun wrote:
>> Hi, Igor!
>> Thanks for the patch!
>> LGTM, except a single nit regarding the commit message.
>>
>> On 13.02.23, Igor Munkin wrote:
>> > struct GCtrace is defined only if LJ_HASJIT is set. Hence all spots
>>
>> Typo: s/Hence,/Hence/
>
>Fixed, force-pushed.
>
>>
>> > where GCtrace is used should be also moved under LJ_HASJIT define.
>> >
>> > Relates to tarantool/tarantool#8252
>>
>> Side note: I see "Relates" and "Related" in our commit logs, so I
>> suggest to use "Relates" for future commits.
>
>I always use "Relates" (except maybe some early commits).
>
>>
>> >
>> > Signed-off-by: Igor Munkin < imun@tarantool.org >
>>
>> <snipped>
>>
>> > --
>> > 2.30.2
>> >
>>
>> --
>> Best regards,
>> Sergey Kaplun
>
>--
>Best regards,
>IM
 

[-- Attachment #2: Type: text/html, Size: 1686 bytes --]

Re: [Tarantool-patches] [PATCH luajit 3/7] test: stop using utils.selfrun in tests

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 4704 bytes --]


Hi, Igor!
Thanks for the fixes!
LGTM.
--
Best regards,
Maxim Kokryashkin
 
  
>Понедельник, 27 февраля 2023, 12:19 +03:00 от Igor Munkin <imun@tarantool.org>:
> 
>Max,
>
>Thanks for your review! See my answers below.
>
>On 16.02.23, Maxim Kokryashkin wrote:
>>
>> Hi, Igor!
>> LGTM, except for a few nits below.
>>  
>
><snipped>
>
>> >>diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>> >>index dd02130c..f4795db0 100644
>> >>--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>> >>+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>> >>@@ -3,52 +3,43 @@ local utils = require('utils')
>> ><snipped>
>> >>+ -- TODO: Leave another toxic comment regarding SIP on macOS.
>> >That comment is unnecessary. Here and below.
>
>================================================================================
>
>diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>index ff3eaf01..ad06c329 100644
>--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
>@@ -6,8 +6,23 @@ local test = tap.test('gh-4427-ffi-sandwich'):skipcond({
> 
> test:plan(2)
> 
>+-- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
>+-- with the given environment, launch options and CLI arguments.
> local script = require('utils').makecmd(arg, {
>- -- TODO: Leave another toxic comment regarding SIP on macOS.
>+ -- XXX: Apple tries their best to "protect their users from
>+ -- malware". As a result SIP (see the link[1] below) has been
>+ -- designed and released. Now, Apple developers are so
>+ -- protected, that they can load nothing being not installed in
>+ -- the system, since the environment is sanitized before the
>+ -- child process is launched. In particular, environment
>+ -- variables starting with DYLD_ and LD_ are unset for child
>+ -- process. For more info, see the docs[2] below.
>+ --
>+ -- The environment variable below is used by FFI machinery to
>+ -- find the proper shared library.
>+ --
>+ -- [1]:  https://support.apple.com/en-us/HT204899
>+ -- [2]:  https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
>   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
>   redirect = '2>&1',
> })
>
>================================================================================
>
>> > 
>> ><snipped>
>> >>
>> >>diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
>> >>index eb11d40d..41a7c22a 100644
>> >>--- a/test/tarantool-tests/utils.lua
>> >>+++ b/test/tarantool-tests/utils.lua
>
><snipped>
>
>> >>+ }, {
>> >>+ __call = function(self, ...)
>> >>+ local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
>> >>+ .. (' %s'):rep(select('#', ...)):format(...)
>> >>+ return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
>> >>+ end
>> >>+ })
>> >That part is barely readable, so I think you should either drop
>> >a comment with explanation, or rewrite it in a more readable way.
>
>Well, yeah... my Perl background is coming out ;)
>
>Anyway, here is the comment:
>
>================================================================================
>
>diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
>index ac04c01d..f5e08043 100644
>--- a/test/tarantool-tests/tap.lua
>+++ b/test/tarantool-tests/tap.lua
>@@ -60,8 +65,17 @@ function M.makecmd(arg, opts)
>     REDIRECT = opts and opts.redirect or '',
>   }, {
>     __call = function(self, ...)
>+ -- This line just makes the command for <io.popen> by the
>+ -- following steps:
>+ -- 1. Replace the placeholders with the corresponding values
>+ -- given to the command constructor (e.g. script, env)
>+ -- 2. Join all CLI arguments given to the __call metamethod
>+ -- 3. Concatenate the results of step 1 and step 2 to obtain
>+ -- the resulting command.
>       local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
>                   .. (' %s'):rep(select('#', ...)):format(...)
>+ -- Trim both leading and trailing whitespace from the output
>+ -- produced by the child process.
>       return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
>     end
>   })
>
>================================================================================
>
>> >> end
>> >> 
>> >> function M.skipcond(condition, message)
>> >>--
>> >>2.30.2
>> >--
>> >Best regards,
>> >Maxim Kokryashkin
>> > 
>
>--
>Best regards,
>IM
 

[-- Attachment #2: Type: text/html, Size: 6313 bytes --]

Re: [Tarantool-patches] [PATCH luajit 4/7] test: make skipcond helper more convenient

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 14002 bytes --]


Hi, Igor!
Thanks for the fixes!
I am still not satisfied with `snap-restore` case,
but whatever.
--
Best regards,
Maxim Kokryashkin
 
 
> 
>>Sergey,
>>
>>Thanks for the patch!
>>
>>On 15.02.23, Sergey Kaplun wrote:
>>> Hi, Igor!
>>> Thanks for the patch!
>>> `skipcond` improvement is delightful, I realy like it!
>>>
>>> Please, consider my comments below.
>>>
>>> On 13.02.23, Igor Munkin wrote:
>>> > This patch provides two enhancements for <utils.skipcond>:
>>> > 1. As a result of the patch, <utils.skipcond> becomes multi-conditional,
>>> > so there is no need to concatenate one huge and complex condition
>>> > with one huge unreadable reason. Now skipcond receives the table with
>>> > conditions and skips the test if one of the given conditions is met.
>>> > 2. <utils.skipcond> yields the test object, that allows to chain
>>> > skipcond call right next to the test constructor.
>>> >
>>> > Finally as a result of the aforementioned changes <utils.skipcond> is
>>> > moved to tap.lua.
>>> >
>>> > Relates to tarantool/tarantool#8252
>>> >
>>> > Co-authored-by: Sergey Kaplun < skaplun@tarantool.org >
>>> > Signed-off-by: Igor Munkin < imun@tarantool.org >
>>> > ---
>>
>><snipped>
>>
>>> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
>>> > index e0b6d86d..019fed29 100644
>>> > --- a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
>>> > +++ b/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
>>>
>>> <snipped>
>>>
>>> >
>>> > -jit.off()
>>> > -jit.flush()
>>> > +-- XXX: Run JIT tuning functions in a safe frame to avoid errors
>>> > +-- thrown when LuaJIT is compiled with JIT engine disabled.
>>> > +pcall(jit.off)
>>> > +pcall(jit.flush)
>>>
>>> Should this be the part of the next patch?
>>
>>Yes, thanks for noticing. Moved as well as all other occurrences.
>>
>>>
>>> >
>>> > local bufread = require "utils.bufread"
>>> > local symtab = require "utils.symtab"
>>
>><snipped>
>>
>>> > diff --git a/test/tarantool-tests/lj-603-err-snap-restore.test.lua b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
>>> > index b5353e85..c67da00e 100644
>>> > --- a/test/tarantool-tests/lj-603-err-snap-restore.test.lua
>>> > +++ b/test/tarantool-tests/lj-603-err-snap-restore.test.lua
>>> > @@ -1,9 +1,15 @@
>>> > local tap = require('tap')
>>> > -
>>> > -- Test to demonstrate the incorrect JIT behaviour when an error
>>> > -- is raised on restoration from the snapshot.
>>> > -- See also  https://github.com/LuaJIT/LuaJIT/issues/603 .
>>> > -local test = tap.test('lj-603-err-snap-restore.test.lua')
>>> > +local test = tap.test('lj-603-err-snap-restore'):skipcond({
>>> > + ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>>>
>>> Side note: discussed offline that we can disable the whole test file,
>>> even that the first test check the behaviour for the VM too.
>>
>>I guess we'll go back to your original approach here (see Max
>>suggestion in the next thread).
>>
>>>
>>> > + -- XXX: The different amount of stack slots is in-use for
>>> > + -- Tarantool at start, so just skip test for it.
>>> > + -- luacheck: no global
>>> > + ['Disable test for Tarantool'] = _TARANTOOL,
>>> > +})
>>> > +
>>> > test:plan(2)
>>> >
>>> > -- XXX: This is fragile. We need a specific amount of Lua stack
>>> > @@ -15,7 +21,7 @@ test:plan(2)
>>> > -- error handling"), etc.).
>>> > -- This amount is suited well for GC64 and non-GC64 mode.
>>> > -- luacheck: no unused
>>> > -local _, _, _, _, _, _
>>> > +local _, _, _, _, _, _, _, _, _, _, _, _, _, _, _
>>>
>>> Side note: Is necessary due to different allocations and memory mapping,
>>> so we can hit `ERRERR` error during raising the `STKOV` error and
>>> handler will be called.
>>
>>Well, I can't agree with Max here: I guess this should be stopped within
>>this patchset. See the next thread for the changes.
>>
>>>
>>> >
>>> > local handler_is_called = false
>>> > local recursive_f
>>
>><snipped>
>>
>>> > diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>> > index 42a9adf9..a41a1c7a 100644
>>> > --- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>> > +++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>> > @@ -1,12 +1,15 @@
>>>
>>> <snipped>
>>>
>>> > +local maxnins = require('utils').const.maxnins
>>>
>>> Minor: since this is the constant for the test should we name it in the
>>> upper-case?
>>
>>Well, I don't have any arguments for or against this approach, so I just
>>do it, if you want. The diff is below.
>>
>>================================================================================
>>
>>diff --git a/test/tarantool-tests/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>index c8879f57..654e5545 100644
>>--- a/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>+++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
>>@@ -10,7 +10,7 @@ local path = arg[0]:gsub('%.test%.lua', '')
>> local suffix = package.cpath:match('?.(%a+);')
>> package.cpath = ('%s/?.%s;'):format(path, suffix)..package.cpath
>> 
>>-local maxnins = require('utils').const.maxnins
>>+local MAXNINS = require('utils').const.maxnins
>> local jit_opt_default = {
>>     3, -- level
>>     "hotloop=56",
>>@@ -95,7 +95,7 @@ end))
>> 
>> -- Compiled loop with a side exit which does not get compiled.
>> test:ok(testgetmetrics.snap_restores(function()
>>- jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
>>+ jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(MAXNINS))
>> 
>>     local function foo(i)
>>         -- math.fmod is not yet compiled!
>>
>>================================================================================
>>
>>>
>>> > local jit_opt_default = {
>>
>><snipped>
>>
>>> > diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>> > index 0c170d0c..e5ee7902 100644
>>> > --- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>> > +++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>>
>>> <snipped>
>>>
>>> >
>>> > +local maxnins = require('utils').const.maxnins
>>>
>>> Ditto.
>>
>>Ditto, see the diff below:
>>
>>================================================================================
>>
>>diff --git a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>index b1f86cea..881e717b 100644
>>--- a/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>+++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
>>@@ -10,7 +10,7 @@ local test = tap.test("lib-misc-getmetrics"):skipcond({
>> 
>> test:plan(10)
>> 
>>-local maxnins = require('utils').const.maxnins
>>+local MAXNINS = require('utils').const.maxnins
>> local jit_opt_default = {
>>     3, -- level
>>     "hotloop=56",
>>@@ -280,7 +280,7 @@ test:test("snap-restores-loop-side-exit-non-compiled", function(subtest)
>>     -- Compiled loop with a side exit which does not get compiled.
>>     subtest:plan(1)
>> 
>>- jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(maxnins))
>>+ jit.opt.start(0, "hotloop=1", "hotexit=2", ("minstitch=%d"):format(MAXNINS))
>> 
>>     local function foo(i)
>>         -- math.fmod is not yet compiled!
>>
>>================================================================================
>>
>>>
>>> > local jit_opt_default = {
>>> > 3, -- level
>>> > "hotloop=56",
>>>
>>> <snipped>
>>>
>>> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>> > index bae0c27c..accb1ee1 100644
>>> > --- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>> > +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>>
>>> <snipped>
>>>
>>> > @@ -28,8 +28,8 @@ local memprof = require "memprof.parse"
>>> > local process = require "memprof.process"
>>> > local symtab = require "utils.symtab"
>>> >
>>> > -local TMP_BINFILE = utils.profilename("memprofdata.tmp.bin")
>>> > -local BAD_PATH = utils.profilename("memprofdata/tmp.bin")
>>> > +local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")
>>>
>>> Don't get the point.
>>> Why do not require this module once, so there is no need to change these
>>> lines?
>>
>>Hm. Ya prosto objebalsa tut. You and Max are right, I've fixed this the
>>following way (the diff is below):
>>
>>================================================================================
>>
>>diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>index e2701c13..1d3f734e 100644
>>--- a/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>+++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
>>@@ -27,9 +27,10 @@ local bufread = require "utils.bufread"
>> local memprof = require "memprof.parse"
>> local process = require "memprof.process"
>> local symtab = require "utils.symtab"
>>+local profilename = require("utils").profilename
>>
>>-local TMP_BINFILE = require("utils").profilename("memprofdata.tmp.bin")
>>-local BAD_PATH = require("utils").profilename("memprofdata/tmp.bin")
>>+local TMP_BINFILE = profilename("memprofdata.tmp.bin")
>>+local BAD_PATH = profilename("memprofdata/tmp.bin")
>> local SRC_PATH = "@"..arg[0]
>>
>> local function default_payload()
>>
>>================================================================================
>>
>>>
>>> > +local BAD_PATH = require("utils").profilename("memprofdata/tmp.bin")
>>> > local SRC_PATH = "@"..arg[0]
>>> >
>>> > local function default_payload()
>>
>><snipped>
>>
>>> > diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>> > index 4bf10e8d..8dc36592 100644
>>> > --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>> > +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>>
>>> <snipped>
>>>
>>> > @@ -18,8 +13,8 @@ local bufread = require("utils.bufread")
>>> > local symtab = require("utils.symtab")
>>> > local sysprof = require("sysprof.parse")
>>> >
>>> > -local TMP_BINFILE = utils.profilename("sysprofdata.tmp.bin")
>>> > -local BAD_PATH = utils.profilename("sysprofdata/tmp.bin")
>>> > +local TMP_BINFILE = require("utils").profilename("sysprofdata.tmp.bin")
>>> > +local BAD_PATH = require("utils").profilename("sysprofdata/tmp.bin")
>>>
>>> Ditto.
>>
>>Ditto, see the diff below:
>>
>>================================================================================
>>
>>diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>index dcde81b9..3e7c0005 100644
>>--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
>>@@ -14,9 +14,10 @@ pcall(jit.flush)
>> local bufread = require("utils.bufread")
>> local symtab = require("utils.symtab")
>> local sysprof = require("sysprof.parse")
>>+local profilename = require("utils").profilename
>>
>>-local TMP_BINFILE = require("utils").profilename("sysprofdata.tmp.bin")
>>-local BAD_PATH = require("utils").profilename("sysprofdata/tmp.bin")
>>+local TMP_BINFILE = profilename("sysprofdata.tmp.bin")
>>+local BAD_PATH = profilename("sysprofdata/tmp.bin")
>>
>> local function payload()
>>   local function fib(n)
>>
>>================================================================================
>>
>>>
>>> >
>>> > local function payload()
>>> > local function fib(n)
>>> > diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
>>> > index a1f54d20..ac04c01d 100644
>>> > --- a/test/tarantool-tests/tap.lua
>>> > +++ b/test/tarantool-tests/tap.lua
>>> > @@ -304,6 +304,17 @@ local function check(test)
>>> > return test.planned == test.total and test.failed == 0
>>> > end
>>> >
>>> > +local function skipcond(test, conditions)
>>> > + for message, condition in pairs(conditions) do
>>> > + if condition then
>>> > + test:plan(1)
>>> > + test:skip(message)
>>> > + os.exit(test:check() and 0 or 1)
>>>
>>> Does this mean that the subtest should be the last test to run in the
>>> file? It's kinda not obvious. Also, I suggest the following approach:
>>> Keep that condition search result, and skip every test (and subtest) if
>>> the condition met. We can still skip full test, like you done, if it has
>>> no parent test to run (has no test.parent).
>>
>>It means that ya objebalsa one more time... Well, I have to declare that
>>this approach doesn't work for subtests. Hence, I suggest the following:
>>1. Proceed with this patchset for the root (i.e. parent) tests, since
>>   this is the most popular way to write tests in Tarantool suite.
>>2. Think a little how to solve the problem for subtests. For now it's
>>   too complex: plain Lua assertions, JIT engine tuning, etc --
>>   everything need to be considered and carefully handled.
>>
>>Anyway, to prevent skipcond misuse I added the next assertion:
>>
>>================================================================================
>>
>>diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
>>index ac04c01d..f7be9c81 100644
>>--- a/test/tarantool-tests/tap.lua
>>+++ b/test/tarantool-tests/tap.lua
>>@@ -305,6 +305,7 @@ local function check(test)
>> end
>> 
>> local function skipcond(test, conditions)
>>+ assert(test.parent ~= nil, 'FIXME: test:skipcond works only for parent tests')
>>   for message, condition in pairs(conditions) do
>>     if condition then
>>       test:plan(1)
>>
>>================================================================================
>>
>>>
>>> > + end
>>> > + end
>>> > + return test
>>> > +end
>>> > +
>>> > test_mt = {
>>> > __index = {
>>> > test = new,
>>
>><snipped>
>>
>>> > --
>>> > 2.30.2
>>> >
>>>
>>> --
>>> Best regards,
>>> Sergey Kaplun
>>
>>--
>>Best regards,
>>IM
> 

[-- Attachment #2: Type: text/html, Size: 16616 bytes --]

Re: [Tarantool-patches] [PATCH luajit 6/7] test: fix lua-Harness JIT-related tests

[Igor Munkin via Tarantool-patches]

Sergey,

Thanks for your review! I've fixed both typos in commit message and
added the new patch at the bottom.

On 14.02.23, Sergey Kaplun wrote:
> Hi, Igor!
> Thanks for the patch!
> Please, consider my comment below.
> 
> On 13.02.23, Igor Munkin wrote:

<snipped>

> 
> I afraid that this check isn't fully correct either:
> `jit.status()` yields CPU-specific flags and JIT optimization flags.
> For ARM64 the subset of CPU-specific flags is empty. Hence, if someone
> runs the tests with `jit.opt.start(0)` to disable all JIT optimization
> this check will return false for LuaJIT compiled with JIT.
> 
> | $ src/luajit -e 'jit.opt.start(0) print(jit.arch, jit.status(), select("#", jit.status()) > 1)'
> | arm64   true    false
> 

Ouch, you're right. I've fixed the issue the way you suggested below.

> Also, I found the easier way to check `LJ_HASJIT` is set or not.
> We can simply check the existance of `jit.opt` module:
> 
> | $ src/luajit -e 'print(jit.opt)'
> | nil
> 

<snipped>

> > -- 
> > 2.30.2
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

Here is the new patch:

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

    test: fix lua-Harness JIT-related tests

    lua-Harness considers whether JIT is enabled or not in the scope of the
    403-jit.t and 411-luajit.t tests. However, the original condition is
    wrong, since <jit.status> yields false for both cases, when JIT is just
    turned off and when LuaJIT is built without compiler support. So, if
    <jit.status> yields false, the latter case is considered. The condition
    is fixed to differ both aforementioned cases in the following way: when
    jit.opt is nil, LuaJIT is built without compiler; otherwise, JIT support
    is on board.

    Part of tarantool/tarantool#8252

diff --git a/test/lua-Harness-tests/403-jit.t b/test/lua-Harness-tests/403-jit.t
index 0f986da9..1a88564b 100755
--- a/test/lua-Harness-tests/403-jit.t
+++ b/test/lua-Harness-tests/403-jit.t
@@ -31,7 +31,7 @@ if not jit then
     skip_all("only with LuaJIT")
 end
 
-local compiled_with_jit = jit.status()
+local compiled_with_jit = jit.opt ~= nil
 local luajit20 = jit.version_num < 20100 and not jit.version:match'RaptorJIT'
 local has_jit_opt = compiled_with_jit
 local has_jit_security = jit.security
diff --git a/test/lua-Harness-tests/411-luajit.t b/test/lua-Harness-tests/411-luajit.t
index 3a9a7b8f..6cfd6837 100755
--- a/test/lua-Harness-tests/411-luajit.t
+++ b/test/lua-Harness-tests/411-luajit.t
@@ -37,7 +37,7 @@ if not pcall(io.popen, lua .. [[ -e "a=1"]]) then
     skip_all("io.popen not supported")
 end
 
-local compiled_with_jit = jit.status()
+local compiled_with_jit = jit.opt ~= nil
 local has_jutil = pcall(require, 'jit.util')
 local has_openresty_listing = profile.openresty or jit.version:match'moonjit'
 

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

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 5/7] test: add skipcond for all JIT-related tests

[Igor Munkin via Tarantool-patches]

Max,

Thanks for your review! I've fixed the comments you've left.

On 16.02.23, Maxim Kokryashkin wrote:
> 
> Hi, Igor!
> Thanks for the patch!
> Please consider my comments below.
>  
> `lj-906-fix-err-mem.test.lua` has a `jit.off()`
> call in the line 78, which should be wrapped in a pcall.

Hm, thanks for noticing! Fun fact: jit.off doesn't raise the error when
JIT is not compiled. I guess we can left pcall only for jit.flush calls.

>  
> This change should be moved to commit
> «build: fix build with JIT disabled»
> (a835fb0f778db6e9f0109a66ea1d2ac78fe682e4)
> along with the change that I pointed out below. 

As we discussed offline, these changes should be left here. Ignoring.

> > 

<snipped>

> --
> Best regards,
> Maxim Kokryashkin
>  

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 6/7] test: fix lua-Harness JIT-related tests

[Igor Munkin via Tarantool-patches]

Max,

Thanks for your review! I've fixed all the comments you've left. The
branch is force-pushed.

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 0/7] Adjust tests to be run when JIT is disabled

[Igor Munkin via Tarantool-patches]

I've sent the second series[1] for #8252[2] that also consists fixed for
all comments you've all left here.

On 27.02.23, Igor Munkin wrote:
> Hi folks,
> 
> I've finally decided to split this series into two patchsets:
> 1. One with TAP and test-related improvements[1].
> 2. Another one related to #8252[2] (I'll send, when the first one is
>    merged into the trunk).
> 
> Anyway, I'll consider all the comments you've left for the patches in
> this thread, and fix them in the upcoming series.
> 
> [1]: https://lists.tarantool.org/tarantool-patches/cover.1677236706.git.imun@tarantool.org/T/#t
> [2]: https://github.com/tarantool/tarantool/issues/8252
> 
> -- 
> Best regards,
> IM

[1]: https://lists.tarantool.org/tarantool-patches/cover.1677607479.git.imun@tarantool.org/T/#t
[2]: https://github.com/tarantool/tarantool/issues/8252

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH luajit 5/7] test: add skipcond for all JIT-related tests

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]


Hi, Igor!
Thanks for the fixes!
LGTM
--
Best regards,
Maxim Kokryashkin
 
 
> 
>>Max,
>>
>>Thanks for your review! I've fixed the comments you've left.
>>
>>On 16.02.23, Maxim Kokryashkin wrote:
>>>
>>> Hi, Igor!
>>> Thanks for the patch!
>>> Please consider my comments below.
>>>  
>>> `lj-906-fix-err-mem.test.lua` has a `jit.off()`
>>> call in the line 78, which should be wrapped in a pcall.
>>
>>Hm, thanks for noticing! Fun fact: jit.off doesn't raise the error when
>>JIT is not compiled. I guess we can left pcall only for jit.flush calls.
>>
>>>  
>>> This change should be moved to commit
>>> «build: fix build with JIT disabled»
>>> (a835fb0f778db6e9f0109a66ea1d2ac78fe682e4)
>>> along with the change that I pointed out below.
>>
>>As we discussed offline, these changes should be left here. Ignoring.
>>
>>> > 
>>
>><snipped>
>>
>>> --
>>> Best regards,
>>> Maxim Kokryashkin
>>>  
>>
>>--
>>Best regards,
>>IM
> 

[-- Attachment #2: Type: text/html, Size: 1588 bytes --]

Re: [Tarantool-patches] [PATCH luajit 6/7] test: fix lua-Harness JIT-related tests

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 222 bytes --]


Hi, Igor!
Thanks for the fixes!
LGTM
--
Best regards,
Maxim Kokryashkin
 
 
> 
>>Max,
>>
>>Thanks for your review! I've fixed all the comments you've left. The
>>branch is force-pushed.
>>
>>--
>>Best regards,
>>IM
> 

[-- Attachment #2: Type: text/html, Size: 763 bytes --]