pdxcodemonkey commented on a change in pull request #919:
URL: https://github.com/apache/geode-native/pull/919#discussion_r817836093
##########
File path: cppcache/integration/test/FunctionExecutionTest.cpp
##########
@@ -324,7 +324,9 @@ TEST(FunctionExecutionTest,
testThatFunctionExecutionThrowsExceptionNonHA) {
.withType("PARTITION")
.execute();
- cluster.getServers()[2].stop();
+ auto &targetServer = cluster.getServers()[2];
+ targetServer.stop();
+ targetServer.wait();
Review comment:
Why this separation of concerns between stop() and wait()? Is there
ever a circumstance where we call stop() and don't intend for the process to
actually be terminated yet when the function returns?
##########
File path: cppcache/integration/test/PdxInstanceTest.cpp
##########
@@ -395,9 +395,8 @@ TEST(PdxInstanceTest, testInstancePutAfterRestart) {
cv_status.wait(lock, [&status] { return !status; });
}
- std::this_thread::sleep_for(std::chrono::seconds{30});
-
for (auto& server : cluster.getServers()) {
+ server.wait();
Review comment:
Okay I see this used separately here in several places, but... would it
be necessary to call wait() in the other places if stop() just actually made
sure the process was terminated?
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
