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

Fix memcheck script to execute only _TEST files found in bin/gtests/libcudf #13006

Merged
merged 1 commit into from
Mar 24, 2023

Conversation

davidwendt
Copy link
Contributor

The nightly runs of compute-sanitizer started failing due to the test script trying to execute some new extra files found in the $CONDA_PREFIX"/bin/gtests/libcudf/ directory. This change ensures only files ending in _TEST are executed by compute-sanitizer.

For reference errors are here: https://github.com/rapidsai/cudf/actions/runs/4508267264/jobs/7936800047
Example:

Running compute-sanitizer on CTestTestfile.cmake
========= COMPUTE-SANITIZER
========= Error: Target application terminated before first instrumented API call

Follow on issue/PR could explore adding a special make option to execute compute-sanitizer only on test files.

Checklist

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

@davidwendt davidwendt added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change ci labels Mar 24, 2023
@davidwendt davidwendt requested a review from a team as a code owner March 24, 2023 10:15
@davidwendt davidwendt self-assigned this Mar 24, 2023
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.

@davidwendt, it looks like the new files that have showed up in that directory are these:

CTestTestfile.cmake
generate_ctest_json
run_gpu_test.cmake

These look like they're from #12451.

As part of the changes in that PR, @robertmaynard also switched up the way that the GTests are invoked for our regular test_cpp.sh script.

I'm wondering if his changes should also be adapted for this test_cpp_memcheck.sh script.

His method looks a bit more robust than the approach that we've been using previously.

@davidwendt
Copy link
Contributor Author

I'm wondering if his changes should also be adapted for this test_cpp_memcheck.sh script.

His method looks a bit more robust than the approach that we've been using previously.

Yes, this is what I meant about making this work with make/cmake in a future PR.
The test_cpp.sh uses ctest which is a cmake utility.

@ajschmidt8
Copy link
Member

Yes, this is what I meant about making this work with make/cmake in a future PR. The test_cpp.sh uses ctest which is a cmake utility.

Got it. Sorry, missed that part. Approved to get this unblocked for now.

@davidwendt
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 698fcf6 into rapidsai:branch-23.04 Mar 24, 2023
@davidwendt davidwendt deleted the bug-sanitizer-exec branch March 24, 2023 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants