kirklund commented on a change in pull request #6919:
URL: https://github.com/apache/geode/pull/6919#discussion_r720584645



##########
File path: 
geode-core/src/integrationTest/java/org/apache/geode/internal/io/LocatorSerializableObjectFilterIntegrationTest.java
##########
@@ -195,7 +195,7 @@ public void configuresValidateSerializableObjects_onJava8() 
{
         .isTrue();
     assertThat(isValidateSerializableObjectsConfigured())
         .as(VALIDATE_SERIALIZABLE_OBJECTS)
-        .isTrue();
+        .isTrue(); // ??? why not false

Review comment:
       It looks like `assumeThat(isJavaVersionAtLeast(JAVA_9)).isTrue();` on 
the first line of this test (`configuresValidateSerializableObjects_onJava8`) 
is wrong... it should be `isJavaVersionAtMost(JAVA_8)`.
   
   Any tests that result in setting the global serial filter in a VM will need 
to either bounce that VM after the test or live in its own test class so that 
the JVM does away.

##########
File path: 
geode-core/src/integrationTest/java/org/apache/geode/internal/io/LocatorSerializableObjectFilterIntegrationTest.java
##########
@@ -195,7 +195,7 @@ public void configuresValidateSerializableObjects_onJava8() 
{
         .isTrue();
     assertThat(isValidateSerializableObjectsConfigured())
         .as(VALIDATE_SERIALIZABLE_OBJECTS)
-        .isTrue();
+        .isTrue(); // ??? why not false

Review comment:
       `LocatorLauncher` should configure BOTH global serial filter AND 
`validate-serializable-objects` when `start` is invoked and the JVM is Java 8.
   
   I bet my code doesn't yet configure `validate-serializable-objects`.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to