[GitHub] [geode] pivotal-eshu commented on a change in pull request #6415: GEODE-8990: Reworked out of date test case

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

pivotal-eshu commented on a change in pull request #6415:
URL: https://github.com/apache/geode/pull/6415#discussion_r624250606



##########
File path: geode-assembly/src/distributedTest/java/org/apache/geode/session/tests/Jetty9CachingClientServerTest.java
##########
@@ -44,29 +45,37 @@ public ContainerInstall getInstall(IntSupplier portSupplier)
    * request
    */
   @Test
-  public void shouldCacheSessionOnClient()
+  public void shouldUpdateSessionAttributesFromServer()
       throws Exception {
     manager.startAllInactiveContainers();
     await().until(() -> {
       ServerContainer container = manager.getContainer(0);
       return container.getState().isStarted();
     });
     String key = "value_testSessionExpiration";
-    String value = "Foo";
+    String localValue = "bogus";
+    String remoteValue = "Foo";
 
     client.setPort(Integer.parseInt(manager.getContainerPort(0)));
-    Client.Response resp = client.set(key, value);
+    Client.Response resp = client.set(key, localValue);
     assertThat(resp.getResponse()).isEqualTo("");
     assertThat(resp.getSessionCookie()).isNotEqualTo("");
 
+    // Make sure the client correctly set its original cached value
+    resp = client.get(key);
+    assertThat(resp.getResponse()).isEqualTo(localValue);
+
     serverVM.invoke("set bogus session key", () -> {

Review comment:
       Probably should change the runnable name here as the test expectation has been changed.

##########
File path: geode-assembly/src/distributedTest/java/org/apache/geode/session/tests/Jetty9CachingClientServerTest.java
##########
@@ -44,29 +45,37 @@ public ContainerInstall getInstall(IntSupplier portSupplier)
    * request
    */
   @Test
-  public void shouldCacheSessionOnClient()
+  public void shouldUpdateSessionAttributesFromServer()
       throws Exception {
     manager.startAllInactiveContainers();
     await().until(() -> {
       ServerContainer container = manager.getContainer(0);
       return container.getState().isStarted();
     });
     String key = "value_testSessionExpiration";
-    String value = "Foo";
+    String localValue = "bogus";
+    String remoteValue = "Foo";
 
     client.setPort(Integer.parseInt(manager.getContainerPort(0)));
-    Client.Response resp = client.set(key, value);
+    Client.Response resp = client.set(key, localValue);
     assertThat(resp.getResponse()).isEqualTo("");
     assertThat(resp.getSessionCookie()).isNotEqualTo("");
 
+    // Make sure the client correctly set its original cached value
+    resp = client.get(key);
+    assertThat(resp.getResponse()).isEqualTo(localValue);
+
     serverVM.invoke("set bogus session key", () -> {
       final InternalCache cache = ClusterStartupRule.memberStarter.getCache();
       Region<String, HttpSession> region = cache.getRegion("gemfire_modules_sessions");
-      region.values().forEach(session -> session.setAttribute(key, "bogus"));
+      region.values().forEach(session -> session.setAttribute(key, remoteValue));
     });
 
-    // Make sure the client still sees its original cached value
-    resp = client.get(key);
-    assertThat(resp.getResponse()).isEqualTo(value);
+    // Make sure the client is now retrieving the value changed in the cluster
+    GeodeAwaitility.await().untilAsserted(() -> {
+      Client.Response response = client.get(key);
+      assertThat(response.getResponse()).isEqualTo(remoteValue);
+    });
+    // assertThat(resp.getResponse()).isEqualTo(remoteValue);

Review comment:
       The commented out code can be removed.




-- 
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