[ https://issues.apache.org/jira/browse/GEODE-9108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17314016#comment-17314016 ]
ASF GitHub Bot commented on GEODE-9108: --------------------------------------- pdxcodemonkey commented on a change in pull request #780: URL: https://github.com/apache/geode-native/pull/780#discussion_r606356825 ########## File path: cppcache/integration-test/ThinClientFailoverRegex.hpp ########## @@ -292,7 +292,7 @@ const char* vals[] = {"Value-1", "Value-2", "Value-3", "Value-4"}; const char* nvals[] = {"New Value-1", "New Value-2", "New Value-3", "New Value-4"}; -const char* regionNames[] = {"DistRegionAck", "DistRegionNoAck"}; +const char* regionNames[] = {"DistRegionAck", "not-used"}; Review comment: And again ########## File path: cppcache/integration-test/ThinClientFailoverInterest.hpp ########## @@ -290,7 +290,7 @@ const char* vals[] = {"Value-1", "Value-2", "Value-3", "Value-4"}; const char* nvals[] = {"New Value-1", "New Value-2", "New Value-3", "New Value-4"}; -const char* regionNames[] = {"DistRegionAck", "DistRegionNoAck"}; +const char* regionNames[] = {"DistRegionAck", "not-used"}; Review comment: Above, you changed this to just a static string. Why leave it an array here? Let's get rid of "not-used," unless of course it _is_ used, in which case "not-used" is a bad name. ########## File path: cppcache/integration-test/testThinClientRemoteQueryTimeout.cpp ########## @@ -290,7 +291,7 @@ DUNIT_TASK_DEFINITION(CLIENT1, StepSix) try { LOG("EXECUTE 4 START"); - results = qry->execute(std::chrono::seconds(850)); + results = qry->execute(std::chrono::seconds(850000)); Review comment: 850000 seconds is approximately 9.8 days. Seems a little extreme as a timeout value, you probably wanted milliseconds ########## File path: cppcache/integration-test/ThinClientHelper.hpp ########## @@ -134,7 +134,7 @@ const char* vals[] = {"Value-1", "Value-2", "Value-3", const char* nvals[] = {"New Value-1", "New Value-2", "New Value-3", "New Value-4", "New Value-5", "New Value-6"}; -const char* regionNames[] = {"DistRegionAck", "DistRegionNoAck"}; +const char* regionNames[] = {"DistRegionAck", "not-used"}; Review comment: One more instance ########## File path: cppcache/integration-test/testThinClientRemoteQueryTimeout.cpp ########## @@ -491,7 +493,8 @@ DUNIT_TASK_DEFINITION(CLIENT1, verifyLargeValueTimeout) LOG(logmsg.c_str()); } - SLEEP(150000); // sleep 2.5 min to allow server query to complete + // SLEEP(150000); // sleep 2.5 min to allow server query to complete + SLEEP(15000); // sleep 2.5 min to allow server query to complete Review comment: Same ########## File path: cppcache/integration-test/ThinClientFailoverInterest2.hpp ########## @@ -289,7 +289,7 @@ const char* vals[] = {"Value-1", "Value-2", "Value-3", "Value-4"}; const char* nvals[] = {"New Value-1", "New Value-2", "New Value-3", "New Value-4"}; -const char* regionNames[] = {"DistRegionAck", "DistRegionNoAck"}; +const char* regionNames[] = {"DistRegionAck", "not-used"}; Review comment: Same ########## File path: cppcache/integration-test/testThinClientRemoteQueryTimeout.cpp ########## @@ -447,7 +448,8 @@ DUNIT_TASK_DEFINITION(CLIENT1, verifyNegativeValueTimeout) LOG(logmsg.c_str()); } - SLEEP(150000); // sleep 2.5 min to allow server query to complete + // SLEEP(150000); // sleep 2.5 min to allow server query to complete + SLEEP(15000); // sleep 2.5 min to allow server query to complete Review comment: Comment is incorrect, this is now .25 min (15 sec) ########## File path: cppcache/integration-test/testThinClientRemoteQueryTimeout.cpp ########## @@ -389,7 +390,7 @@ DUNIT_TASK_DEFINITION(CLIENT1, StepEight) } } - results = qry->execute(paramList, std::chrono::seconds(850)); + results = qry->execute(paramList, std::chrono::seconds(850000)); Review comment: milliseconds? -- 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: us...@infra.apache.org > Remove no-ack from existing tests > --------------------------------- > > Key: GEODE-9108 > URL: https://issues.apache.org/jira/browse/GEODE-9108 > Project: Geode > Issue Type: Test > Components: native client > Reporter: Michael Martell > Priority: Major > Labels: pull-request-available > > Many of the geode-native legacy tests use no-ack for the region scope > attribute. This has been shown to be the source of intermittent failures of > many of the tests, and should should be removed. As region scope is purely > server side functionality, there should little to no need to test it from a > client's perspective. If there is a use case for such a no-ack test from > geode-native, it should be captured in a new test. -- This message was sent by Atlassian Jira (v8.3.4#803005)