-
Notifications
You must be signed in to change notification settings - Fork 215
Test more webrtc connectivity scenarios #127
Conversation
The automated tests are failing in Firefox. From what I can tell, the FF build being used by Travis does not trigger the onerror callback for getUserMedia - I think this prevents the dialog from ever showing the "continue without audio/or video" button and so the tests just hang... |
4d1a80a
to
e7217af
Compare
nvm, that's just what happens in FF on my machine... tried on another box and saw the real problem: e7217af |
iceServers: [ | ||
{ | ||
urls: [ | ||
'stun:23.21.150.121' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use stun.l.google.com:19302 instead?
4a3ef60
to
3d01b6d
Compare
@andresusanopinto do you have anything else you want to add? |
// and verify data can be transmitted and received | ||
// (packets should stay on the link if behind a router doing NAT) | ||
function reflexiveConnectivityTest() { | ||
runConnectivityTest(Call.isReflexive, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to hard-code a stun server here. This should still use the stun or turn server provided by Call.asyncCreateTurnConfig.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, that makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let me know what you think of 9f15cd7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use the turn servers provided by https://github.com/webrtc/testrtc/blob/master/src/js/call.js#L160. Do note that you will get a list of TURN servers, however they do STUN as well, just change the URL to stun when stun is required:
turn:23.251.129.26:3479 > stun:23.251.129.26:3479
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok try c49cd65
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks good! Add it to this PR and then lets merge ;)
@KaptenJansson it's already here 👆 |
Doh, I looked for the matching commit string but obviously missed it ;). LGTM! Thanks for your patience and contribution! |
Test more webrtc connectivity scenarios
I discovered this project while trying to troubleshoot a specific WebRTC connectivity issue - host to host behind the same NAT. Currently testrtc only appears to test WebRTC connectivity through a relay, leaving out the server reflexive and host scenarios. This pull request adds tests for these scenarios, and reduces possible confusion by splitting out the network capability tests and throughput tests into their own suites.
See #125 for prior relevant discussion.
cc @KaptenJansson @andresusanopinto