[GitHub] [geode-native] mreddington commented on a change in pull request #658: GEODE-8129: A test that breaks and reestablishes the proxy connection.

[GitBox <gi...@apache.org>]

mreddington commented on a change in pull request #658:
URL: https://github.com/apache/geode-native/pull/658#discussion_r497671089



##########
File path: cppcache/acceptance-test/SNITest.cpp
##########
@@ -174,4 +175,53 @@ TEST_F(SNITest, connectWithoutProxyFails) {
   cache.close();
 }
 
+TEST_F(SNITest, dropSNIProxyTest) {
+  const auto clientTruststore =
+      (clientSslKeysDir / boost::filesystem::path("/truststore_sni.pem"));
+
+  auto cache = CacheFactory()
+                   .set("log-level", "debug")
+                   .set("log-file", "SNITest.log")
+                   .set("ssl-enabled", "true")
+                   .set("ssl-truststore", clientTruststore.string())
+                   .create();
+
+  auto portString = runDockerCommand("docker port haproxy");
+  auto portNumber = parseProxyPort(portString);
+
+  cache.getPoolManager()
+      .createFactory()
+      .setSniProxy("localhost", portNumber)
+      .addLocator("locator-maeve", 10334)
+      .create("pool");
+
+  auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+                    .setPoolName("pool")
+                    .create("jellyfish");
+
+  auto systemRVal = std::system("docker pause haproxy");
+  if (systemRVal == -1) {
+    BOOST_LOG_TRIVIAL(error) << "std::system returned: " << systemRVal;

Review comment:
       Honestly I don't know. This test is cobbled out of the existing SNI tests, this is the same structure we use to initiate the docker images - invoke the command, check the result, log the error. I figured when in Rome...
   
   The pseudo-dependency is already there because this is how we start the proxy in the test fixture.
   
   I don't think an expectation is appropriate here. We're not testing whether or not a system command works... You know? I'd love if GTest had an "Incomplete" state for tests that incur an error beyond the scope of the test, in the environment. But then again, we shouldn't be writing big tests like this in the first place with a unit test framework.




----------------------------------------------------------------
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:
users@infra.apache.org