Skip to content

[cppyy] Add CMakeLists.txt for cppyy/test #18671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Vipul-Cariappa
Copy link
Contributor

Enabling running tests using ctest

Enabling running tests using `ctest`
@Vipul-Cariappa Vipul-Cariappa force-pushed the dev/test/cppyy-cmake branch from 0a643e1 to e8e6b75 Compare May 9, 2025 10:37
Copy link

github-actions bot commented May 9, 2025

Test Results

    18 files      18 suites   3d 21h 24m 14s ⏱️
 2 763 tests  2 741 ✅ 0 💤 22 ❌
48 357 runs  48 276 ✅ 0 💤 81 ❌

For more details on these failures, see this check.

Results for commit c438360.

♻️ This comment has been updated with latest results.

@aaronj0
Copy link
Contributor

aaronj0 commented May 9, 2025

Thanks for this very nice PR! Looks like the bots fail because the makefile requires rootcling, which is built as a part of ROOT and unavailable at the build time of cppyy and its tests.

foreach(CPPYY_TEST_FILE ${CPPYY_TEST_SRC})
get_filename_component(CPPYY_TEST_FILE_NAME ${CPPYY_TEST_FILE} NAME)
string(REPLACE ".py" "" CPPYY_TEST_FILE_NO_EXT ${CPPYY_TEST_FILE_NAME})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need something like: https://github.com/root-project/root/blob/master/roottest/cling/stl/dicts/CMakeLists.txt, where the ROOT_GENERATE_DICTIONARY macro can be used in the CMake to perform the generation of the dictionaries, followed by ROOT_LINKER_LIBRARY with the sources for each test. We can then drop the makefile completely :)

Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR - very useful increase of coverage.


add_custom_target(cppyy_tests_dict ALL COMMAND make all WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/bindings/pyroot/cppyy/cppyy/test)

file(GLOB CPPYY_TEST_SRC "test_*.py")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to avoid globs in general: could we perhaps be explicit listing the test files we want to add to the list?

@Vipul-Cariappa Vipul-Cariappa marked this pull request as draft May 13, 2025 12:09
Generating rootmaps and shared libraries for the tests using cmake.
Fix formatting.
@Vipul-Cariappa Vipul-Cariappa force-pushed the dev/test/cppyy-cmake branch from d811bd6 to c438360 Compare May 13, 2025 12:28
@guitargeek
Copy link
Contributor

Hi @Vipul-Cariappa! I see there are some failures in the CI. Can you investigate next week please? Thanks!

@Vipul-Cariappa
Copy link
Contributor Author

Hi @Vipul-Cariappa! I see there are some failures in the CI. Can you investigate next week please? Thanks!

Yes. I plan on doing that. I had a long conversation with Aaron yesterday on some weird behaviour. Maybe I can share the details with you in the office on Monday.

@guitargeek
Copy link
Contributor

Excellent, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants