[Tarantool-patches] [PATCH luajit 2/2] test: skip sysprof tests with LUAJIT_DISABLE_SYSPROF
Sergey Kaplun
skaplun at tarantool.org
Fri Dec 6 11:08:57 MSK 2024
Hi, Sergey!
Thanks for the patch!
LGTM, with a minor comment below.
On 06.12.24, Sergey Bronnikov wrote:
> From: Sergey Bronnikov <sergeyb at tarantool.org>
>
> LuaJIT has a macro LUAJIT_DISABLE_SYSPROF that disable
Typo: s/disable/disables/
> sysprof support. Sysprof tests doesn't respect this macro
Typo: s/doesn't/don't/
Typo: s/macro/macro,/
> and therefore some of them failed when macro is enabled.
Typo: /macro/macro,/
>
> The proposed patch:
This isn't proposed patch, but it is just patch, so:
s/proposed //
>
> - skip sysprof testcases in a suite tarantool-c-tests
Typo: /skip/Skips/
Missed dot at the end of the sentence.
> - introduce an environment variable LUAJIT_DISABLE_SYSPROF in
Typo: s/introduce/Introduces/
> a suite tarantool-tests that set to 1 when sysprof support is
Typo: s/set/is set/
> disabled
Missed dot at the end of the sentence.
> - propagate a status of sysprof support to Lua tests and skip
Typo: s/propagate/Propagates
> testing when sysprof is disabled
Missed dot at the end of the sentence.
> ---
> test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c | 4 ++++
> test/tarantool-c-tests/misclib-sysprof-capi.test.c | 4 ++++
> test/tarantool-tests/CMakeLists.txt | 4 ++++
> test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua | 2 ++
> .../profilers/gh-5813-resolving-of-c-symbols.test.lua | 2 ++
> .../gh-7264-add-proto-trace-sysprof-default.test.lua | 2 ++
> .../profilers/gh-9217-profile-parsers-error-handling.test.lua | 2 ++
> test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua | 2 ++
> 8 files changed, 22 insertions(+)
>
> diff --git a/test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c b/test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c
> index a8de3358..de4fb2c4 100644
> --- a/test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c
> +++ b/test/tarantool-c-tests/gh-8594-sysprof-ffunc-crash.test.c
> @@ -277,8 +277,12 @@ static int test_tostring_call(void *ctx)
>
> int main(void)
> {
> +#if LUAJIT_DISABLE_SYSPROF
> + return skip_all("Sysprof is disabled (LUAJIT_DISABLE_SYSPROF)");
Minor: I suppose we can drop the LUAJIT_DISABLE_SYSPROF mentioning since
it becomes "maslo maslyanoe", for my taste.
Feel free to ignore.
> +#else /* LUAJIT_DISABLE_SYSPROF */
<snipped>
> diff --git a/test/tarantool-c-tests/misclib-sysprof-capi.test.c b/test/tarantool-c-tests/misclib-sysprof-capi.test.c
> index 6f53d488..ec3354cc 100644
> --- a/test/tarantool-c-tests/misclib-sysprof-capi.test.c
> +++ b/test/tarantool-c-tests/misclib-sysprof-capi.test.c
> @@ -301,6 +301,9 @@ static int profile_func_jiton(void *test_state)
>
> int main(void)
> {
> +#if LUAJIT_DISABLE_SYSPROF
> + return skip_all("Sysprof is disabled (LUAJIT_DISABLE_SYSPROF)");
Ditto.
> +#else /* LUAJIT_DISABLE_SYSPROF */
> if (LUAJIT_OS != LUAJIT_OS_LINUX)
> return skip_all("Sysprof is implemented for Linux only");
> if (LUAJIT_TARGET != LUAJIT_ARCH_X86
> @@ -322,4 +325,5 @@ int main(void)
> const int test_result = test_run_group(tgroup, L);
> utils_lua_close(L);
> return test_result;
> +#endif /* LUAJIT_DISABLE_SYSPROF */
> }
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 8bdb2cf3..a6e39f6c 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -89,6 +89,10 @@ if(LUAJIT_ENABLE_TABLE_BUMP)
> list(APPEND LUA_TEST_ENV_MORE LUAJIT_TABLE_BUMP=1)
> endif()
>
> +if(LUAJIT_DISABLE_SYSPROF)
> + list(APPEND LUA_TEST_ENV_MORE LUAJIT_DISABLE_SYSPROF=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 f935dc5b..379ccf8e 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
> @@ -1,4 +1,5 @@
> local tap = require('tap')
> +local no_sysprof = os.getenv('LUAJIT_DISABLE_SYSPROF')
> local test = tap.test('gh-5688-tool-cli-flag'):skipcond({
> ['Profile tools are implemented for x86_64 only'] = jit.arch ~= 'x86' and
> jit.arch ~= 'x64',
> @@ -7,6 +8,7 @@ local test = tap.test('gh-5688-tool-cli-flag'):skipcond({
> ['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'),
> + ['Disabled due to LUAJIT_DISABLE_SYSPROF'] = no_sysprof,
Minor: "disabled due to disable" sounds like "maslo maslyanoe" for my
taste. I suppose "Sysprof is disabled" is just enough. As a minor bonus,
we may avoid the declaration of the `no_sysprof` variable since we can
just use one-line `os.getenv()` that fits into this line now.
Same for the other Lua tests.
> })
>
> 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 74bcd9e8..ff745225 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
> @@ -1,10 +1,12 @@
> local tap = require("tap")
> +local no_sysprof = os.getenv("LUAJIT_DISABLE_SYSPROF")
> 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"),
> + ["Disabled due to LUAJIT_DISABLE_SYSPROF"] = no_sysprof,
> })
>
> test:plan(5)
> 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 176c1a15..e09016e7 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
> @@ -1,4 +1,5 @@
> local tap = require('tap')
> +local no_sysprof = os.getenv('LUAJIT_DISABLE_SYSPROF')
> 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
> @@ -6,6 +7,7 @@ local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
> ['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'),
> + ['Disabled due to LUAJIT_DISABLE_SYSPROF'] = no_sysprof,
> })
>
> 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 65c51198..012b1501 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
> @@ -1,4 +1,5 @@
> local tap = require('tap')
> +local no_sysprof = os.getenv('LUAJIT_DISABLE_SYSPROF')
> local test = tap.test('gh-9217-profile-parsers-error-handling'):skipcond({
> ['Profile tools are implemented for x86_64 only'] = jit.arch ~= 'x86' and
> jit.arch ~= 'x64',
> @@ -7,6 +8,7 @@ local test = tap.test('gh-9217-profile-parsers-error-handling'):skipcond({
> ['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'),
> + ['Disabled due to LUAJIT_DISABLE_SYSPROF'] = no_sysprof,
> })
>
> jit.off()
> diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> index 26c277cd..ea5ca9a9 100644
> --- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> +++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> @@ -1,10 +1,12 @@
> local tap = require("tap")
> +local no_sysprof = os.getenv('LUAJIT_DISABLE_SYSPROF')
> 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"),
> + ["Disabled due to LUAJIT_DISABLE_SYSPROF"] = no_sysprof,
> })
>
> test:plan(19)
> --
> 2.34.1
>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list