[
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)