Re: [Tarantool-patches] [PATCH v2 luajit 6/7] test: skip flaky tests with enabled table bump

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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

Hi, Sergey,

On 24.09.2024 14:45, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
>
> On 24.09.24, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> test statuses from TAP report produced by tests is not propagated to CTest.
>>
>> I propose to disable these tests in CTest:
>>
>>
>> --- a/test/tarantool-tests/CMakeLists.txt
>> +++ b/test/tarantool-tests/CMakeLists.txt
>> @@ -166,3 +167,14 @@ if(LUAJIT_USE_ASAN AND CMAKE_C_COMPILER_ID STREQUAL
>> "GNU")
>>        LD_PRELOAD ${LIB_ASAN}
>>      )
>>    endif()
>> +
>> +set_tests_properties(
>> +  test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua
>> + test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
>> + test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
>> +
>> test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
>> +
>> test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
>> +  test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua
>> +  test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
>> +  PROPERTIES DISABLED TRUE
>> +)
>>
>>
>> CTest will always report that these tests are disabled and we will never
>> forget to enable them back:
>>
>>
>> 100% tests passed, 0 tests failed out of 158
>>
>> Label Time Summary:
>> tarantool-tests    =  14.66 sec*proc (165 tests)
>>
>> Total Test time (real) =  14.73 sec
>>
>> The following tests did not run:
>>           222 -
>> test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua (Disabled)
>>           223 -
>> test/tarantool-tests/profilers/gh-5813-resolving-of-c-symbols.test.lua
>> (Disabled)
>>           224 -
>> test/tarantool-tests/profilers/gh-5994-memprof-human-readable.test.lua
>> (Disabled)
>>           225 -
>> test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
>> (Disabled)
>>           226 -
>> test/tarantool-tests/profilers/gh-9217-profile-parsers-error-handling.test.lua
>> (Disabled)
>>           227 -
>> test/tarantool-tests/profilers/misclib-memprof-lapi.test.lua (Disabled)
>>           228 -
>> test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua (Disabled)
>>
>>
>> What do you think?
>>
> Side note: this is new in 3.9, so we need to update CMake first.
>
> I'm not sure about this one.
> We also have all skipconditions inside the test itself, which makes it
> "self-sufficient" -- so if we look at the test, we know all builds when
> it is disabled. Adding another location may be confusing.

I agree. The ideal option for us is when TAP statuses are propagated to 
CTest,

but it is not an option now.

>
> OTOH, we can see skipped tests in -V mode (but I agree, that it isn't
> very convenient).
>
> I am open for discussion :).

Feel free to ignore, I'll not insist.


LGTM

>> On 24.09.2024 13:29, 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
> <snipped>
>
>>>    test:plan(19)

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