[ 
https://issues.apache.org/jira/browse/GEODE-4143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16301635#comment-16301635
 ] 

ASF GitHub Bot commented on GEODE-4143:
---------------------------------------

pivotal-jbarrett closed pull request #172: GEODE-4143: Improves integration 
test stability.
URL: https://github.com/apache/geode-native/pull/172
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/clicache/integration-test/CacheHelperN.cs 
b/clicache/integration-test/CacheHelperN.cs
index 72266b7b..d865c9b6 100644
--- a/clicache/integration-test/CacheHelperN.cs
+++ b/clicache/integration-test/CacheHelperN.cs
@@ -1990,7 +1990,7 @@ static int getLocatorPort(int num)
         {
           extraLocatorArgs = locatorPort;
         }
-        string locatorArgs = LocatorStartArgs + " --name=" + serverName + 
startDir + extraLocatorArgs;
+        string locatorArgs = LocatorStartArgs + " --name=" + serverName + 
startDir + extraLocatorArgs + " --http-service-port=0";
 
         if (!Util.StartProcess(locatorPath, locatorArgs, false, null, true,
           false, false, true, out javaProc))
diff --git a/clicache/integration-test/test.bat.in 
b/clicache/integration-test/test.bat.in
index 92e3e01f..c0b7735d 100644
--- a/clicache/integration-test/test.bat.in
+++ b/clicache/integration-test/test.bat.in
@@ -45,7 +45,6 @@ set 
GF_CLASSPATH=%GF_CLASSPATH%;$<SHELL_PATH:${CMAKE_BINARY_DIR}>\tests\javaobje
 set PROFILERCMD=
 set BUG481=
 set TESTNAME=${TEST}
-set LOG=${TEST}.log
 
 rmdir /q /s "%TEST_DIR%" 2>nul
 mkdir "%TEST_DIR%"
@@ -53,17 +52,7 @@ if %errorlevel% neq 0 exit /b %errorlevel%
 pushd "%TEST_DIR%"
 if %errorlevel% neq 0 exit /b %errorlevel%
 
-rem In Windows, pipes to tee return tee's exit code instead of executable's
-rem exit code. As a workaround we write exit codes to files.
-
-(${NUNIT_CONSOLE} /labels /run:${NAMESPACE}.${TESTCLASS} 
..\..\$<CONFIG>\UnitTests.dll 2>&1 && echo 0 >${TEST}.errorlevel || echo 1 
>${TEST}.errorlevel) | tee %LOG%
-
-rem Our testing framework sometimes leaves lingering server/locator processes.
-rem Let's clean up after ourselves so that we do not affect subsequent runs.
-rem Unfortunately this also means that our tests can only run serially.
-taskkill.exe /F /IM java.exe
-
-set /p errorlevel= <${TEST}.errorlevel
+${NUNIT_CONSOLE} /labels /run:${NAMESPACE}.${TESTCLASS} 
..\..\$<CONFIG>\UnitTests.dll 
 if %errorlevel% neq 0 exit /b %errorlevel%
 
 popd
diff --git a/clicache/src/ExceptionTypes.hpp b/clicache/src/ExceptionTypes.hpp
index 066c89fc..4c5ec607 100644
--- a/clicache/src/ExceptionTypes.hpp
+++ b/clicache/src/ExceptionTypes.hpp
@@ -164,7 +164,8 @@ namespace Apache
                 cause = GeodeException::GetNative(ex->InnerException);
               }
               return std::make_shared<apache::geode::client::Exception>(
-                  marshal_as<std::string>(MgSysExPrefix + ex->ToString()) + 
cause->getMessage());
+                marshal_as<std::string>(MgSysExPrefix + ex->ToString()) 
+                  + (cause ? cause->getMessage() : ""));
             }
           }
           return nullptr;
@@ -181,7 +182,8 @@ namespace Apache
             cause = GeodeException::GetNative(this->InnerException);
           }
           return std::make_shared<apache::geode::client::Exception>(
-            marshal_as<std::string>(this->Message + ": " + this->StackTrace) + 
cause->getMessage());
+            marshal_as<std::string>(this->Message + ": " + this->StackTrace)
+              + (cause ? cause->getMessage() : ""));
         }
 
         /// <summary>
@@ -206,7 +208,8 @@ namespace Apache
               cause = GeodeException::GetNative(ex->InnerException);
             }
             throw apache::geode::client::Exception(
-              marshal_as<std::string>(MgSysExPrefix + ex->ToString()) + 
cause->getMessage());
+              marshal_as<std::string>(MgSysExPrefix + ex->ToString())
+                + (cause ? cause->getMessage() : ""));
           }
         }
 
