From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Maxim Kokryashkin <max.kokryashkin@gmail.com>, tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, imun@tarantool.org Subject: Re: [Tarantool-patches] [PATCH luajit v5 2/2] test: add tests for debugging extensions Date: Tue, 19 Dec 2023 11:16:22 +0300 [thread overview] Message-ID: <b4c7a4fd-035b-4076-8364-eb92f34f2557@tarantool.org> (raw) In-Reply-To: <dcc57f3b7fce3775eb92b5f3fc80dac2091afeac.1701872037.git.m.kokryashkin@tarantool.org> Hi, Max! many thanks for the patch! LGTM with a a number of minor comments, please see below. Sergey On 12/6/23 17:14, Maxim Kokryashkin wrote: > From: Maksim Kokryashkin <max.kokryashkin@gmail.com> > > This patch adds tests for LuaJIT debugging > extensions for lldb and gdb. > --- > .flake8rc | 4 + > src/luajit_dbg.py | 11 +- > test/CMakeLists.txt | 3 + > .../CMakeLists.txt | 102 +++++++ > .../debug-extension-tests.py | 262 ++++++++++++++++++ > 5 files changed, 376 insertions(+), 6 deletions(-) > create mode 100644 test/LuaJIT-debug-extensions-tests/CMakeLists.txt > create mode 100644 test/LuaJIT-debug-extensions-tests/debug-extension-tests.py <snipped> > > if(LUAJIT_USE_TEST) > diff --git a/test/LuaJIT-debug-extensions-tests/CMakeLists.txt b/test/LuaJIT-debug-extensions-tests/CMakeLists.txt > new file mode 100644 > index 00000000..78eafd02 > --- /dev/null > +++ b/test/LuaJIT-debug-extensions-tests/CMakeLists.txt > @@ -0,0 +1,102 @@ > +add_custom_target(LuaJIT-gdb-extension-tests > + DEPENDS ${LUAJIT_TEST_BINARY} > +) > + > +add_custom_target(LuaJIT-lldb-extension-tests > + DEPENDS ${LUAJIT_TEST_BINARY} > +) > + > +set(DUMMY_GDB_MSG "LuaJIT-gdb-extension-tests is a dummy target") > +set(DUMMY_LLDB_MSG "LuaJIT-lldb-extension-tests is a dummy target") > + > +# Debug info is required for testing of extensions. > +if(NOT (CMAKE_BUILD_TYPE MATCHES Debug)) > + message(WARNING > + "not a DEBUG build, LuaJIT-lldb-extension-tests and " > + "LuaJIT-gdb-extension-tests are dummy" > + ) > + add_custom_command(TARGET LuaJIT-gdb-extension-tests > + COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${DUMMY_GDB_MSG} > + ) > + add_custom_command(TARGET LuaJIT-lldb-extension-tests > + COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${DUMMY_LLDB_MSG} > + ) > + return() > +endif() > + > +# MacOS asks for permission to debug a process even when the > +# machine is set into development mode. To solve the issue, > +# it is required to add relevant users to the `_developer` user > +# group in MacOS. Disabled for now. > +if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND DEFINED ENV{CI}) > + message(WARNING > + "Interactive debugging is unavailable for macOS CI builds," > + "LuaJIT-lldb-extension-tests is dummy" > + ) > + add_custom_command(TARGET LuaJIT-lldb-extension-tests > + COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${DUMMY_LLDB_MSG} > + ) > + return() > +endif() > + > +find_package(PythonInterp) CMake Warning (dev) at test/LuaJIT-debug-extensions-tests/CMakeLists.txt:30 (find_package): Policy CMP0148 is not set: The FindPythonInterp and FindPythonLibs modules are removed. Run "cmake --help-policy CMP0148" for policy details. Use the cmake_policy command to set the policy and suppress this warning. This warning is for project developers. Use -Wno-dev to suppress it. See https://cmake.org/cmake/help/latest/policy/CMP0148.html Let's fix this warning. > +if(NOT PYTHONINTERP_FOUND) > + message(WARNING > + "`python` is not found, LuaJIT-lldb-extension-tests and " > + "LuaJIT-gdb-extension-tests are dummy" > + ) > + add_custom_command(TARGET LuaJIT-gdb-extension-tests > + COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${DUMMY_GDB_MSG} > + ) > + add_custom_command(TARGET LuaJIT-lldb-extension-tests > + COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${DUMMY_LLDB_MSG} > + ) > + return() > +endif() > + > +set(DEBUGGER_TEST_ENV > + "LUAJIT_TEST_BINARY=${LUAJIT_TEST_BINARY}" > + # Suppresses __pycache__ generation. > + "PYTHONDONTWRITEBYTECODE=1" > + "DEBUGGER_EXTENSION_PATH=${PROJECT_SOURCE_DIR}/src/luajit_dbg.py" > +) > + > +set(TEST_SCRIPT_PATH > + ${PROJECT_SOURCE_DIR}/test/LuaJIT-debug-extensions-tests/debug-extension-tests.py > +) > + > +find_program(GDB gdb) > +if(GDB) > + set(GDB_TEST_ENV ${DEBUGGER_TEST_ENV} > + "DEBUGGER_COMMAND=${GDB}" > + ) > + add_custom_command(TARGET LuaJIT-gdb-extension-tests > + COMMENT "Running luajit_dbg.py tests with gdb" Nit: I would omit a filename in a comment > + COMMAND > + ${GDB_TEST_ENV} ${PYTHON_EXECUTABLE} ${TEST_SCRIPT_PATH} > + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} > + ) > +else() > + message(WARNING "`gdb' is not found, so LuaJIT-gdb-extension-tests is dummy") > + add_custom_command(TARGET LuaJIT-gdb-extension-tests > + COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red ${DUMMY_GDB_MSG} > + ) > +endif() > + > +find_program(LLDB lldb) > +if(LLDB) > + set(LLDB_TEST_ENV ${DEBUGGER_TEST_ENV} > + "DEBUGGER_COMMAND=${LLDB}" > + ) > + add_custom_command(TARGET LuaJIT-lldb-extension-tests > + COMMENT "Running luajit_dbg.py tests with lldb" Nit: I would omit a filename in a comment > + <snipped> > + > +for test_cls in TestCaseBase.__subclasses__(): > + test_cls.test = lambda self: self.check() > + > +if __name__ == '__main__': > + unittest.main(verbosity=2) there are three verbosity levels: quiet (0), default (1) and verbose (3). I would left a comment that "2" is a verbose level. Feel free to ignore.
next prev parent reply other threads:[~2023-12-19 8:16 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-12-06 14:14 [Tarantool-patches] [PATCH luajit v5 0/2] debug: generalized extension Maxim Kokryashkin via Tarantool-patches 2023-12-06 14:14 ` [Tarantool-patches] [PATCH luajit v5 1/2] " Maxim Kokryashkin via Tarantool-patches 2023-12-19 8:01 ` Sergey Bronnikov via Tarantool-patches 2023-12-06 14:14 ` [Tarantool-patches] [PATCH luajit v5 2/2] test: add tests for debugging extensions Maxim Kokryashkin via Tarantool-patches 2023-12-19 8:16 ` Sergey Bronnikov via Tarantool-patches [this message] 2023-12-19 14:34 ` Maxim Kokryashkin via Tarantool-patches 2023-12-20 12:23 ` Sergey Bronnikov via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b4c7a4fd-035b-4076-8364-eb92f34f2557@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=imun@tarantool.org \ --cc=max.kokryashkin@gmail.com \ --cc=sergeyb@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit v5 2/2] test: add tests for debugging extensions' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox