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

Remove the mr from the base fixture #14057

Merged
merged 5 commits into from
Sep 9, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Sep 7, 2023

Description

This mr is just an alias for the current memory resource, so we don't really need it. This came up in #14010 (comment). This PR removes all uses of it, but does not actually remove the mr yet. That will be done in a follow-up (see #14057 (comment)).

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 3 - Ready for Review Ready for review by team tests Unit testing for project non-breaking Non-breaking change labels Sep 7, 2023
@vyasr vyasr requested a review from davidwendt September 7, 2023 17:50
@vyasr vyasr self-assigned this Sep 7, 2023
@vyasr vyasr requested a review from a team as a code owner September 7, 2023 17:50
@vyasr vyasr requested a review from ttnghia September 7, 2023 17:50
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Sep 7, 2023
@vyasr vyasr added the improvement Improvement / enhancement to an existing function label Sep 7, 2023
@vyasr
Copy link
Contributor Author

vyasr commented Sep 8, 2023

There's something very strange happening here. I can remove all uses of the mr from the fixture, but removing the mr itself is causing a single test failure in a test that isn't otherwise modified in this PR and does not use the mr function. Furthermore, the test only fails on arm, and only for certain types. In the interest of debugging a minimal example, I'm going to revert just that part of this PR, merge it, and then debug the issue separately (will probably need to get access to an arm machine). It seems likely that the issue is not directly related to the mr, but is some other problem perhaps caused by somewhere that we're not initializing/tearing down/otherwise interacting with GTest improperly.

@vyasr
Copy link
Contributor Author

vyasr commented Sep 9, 2023

/merge

@rapids-bot rapids-bot bot merged commit 886e189 into rapidsai:branch-23.10 Sep 9, 2023
54 checks passed
@vyasr vyasr deleted the chore/remove_test_mr branch September 11, 2023 17:09
@vyasr vyasr mentioned this pull request Sep 11, 2023
3 tasks
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 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.

3 participants