Fix flaky tests due to race condition in call state changes

jitsi · May 17, 2022 · 3bb3db7 · 3bb3db7
1 parent e5b9f40
commit 3bb3db7
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 60 deletions.
diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml
@@ -36,6 +36,12 @@ jobs:
     - name: Build with Maven
       run: mvn verify -B -Pcoverage
 
+    - name: Publish Test Report
+      uses: mikepenz/action-junit-report@41a3188dde10229782fd78cd72fc574884dd7686
+      if: always() # always run even if the previous step fails
+      with:
+        report_paths: '**/target/surefire-reports/TEST-*.xml'
+
     - name: Upload coverage report
       if: matrix.java == env.RELEASE_JAVA_VERSION
       uses: codecov/codecov-action@v2
diff --git a/src/test/java/net/java/sip/communicator/service/protocol/mock/MockBasicTeleOpSet.java b/src/test/java/net/java/sip/communicator/service/protocol/mock/MockBasicTeleOpSet.java
@@ -65,7 +65,7 @@ public void answerCallPeer(CallPeer peer)
 
         ((MockCallPeer) peer).setState(CallPeerState.CONNECTED);
 
-        ((MockCall) peer.getCall()).setCallState(CallState.CALL_IN_PROGRESS);
+        ((MockCall) peer.getCall()).setInProgress();
     }
 
     @Override
@@ -108,11 +108,11 @@ public MockProtocolProvider getProtocolProvider()
         return protocolProvider;
     }
 
-    public synchronized MockCall createIncomingCall(String calee)
+    public synchronized MockCall createIncomingCall(String callee)
     {
         MockCall incomingCall = new MockCall(this);
 
-        MockCallPeer peer = new MockCallPeer(calee, incomingCall);
+        MockCallPeer peer = new MockCallPeer(callee, incomingCall);
 
         incomingCall.addCallPeer(peer);
 

diff --git a/src/test/java/net/java/sip/communicator/service/protocol/mock/MockCall.java b/src/test/java/net/java/sip/communicator/service/protocol/mock/MockCall.java
@@ -78,8 +78,15 @@ public String toString()
         return super.toString() + " " + getProtocolProvider().getProtocolName();
     }
 
-    public void setCallState(CallState newState)
+    public synchronized void setInProgress()
     {
-        super.setCallState(newState);
+        if (getCallState() != CallState.CALL_ENDED)
+        {
+            setCallState(CallState.CALL_IN_PROGRESS);
+        }
+        else
+        {
+            throw new RuntimeException("Call already ended");
+        }
     }
 }
diff --git a/src/test/java/org/jitsi/jigasi/CallStateListener.java b/src/test/java/org/jitsi/jigasi/CallStateListener.java
@@ -47,25 +47,21 @@ public void callStateChanged(CallChangeEvent evt)
     }
 
     public void waitForState(Call      watchedCall,
-                             CallState targetState,
-                             long      timeout)
+                             CallState targetState)
         throws InterruptedException
     {
         this.targetState = targetState;
 
-        // FIXME: we can miss call state anyway ?(but timeout will release)
+        watchedCall.addCallChangeListener(this);
         if (!targetState.equals(watchedCall.getCallState()))
         {
             synchronized (this)
             {
-                watchedCall.addCallChangeListener(this);
-
-                this.wait(timeout);
+                this.wait();
             }
         }
 
         watchedCall.removeCallChangeListener(this);
-
         assertEquals(targetState, watchedCall.getCallState());
     }
 }
diff --git a/src/test/java/org/jitsi/jigasi/CallsHandlingTest.java b/src/test/java/org/jitsi/jigasi/CallsHandlingTest.java
@@ -25,7 +25,6 @@
 import org.jitsi.jigasi.xmpp.*;
 import org.jitsi.service.configuration.*;
 import org.jitsi.xmpp.extensions.rayo.*;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.*;
 import org.jxmpp.jid.*;
 import org.jxmpp.jid.impl.*;
@@ -43,6 +42,7 @@
  * @author Pawel Domas
  * @author Nik Vaessen
  */
+@Timeout(30)
 public class CallsHandlingTest
 {
     private static OSGiHandler osgi;
@@ -65,7 +65,6 @@ public static void setUpClass()
         osgi = new OSGiHandler();
         var fw = osgi.init();
         var start = System.nanoTime();
-        Thread.sleep(5000);
         while (fw.getState() != Framework.ACTIVE)
         {
             if (System.nanoTime() - start > TimeUnit.SECONDS.toNanos(5))
@@ -139,7 +138,7 @@ public void testIncomingSipCall()
 
         // SipGateway ought to accept incoming sip call
         // once JVB conference is joined.
-        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS, 1000);
+        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS);
 
         SipGatewaySession session
             = osgi.getSipGateway().getActiveSessions().get(0);
@@ -155,7 +154,7 @@ public void testIncomingSipCall()
         CallManager.hangupCall(sipCall);
 
         callStateWatch.waitForState(
-            session.getJvbCall(), CallState.CALL_ENDED, 1000);
+            session.getJvbCall(), CallState.CALL_ENDED);
         assertFalse(jvbConfRoom.isJoined());
     }
 
@@ -202,7 +201,7 @@ public void testOutgoingSipCall()
         // Remote SIP peer accepts
         CallManager.acceptCall(sipCall);
 
-        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS, 1000);
+        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS);
 
         GatewaySessionAsserts sessionWatch = new GatewaySessionAsserts();
         sessionWatch.assertJvbRoomJoined(session, 2000);
@@ -213,16 +212,16 @@ public void testOutgoingSipCall()
 
         assertNotNull(chatRoom);
         assertTrue(chatRoom.isJoined());
-        callStateWatch.waitForState(jvbCall, CallState.CALL_IN_PROGRESS, 1000);
+        callStateWatch.waitForState(jvbCall, CallState.CALL_IN_PROGRESS);
         assertEquals(CallState.CALL_IN_PROGRESS, jvbCall.getCallState());
 
         // Now tear down, SIP calee ends the call
         // then XMPP cal should be terminated and MUC room left
 
         CallManager.hangupCall(sipCall);
 
-        callStateWatch.waitForState(sipCall, CallState.CALL_ENDED, 1000);
-        callStateWatch.waitForState(jvbCall, CallState.CALL_ENDED, 1000);
+        callStateWatch.waitForState(sipCall, CallState.CALL_ENDED);
+        callStateWatch.waitForState(jvbCall, CallState.CALL_ENDED);
         assertFalse(chatRoom.isJoined());
     }
 
@@ -270,12 +269,11 @@ public void testFocusLeftTheRoomWithNoResume()
         // Focus will leave the room after inviting us to the conference
         focus.setLeaveRoomAfterInvite(true);
 
-        CallStateListener callStateWatch = new CallStateListener();
-
         // Create incoming call
-        MockCall sipCall = sipProvider.getTelephony().mockIncomingGatewayCall("calee", roomName);
+        MockCall sipCall = sipProvider.getTelephony().mockIncomingGatewayCall("callee", roomName);
 
-        callStateWatch.waitForState(sipCall, CallState.CALL_ENDED, 2000);
+        var callStateWatch = new CallStateListener();
+        callStateWatch.waitForState(sipCall, CallState.CALL_ENDED);
 
         // Now we expect SIP call to be terminated
         assertEquals(CallState.CALL_ENDED, focus.getCall().getCallState());
@@ -302,10 +300,10 @@ public void testFocusLeftTheRoomWithResume()
         // Create incoming call
         MockCall sipCall = sipProvider.getTelephony().mockIncomingGatewayCall("calee", roomName);
 
-        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS, 2000);
+        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS);
 
         // Now we expect SIP call to be in progress, but xmpp call ended
-        callStateWatch.waitForState(focus.getCall(), CallState.CALL_ENDED, 2000);
+        callStateWatch.waitForState(focus.getCall(), CallState.CALL_ENDED);
         assertNull(focus.getChatRoom());
 
         AbstractGateway.setJvbInviteTimeout(origValue);
@@ -343,11 +341,8 @@ public void testCallControl()
         CallContext ctx = new CallContext(this);
         ctx.setDomain(serverName);
 
-        org.jivesoftware.smack.packet.IQ result = callControl.handleDialIq(dialIq, ctx, null);
-
-        assertNotNull(result);
-
-        RefIq callRef = (RefIq) result;
+        var callRef = callControl.handleDialIq(dialIq, ctx, null);
+        assertNotNull(callRef);
 
         String callUri = callRef.getUri();
         assertEquals("xmpp:", callUri.substring(0, 5));
@@ -364,7 +359,7 @@ public void testCallControl()
 
         CallStateListener callStateWatch = new CallStateListener();
 
-        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS, 1000);
+        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS);
 
         Jid callResource = JidCreate.from(callUri.substring(5)); //remove xmpp:
 
@@ -377,7 +372,7 @@ public void testCallControl()
         Call xmppCall = session.getJvbCall();
 
         // We joined JVB conference call
-        callStateWatch.waitForState(xmppCall, CallState.CALL_IN_PROGRESS, 1000);
+        callStateWatch.waitForState(xmppCall, CallState.CALL_IN_PROGRESS);
 
         ChatRoom conferenceChatRoom = session.getJvbChatRoom();
 
@@ -389,8 +384,8 @@ public void testCallControl()
         // FIXME: validate result
         callControl.handleHangUp(hangUp);
 
-        callStateWatch.waitForState(xmppCall, CallState.CALL_ENDED, 1000);
-        callStateWatch.waitForState(sipCall, CallState.CALL_ENDED, 1000);
+        callStateWatch.waitForState(xmppCall, CallState.CALL_ENDED);
+        callStateWatch.waitForState(sipCall, CallState.CALL_ENDED);
         assertFalse(conferenceChatRoom.isJoined());
     }
 
@@ -419,7 +414,7 @@ public void testDefaultJVbRoomProperty()
 
         // SipGateway ought to accept incoming sip call
         // once JVB conference is joined.
-        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS, 2000);
+        callStateWatch.waitForState(sipCall, CallState.CALL_IN_PROGRESS);
 
         // Now tear down, SIP calee ends the call
         // then XMPP cal should be terminated and MUC room left
@@ -435,8 +430,8 @@ public void testDefaultJVbRoomProperty()
 
         CallManager.hangupCall(sipCall);
 
-        callStateWatch.waitForState(xmppCall, CallState.CALL_ENDED, 1000);
-        callStateWatch.waitForState(sipCall, CallState.CALL_ENDED, 1000);
+        callStateWatch.waitForState(xmppCall, CallState.CALL_ENDED);
+        callStateWatch.waitForState(sipCall, CallState.CALL_ENDED);
         assertFalse(jvbRoom.isJoined());
     }
 
@@ -458,9 +453,9 @@ public void testSimultaneousCalls()
 
         CallStateListener callStateWatch = new CallStateListener();
 
-        callStateWatch.waitForState(sipCall1, CallState.CALL_IN_PROGRESS, 1000);
-        callStateWatch.waitForState(sipCall2, CallState.CALL_IN_PROGRESS, 1000);
-        callStateWatch.waitForState(sipCall3, CallState.CALL_IN_PROGRESS, 1000);
+        callStateWatch.waitForState(sipCall1, CallState.CALL_IN_PROGRESS);
+        callStateWatch.waitForState(sipCall2, CallState.CALL_IN_PROGRESS);
+        callStateWatch.waitForState(sipCall3, CallState.CALL_IN_PROGRESS);
 
         // Check peers are not on hold
         CallPeerStateListener peerStateWatch = new CallPeerStateListener();
@@ -492,9 +487,9 @@ public void testSimultaneousCalls()
         CallManager.hangupCall(sipCall2);
         CallManager.hangupCall(sipCall3);
 
-        callStateWatch.waitForState(jvbCall1, CallState.CALL_ENDED, 1000);
-        callStateWatch.waitForState(jvbCall2, CallState.CALL_ENDED, 1000);
-        callStateWatch.waitForState(jvbCall3, CallState.CALL_ENDED, 1000);
+        callStateWatch.waitForState(jvbCall1, CallState.CALL_ENDED);
+        callStateWatch.waitForState(jvbCall2, CallState.CALL_ENDED);
+        callStateWatch.waitForState(jvbCall3, CallState.CALL_ENDED);
 
         assertEquals(CallState.CALL_ENDED, sipCall1.getCallState());
         assertEquals(CallState.CALL_ENDED, sipCall2.getCallState());
@@ -530,7 +525,7 @@ public void testNoFocusInTheRoom()
 
         // Assert incoming call state
         callStateWatch.waitForState(
-            sipCall1, CallState.CALL_INITIALIZATION, 1000);
+            sipCall1, CallState.CALL_INITIALIZATION);
 
         // We expect to have 1 active sessions
         List<SipGatewaySession> sessions = gatewaySessions.getSessions(1000);
@@ -551,7 +546,7 @@ public void testNoFocusInTheRoom()
         assertNull(jvbCall1);
 
         callStateWatch.waitForState(
-            sipCall1, CallState.CALL_ENDED, jvbInviteTimeout + 200);
+            sipCall1, CallState.CALL_ENDED);
 
         assertFalse(jvbRoom1.isJoined());
 

diff --git a/src/test/java/org/jitsi/jigasi/MockJvbConferenceFocus.java b/src/test/java/org/jitsi/jigasi/MockJvbConferenceFocus.java
@@ -87,11 +87,8 @@ public void serviceChanged(ServiceEvent serviceEvent)
 
                         setXmppProvider(protocol);
                     }
-                    catch (OperationFailedException e)
-                    {
-                        logger.error(e, e);
-                    }
-                    catch (OperationNotSupportedException e)
+                    catch (OperationFailedException |
+                           OperationNotSupportedException e)
                     {
                         logger.error(e, e);
                     }
@@ -143,15 +140,7 @@ private void inviteToConference(ChatRoomMember member)
         if (leaveRoomAfterInvite)
         {
             logger.info(myName + " invited peer will leave the room");
-            new Thread(new Runnable()
-            {
-                @Override
-                public void run()
-                {
-                    logger.info(myName + " leaving the room");
-                    tearDown();
-                }
-            }).start();
+            tearDown();
         }
     }