diff --git a/cppcache/integration-test/CMakeLists.txt 
b/cppcache/integration-test/CMakeLists.txt
index dd437839..74571bae 100644
--- a/cppcache/integration-test/CMakeLists.txt
+++ b/cppcache/integration-test/CMakeLists.txt
@@ -139,7 +139,6 @@ set_property(TEST testThinClientCacheables PROPERTY LABELS 
FLAKY)
 set_property(TEST testThinClientCq PROPERTY LABELS FLAKY)
 set_property(TEST testThinClientCqFailover PROPERTY LABELS FLAKY)
 set_property(TEST testThinClientCqHAFailover PROPERTY LABELS FLAKY)
-set_property(TEST testThinClientDistOpsUpdateLocatorList PROPERTY LABELS FLAKY)
 set_property(TEST testThinClientDurableConnect PROPERTY LABELS FLAKY)
 set_property(TEST testThinClientDurableDisconnectNormal PROPERTY LABELS FLAKY)
 set_property(TEST testThinClientDurableDisconnectTimeout PROPERTY LABELS FLAKY)
diff --git a/cppcache/integration-test/CacheHelper.cpp 
b/cppcache/integration-test/CacheHelper.cpp
index d4401ecf..832c563e 100644
--- a/cppcache/integration-test/CacheHelper.cpp
+++ b/cppcache/integration-test/CacheHelper.cpp
@@ -159,8 +159,8 @@ CacheHelper::CacheHelper(const bool isThinclient,
   }
   try {
     LOG(" in cachehelper before createCacheFactory");
-    auto cacheFactory = CacheFactory::createCacheFactory(pp)
-                        ->setAuthInitialize(authInitialize);
+    auto cacheFactory =
+        
CacheFactory::createCacheFactory(pp)->setAuthInitialize(authInitialize);
     cachePtr = std::make_shared<Cache>(cacheFactory->create());
     m_doDisconnect = false;
   } catch (const Exception& excp) {
@@ -1187,7 +1187,8 @@ void CacheHelper::cleanupServerInstances() {
 void CacheHelper::initServer(int instance, const char* xml,
                              const char* locHostport, const char* authParam,
                              bool ssl, bool enableDelta, bool multiDS,
-                             bool testServerGC, bool untrustedCert, bool 
useSecurityManager) {
+                             bool testServerGC, bool untrustedCert,
+                             bool useSecurityManager) {
   if (!isServerCleanupCallbackRegistered &&
       gClientCleanup.registerCallback(&CacheHelper::cleanupServerInstances)) {
     isServerCleanupCallbackRegistered = true;
@@ -1340,8 +1341,8 @@ void CacheHelper::initServer(int instance, const char* 
xml,
   }
 
   if (locHostport != nullptr) {  // check number of locator host port.
-    std::string geodeProperties =
-        generateGeodeProperties(currDir, ssl, -1, 0, untrustedCert, 
useSecurityManager);
+    std::string geodeProperties = generateGeodeProperties(
+        currDir, ssl, -1, 0, untrustedCert, useSecurityManager);
 
     sprintf(
         cmd,
@@ -1352,7 +1353,7 @@ void CacheHelper::initServer(int instance, const char* 
xml,
         "--J=-Dgemfire.tombstone-gc-hreshold=%ld "
         "--J=-Dgemfire.security-log-level=%s --J=-Xmx1024m --J=-Xms128m 2>&1",
         gfjavaenv, GFSH, classpath, sname.c_str(), xmlFile.c_str(),
-        useSecurityManager ?  "--user=root --password=root-password" : "",
+        useSecurityManager ? "--user=root --password=root-password" : "",
         currDir.c_str(), portNum, gfLogLevel, geodeProperties.c_str(),
         authParam, deltaProperty.c_str(),
         testServerGC ? userTombstone_timeout : defaultTombstone_timeout,
@@ -1696,7 +1697,8 @@ void CacheHelper::cleanupLocatorInstances() {
 
 // starting locator
 void CacheHelper::initLocator(int instance, bool ssl, bool multiDS, int dsId,
-                              int remoteLocator, bool untrustedCert, bool 
useSecurityManager) {
+                              int remoteLocator, bool untrustedCert,
+                              bool useSecurityManager) {
   if (!isLocatorCleanupCallbackRegistered &&
       gClientCleanup.registerCallback(&CacheHelper::cleanupLocatorInstances)) {
     isLocatorCleanupCallbackRegistered = true;
@@ -1750,8 +1752,8 @@ void CacheHelper::initLocator(int instance, bool ssl, 
bool multiDS, int dsId,
 
   ACE_OS::mkdir(locDirname.c_str());
 
-  std::string geodeFile =
-      generateGeodeProperties(currDir, ssl, dsId, remoteLocator, 
untrustedCert, useSecurityManager);
+  std::string geodeFile = generateGeodeProperties(
+      currDir, ssl, dsId, remoteLocator, untrustedCert, useSecurityManager);
 
   sprintf(cmd, "%s/bin/%s stop locator --dir=%s --properties-file=%s ",
           gfjavaenv, GFSH, currDir.c_str(), geodeFile.c_str());
@@ -1760,9 +1762,10 @@ void CacheHelper::initLocator(int instance, bool ssl, 
bool multiDS, int dsId,
   ACE_OS::system(cmd);
 
   static char* classpath = ACE_OS::getenv("GF_CLASSPATH");
-  std::string propertiesFile = useSecurityManager ?
-    std::string("--security-properties-file=") + geodeFile :
-    std::string("--properties-file=") + geodeFile;
+  std::string propertiesFile =
+      useSecurityManager
+          ? std::string("--security-properties-file=") + geodeFile
+          : std::string("--properties-file=") + geodeFile;
   sprintf(cmd,
           "%s/bin/%s start locator --name=%s --port=%d --dir=%s "
           "%s --http-service-port=0 --classpath=%s",
@@ -1848,16 +1851,17 @@ int CacheHelper::getRandomAvailablePort() {
 }
 
 std::string CacheHelper::unitTestOutputFile() {
-  char currWDPath[512];
-  char* wdPath ATTR_UNUSED = ACE_OS::getcwd(currWDPath, 512);
+  char cwd[1024];
+  if (!ACE_OS::getcwd(cwd, sizeof(cwd))) {
+    throw Exception("Failed to get current working directory.");
+  }
 
-  char* testName = ACE_OS::getenv("TESTNAME");
-  strcat(currWDPath, "/");
-  strcat(currWDPath, testName);
-  strcat(currWDPath, ".log");
+  std::string outputFile(cwd);
+  outputFile += "/";
+  outputFile += ACE_OS::getenv("TESTNAME");
+  outputFile += ".log";
 
-  std::string str(currWDPath);
-  return str;
+  return outputFile;
 }
 
 int CacheHelper::getNumLocatorListUpdates(const char* s) {
@@ -1875,11 +1879,10 @@ int CacheHelper::getNumLocatorListUpdates(const char* 
s) {
   return numMatched;
 }
 
-std::string CacheHelper::generateGeodeProperties(const std::string& path,
-                                                 const bool ssl, const int 
dsId,
-                                                 const int remoteLocator,
-                                                 const bool untrustedCert,
-                                                 const bool 
useSecurityManager) {
+std::string CacheHelper::generateGeodeProperties(
+    const std::string& path, const bool ssl, const int dsId,
+    const int remoteLocator, const bool untrustedCert,
+    const bool useSecurityManager) {
   char cmd[2048];
   std::string keystore = std::string(ACE_OS::getenv("TESTSRC")) + "/keystore";
 
diff --git a/cppcache/integration-test/ThinClientDistOps.hpp 
b/cppcache/integration-test/ThinClientDistOps.hpp
index 2f3247fb..73ae799d 100644
--- a/cppcache/integration-test/ThinClientDistOps.hpp
+++ b/cppcache/integration-test/ThinClientDistOps.hpp
@@ -65,18 +65,25 @@ DUNIT_TASK_DEFINITION(CLIENT2, Alter_Client_Grid_Property_2)
   { g_isGridClient = !g_isGridClient; }
 END_TASK_DEFINITION
 
-void initClient(const bool isthinClient) {
+void initClient(const bool isthinClient, const bool redirectLog) {
   if (cacheHelper == nullptr) {
     auto config = Properties::create();
     if (g_isGridClient) {
       config->insert("grid-client", "true");
     }
     config->insert("log-level", "finer");
+
+    if (redirectLog) {
+      config->insert("log-file", CacheHelper::unitTestOutputFile());
+    }
+
     cacheHelper = new CacheHelper(isthinClient, config);
   }
   ASSERT(cacheHelper, "Failed to create a CacheHelper client instance.");
 }
 
+void initClient(const bool isthinClient) { initClient(isthinClient, false); }
+
 void cleanProc() {
   if (cacheHelper != nullptr) {
     delete cacheHelper;
@@ -562,7 +569,7 @@ DUNIT_TASK_DEFINITION(CLIENT1, 
CreatePoolForUpdateLocatorList)
     = -1, int connections = -1, int loadConditioningInterval = - 1, bool
     isMultiuserMode = false, int updateLocatorListInterval = 5000 )
     */
-    initClient(true);
+    initClient(true, true);
     getHelper()->createPool("__TESTPOOL1_", locatorsG, nullptr, 0, false,
                             std::chrono::milliseconds::zero(), -1, -1, false);
     LOG("CreatePoolForUpdateLocatorList complete.");
@@ -578,7 +585,7 @@ DUNIT_TASK_DEFINITION(CLIENT1, 
CreatePoolForDontUpdateLocatorList)
     = -1, int connections = -1, int loadConditioningInterval = - 1, bool
     isMultiuserMode = false, int updateLocatorListInterval = 5000 )
     */
-    initClient(true);
+    initClient(true, true);
     getHelper()->createPool("__TESTPOOL1_", locatorsG, nullptr, 0, false,
                             std::chrono::milliseconds::zero(), -1, -1, false);
     LOG("CreatePoolForDontUpdateLocatorList complete.");
@@ -591,7 +598,8 @@ DUNIT_TASK_DEFINITION(CLIENT1, 
VerifyUpdateLocatorListThread)
     dunit::sleep(sleepSeconds * 1000);
 
     auto pptr = getHelper()->getCache()->getPoolManager().find("__TESTPOOL1_");
-    int updateIntervalSeconds = pptr->getUpdateLocatorListInterval().count() / 
1000;
+    int updateIntervalSeconds =
+        pptr->getUpdateLocatorListInterval().count() / 1000;
 
     int numLocatorListUpdates =
         CacheHelper::getNumLocatorListUpdates("Querying locator list at:");
diff --git a/cppcache/integration-test/test.bat.in 
b/cppcache/integration-test/test.bat.in
index f44abeb8..ba99c085 100644
--- a/cppcache/integration-test/test.bat.in
+++ b/cppcache/integration-test/test.bat.in
@@ -39,7 +39,6 @@ set 
GF_CLASSPATH=%GF_CLASSPATH%;${CMAKE_BINARY_DIR}/tests/javaobject/javaobject.
 set PROFILERCMD=
 set BUG481=
 set TESTNAME=${TEST}
-set LOG=${TEST}.log
 
 set
 
@@ -49,10 +48,7 @@ if %errorlevel% neq 0 exit /b %errorlevel%
 pushd "$<SHELL_PATH:${TEST_DIR}>"
 if %errorlevel% neq 0 exit /b %errorlevel%
 
-rem In Windows, pipes to tee return tee's exit code instead of executable's
-rem exit code. As a workaround we write exit codes to files.
-("$<SHELL_PATH:$<TARGET_FILE:${TEST}>>" && echo 0 >${TEST}.errorlevel || echo 
1 >${TEST}.errorlevel) | tee %LOG%
-set /p errorlevel= <${TEST}.errorlevel
+"$<SHELL_PATH:$<TARGET_FILE:${TEST}>>"
 if %errorlevel% neq 0 exit /b %errorlevel%
 
 popd
diff --git a/cppcache/integration-test/test.sh.in 
b/cppcache/integration-test/test.sh.in
index 23d2aec9..f137dbca 100644
--- a/cppcache/integration-test/test.sh.in
+++ b/cppcache/integration-test/test.sh.in
@@ -40,7 +40,6 @@ export 
GF_CLASSPATH=$GF_CLASSPATH:${CMAKE_BINARY_DIR}/tests/javaobject/javaobjec
 export PROFILERCMD=
 export BUG481=
 export TESTNAME=${TEST}
-export LOG=${TEST}.log
 
 rm -rf "${TEST_DIR}"
 mkdir -p "${TEST_DIR}"
@@ -51,7 +50,7 @@ if [ `uname` = "Darwin" ]; then
   export DYLD_LIBRARY_PATH=$LD_LIBRARY_PATH
 fi
 
-$DEBUG $<TARGET_FILE:${TEST}> | tee $LOG
+$DEBUG $<TARGET_FILE:${TEST}>
 
 # hack: This is _not_ ideal. We're potentially also masking real product bugs.
 #       For now, we just want something that produces results. TODO: REMOVE 
ASAP


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> Improve test stability
> ----------------------
>
>                 Key: GEODE-4143
>                 URL: https://issues.apache.org/jira/browse/GEODE-4143
>             Project: Geode
>          Issue Type: Test
>          Components: native client
>            Reporter: Jacob S. Barrett
>            Assignee: Jacob S. Barrett
>
> Remove explicitly output logging at the test level and leverage the test 
> runner. This removes tee which tends to hang on windows, causing the test to 
> hang. 



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to