From d988247134fae0851326d20d02d0c59971dbf61e Mon Sep 17 00:00:00 2001 From: Ingo Bauersachs Date: Wed, 18 May 2022 00:18:22 +0200 Subject: [PATCH] Fix flaky tests due to race condition in call state changes --- .../protocol/mock/MockBasicTeleOpSet.java | 6 +- .../service/protocol/mock/MockCall.java | 11 +++- .../org/jitsi/jigasi/CallStateListener.java | 10 +-- .../org/jitsi/jigasi/CallsHandlingTest.java | 63 +++++++++---------- .../jitsi/jigasi/MockJvbConferenceFocus.java | 17 +---- 5 files changed, 47 insertions(+), 60 deletions(-) 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 index fd1f4830a..3ad5c903c 100644 --- 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 index 02e8f61bc..00db7cf8b 100644 --- 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 index 4b0677d73..a8dffa0ce 100644 --- 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 index 3a49162c8..7d915c896 100644 --- 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,7 +212,7 @@ 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 @@ -221,8 +220,8 @@ public void testOutgoingSipCall() 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 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 index 3123c0bbb..32f1a71f2 100644 --- 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(); } }