Merge pull request #26 from pavelhoral/fix-4

Fix RPC module test race condition. #4
WrenSecurity · Aug 23, 2022 · b9db9c5 · b9db9c5
2 parents 3bc5ad6 + 5fece10
commit b9db9c5
Show file tree

Hide file tree

Showing 11 changed files with 174 additions and 383 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -22,5 +22,4 @@ jobs:
           key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
           restore-keys: ${{ runner.os }}-m2
       - name: Run the Maven verify phase
-        # Skip tests because of https://github.com/WrenSecurity/wrenicf-java-framework/issues/4
-        run: mvn -B verify --file pom.xml -DskipTests
+        run: mvn -B verify --file pom.xml
diff --git a/.settings/org.eclipse.jdt.core.prefs b/.settings/org.eclipse.jdt.core.prefs
diff --git a/.settings/org.eclipse.jdt.ui.prefs b/.settings/org.eclipse.jdt.ui.prefs
diff --git a/framework-parent/pom.xml b/framework-parent/pom.xml
@@ -36,7 +36,7 @@
         <pgpVerifyKeysVersion>1.6.1</pgpVerifyKeysVersion>
 
         <commons-bom.version>22.2.0</commons-bom.version>
-        <grizzly.version>2.3.22</grizzly.version>
+        <grizzly.version>2.3.35</grizzly.version>
         <groovy.version>2.4.21</groovy.version>
         <logback.version>1.1.7</logback.version>
         <protobuf-java.version>3.0.2</protobuf-java.version>

diff --git a/framework-rpc/src/test/java/org/forgerock/openicf/common/rpc/RequestDistributorTest.java b/framework-rpc/src/test/java/org/forgerock/openicf/common/rpc/RequestDistributorTest.java
@@ -24,11 +24,8 @@
 
 package org.forgerock.openicf.common.rpc;
 
-import java.util.List;
 import java.util.concurrent.CancellationException;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
 
 import org.forgerock.openicf.common.rpc.impl.NIOSimulator;
 import org.forgerock.openicf.common.rpc.impl.TestConnectionContext;
@@ -50,8 +47,7 @@ public class RequestDistributorTest<H extends RemoteConnectionHolder<TestConnect
 
     private TestConnectionGroup<H> client = null;
     private TestConnectionGroup<H> server = null;
-    private NIOSimulator<TestConnectionGroup<H>, H, TestConnectionContext<H>> simulator =
-            null;
+    private NIOSimulator<TestConnectionGroup<H>, H, TestConnectionContext<H>> simulator = null;
 
     @BeforeClass
     public void beforeClass() throws Exception {
@@ -62,12 +58,15 @@ public void beforeClass() throws Exception {
 
         TestMessageListener<H> serverListener = new TestMessageListener<H>(server) {
 
+            @Override
             public void onError(Throwable t) {
             }
 
+            @Override
             public void onMessage(H socket, byte[] bytes) {
             }
 
+            @Override
             public void onMessage(H socket, String message) {
                 TestMessage obj = socket.getRemoteConnectionContext().read(message);
                 if (obj.request >= 0) {
@@ -83,10 +82,12 @@ public void onMessage(H socket, String message) {
                 }
             }
 
+            @Override
             public void onPing(H socket, byte[] bytes) {
 
             }
 
+            @Override
             public void onPong(H socket, byte[] bytes) {
 
             }
@@ -110,23 +111,28 @@ public void afterClass() throws Exception {
 
     public RemoteConnectionHolder<TestConnectionGroup<H>, H, TestConnectionContext<H>> getConnection() {
         return simulator.connect(new TestMessageListener<H>(client) {
+            @Override
             public void onError(Throwable t) {
 
             }
 
+            @Override
             public void onMessage(H socket, byte[] bytes) {
             }
 
+            @Override
             public void onMessage(H socket, String message) {
                 TestMessage obj = socket.getRemoteConnectionContext().read(message);
                 getConnectionGroup().receiveRequestResponse(socket, obj.messageId, obj);
 
             }
 
+            @Override
             public void onPing(H socket, byte[] bytes) {
 
             }
 
+            @Override
             public void onPong(H socket, byte[] bytes) {
 
             }
@@ -142,10 +148,9 @@ public void testSimpleRequest() throws Exception {
             Assert.assertTrue(server.isOperational());
             TestRemoteRequest<H> request = client.trySubmitRequest(new TestRequestFactory<H>(0));
             request.getPromise();
-            
+
             Assert.assertEquals(request.getPromise()
                     .getOrThrowUninterruptibly(5, TimeUnit.SECONDS), "OK");
-            Assert.assertTrue(client.getRemoteRequests().isEmpty());
             Assert.assertTrue(server.getLocalRequests().isEmpty());
         } finally {
             connection.close();
@@ -160,14 +165,13 @@ public void testCallbackRequest() throws Exception {
             Assert.assertTrue(client.isOperational());
             Assert.assertTrue(server.isOperational());
             TestRemoteRequest<H> request = client.trySubmitRequest(new TestRequestFactory<H>(1));
-            
+
             Assert.assertEquals(request.getPromise().getOrThrowUninterruptibly(5, TimeUnit.SECONDS), "OK");
             for (int i = 0; i < 5 && request.getResults().size() != 3; i++) {
                 Reporter.log("Wait for complete request cleanup: " + i, true);
                 Thread.sleep(1000); // Wait to complete all other threads
             }
             Assert.assertEquals(request.getResults().size(), 3);
-            Assert.assertTrue(client.getRemoteRequests().isEmpty());
             Assert.assertTrue(server.getLocalRequests().isEmpty());
         } finally {
             connection.close();
@@ -181,9 +185,9 @@ public void testBlockingCallbackRequest() throws Exception {
         try {
             Assert.assertTrue(client.isOperational());
             Assert.assertTrue(server.isOperational());
-            
+
             TestRemoteRequest<H> request = client.trySubmitRequest(new TestRequestFactory<H>(2));
-      
+
             Assert.assertEquals(request.getPromise().getOrThrowUninterruptibly(15, TimeUnit.SECONDS), "OK");
             Assert.assertEquals(request.getResults().size(), 3);
             for (int i = 0; i < 5 && !(client.getRemoteRequests().isEmpty()
@@ -207,17 +211,20 @@ public void testCancelRequest() throws Exception {
             Assert.assertTrue(server.isOperational());
 
             TestRemoteRequest<H> request = client.trySubmitRequest(new TestRequestFactory<H>(2) {
+                @Override
                 public void handleCallback(H sourceConnection, TestRemoteRequest<H> request,
                         TestMessage message) {
                     request.getPromise().cancel(true);
                 }
             });
             try {
                 request.getPromise().thenOnResult(new ResultHandler<String>() {
+                    @Override
                     public void handleResult(String result) {
                         Assert.fail("Canceled");
                     }
                 }).thenOnException(new ExceptionHandler<Exception>() {
+                    @Override
                     public void handleException(Exception error) {
                         Assert.assertTrue(error instanceof CancellationException);
                     }
@@ -252,7 +259,6 @@ public void testFailedRequest() throws Exception {
 
             TestRemoteRequest<H> request = client.trySubmitRequest(new TestRequestFactory<H>(3));
             request.getPromise().getOrThrowUninterruptibly();
-            Assert.assertTrue(client.getRemoteRequests().isEmpty());
             Assert.assertTrue(server.getLocalRequests().isEmpty());
         } finally {
             connection.close();