Github user pivotal-amurmann commented on a diff in the pull request:
https://github.com/apache/geode/pull/737#discussion_r135156514
--- Diff:
geode-protobuf/src/test/java/org/apache/geode/protocol/RoundTripCacheConnectionJUnitTest.java
---
@@ -119,7 +119,14 @@ public void setup() throws Exception {
}
CacheFactory cacheFactory = new CacheFactory(properties);
- cacheFactory.set("mcast-port", "0"); // sometimes it isn't due to
other tests.
+ cacheFactory.set(ConfigurationProperties.MCAST_PORT, "0"); //
sometimes it isn't due to other
+ // tests.
+ cacheFactory.set(ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION,
"false"); // sometimes it
+
// isn't due to
+
// other tests.
--- End diff --
I realize this is following existing style, but could we be more explicit
in the comment and say what `it isn't`? Something like `sometimes it is not set
to "false" due to test pollution` would be clearer. Or Even just `guard against
test pollution`. II had to read the comment multiple times to understand what
was going on.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
