[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #609: GEODE-7508: remove duplicate SSL keys

[GitBox <gi...@apache.org>]

pivotal-jbarrett commented on a change in pull request #609:
URL: https://github.com/apache/geode-native/pull/609#discussion_r434068548



##########
File path: cppcache/integration/framework/TestConfig.cpp.in
##########
@@ -22,6 +22,8 @@ const char *getFrameworkString(FrameworkVariable name) {
     case FrameworkVariable::JavaObjectJarPath: return "@JAVAOBJECT_JAR_PATH@";
     case FrameworkVariable::GfShExecutable: return "@Geode_gfsh_EXECUTABLE@";
     case FrameworkVariable::TestCacheXmlDir: return "@CMAKE_CURRENT_SOURCE_DIR@/../../integration-test/resources";
+    case FrameworkVariable::TestClientSslKeysDir: return "@CMAKE_CURRENT_SOURCE_DIR@/../../../ssl_keys/client_keys";
+    case FrameworkVariable::TestServerSslKeysDir: return "@CMAKE_CURRENT_SOURCE_DIR@/../../../ssl_keys/server_keys";

Review comment:
       Why is this a giant case statement. Why not make these all global `const` values in a header. Would be nice to have this be a compile time failure if you are using something not defined than runtime. 

##########
File path: cppcache/integration/test/SslOneWayTest.cpp
##########
@@ -42,7 +42,10 @@ class SslOneWayTest : public ::testing::Test {
   SslOneWayTest() {
     // You can do set-up work for each test here.
     certificatePassword = std::string("apachegeode");
-    currentWorkingDirectory = boost::filesystem::current_path();
+    serverSslKeysDir = boost::filesystem::path(
+        getFrameworkString(FrameworkVariable::TestServerSslKeysDir));

Review comment:
       See comment above. This could simply be:
   `serverSslKeysDir = FrameworkVariable::TestServerSslKeysDir`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org