pivotal-jbarrett commented on a change in pull request #826:
URL: https://github.com/apache/geode-native/pull/826#discussion_r661123304
##########
File path: cppcache/integration-test/CMakeLists.txt
##########
@@ -205,7 +205,7 @@ set_tests_properties(
testThinClientPoolAttrTest
testThinClientPoolLocator
testThinClientPoolRedundancy
- testThinClientSecurityAuthentication
+ #testThinClientSecurityAuthentication
Review comment:
Delete?
##########
File path: cppcache/integration-test/testThinClientSecurityAuthentication.cpp
##########
@@ -138,7 +140,7 @@ DUNIT_TASK_DEFINITION(LOCATORSERVER, CreateServer1)
printf("Input to server cmd is --> %s",
cmdServerAuthenticator.c_str());
CacheHelper::initServer(
- 1, nullptr, locHostPort,
+ 1, "cacheserver_notify_subscription.xml", locHostPort,
Review comment:
Wouldn't this change the intent of the test now that it starts with a
cache xml file?
##########
File path: cppcache/integration-test/testThinClientSecurityAuthentication.cpp
##########
@@ -138,7 +140,7 @@ DUNIT_TASK_DEFINITION(LOCATORSERVER, CreateServer1)
printf("Input to server cmd is --> %s",
cmdServerAuthenticator.c_str());
CacheHelper::initServer(
- 1, nullptr, locHostPort,
+ 1, "cacheserver_notify_subscription.xml", locHostPort,
const_cast<char *>(cmdServerAuthenticator.c_str()));
Review comment:
This should be fixed too. Really bad to be taking stripping the `const`
off here. The funny thing is the method takes a `const`. The method should
really be change to take a `const std::string&`. I know this might feel out of
scope but it's right there!
##########
File path: cppcache/integration-test/testThinClientSecurityAuthorization.cpp
##########
@@ -54,13 +54,15 @@ const std::string locHostPort =
std::shared_ptr<CredentialGenerator> credentialGeneratorHandler;
std::string getXmlPath() {
- char xmlPath[1000] = {'\0'};
- const char *path = std::getenv("TESTSRC");
- ASSERT(path != nullptr,
+ std::string path = std::string(std::getenv("TESTSRC"));
Review comment:
Use `auto` on the lefthand side whenever you can. Almost all the new
code should be corrected to use auto for all these local variables.
##########
File path: cppcache/integration-test/ThinClientSecurityHelper.hpp
##########
@@ -59,13 +59,15 @@ const char* regionNamesAuth[] = {"DistRegionAck"};
std::shared_ptr<CredentialGenerator> credentialGeneratorHandler;
std::string getXmlPath() {
- char xmlPath[1000] = {'\0'};
- const char* path = std::getenv("TESTSRC");
- ASSERT(path != nullptr,
+ std::string path = std::string(std::getenv("TESTSRC"));
+
+ int indexOfCppcache = path.find("cppcache");
Review comment:
We use boost::filesystem elsewhere so using it here to make file path
operations safe would be nice.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
