Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky tests due to race condition in call state changes #415

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/maven.yml
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
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
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
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
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
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