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

Move stream-related test configuration to CMake #13513

Merged
merged 15 commits into from
Jun 9, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Jun 6, 2023

Description

Instead of configuring test settings with environment variables in the bash script for CI, these changes allow all that configuration to happen directly in CMake, which is more robust and also makes the test configuration portable. With these changes any execution of the tests with ctest will use the preload library with all the other necessary environment variables configured as well. Running the test executables directly, on the other hand, will skip stream validation but still run correctly (except for the one test that explicitly tests the stream validation preload lib itself). These changes are now possible because our CI test scripts now run ctest instead of the executables (as of #12451) and due to full support for generator expression propagation even with the parallel testing feature (as of rapidsai/rapids-cmake#410).

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@vyasr vyasr added tests Unit testing for project CMake CMake build issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 6, 2023
@vyasr vyasr added this to the Enable streams milestone Jun 6, 2023
@vyasr vyasr requested a review from a team as a code owner June 6, 2023 02:07
@vyasr vyasr self-assigned this Jun 6, 2023
@vyasr vyasr requested a review from a team as a code owner June 6, 2023 02:07
@github-actions github-actions bot added ci libcudf Affects libcudf (C++/CUDA) code. labels Jun 6, 2023
@vyasr vyasr requested a review from robertmaynard June 6, 2023 02:07
@vyasr vyasr mentioned this pull request Jun 6, 2023
3 tasks
@vyasr vyasr mentioned this pull request Jun 6, 2023
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Jun 7, 2023
The preload lib is already being specified in the CI testing environment via bash, so this change has no short-term effect. The long-term solution is being worked on in #13513.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - David Wendt (https://github.com/davidwendt)

URL: #13519
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

rapids-bot bot pushed a commit to rapidsai/rapids-cmake that referenced this pull request Jun 8, 2023
rapidsai/cudf#13513 found 2 issues in the `install_relocatable` logic that caused LD_PRELOAD setup to fail. This PR corrects those two issues:

1. The `ENVIRONMENT` value always needs to be quoted to properly support multiple variables to be set
2. We need to scan for all `cmake_install.cmake` starting from the root build directory

Authors:
  - Robert Maynard (https://github.com/robertmaynard)
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #423
.github/CODEOWNERS Outdated Show resolved Hide resolved
@vyasr
Copy link
Contributor Author

vyasr commented Jun 9, 2023

/merge

@rapids-bot rapids-bot bot merged commit 7e8d534 into rapidsai:branch-23.08 Jun 9, 2023
@vyasr vyasr deleted the feat/stream_test_config_cmake branch June 9, 2023 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake CMake build issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change tests Unit testing for project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants