Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests
@ 2024-09-23  7:18 Sergey Kaplun via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 1/7] test: move profilers tests to subdirectory Sergey Kaplun via Tarantool-patches
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:18 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

This patchset shrinks the environment variables that are used in the
tarantool-tests suite. There will be no more huge, inconvenient
copy-pasting of LD_LIBRARY_PATH, LUA_CPATH. Also, it disables the flaky
profilers tests with the enabled table bump optimization.

Branch: https://github.com/tarantool/luajit/tree/skaplun/shrink-test-env
Related issue: https://github.com/tarantool/tarantool/issues/9898

Sergey Kaplun (7):
  test: move profilers tests to subdirectory
  test: rename <arm64-ccall-fp-convention.test.lua>
  cmake: introduce AppendTestEnvVar macro
  test: shrink LUA_PATH environment variable
  test: shrink LUA_CPATH and {DY}LD_LIBRARY_PATH
  test: skip flaky tests with enabled table bump
  test: set LD_PRELOAD only when necessary

 test/tarantool-tests/CMakeLists.txt           | 202 +++++++++++-------
 ...=> ffi-ccall-arm64-fp-convention.test.lua} |   2 +-
 .../gh-5688-tool-cli-flag.test.lua            |   2 +
 .../gh-5813-resolving-of-c-symbols.test.lua   |   2 +
 .../both/CMakeLists.txt                       |   0
 .../both/resboth.c                            |   0
 .../gnuhash/CMakeLists.txt                    |   0
 .../gnuhash/resgnuhash.c                      |   0
 .../hash/CMakeLists.txt                       |   0
 .../hash/reshash.c                            |   0
 .../stripped/CMakeLists.txt                   |   0
 .../stripped/resstripped.c                    |   0
 .../gh-5994-memprof-human-readable.test.lua   |   2 +
 ...4-add-proto-trace-sysprof-default.test.lua |   2 +
 ...17-profile-parsers-error-handling.test.lua |   2 +
 .../misclib-memprof-lapi.test.lua             |  16 +-
 .../misclib-sysprof-lapi.test.lua             |   2 +
 .../{ => profilers}/tools-utils-avl.test.lua  |   0
 18 files changed, 152 insertions(+), 80 deletions(-)
 rename test/tarantool-tests/{arm64-ccall-fp-convention.test.lua => ffi-ccall-arm64-fp-convention.test.lua} (96%)
 rename test/tarantool-tests/{ => profilers}/gh-5688-tool-cli-flag.test.lua (95%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols.test.lua (95%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/both/resboth.c (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/hash/reshash.c (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/stripped/resstripped.c (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5994-memprof-human-readable.test.lua (93%)
 rename test/tarantool-tests/{ => profilers}/gh-7264-add-proto-trace-sysprof-default.test.lua (90%)
 rename test/tarantool-tests/{ => profilers}/gh-9217-profile-parsers-error-handling.test.lua (94%)
 rename test/tarantool-tests/{ => profilers}/misclib-memprof-lapi.test.lua (92%)
 rename test/tarantool-tests/{ => profilers}/misclib-sysprof-lapi.test.lua (96%)
 rename test/tarantool-tests/{ => profilers}/tools-utils-avl.test.lua (100%)

-- 
2.46.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Tarantool-patches] [PATCH luajit 1/7] test: move profilers tests to subdirectory
  2024-09-23  7:18 [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:18 ` Sergey Kaplun via Tarantool-patches
  2024-09-23  7:40   ` Sergey Bronnikov via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 2/7] test: rename <arm64-ccall-fp-convention.test.lua> Sergey Kaplun via Tarantool-patches
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:18 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

It is useful when we want to change the properties for these tests only.

Needed for tarantool/tarantool#9898
---
 test/tarantool-tests/CMakeLists.txt                    | 10 +++++-----
 .../{ => profilers}/gh-5688-tool-cli-flag.test.lua     |  0
 .../gh-5813-resolving-of-c-symbols.test.lua            |  0
 .../gh-5813-resolving-of-c-symbols/both/CMakeLists.txt |  0
 .../gh-5813-resolving-of-c-symbols/both/resboth.c      |  0
 .../gnuhash/CMakeLists.txt                             |  0
 .../gnuhash/resgnuhash.c                               |  0
 .../gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt |  0
 .../gh-5813-resolving-of-c-symbols/hash/reshash.c      |  0
 .../stripped/CMakeLists.txt                            |  0
 .../stripped/resstripped.c                             |  0
 .../gh-5994-memprof-human-readable.test.lua            |  0
 .../gh-7264-add-proto-trace-sysprof-default.test.lua   |  0
 .../gh-9217-profile-parsers-error-handling.test.lua    |  0
 .../{ => profilers}/misclib-memprof-lapi.test.lua      |  0
 .../{ => profilers}/misclib-sysprof-lapi.test.lua      |  0
 .../{ => profilers}/tools-utils-avl.test.lua           |  0
 17 files changed, 5 insertions(+), 5 deletions(-)
 rename test/tarantool-tests/{ => profilers}/gh-5688-tool-cli-flag.test.lua (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols.test.lua (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/both/resboth.c (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/hash/reshash.c (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/stripped/resstripped.c (100%)
 rename test/tarantool-tests/{ => profilers}/gh-5994-memprof-human-readable.test.lua (100%)
 rename test/tarantool-tests/{ => profilers}/gh-7264-add-proto-trace-sysprof-default.test.lua (100%)
 rename test/tarantool-tests/{ => profilers}/gh-9217-profile-parsers-error-handling.test.lua (100%)
 rename test/tarantool-tests/{ => profilers}/misclib-memprof-lapi.test.lua (100%)
 rename test/tarantool-tests/{ => profilers}/misclib-sysprof-lapi.test.lua (100%)
 rename test/tarantool-tests/{ => profilers}/tools-utils-avl.test.lua (100%)

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 0bd3e6fc..11a84496 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -21,10 +21,6 @@ endmacro()
 add_subdirectory(ffi-ccall)
 add_subdirectory(fix-bit-shift-generation)
 add_subdirectory(gh-4427-ffi-sandwich)
-add_subdirectory(gh-5813-resolving-of-c-symbols/both)
-add_subdirectory(gh-5813-resolving-of-c-symbols/hash)
-add_subdirectory(gh-5813-resolving-of-c-symbols/gnuhash)
-add_subdirectory(gh-5813-resolving-of-c-symbols/stripped)
 add_subdirectory(gh-6098-fix-side-exit-patching-on-arm64)
 add_subdirectory(gh-6189-cur_L)
 add_subdirectory(lj-416-xor-before-jcc)
@@ -38,6 +34,10 @@ add_subdirectory(lj-flush-on-trace)
 add_subdirectory(lj-1004-oom-error-frame)
 add_subdirectory(lj-1066-fix-cur_L-after-coroutine-resume)
 add_subdirectory(lj-1166-error-stitch)
+add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/both)
+add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/gnuhash)
+add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/hash)
+add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/stripped)
 
 # The part of the memory profiler toolchain is located in tools
 # directory, jit, profiler, and bytecode toolchains are located
@@ -121,7 +121,7 @@ add_test_suite_target(tarantool-tests
 
 file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")
 foreach(test_path ${tests})
-  get_filename_component(test_name ${test_path} NAME)
+  file(RELATIVE_PATH test_name ${CMAKE_CURRENT_SOURCE_DIR} ${test_path})
   set(test_title "test/${TEST_SUITE_NAME}/${test_name}")
   add_test(NAME ${test_title}
     COMMAND ${LUAJIT_TEST_COMMAND} ${test_path}
diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
similarity index 100%
rename from test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
rename to test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/resboth.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/resboth.c
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/resboth.c
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/resboth.c
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/reshash.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/reshash.c
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/reshash.c
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/reshash.c
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/resstripped.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
similarity index 100%
rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
diff --git a/test/tarantool-tests/gh-5994-memprof-human-readable.test.lua b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
similarity index 100%
rename from test/tarantool-tests/gh-5994-memprof-human-readable.test.lua
rename to test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
similarity index 100%
rename from test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
rename to test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
diff --git a/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
similarity index 100%
rename from test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
rename to test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
similarity index 100%
rename from test/tarantool-tests/misclib-memprof-lapi.test.lua
rename to test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
similarity index 100%
rename from test/tarantool-tests/misclib-sysprof-lapi.test.lua
rename to test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
diff --git a/test/tarantool-tests/tools-utils-avl.test.lua b/test/tarantool-tests/profilers/tools-utils-avl.test.lua
similarity index 100%
rename from test/tarantool-tests/tools-utils-avl.test.lua
rename to test/tarantool-tests/profilers/tools-utils-avl.test.lua
-- 
2.46.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Tarantool-patches] [PATCH luajit 2/7] test: rename <arm64-ccall-fp-convention.test.lua>
  2024-09-23  7:18 [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 1/7] test: move profilers tests to subdirectory Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:18 ` Sergey Kaplun via Tarantool-patches
  2024-09-23  7:45   ` Sergey Bronnikov via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 3/7] cmake: introduce AppendTestEnvVar macro Sergey Kaplun via Tarantool-patches
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:18 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

This patch renames the aforementioned file to
<ffi-ccall-arm64-fp-convention.test.lua> to make the prefix (ffi-ccall)
match the name of the directory containing the C library for the test.

Needed for tarantool/tarantool#9898
---
 ...nvention.test.lua => ffi-ccall-arm64-fp-convention.test.lua} | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename test/tarantool-tests/{arm64-ccall-fp-convention.test.lua => ffi-ccall-arm64-fp-convention.test.lua} (96%)

diff --git a/test/tarantool-tests/arm64-ccall-fp-convention.test.lua b/test/tarantool-tests/ffi-ccall-arm64-fp-convention.test.lua
similarity index 96%
rename from test/tarantool-tests/arm64-ccall-fp-convention.test.lua
rename to test/tarantool-tests/ffi-ccall-arm64-fp-convention.test.lua
index c58ba697..053b7a3a 100644
--- a/test/tarantool-tests/arm64-ccall-fp-convention.test.lua
+++ b/test/tarantool-tests/ffi-ccall-arm64-fp-convention.test.lua
@@ -3,7 +3,7 @@ local tap = require('tap')
 
 local ffi_ccall = ffi.load('libfficcall')
 
-local test = tap.test('arm64-ccall-fp-convention')
+local test = tap.test('ffi-ccall-arm64-fp-convention')
 test:plan(3)
 
 ffi.cdef[[
-- 
2.46.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Tarantool-patches] [PATCH luajit 3/7] cmake: introduce AppendTestEnvVar macro
  2024-09-23  7:18 [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 1/7] test: move profilers tests to subdirectory Sergey Kaplun via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 2/7] test: rename <arm64-ccall-fp-convention.test.lua> Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:18 ` Sergey Kaplun via Tarantool-patches
  2024-09-23  7:51   ` Sergey Bronnikov via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 4/7] test: shrink LUA_PATH environment variable Sergey Kaplun via Tarantool-patches
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:18 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

It is useful to update the environment variable for some tests. For
CMake versions >= 3.22, we can use ENVIRONMENT_MODIFICATION [1] instead.
But unless we bump the CMake version, this macro is a workaround.

[1]: https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT_MODIFICATION.html

Part of tarantool/tarantool#9898
---
 test/tarantool-tests/CMakeLists.txt | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 11a84496..4530c9fd 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -18,6 +18,29 @@ macro(BuildTestCLib lib sources)
   set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
 endmacro()
 
+# FIXME: This is used only due to ancient CMake requirements.
+# If we update to CMake >= 3.22, we can use
+# ENVIRONMENT_MODIFICATION [1] instead.
+# [1]: https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT_MODIFICATION.html
+macro(AppendTestEnvVar testname var value)
+  get_test_property(${testname} ENVIRONMENT old_env)
+  foreach(loopvar "${old_env}")
+    if(loopvar MATCHES "^${var}=(.*)")
+      set(envvar_found TRUE)
+      set(loopvar "${var}=${value}${CMAKE_MATCH_1}")
+    endif()
+    list(APPEND new_env "${loopvar}")
+  endforeach()
+  if(NOT "${envvar_found}")
+    list(APPEND new_env "${var}=${value}")
+  endif()
+  set_tests_properties(${testname} PROPERTIES ENVIRONMENT "${new_env}")
+
+  unset(envvar_found)
+  unset(old_env)
+  unset(new_env)
+endmacro()
+
 add_subdirectory(ffi-ccall)
 add_subdirectory(fix-bit-shift-generation)
 add_subdirectory(gh-4427-ffi-sandwich)
-- 
2.46.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Tarantool-patches] [PATCH luajit 4/7] test: shrink LUA_PATH environment variable
  2024-09-23  7:18 [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
                   ` (2 preceding siblings ...)
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 3/7] cmake: introduce AppendTestEnvVar macro Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:18 ` Sergey Kaplun via Tarantool-patches
  2024-09-23  8:47   ` Sergey Bronnikov via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 5/7] test: shrink LUA_CPATH and {DY}LD_LIBRARY_PATH Sergey Kaplun via Tarantool-patches
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:18 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

This patch removes the default adding of the tools directory to the
LUA_PATH. Now it is done only for profilers tests.

Part of tarantool/tarantool#9898
---
 test/tarantool-tests/CMakeLists.txt | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 4530c9fd..a1339100 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -62,18 +62,16 @@ add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/gnuhash)
 add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/hash)
 add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/stripped)
 
-# The part of the memory profiler toolchain is located in tools
-# directory, jit, profiler, and bytecode toolchains are located
-# in src/ directory, jit/vmdef.lua is autogenerated file also
-# located in src/ directory, but in the scope of the binary
-# artefacts tree and auxiliary tests-related modules are located
+# JIT, profiler, and bytecode toolchains are located in the <src/>
+# directory, <jit/vmdef.lua> is the autogenerated file also
+# located in the <src/> directory, but in the scope of the binary
+# artifacts tree, and auxiliary tests-related modules are located
 # in the current directory (but tests are run in the binary
-# directory), so LUA_PATH need to be updated.
+# directory), so LUA_PATH needs to be updated.
 make_lua_path(LUA_PATH
   PATHS
     ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
     ${CMAKE_CURRENT_SOURCE_DIR}/?/init.lua
-    ${PROJECT_SOURCE_DIR}/tools/?.lua
     ${LUAJIT_SOURCE_DIR}/?.lua
     ${LUAJIT_BINARY_DIR}/?.lua
 )
@@ -159,3 +157,15 @@ foreach(test_path ${tests})
     DEPENDS tarantool-tests-deps
   )
 endforeach()
+
+# The part of the profilers toolchain is located in the <tools/>
+# directory, so LUA_PATH needs to be updated.
+file(GLOB_RECURSE profilers_tests
+  RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
+  "profilers/*${LUA_TEST_SUFFIX}"
+)
+foreach(test_name ${profilers_tests})
+  AppendTestEnvVar("test/${TEST_SUITE_NAME}/${test_name}"
+    LUA_PATH "${PROJECT_SOURCE_DIR}/tools/?.lua\;"
+  )
+endforeach()
-- 
2.46.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Tarantool-patches] [PATCH luajit 5/7] test: shrink LUA_CPATH and {DY}LD_LIBRARY_PATH
  2024-09-23  7:18 [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
                   ` (3 preceding siblings ...)
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 4/7] test: shrink LUA_PATH environment variable Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:18 ` Sergey Kaplun via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 6/7] test: skip flaky tests with enabled table bump Sergey Kaplun via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 7/7] test: set LD_PRELOAD only when necessary Sergey Kaplun via Tarantool-patches
  6 siblings, 0 replies; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:18 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

This patch sets the unique value of each of these variables for each of
the tests and only where they are needed. Also, it drops the comment
about SIP [1] tricks since it is obsolete after
29897567ee5ed57e961c730432c056a3dbaa8f09 ("test: stop using
utils.selfrun in tests").

[1]: https://support.apple.com/en-us/HT204899

Resolves tarantool/tarantool#9898
---
 test/tarantool-tests/CMakeLists.txt | 110 ++++++++++++++++------------
 1 file changed, 63 insertions(+), 47 deletions(-)

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index a1339100..6f1b1ae1 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -8,14 +8,13 @@ add_custom_target(tarantool-tests-libs
 macro(BuildTestCLib lib sources)
   AddTestLib(${lib} ${sources})
   add_dependencies(tarantool-tests-libs ${lib})
-  # Add the directory where the lib is built to the list with
-  # entries for LUA_CPATH environment variable, so LuaJIT can find
-  # and load it. See the comment about extending the list in the
-  # parent scope few lines above.
-  set(LUA_CPATHS "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX};${LUA_CPATHS}" PARENT_SCOPE)
-  # Also add this directory to LD_LIBRARY_PATH environment
-  # variable, so FFI machinery can find and load it.
-  set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
+  # Add libraries to the list to be proceeded with after test
+  # targets are created.
+  # XXX: Workaround for profilers c-related tests, see the comment
+  # below.
+  if(NOT "${CMAKE_CURRENT_BINARY_DIR}" MATCHES profilers)
+    set(CLIB_DIRS "${CMAKE_CURRENT_BINARY_DIR};${CLIB_DIRS}" PARENT_SCOPE)
+  endif()
 endmacro()
 
 # FIXME: This is used only due to ancient CMake requirements.
@@ -76,46 +75,8 @@ make_lua_path(LUA_PATH
     ${LUAJIT_BINARY_DIR}/?.lua
 )
 
-# Update LUA_CPATH with the library paths collected within
-# <BuildTestLib> macro.
-make_lua_path(LUA_CPATH PATHS ${LUA_CPATHS})
-
 set(LUA_TEST_SUFFIX .test.lua)
 
-# XXX: Since the auxiliary libraries are built as a dynamically
-# loaded modules on MacOS instead of shared libraries as it is
-# done on Linux and BSD, another environment variable should be
-# used to guide <ffi.load> while searching the extension.
-# XXX: Be noticed that we shouldn't use `"` here to wrap
-# the variable's content. If we do this, the variable value will
-# contain `"` at the beginning and the end, so this `"` at the
-# beginning will be treated as the directory for the first entry
-# (the last subdirectory added).
-if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
-  # 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 some programs sanitize the environment before they start
-  # child processes. Specifically, environment variables starting
-  # with DYLD_ and LD_ are unset for child process started by
-  # other programs (like `ctest` using for launching tests).
-  # For more info, see the docs[2] below.
-  #
-  # These environment variables are used by FFI machinery to find
-  # the proper shared library, hence we can still tweak testing
-  # environment before calling <ffi.load>. However, the value
-  # can't be passed via the standard environment variable, so we
-  # use ENVIRONMENT property in `set_tests_properties` to get
-  # around SIP magic tricks.
-  #
-  # [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
-  list(APPEND LUA_TEST_ENV_MORE DYLD_LIBRARY_PATH=${LD_LIBRARY_PATH})
-else()
-  list(APPEND LUA_TEST_ENV_MORE LD_LIBRARY_PATH=${LD_LIBRARY_PATH})
-endif()
-
 # Some tests use `LD_PRELOAD` to mock system calls (like
 # <lj-802-panic-at-mcode-protfail.test.lua> overwrites
 # `mprotect()`. When compiling with ASan support under GCC, it is
@@ -152,12 +113,67 @@ foreach(test_path ${tests})
     # LUA_CPATH and LD_LIBRARY_PATH variables and also
     # dependencies list with libraries are set in scope of
     # BuildTestLib macro.
-    ENVIRONMENT "LUA_PATH=${LUA_PATH};LUA_CPATH=${LUA_CPATH};${LUA_TEST_ENV_MORE}"
+    ENVIRONMENT "LUA_PATH=${LUA_PATH};${LUA_TEST_ENV_MORE}"
     LABELS ${TEST_SUITE_NAME}
     DEPENDS tarantool-tests-deps
   )
 endforeach()
 
+# We use the following naming convention: the name of the
+# directory containing the C library to be loaded should match the
+# prefix of the test itself. The same library may be used in
+# several tests. See <lj-1166-error-stitch*>, for example.
+foreach(clib_dir ${CLIB_DIRS})
+  get_filename_component(clib_dir_prefix ${clib_dir} NAME)
+  file(GLOB_RECURSE tests_using_clib
+    RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
+    "${clib_dir_prefix}*${LUA_TEST_SUFFIX}"
+  )
+  foreach(test_name ${tests_using_clib})
+    set(test_title "test/${TEST_SUITE_NAME}/${test_name}")
+    # Add the directory where the library is built to the list
+    # with entries for the LUA_CPATH environment variable, so
+    # LuaJIT can find and load it.
+    AppendTestEnvVar(${test_title}
+      LUA_CPATH "${clib_dir}/?${CMAKE_SHARED_LIBRARY_SUFFIX}\;"
+    )
+    # Also, add this directory to the LD_LIBRARY_PATH environment
+    # variable so FFI machinery can find and load it.
+    # XXX: Be noticed that we shouldn't use `"` here to wrap the
+    # variable's content. If we do this, the variable value will
+    # contain `"` at the beginning and the end, so this `"` at the
+    # beginning will be treated as the directory for the entry.
+    # XXX: Since the auxiliary libraries are built as dynamically
+    # loaded modules on MacOS instead of shared libraries as it is
+    # done on Linux and BSD, another environment variable should
+    # be used to guide <ffi.load> while searching the extension.
+    if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
+      AppendTestEnvVar(${test_title} DYLD_LIBRARY_PATH ${clib_dir}:)
+    else()
+      AppendTestEnvVar(${test_title} LD_LIBRARY_PATH ${clib_dir}:)
+    endif()
+  endforeach()
+endforeach()
+
+# XXX: Special workaround for several different loaded libraries
+# in the <gh-5813-resolving-of-c-symbols.test.lua>. Since the libs
+# can't be flatterned to one directory (see the comment in their
+# <CMakeLists.txt>), just set them manually here.
+set(CSYMBOLS_DIR
+  ${CMAKE_CURRENT_BINARY_DIR}/profilers/gh-5813-resolving-of-c-symbols
+)
+make_lua_path(CSYMBOLS_CPATHS
+  PATHS
+    ${CSYMBOLS_DIR}/both/?${CMAKE_SHARED_LIBRARY_SUFFIX}
+    ${CSYMBOLS_DIR}/gnuhash/?${CMAKE_SHARED_LIBRARY_SUFFIX}
+    ${CSYMBOLS_DIR}/hash/?${CMAKE_SHARED_LIBRARY_SUFFIX}
+    ${CSYMBOLS_DIR}/stripped/?${CMAKE_SHARED_LIBRARY_SUFFIX}
+)
+AppendTestEnvVar(
+  test/${TEST_SUITE_NAME}/profilers/gh-5813-resolving-of-c-symbols.test.lua
+  LUA_CPATH "${CSYMBOLS_CPATHS}"
+)
+
 # The part of the profilers toolchain is located in the <tools/>
 # directory, so LUA_PATH needs to be updated.
 file(GLOB_RECURSE profilers_tests
-- 
2.46.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Tarantool-patches] [PATCH luajit 6/7] test: skip flaky tests with enabled table bump
  2024-09-23  7:18 [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
                   ` (4 preceding siblings ...)
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 5/7] test: shrink LUA_CPATH and {DY}LD_LIBRARY_PATH Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:18 ` Sergey Kaplun via Tarantool-patches
  2024-09-23  9:44   ` Sergey Bronnikov via Tarantool-patches
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 7/7] test: set LD_PRELOAD only when necessary Sergey Kaplun via Tarantool-patches
  6 siblings, 1 reply; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:18 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

Profilers tests with LUAJIT_ENABLE_TABLE_BUMP are flaky due to the bug
in the TDUP patching. Disable them for now.
---
 test/tarantool-tests/CMakeLists.txt              |  6 ++++++
 .../profilers/gh-5688-tool-cli-flag.test.lua     |  2 ++
 .../gh-5813-resolving-of-c-symbols.test.lua      |  2 ++
 .../gh-5994-memprof-human-readable.test.lua      |  2 ++
 ...7264-add-proto-trace-sysprof-default.test.lua |  2 ++
 ...-9217-profile-parsers-error-handling.test.lua |  2 ++
 .../profilers/misclib-memprof-lapi.test.lua      | 16 +++++++++-------
 .../profilers/misclib-sysprof-lapi.test.lua      |  2 ++
 8 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 6f1b1ae1..fe1d8165 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -92,6 +92,12 @@ if(LUAJIT_USE_ASAN AND CMAKE_C_COMPILER_ID STREQUAL "GNU")
   list(APPEND LUA_TEST_ENV_MORE LD_PRELOAD=${LIB_ASAN})
 endif()
 
+# FIXME: This is needed for disabling some flaky tests (like
+# profilers), until LuaJIT/LuaJIT#606 will not be resolved.
+if(LUAJIT_ENABLE_TABLE_BUMP)
+  list(APPEND LUA_TEST_ENV_MORE LUAJIT_TABLE_BUMP=1)
+endif()
+
 set(TEST_SUITE_NAME "tarantool-tests")
 
 # XXX: The call produces both test and target
diff --git a/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
index dd0fd260..f935dc5b 100644
--- a/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
+++ b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
@@ -5,6 +5,8 @@ local test = tap.test('gh-5688-tool-cli-flag'):skipcond({
   ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',
   -- XXX: Tarantool integration is required to run this test properly.
   ['No profile tools CLI option integration'] = _TARANTOOL,
+  -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
+  ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
 })
 
 test:plan(3)
diff --git a/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
index 1581ee4b..74bcd9e8 100644
--- a/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
+++ b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
@@ -3,6 +3,8 @@ 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",
+  -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
+  ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
 })
 
 test:plan(5)
diff --git a/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
index 7c3ff94d..f3041779 100644
--- a/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
+++ b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
@@ -5,6 +5,8 @@ local test = tap.test('gh-5994-memprof-human-readable'):skipcond({
   ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',
   -- XXX: Tarantool integration is required to run this test properly.
   ['No profile tools CLI option integration'] = _TARANTOOL,
+  -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
+  ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
 })
 
 local utils = require('utils')
diff --git a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
index c1d68e3c..176c1a15 100644
--- a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
+++ b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
@@ -4,6 +4,8 @@ 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',
+  -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
+  ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
 })
 
 test:plan(2)
diff --git a/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
index 92f9f59d..65c51198 100644
--- a/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
+++ b/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
@@ -5,6 +5,8 @@ local test = tap.test('gh-9217-profile-parsers-error-handling'):skipcond({
   ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',
   -- XXX: Tarantool integration is required to run this test properly.
   ['No profile tools CLI option integration'] = _TARANTOOL,
+  -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
+  ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
 })
 
 jit.off()
diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
index 728a7ab7..ce41e4d5 100644
--- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
@@ -6,6 +6,8 @@ local test = tap.test("misc-memprof-lapi"):skipcond({
   ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
   ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
                                                jit.arch ~= "x64",
+  -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
+  ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
 })
 
 test:plan(5)
@@ -162,9 +164,9 @@ test:test("output", function(subtest)
   -- one is the number of allocations. 1 event - allocation of
   -- table by itself + 1 allocation of array part as far it is
   -- bigger than LJ_MAX_COLOSIZE (16).
-  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
+  subtest:ok(check_alloc_report(alloc, { line = 39, linedefined = 37 }, 2))
   -- 20 strings allocations.
-  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 20))
+  subtest:ok(check_alloc_report(alloc, { line = 44, linedefined = 37 }, 20))
 
   -- Collect all previous allocated objects.
   subtest:ok(free.INTERNAL == 22)
@@ -172,8 +174,8 @@ test:test("output", function(subtest)
   -- Tests for leak-only option.
   -- See also https://github.com/tarantool/tarantool/issues/5812.
   local heap_delta = process.form_heap_delta(events)
-  local tab_alloc_stats = heap_delta[form_source_line(37)]
-  local str_alloc_stats = heap_delta[form_source_line(42)]
+  local tab_alloc_stats = heap_delta[form_source_line(39)]
+  local str_alloc_stats = heap_delta[form_source_line(44)]
   subtest:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
   subtest:ok(tab_alloc_stats.dbytes == 0)
   subtest:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree)
@@ -258,10 +260,10 @@ test:test("jit-output", function(subtest)
   -- 10 allocations in interpreter mode, 1 allocation for a trace
   -- recording and assembling and next 9 allocations will happen
   -- while running the trace.
-  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 11))
-  subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 40 }, 9))
+  subtest:ok(check_alloc_report(alloc, { line = 44, linedefined = 37 }, 11))
+  subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 42 }, 9))
   -- See same checks with jit.off().
-  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
+  subtest:ok(check_alloc_report(alloc, { line = 39, linedefined = 37 }, 2))
 
   -- Restore default JIT settings.
   jit.opt.start(unpack(jit_opt_default))
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index fdaed46a..26c277cd 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -3,6 +3,8 @@ 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",
+  -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
+  ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
 })
 
 test:plan(19)
-- 
2.46.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Tarantool-patches] [PATCH luajit 7/7] test: set LD_PRELOAD only when necessary
  2024-09-23  7:18 [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
                   ` (5 preceding siblings ...)
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 6/7] test: skip flaky tests with enabled table bump Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:18 ` Sergey Kaplun via Tarantool-patches
  6 siblings, 0 replies; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:18 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: tarantool-patches

This patch sets LD_PRELOAD for ASan build for the required tests instead
of all tests.

Follows up tarantool/tarantool#9898
---
 test/tarantool-tests/CMakeLists.txt | 33 ++++++++++++++++-------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index fe1d8165..d4c7a5a0 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -77,21 +77,6 @@ make_lua_path(LUA_PATH
 
 set(LUA_TEST_SUFFIX .test.lua)
 
-# Some tests use `LD_PRELOAD` to mock system calls (like
-# <lj-802-panic-at-mcode-protfail.test.lua> overwrites
-# `mprotect()`. When compiling with ASan support under GCC, it is
-# required that the ASan library go first in the `LD_PRELOAD`
-# list. Set it manually. The test will append it to the executed
-# process.
-if(LUAJIT_USE_ASAN AND CMAKE_C_COMPILER_ID STREQUAL "GNU")
-  # FIXME: We should set this environment variable only
-  # for the corresponding tests to avoid warnings from
-  # the GNU libc and other libc implementations.
-  # See https://github.com/tarantool/tarantool/issues/9898.
-  LibRealPath(LIB_ASAN libasan.so)
-  list(APPEND LUA_TEST_ENV_MORE LD_PRELOAD=${LIB_ASAN})
-endif()
-
 # FIXME: This is needed for disabling some flaky tests (like
 # profilers), until LuaJIT/LuaJIT#606 will not be resolved.
 if(LUAJIT_ENABLE_TABLE_BUMP)
@@ -191,3 +176,21 @@ foreach(test_name ${profilers_tests})
     LUA_PATH "${PROJECT_SOURCE_DIR}/tools/?.lua\;"
   )
 endforeach()
+
+# Some tests use `LD_PRELOAD` to mock system calls (like
+# <lj-802-panic-at-mcode-protfail.test.lua> overwrites
+# `mprotect()`). When compiling with ASan support under GCC, it is
+# required that the ASan library go first in the `LD_PRELOAD`
+# list. Set it manually. The test will append it to the executed
+# process.
+if(LUAJIT_USE_ASAN AND CMAKE_C_COMPILER_ID STREQUAL "GNU")
+  LibRealPath(LIB_ASAN libasan.so)
+  AppendTestEnvVar(
+    "test/${TEST_SUITE_NAME}/lj-522-fix-dlerror-return-null.test.lua"
+    LD_PRELOAD ${LIB_ASAN}
+  )
+  AppendTestEnvVar(
+    "test/${TEST_SUITE_NAME}/lj-802-panic-at-mcode-protfail.test.lua"
+    LD_PRELOAD ${LIB_ASAN}
+  )
+endif()
-- 
2.46.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/7] test: move profilers tests to subdirectory
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 1/7] test: move profilers tests to subdirectory Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:40   ` Sergey Bronnikov via Tarantool-patches
  2024-09-23  7:51     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 16+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-09-23  7:40 UTC (permalink / raw)
  To: Sergey Kaplun, Maxim Kokryashkin; +Cc: tarantool-patches

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

Hi, Sergey


thanks for the patch! See a comment below.


On 23.09.2024 10:18, Sergey Kaplun wrote:
> It is useful when we want to change the properties for these tests only.
>
> Needed for tarantool/tarantool#9898
> ---
>   test/tarantool-tests/CMakeLists.txt                    | 10 +++++-----
>   .../{ => profilers}/gh-5688-tool-cli-flag.test.lua     |  0
>   .../gh-5813-resolving-of-c-symbols.test.lua            |  0
>   .../gh-5813-resolving-of-c-symbols/both/CMakeLists.txt |  0
>   .../gh-5813-resolving-of-c-symbols/both/resboth.c      |  0
>   .../gnuhash/CMakeLists.txt                             |  0
>   .../gnuhash/resgnuhash.c                               |  0
>   .../gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt |  0
>   .../gh-5813-resolving-of-c-symbols/hash/reshash.c      |  0
>   .../stripped/CMakeLists.txt                            |  0
>   .../stripped/resstripped.c                             |  0
>   .../gh-5994-memprof-human-readable.test.lua            |  0
>   .../gh-7264-add-proto-trace-sysprof-default.test.lua   |  0
>   .../gh-9217-profile-parsers-error-handling.test.lua    |  0
>   .../{ => profilers}/misclib-memprof-lapi.test.lua      |  0
>   .../{ => profilers}/misclib-sysprof-lapi.test.lua      |  0
>   .../{ => profilers}/tools-utils-avl.test.lua           |  0
>   17 files changed, 5 insertions(+), 5 deletions(-)
>   rename test/tarantool-tests/{ => profilers}/gh-5688-tool-cli-flag.test.lua (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols.test.lua (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/both/resboth.c (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/hash/reshash.c (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/stripped/resstripped.c (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-5994-memprof-human-readable.test.lua (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-7264-add-proto-trace-sysprof-default.test.lua (100%)
>   rename test/tarantool-tests/{ => profilers}/gh-9217-profile-parsers-error-handling.test.lua (100%)
>   rename test/tarantool-tests/{ => profilers}/misclib-memprof-lapi.test.lua (100%)
>   rename test/tarantool-tests/{ => profilers}/misclib-sysprof-lapi.test.lua (100%)
>   rename test/tarantool-tests/{ => profilers}/tools-utils-avl.test.lua (100%)
>
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 0bd3e6fc..11a84496 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -21,10 +21,6 @@ endmacro()
>   add_subdirectory(ffi-ccall)
>   add_subdirectory(fix-bit-shift-generation)
>   add_subdirectory(gh-4427-ffi-sandwich)
> -add_subdirectory(gh-5813-resolving-of-c-symbols/both)
> -add_subdirectory(gh-5813-resolving-of-c-symbols/hash)
> -add_subdirectory(gh-5813-resolving-of-c-symbols/gnuhash)
> -add_subdirectory(gh-5813-resolving-of-c-symbols/stripped)

Should we move these tests as well?

test/tarantool-tests/lj-726-profile-flush-close.test.lua

test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua


or did you mean only tarantool's profilers in commit message.

Please clarify it in commit message if it is so.

>   add_subdirectory(gh-6098-fix-side-exit-patching-on-arm64)
>   add_subdirectory(gh-6189-cur_L)
>   add_subdirectory(lj-416-xor-before-jcc)
> @@ -38,6 +34,10 @@ add_subdirectory(lj-flush-on-trace)
>   add_subdirectory(lj-1004-oom-error-frame)
>   add_subdirectory(lj-1066-fix-cur_L-after-coroutine-resume)
>   add_subdirectory(lj-1166-error-stitch)
> +add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/both)
> +add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/gnuhash)
> +add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/hash)
> +add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/stripped)
>   
>   # The part of the memory profiler toolchain is located in tools
>   # directory, jit, profiler, and bytecode toolchains are located
> @@ -121,7 +121,7 @@ add_test_suite_target(tarantool-tests
>   
>   file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")
>   foreach(test_path ${tests})
> -  get_filename_component(test_name ${test_path} NAME)
> +  file(RELATIVE_PATH test_name ${CMAKE_CURRENT_SOURCE_DIR} ${test_path})
>     set(test_title "test/${TEST_SUITE_NAME}/${test_name}")
>     add_test(NAME ${test_title}
>       COMMAND ${LUAJIT_TEST_COMMAND} ${test_path}
> diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
> similarity index 100%
> rename from test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
> rename to test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/resboth.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/resboth.c
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/resboth.c
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/resboth.c
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/reshash.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/reshash.c
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/reshash.c
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/reshash.c
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
> diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/resstripped.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
> similarity index 100%
> rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
> rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
> diff --git a/test/tarantool-tests/gh-5994-memprof-human-readable.test.lua b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
> similarity index 100%
> rename from test/tarantool-tests/gh-5994-memprof-human-readable.test.lua
> rename to test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
> diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> similarity index 100%
> rename from test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> rename to test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> diff --git a/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
> similarity index 100%
> rename from test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
> rename to test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
> diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
> similarity index 100%
> rename from test/tarantool-tests/misclib-memprof-lapi.test.lua
> rename to test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
> diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> similarity index 100%
> rename from test/tarantool-tests/misclib-sysprof-lapi.test.lua
> rename to test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> diff --git a/test/tarantool-tests/tools-utils-avl.test.lua b/test/tarantool-tests/profilers/tools-utils-avl.test.lua
> similarity index 100%
> rename from test/tarantool-tests/tools-utils-avl.test.lua
> rename to test/tarantool-tests/profilers/tools-utils-avl.test.lua

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/7] test: rename <arm64-ccall-fp-convention.test.lua>
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 2/7] test: rename <arm64-ccall-fp-convention.test.lua> Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:45   ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 16+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-09-23  7:45 UTC (permalink / raw)
  To: Sergey Kaplun, Maxim Kokryashkin; +Cc: tarantool-patches

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

Hi, Sergey,

thanks for the patch! LGTM

On 23.09.2024 10:18, Sergey Kaplun wrote:
> This patch renames the aforementioned file to
> <ffi-ccall-arm64-fp-convention.test.lua> to make the prefix (ffi-ccall)
> match the name of the directory containing the C library for the test.
>
> Needed for tarantool/tarantool#9898
> ---
>   ...nvention.test.lua => ffi-ccall-arm64-fp-convention.test.lua} | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>   rename test/tarantool-tests/{arm64-ccall-fp-convention.test.lua => ffi-ccall-arm64-fp-convention.test.lua} (96%)
>
> diff --git a/test/tarantool-tests/arm64-ccall-fp-convention.test.lua b/test/tarantool-tests/ffi-ccall-arm64-fp-convention.test.lua
> similarity index 96%
> rename from test/tarantool-tests/arm64-ccall-fp-convention.test.lua
> rename to test/tarantool-tests/ffi-ccall-arm64-fp-convention.test.lua
> index c58ba697..053b7a3a 100644
> --- a/test/tarantool-tests/arm64-ccall-fp-convention.test.lua
> +++ b/test/tarantool-tests/ffi-ccall-arm64-fp-convention.test.lua
> @@ -3,7 +3,7 @@ local tap = require('tap')
>   
>   local ffi_ccall = ffi.load('libfficcall')
>   
> -local test = tap.test('arm64-ccall-fp-convention')
> +local test = tap.test('ffi-ccall-arm64-fp-convention')
>   test:plan(3)
>   
>   ffi.cdef[[

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/7] cmake: introduce AppendTestEnvVar macro
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 3/7] cmake: introduce AppendTestEnvVar macro Sergey Kaplun via Tarantool-patches
@ 2024-09-23  7:51   ` Sergey Bronnikov via Tarantool-patches
  2024-09-23  8:18     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 16+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-09-23  7:51 UTC (permalink / raw)
  To: Sergey Kaplun, Maxim Kokryashkin; +Cc: tarantool-patches

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

Hi, Sergey,

thanks for the patch! See comments below.

On 23.09.2024 10:18, Sergey Kaplun wrote:
> It is useful to update the environment variable for some tests. For
> CMake versions >= 3.22, we can use ENVIRONMENT_MODIFICATION [1] instead.
> But unless we bump the CMake version, this macro is a workaround.
>
> [1]:https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT_MODIFICATION.html
>
> Part of tarantool/tarantool#9898
> ---
>   test/tarantool-tests/CMakeLists.txt | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 11a84496..4530c9fd 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -18,6 +18,29 @@ macro(BuildTestCLib lib sources)
>     set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
>   endmacro()
>   
> +# FIXME: This is used only due to ancient CMake requirements.
> +# If we update to CMake >= 3.22, we can use
> +# ENVIRONMENT_MODIFICATION [1] instead.
> +# [1]:https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT_MODIFICATION.html
I would add a small description for a macro.
> +macro(AppendTestEnvVar testname var value)

I would rename macro to something like "ModifyTestEnv"

because it appends env var when variable with the same name was not found

and modifies it when variable exist.


> +  get_test_property(${testname} ENVIRONMENT old_env)
> +  foreach(loopvar "${old_env}")
> +    if(loopvar MATCHES "^${var}=(.*)")
> +      set(envvar_found TRUE)
> +      set(loopvar "${var}=${value}${CMAKE_MATCH_1}")
> +    endif()
> +    list(APPEND new_env "${loopvar}")
> +  endforeach()
> +  if(NOT "${envvar_found}")
> +    list(APPEND new_env "${var}=${value}")
> +  endif()
> +  set_tests_properties(${testname} PROPERTIES ENVIRONMENT "${new_env}")
> +
> +  unset(envvar_found)
> +  unset(old_env)
> +  unset(new_env)
> +endmacro()
> +
>   add_subdirectory(ffi-ccall)
>   add_subdirectory(fix-bit-shift-generation)
>   add_subdirectory(gh-4427-ffi-sandwich)

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/7] test: move profilers tests to subdirectory
  2024-09-23  7:40   ` Sergey Bronnikov via Tarantool-patches
@ 2024-09-23  7:51     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  7:51 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Hi, Sergey!
Thanks for the review!

On 23.09.24, Sergey Bronnikov wrote:
> Hi, Sergey
> 
> 
> thanks for the patch! See a comment below.

Updated the commit message.

> 
> 
> On 23.09.2024 10:18, Sergey Kaplun wrote:
> > It is useful when we want to change the properties for these tests only.
> >
> > Needed for tarantool/tarantool#9898
> > ---
> >   test/tarantool-tests/CMakeLists.txt                    | 10 +++++-----
> >   .../{ => profilers}/gh-5688-tool-cli-flag.test.lua     |  0
> >   .../gh-5813-resolving-of-c-symbols.test.lua            |  0
> >   .../gh-5813-resolving-of-c-symbols/both/CMakeLists.txt |  0
> >   .../gh-5813-resolving-of-c-symbols/both/resboth.c      |  0
> >   .../gnuhash/CMakeLists.txt                             |  0
> >   .../gnuhash/resgnuhash.c                               |  0
> >   .../gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt |  0
> >   .../gh-5813-resolving-of-c-symbols/hash/reshash.c      |  0
> >   .../stripped/CMakeLists.txt                            |  0
> >   .../stripped/resstripped.c                             |  0
> >   .../gh-5994-memprof-human-readable.test.lua            |  0
> >   .../gh-7264-add-proto-trace-sysprof-default.test.lua   |  0
> >   .../gh-9217-profile-parsers-error-handling.test.lua    |  0
> >   .../{ => profilers}/misclib-memprof-lapi.test.lua      |  0
> >   .../{ => profilers}/misclib-sysprof-lapi.test.lua      |  0
> >   .../{ => profilers}/tools-utils-avl.test.lua           |  0
> >   17 files changed, 5 insertions(+), 5 deletions(-)
> >   rename test/tarantool-tests/{ => profilers}/gh-5688-tool-cli-flag.test.lua (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols.test.lua (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/both/resboth.c (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/hash/reshash.c (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5813-resolving-of-c-symbols/stripped/resstripped.c (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-5994-memprof-human-readable.test.lua (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-7264-add-proto-trace-sysprof-default.test.lua (100%)
> >   rename test/tarantool-tests/{ => profilers}/gh-9217-profile-parsers-error-handling.test.lua (100%)
> >   rename test/tarantool-tests/{ => profilers}/misclib-memprof-lapi.test.lua (100%)
> >   rename test/tarantool-tests/{ => profilers}/misclib-sysprof-lapi.test.lua (100%)
> >   rename test/tarantool-tests/{ => profilers}/tools-utils-avl.test.lua (100%)
> >
> > diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> > index 0bd3e6fc..11a84496 100644
> > --- a/test/tarantool-tests/CMakeLists.txt
> > +++ b/test/tarantool-tests/CMakeLists.txt
> > @@ -21,10 +21,6 @@ endmacro()
> >   add_subdirectory(ffi-ccall)
> >   add_subdirectory(fix-bit-shift-generation)
> >   add_subdirectory(gh-4427-ffi-sandwich)
> > -add_subdirectory(gh-5813-resolving-of-c-symbols/both)
> > -add_subdirectory(gh-5813-resolving-of-c-symbols/hash)
> > -add_subdirectory(gh-5813-resolving-of-c-symbols/gnuhash)
> > -add_subdirectory(gh-5813-resolving-of-c-symbols/stripped)
> 
> Should we move these tests as well?
> 
> test/tarantool-tests/lj-726-profile-flush-close.test.lua
> 
> test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> 
> 
> or did you mean only tarantool's profilers in commit message.

Yes this one.

> 
> Please clarify it in commit message if it is so.

The new commit message is the following. Branch is force-pushed.

| test: move profilers tests to subdirectory
|
| This patch moves tests for Tarantool's profilers like memprof and
| sysprof to the corresponding subdirectory.
| It is useful when we want to change the properties for these tests only.
|
| Needed for tarantool/tarantool#9898

> 
> >   add_subdirectory(gh-6098-fix-side-exit-patching-on-arm64)
> >   add_subdirectory(gh-6189-cur_L)
> >   add_subdirectory(lj-416-xor-before-jcc)
> > @@ -38,6 +34,10 @@ add_subdirectory(lj-flush-on-trace)
> >   add_subdirectory(lj-1004-oom-error-frame)
> >   add_subdirectory(lj-1066-fix-cur_L-after-coroutine-resume)
> >   add_subdirectory(lj-1166-error-stitch)
> > +add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/both)
> > +add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/gnuhash)
> > +add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/hash)
> > +add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/stripped)
> >   
> >   # The part of the memory profiler toolchain is located in tools
> >   # directory, jit, profiler, and bytecode toolchains are located
> > @@ -121,7 +121,7 @@ add_test_suite_target(tarantool-tests
> >   
> >   file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")
> >   foreach(test_path ${tests})
> > -  get_filename_component(test_name ${test_path} NAME)
> > +  file(RELATIVE_PATH test_name ${CMAKE_CURRENT_SOURCE_DIR} ${test_path})
> >     set(test_title "test/${TEST_SUITE_NAME}/${test_name}")
> >     add_test(NAME ${test_title}
> >       COMMAND ${LUAJIT_TEST_COMMAND} ${test_path}
> > diff --git a/test/tarantool-tests/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5688-tool-cli-flag.test.lua
> > rename to test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols.test.lua
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/CMakeLists.txt
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/resboth.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/resboth.c
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/both/resboth.c
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/both/resboth.c
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/CMakeLists.txt
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/gnuhash/resgnuhash.c
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/CMakeLists.txt
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/reshash.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/reshash.c
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/hash/reshash.c
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/hash/reshash.c
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/CMakeLists.txt
> > diff --git a/test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/resstripped.c b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
> > rename to test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols/stripped/resstripped.c
> > diff --git a/test/tarantool-tests/gh-5994-memprof-human-readable.test.lua b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
> > similarity index 100%
> > rename from test/tarantool-tests/gh-5994-memprof-human-readable.test.lua
> > rename to test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
> > diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> > similarity index 100%
> > rename from test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> > rename to test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> > diff --git a/test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
> > similarity index 100%
> > rename from test/tarantool-tests/gh-9217-profile-parsers-error-handling.test.lua
> > rename to test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
> > diff --git a/test/tarantool-tests/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
> > similarity index 100%
> > rename from test/tarantool-tests/misclib-memprof-lapi.test.lua
> > rename to test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
> > diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> > similarity index 100%
> > rename from test/tarantool-tests/misclib-sysprof-lapi.test.lua
> > rename to test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> > diff --git a/test/tarantool-tests/tools-utils-avl.test.lua b/test/tarantool-tests/profilers/tools-utils-avl.test.lua
> > similarity index 100%
> > rename from test/tarantool-tests/tools-utils-avl.test.lua
> > rename to test/tarantool-tests/profilers/tools-utils-avl.test.lua

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/7] cmake: introduce AppendTestEnvVar macro
  2024-09-23  7:51   ` Sergey Bronnikov via Tarantool-patches
@ 2024-09-23  8:18     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23  8:18 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Hi, Sergey!
Thanks for the review!
See my answers below.

On 23.09.24, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch! See comments below.
> 
> On 23.09.2024 10:18, Sergey Kaplun wrote:
> > It is useful to update the environment variable for some tests. For
> > CMake versions >= 3.22, we can use ENVIRONMENT_MODIFICATION [1] instead.
> > But unless we bump the CMake version, this macro is a workaround.
> >
> > [1]:https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT_MODIFICATION.html
> >
> > Part of tarantool/tarantool#9898
> > ---
> >   test/tarantool-tests/CMakeLists.txt | 23 +++++++++++++++++++++++
> >   1 file changed, 23 insertions(+)
> >
> > diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> > index 11a84496..4530c9fd 100644
> > --- a/test/tarantool-tests/CMakeLists.txt
> > +++ b/test/tarantool-tests/CMakeLists.txt
> > @@ -18,6 +18,29 @@ macro(BuildTestCLib lib sources)
> >     set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
> >   endmacro()
> >   
> > +# FIXME: This is used only due to ancient CMake requirements.
> > +# If we update to CMake >= 3.22, we can use
> > +# ENVIRONMENT_MODIFICATION [1] instead.
> > +# [1]:https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT_MODIFICATION.html
> I would add a small description for a macro.

Added the description. Branch is force-pushed.
===================================================================
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 4530c9fd..a5f3efcd 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -22,6 +22,9 @@ endmacro()
 # If we update to CMake >= 3.22, we can use
 # ENVIRONMENT_MODIFICATION [1] instead.
 # [1]: https://cmake.org/cmake/help/latest/prop_test/ENVIRONMENT_MODIFICATION.html
+# This macro appends to the given test's environment variable the
+# given value. If the variable doesn't exist, it is created equal
+# to the given argument.
 macro(AppendTestEnvVar testname var value)
   get_test_property(${testname} ENVIRONMENT old_env)
   foreach(loopvar "${old_env}")
===================================================================

> > +macro(AppendTestEnvVar testname var value)
> 
> I would rename macro to something like "ModifyTestEnv"
> 
> because it appends env var when variable with the same name was not found
> 
> and modifies it when variable exist.

I prefer to leave AppendTestEnvVar (since "ModifyTestEnv" lacks the
description of the Append part) and clarify its behaviour with a
comment.

> 
> 

<snipped>

> >   add_subdirectory(gh-4427-ffi-sandwich)

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 4/7] test: shrink LUA_PATH environment variable
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 4/7] test: shrink LUA_PATH environment variable Sergey Kaplun via Tarantool-patches
@ 2024-09-23  8:47   ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 16+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-09-23  8:47 UTC (permalink / raw)
  To: Sergey Kaplun, Maxim Kokryashkin; +Cc: tarantool-patches

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

Hi, Sergey

thanks for the patch! See comments below:


I would replace a short commit description with

"shrink LUA_PATH environment variable for profiler tests"


On 23.09.2024 10:18, Sergey Kaplun wrote:
> This patch removes the default adding of the tools directory to the
> LUA_PATH. Now it is done only for profilers tests.
>
> Part of tarantool/tarantool#9898
> ---
>   test/tarantool-tests/CMakeLists.txt | 24 +++++++++++++++++-------
>   1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 4530c9fd..a1339100 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -62,18 +62,16 @@ add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/gnuhash)
>   add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/hash)
>   add_subdirectory(profilers/gh-5813-resolving-of-c-symbols/stripped)
>   
> -# The part of the memory profiler toolchain is located in tools
> -# directory, jit, profiler, and bytecode toolchains are located
> -# in src/ directory, jit/vmdef.lua is autogenerated file also
> -# located in src/ directory, but in the scope of the binary
> -# artefacts tree and auxiliary tests-related modules are located
> +# JIT, profiler, and bytecode toolchains are located in the <src/>
> +# directory, <jit/vmdef.lua> is the autogenerated file also
> +# located in the <src/> directory, but in the scope of the binary
> +# artifacts tree, and auxiliary tests-related modules are located
>   # in the current directory (but tests are run in the binary
> -# directory), so LUA_PATH need to be updated.
> +# directory), so LUA_PATH needs to be updated.
>   make_lua_path(LUA_PATH
>     PATHS
>       ${CMAKE_CURRENT_SOURCE_DIR}/?.lua
>       ${CMAKE_CURRENT_SOURCE_DIR}/?/init.lua
> -    ${PROJECT_SOURCE_DIR}/tools/?.lua
>       ${LUAJIT_SOURCE_DIR}/?.lua
>       ${LUAJIT_BINARY_DIR}/?.lua
>   )
> @@ -159,3 +157,15 @@ foreach(test_path ${tests})
>       DEPENDS tarantool-tests-deps
>     )
>   endforeach()
> +
> +# The part of the profilers toolchain is located in the <tools/>
> +# directory, so LUA_PATH needs to be updated.
> +file(GLOB_RECURSE profilers_tests
> +  RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
> +  "profilers/*${LUA_TEST_SUFFIX}"
> +)
> +foreach(test_name ${profilers_tests})
> +  AppendTestEnvVar("test/${TEST_SUITE_NAME}/${test_name}"
> +    LUA_PATH "${PROJECT_SOURCE_DIR}/tools/?.lua\;"
> +  )
> +endforeach()

discussed verbally,

let's move this to a main foreach:


--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -156,16 +156,12 @@ foreach(test_path ${tests})
      LABELS ${TEST_SUITE_NAME}
      DEPENDS tarantool-tests-deps
    )
-endforeach()

-# The part of the profilers toolchain is located in the <tools/>
-# directory, so LUA_PATH needs to be updated.
-file(GLOB_RECURSE profilers_tests
-  RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
-  "profilers/*${LUA_TEST_SUFFIX}"
-)
-foreach(test_name ${profilers_tests})
-  AppendTestEnvVar("test/${TEST_SUITE_NAME}/${test_name}"
-    LUA_PATH "${PROJECT_SOURCE_DIR}/tools/?.lua\;"
-  )
+  # The part of the profilers toolchain is located in the <tools/>
+  # directory, so LUA_PATH needs to be updated.
+  if(test_name MATCHES "^profilers")
+    AppendTestEnvVar("${test_title}"
+      LUA_PATH "${PROJECT_SOURCE_DIR}/tools/?.lua\;"
+    )
+  endif()
  endforeach()

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 6/7] test: skip flaky tests with enabled table bump
  2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 6/7] test: skip flaky tests with enabled table bump Sergey Kaplun via Tarantool-patches
@ 2024-09-23  9:44   ` Sergey Bronnikov via Tarantool-patches
  2024-09-23 11:08     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 16+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-09-23  9:44 UTC (permalink / raw)
  To: Sergey Kaplun, Maxim Kokryashkin; +Cc: tarantool-patches

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

Hi, Sergey!

thanks for the patch!

I would add our own issue as a reminder instead of LuaJIT issue

and link them to each other.

On 23.09.2024 10:18, Sergey Kaplun wrote:
> Profilers tests with LUAJIT_ENABLE_TABLE_BUMP are flaky due to the bug
> in the TDUP patching. Disable them for now.
> ---
>   test/tarantool-tests/CMakeLists.txt              |  6 ++++++
>   .../profilers/gh-5688-tool-cli-flag.test.lua     |  2 ++
>   .../gh-5813-resolving-of-c-symbols.test.lua      |  2 ++
>   .../gh-5994-memprof-human-readable.test.lua      |  2 ++
>   ...7264-add-proto-trace-sysprof-default.test.lua |  2 ++
>   ...-9217-profile-parsers-error-handling.test.lua |  2 ++
>   .../profilers/misclib-memprof-lapi.test.lua      | 16 +++++++++-------
>   .../profilers/misclib-sysprof-lapi.test.lua      |  2 ++
>   8 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 6f1b1ae1..fe1d8165 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -92,6 +92,12 @@ if(LUAJIT_USE_ASAN AND CMAKE_C_COMPILER_ID STREQUAL "GNU")
>     list(APPEND LUA_TEST_ENV_MORE LD_PRELOAD=${LIB_ASAN})
>   endif()
>   
> +# FIXME: This is needed for disabling some flaky tests (like
> +# profilers), until LuaJIT/LuaJIT#606 will not be resolved.
> +if(LUAJIT_ENABLE_TABLE_BUMP)
> +  list(APPEND LUA_TEST_ENV_MORE LUAJIT_TABLE_BUMP=1)
> +endif()
> +
>   set(TEST_SUITE_NAME "tarantool-tests")
>   
>   # XXX: The call produces both test and target
> diff --git a/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
> index dd0fd260..f935dc5b 100644
> --- a/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
> +++ b/test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
> @@ -5,6 +5,8 @@ local test = tap.test('gh-5688-tool-cli-flag'):skipcond({
>     ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',
>     -- XXX: Tarantool integration is required to run this test properly.
>     ['No profile tools CLI option integration'] = _TARANTOOL,
> +  -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
> +  ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
>   })
>   
>   test:plan(3)
> diff --git a/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
> index 1581ee4b..74bcd9e8 100644
> --- a/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
> +++ b/test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
> @@ -3,6 +3,8 @@ 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",
> +  -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
> +  ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
>   })
>   
>   test:plan(5)
> diff --git a/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
> index 7c3ff94d..f3041779 100644
> --- a/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
> +++ b/test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
> @@ -5,6 +5,8 @@ local test = tap.test('gh-5994-memprof-human-readable'):skipcond({
>     ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',
>     -- XXX: Tarantool integration is required to run this test properly.
>     ['No profile tools CLI option integration'] = _TARANTOOL,
> +  -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
> +  ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
>   })
>   
>   local utils = require('utils')
> diff --git a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> index c1d68e3c..176c1a15 100644
> --- a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> +++ b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> @@ -4,6 +4,8 @@ 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',
> +  -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
> +  ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
>   })
>   
>   test:plan(2)
> diff --git a/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua b/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
> index 92f9f59d..65c51198 100644
> --- a/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
> +++ b/test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
> @@ -5,6 +5,8 @@ local test = tap.test('gh-9217-profile-parsers-error-handling'):skipcond({
>     ['Profile tools are implemented for Linux only'] = jit.os ~= 'Linux',
>     -- XXX: Tarantool integration is required to run this test properly.
>     ['No profile tools CLI option integration'] = _TARANTOOL,
> +  -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
> +  ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
>   })
>   
>   jit.off()
> diff --git a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
> index 728a7ab7..ce41e4d5 100644
> --- a/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
> @@ -6,6 +6,8 @@ local test = tap.test("misc-memprof-lapi"):skipcond({
>     ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
>     ["Memprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
>                                                  jit.arch ~= "x64",
> +  -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
> +  ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
>   })
>   
>   test:plan(5)
> @@ -162,9 +164,9 @@ test:test("output", function(subtest)
>     -- one is the number of allocations. 1 event - allocation of
>     -- table by itself + 1 allocation of array part as far it is
>     -- bigger than LJ_MAX_COLOSIZE (16).
> -  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
> +  subtest:ok(check_alloc_report(alloc, { line = 39, linedefined = 37 }, 2))
>     -- 20 strings allocations.
> -  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 20))
> +  subtest:ok(check_alloc_report(alloc, { line = 44, linedefined = 37 }, 20))
>   
>     -- Collect all previous allocated objects.
>     subtest:ok(free.INTERNAL == 22)
> @@ -172,8 +174,8 @@ test:test("output", function(subtest)
>     -- Tests for leak-only option.
>     -- See alsohttps://github.com/tarantool/tarantool/issues/5812.
>     local heap_delta = process.form_heap_delta(events)
> -  local tab_alloc_stats = heap_delta[form_source_line(37)]
> -  local str_alloc_stats = heap_delta[form_source_line(42)]
> +  local tab_alloc_stats = heap_delta[form_source_line(39)]
> +  local str_alloc_stats = heap_delta[form_source_line(44)]
>     subtest:ok(tab_alloc_stats.nalloc == tab_alloc_stats.nfree)
>     subtest:ok(tab_alloc_stats.dbytes == 0)
>     subtest:ok(str_alloc_stats.nalloc == str_alloc_stats.nfree)
> @@ -258,10 +260,10 @@ test:test("jit-output", function(subtest)
>     -- 10 allocations in interpreter mode, 1 allocation for a trace
>     -- recording and assembling and next 9 allocations will happen
>     -- while running the trace.
> -  subtest:ok(check_alloc_report(alloc, { line = 42, linedefined = 35 }, 11))
> -  subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 40 }, 9))
> +  subtest:ok(check_alloc_report(alloc, { line = 44, linedefined = 37 }, 11))
> +  subtest:ok(check_alloc_report(alloc, { traceno = 1, line = 42 }, 9))
>     -- See same checks with jit.off().
> -  subtest:ok(check_alloc_report(alloc, { line = 37, linedefined = 35 }, 2))
> +  subtest:ok(check_alloc_report(alloc, { line = 39, linedefined = 37 }, 2))
>   
>     -- Restore default JIT settings.
>     jit.opt.start(unpack(jit_opt_default))
> diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> index fdaed46a..26c277cd 100644
> --- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> +++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> @@ -3,6 +3,8 @@ 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",
> +  -- See alsohttps://github.com/LuaJIT/LuaJIT/issues/606.
> +  ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
>   })
>   
>   test:plan(19)

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 6/7] test: skip flaky tests with enabled table bump
  2024-09-23  9:44   ` Sergey Bronnikov via Tarantool-patches
@ 2024-09-23 11:08     ` Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; 16+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-09-23 11:08 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Hi, Sergey!
Thanks for the review!
Please, consider my answer below.

On 23.09.24, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> thanks for the patch!
> 
> I would add our own issue as a reminder instead of LuaJIT issue
> 
> and link them to each other.

I suppose this is excess since the Tarantool isn't affected (since we
don't enable table bump optimization due to the known issues with it).
Also, it is not clear when we can take this issue to work (and even will
we or not).
At the same time, the original issue is referenced. But even then, we
don't know if Mike has any plans for it or if this optimization will
just be dropped anyway in v3.0.

> 
> On 23.09.2024 10:18, Sergey Kaplun wrote:
> > Profilers tests with LUAJIT_ENABLE_TABLE_BUMP are flaky due to the bug
> > in the TDUP patching. Disable them for now.
> > ---

<snipped>

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-09-23 11:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-23  7:18 [Tarantool-patches] [PATCH luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 1/7] test: move profilers tests to subdirectory Sergey Kaplun via Tarantool-patches
2024-09-23  7:40   ` Sergey Bronnikov via Tarantool-patches
2024-09-23  7:51     ` Sergey Kaplun via Tarantool-patches
2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 2/7] test: rename <arm64-ccall-fp-convention.test.lua> Sergey Kaplun via Tarantool-patches
2024-09-23  7:45   ` Sergey Bronnikov via Tarantool-patches
2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 3/7] cmake: introduce AppendTestEnvVar macro Sergey Kaplun via Tarantool-patches
2024-09-23  7:51   ` Sergey Bronnikov via Tarantool-patches
2024-09-23  8:18     ` Sergey Kaplun via Tarantool-patches
2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 4/7] test: shrink LUA_PATH environment variable Sergey Kaplun via Tarantool-patches
2024-09-23  8:47   ` Sergey Bronnikov via Tarantool-patches
2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 5/7] test: shrink LUA_CPATH and {DY}LD_LIBRARY_PATH Sergey Kaplun via Tarantool-patches
2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 6/7] test: skip flaky tests with enabled table bump Sergey Kaplun via Tarantool-patches
2024-09-23  9:44   ` Sergey Bronnikov via Tarantool-patches
2024-09-23 11:08     ` Sergey Kaplun via Tarantool-patches
2024-09-23  7:18 ` [Tarantool-patches] [PATCH luajit 7/7] test: set LD_PRELOAD only when necessary Sergey Kaplun via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox