Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Test more webrtc connectivity scenarios #127

Merged
merged 16 commits into from
Oct 9, 2015

Conversation

jessetane
Copy link
Contributor

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

@jessetane
Copy link
Contributor Author

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...

@jessetane
Copy link
Contributor Author

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'
Copy link
Contributor

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?

@KaptenJansson
Copy link
Contributor

@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, {
Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that makes sense.

Copy link
Contributor Author

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

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok try c49cd65

Copy link
Contributor

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 ;)

@jessetane
Copy link
Contributor Author

@KaptenJansson it's already here 👆

@KaptenJansson
Copy link
Contributor

Doh, I looked for the matching commit string but obviously missed it ;).

LGTM!

Thanks for your patience and contribution!

KaptenJansson added a commit that referenced this pull request Oct 9, 2015
Test more webrtc connectivity scenarios
@KaptenJansson KaptenJansson merged commit b5bcc73 into webrtc:master Oct 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants