Skip to content

Commit

Permalink
Fix flaky tests due to race condition in call state changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@ibauersachs
ibauersachs committed May 17, 2022
1 parent e5b9f40 commit 3bb3db7
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 60 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/maven.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions src/test/java/net/java/sip/communicator/service/protocol/mock/MockBasicTeleOpSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
11 changes: 9 additions & 2 deletions src/test/java/net/java/sip/communicator/service/protocol/mock/MockCall.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
}
10 changes: 3 additions & 7 deletions src/test/java/org/jitsi/jigasi/CallStateListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
63 changes: 29 additions & 34 deletions src/test/java/org/jitsi/jigasi/CallsHandlingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand All @@ -43,6 +42,7 @@
* @author Pawel Domas
* @author Nik Vaessen
*/
@Timeout(30)
public class CallsHandlingTest
{
private static OSGiHandler osgi;
Expand All @@ -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))
Expand Down Expand Up @@ -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);
Expand All @@ -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());
}

Expand Down Expand Up @@ -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);
Expand All @@ -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());
}

Expand Down Expand Up @@ -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());
Expand All @@ -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);
Expand Down Expand Up @@ -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));
Expand All @@ -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:

Expand All @@ -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();

Expand All @@ -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());
}

Expand Down Expand Up @@ -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
Expand All @@ -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());
}

Expand All @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand All @@ -551,7 +546,7 @@ public void testNoFocusInTheRoom()
assertNull(jvbCall1);

callStateWatch.waitForState(
sipCall1, CallState.CALL_ENDED, jvbInviteTimeout + 200);
sipCall1, CallState.CALL_ENDED);

assertFalse(jvbRoom1.isJoined());

Expand Down
17 changes: 3 additions & 14 deletions src/test/java/org/jitsi/jigasi/MockJvbConferenceFocus.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
}
}

Expand Down

0 comments on commit 3bb3db7

Please sign in to comment.