Skip to content
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

Separating test executables #820

Merged

Conversation

cjnolet
Copy link
Member

@cjnolet cjnolet commented Sep 9, 2022

Other RAPIDS projects have been doing this for awhile. It's time RAFT did this as well.

@cjnolet cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 9, 2022
@cjnolet cjnolet requested a review from a team as a code owner September 9, 2022 18:40
@cjnolet cjnolet requested a review from a team as a code owner September 9, 2022 18:40
@cjnolet cjnolet requested a review from a team as a code owner September 9, 2022 22:03
cpp/test/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -194,7 +194,7 @@ if hasArg tests || (( ${NUMARGS} == 0 )); then
COMPILE_DIST_LIBRARY=ON
ENABLE_NN_DEPENDENCIES=ON
COMPILE_NN_LIBRARY=ON
CMAKE_TARGET="${CMAKE_TARGET};test_raft"
CMAKE_TARGET="${CMAKE_TARGET};CLUSTER_TEST;CORE_TEST;DISTANCE_TEST;LABEL_TEST;LINALG_TEST;MATRIX_TEST;RANDOM_TEST;SOLVERS_TEST;SPARSE_TEST;SPARSE_DIST_TEST;SPARSE_NN_TEST;SPATIAL_TEST;STATS_TEST;UTILS_TEST"
Copy link
Member Author

Choose a reason for hiding this comment

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

@robertmaynard I'm hoping to avoid having to do this. The problem is, build.sh is explicitly specifying the artifact that it's expecting to create by manually specifying the target(s) which are built based on the given options. When the tests are split into separate executables, I now need to specify those targets here- so a single test_raft target got split into several smaller targets.

Eventually, I'd like to add an option to build.sh to manually specify which tests get built, but in the meantime this is pretty verbose and it's a chicken and egg problem- the build.sh script doesn't know the targets at the time it invokes cmake so they need to specified manually, but the targets are defined within cmake itself. Do you have any suggestions on how I could avoid such a pattern here? Is there a way I could just use something similar to --component testing to specify all the targets marked testing? (--component testing seems to be defined only in the install() function but is there a similar pattern I could use to reference the full list of executable targets?)

@cjnolet
Copy link
Member Author

cjnolet commented Sep 23, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit f25e4c8 into rapidsai:branch-22.10 Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake cpp gpuCI improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants