-
Notifications
You must be signed in to change notification settings - Fork 311
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
Changes to support gtest version 1.11 #3511
Changes to support gtest version 1.11 #3511
Conversation
@@ -383,6 +383,9 @@ TEST_P(Tests_Louvain_File64, CheckInt64Int64FloatFloat) | |||
override_File_Usecase_with_cmd_line_arguments(GetParam())); | |||
} | |||
|
|||
#if 0 | |||
// FIXME: We should use these tests, gtest-1.11.0 makes it a runtime error | |||
// to define and not instantiate these. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we better run this? Or is this test failing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no current RMAT test defined below. We create the definition here, but we aren't actually using it below.
gtest 1.10 (which we are currently using) ignored this.
gtest 1.11 (which is used by DLFW and I assume we will migrate to shortly) treats the definition of a TEST_P
and not using it as an error.
This is the expedient solution (ifdef out the offending definitions that aren't used). I added the FIXME to identify that we should actually be using these tests. But I needed a simple fix to unblock DLFW and didn't want to spend the time to add and debug the Rmat tests for these algorithms.
/merge |
gtest version 1.11 adds a feature that complains if we define a test but do not instantiate it. We have a few cases where we define a case, but we don't instantiate it at the moment. This PR will ifdef these out so that they will run properly.
Also shrunk the size of an MG rmat test so that it would fit in a 1 GPU test.