Closed Bug 1007196 Opened 5 years ago Closed 4 years ago

Re-enable WebRTC unit-tests

Categories

(Core :: WebRTC: Signaling, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1140584

People

(Reporter: drno, Assigned: drno)

Details

Attachments

(1 file)

To increase the testing coverage we should re-enable the WebRTC unit tests on tbpl.
If we can it probably make sense to do this slowly, e.g. platform by platform.
QA Contact: drno
ekr pinged me about this on IRC. I think the right way to approach this is to put them into the C++ unit test job we're already running. This just involves putting the binaries in the right place in the test package, the C++ unittest runner will run all binaries in that directory:
http://hg.mozilla.org/mozilla-central/annotate/fef1a56f0237/testing/testsuite-targets.mk#l499

n.b. we also have a list of tests to skip on Android:
http://mxr.mozilla.org/mozilla-central/source/testing/android_cppunittest_manifest.txt
Some, like the signaling_unittest are already in the unit test job but abort early because they look for an environment variable in order to run.  I will submit a patch shortly for this one pending some try runs.
Initial try run with signaling unittests back on - https://tbpl.mozilla.org/?tree=Try&rev=2297e5d21cca

It looks like they are just broken for Android.  They do not build or run on Windows or B2G.  I am going to upload a patch that enables them only for Linux and OSX which I think has a good chance of being solidly green.
Assignee: nobody → ethanhugg
Status: NEW → ASSIGNED
Comment on attachment 8418934 [details] [diff] [review]
Re-enable the Signaling unittests for Linux and OSX


CHECK_ENVIRONMENT_FLAG has been used to short-circuit this test on the builders since they don't have MOZ_WEBRTC_TESTS set.  This change will make it run on the builders for Linux and OSX platforms. 

Try is here - https://tbpl.mozilla.org/?tree=Try&rev=702b6baf1d54

If you search the full log for signaling_unittests you'll find the successful runs.
Attachment #8418934 - Flags: review?(ted)
I should also mention that I believe these have a better chance of being green now because of the patch in bug 819549.  The signaling unittests now run the PC calls on the main thread only.
Comment on attachment 8418934 [details] [diff] [review]
Re-enable the Signaling unittests for Linux and OSX

Review of attachment 8418934 [details] [diff] [review]:
-----------------------------------------------------------------

We don't want to run them on the builders, we're moving things out of make check. It shouldn't be much work to get them into the test package and run with the rest of the C++ unit tests, can we just do that?
Attachment #8418934 - Flags: review?(ted)
Attachment #8418934 - Attachment is obsolete: true
Assignee: ethanhugg → nobody
Ted can you point me to any documentation or describe briefly what needs to get done to move the WebRTC unit tests into the test package?
Flags: needinfo?(ted)
We discussed this briefly on IRC, comment 1 contains pretty much everything you need.
Flags: needinfo?(ted)
Assignee: nobody → drno
I thought the signaling_unittests were moved out of make check months ago.  They do not run for me when I do a make check, only when I run ./mach cppunittest.  From the Try runs above you can see the results for the signaling_unittests show up under the 'cpp' and not under the 'B'.  Is this not correct?
Oh, huh, you're absolutely right. I'm way off base here. I guess your original patch is fine then. Sorry, I should have looked at your try push.
Attachment #8418934 - Attachment is obsolete: false
Attachment #8418934 - Flags: review?(ted)
Comment on attachment 8418934 [details] [diff] [review]
Re-enable the Signaling unittests for Linux and OSX

Review of attachment 8418934 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry about that!
Attachment #8418934 - Flags: review?(ted) → review+
Failure was this:
10:56:57     INFO -  [ RUN      ] SignalingAgentTest.CreateOfferSetLocalTrickleTestServer
10:56:59     INFO -  agent: ICE Gathering State: gathering
10:56:59     INFO -  agent: onCreateOfferSuccess =
10:56:59     INFO -      v=0
10:56:59     INFO -      o=Mozilla-SIPUA-32.0a1 6049 0 IN IP4 0.0.0.0
10:56:59     INFO -      s=SIP Call
10:56:59     INFO -      t=0 0
10:56:59     INFO -      a=ice-ufrag:83096e35
10:56:59     INFO -      a=ice-pwd:12adf81afbbe1e05496e3f03873ad0fb
10:56:59     INFO -      a=fingerprint:sha-256 CF:8C:BC:E8:A2:AB:28:B6:9B:9E:C0:5B:EF:57:21:B9:E2:B3:D4:67:04:23:ED:CD:46:8A:E6:30:85:33:E2:49
10:56:59     INFO -      m=audio 56856 RTP/SAVPF 109 0 8 101
10:56:59     INFO -      c=IN IP4 10.12.51.144
10:56:59     INFO -      a=rtpmap:109 opus/48000/2
10:56:59     INFO -      a=ptime:20
10:56:59     INFO -      a=rtpmap:0 PCMU/8000
10:56:59     INFO -      a=rtpmap:8 PCMA/8000
10:56:59     INFO -      a=rtpmap:101 telephone-event/8000
10:56:59     INFO -      a=fmtp:101 0-15
10:56:59     INFO -      a=sendrecv
10:56:59     INFO -      a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level
10:56:59     INFO -      a=setup:actpass
10:56:59     INFO -      a=candidate:0 1 UDP 2130379007 10.12.51.144 56856 typ host
10:56:59     INFO -      a=candidate:0 2 UDP 2130379006 10.12.51.144 60562 typ host
10:56:59     INFO -      a=rtcp-mux
10:56:59     INFO -  agent: SDPSanityCheck flags for offer = 0x3 SHOULD_SEND_AUDIO SHOULD_RECV_AUDIO
10:57:01     INFO -  agent: ICE Gathering State: complete
10:57:01     INFO -  agent: Init Complete
10:57:01     INFO -  agent: Signaling State: SignalingHaveLocalOffer
10:57:01     INFO -  agent: onSetLocalDescriptionSuccess
10:57:02     INFO -  /builds/slave/m-in-osx64-d-00000000000000000/build/media/webrtc/signaling/test/signaling_unittests.cpp:3047: Failure
10:57:02     INFO -  Expected: (2U) <= (agent(0)->MatchingCandidates(kBogusSrflxAddress)), actual: 2 vs 0
10:57:02     INFO -  ICE(PC:1399658217908128): No STUN servers specified
10:57:02     INFO -  ICE(PC:1399658217908128): No TURN servers specified
10:57:02     INFO -  STUN-CLIENT(srflx(IP4:10.12.51.144:56856/UDP|IP4:10.12.51.144:3478/UDP)): Timed out
10:57:02     INFO -  STUN-CLIENT(srflx(IP4:10.12.51.144:60562/UDP|IP4:10.12.51.144:3478/UDP)): Timed out
10:57:02     INFO -  STUN-CLIENT(srflx(IP4:10.12.51.144:50974/UDP|IP4:10.12.51.144:3478/UDP)): Timed out
10:57:02     INFO -  STUN-CLIENT(srflx(IP4:10.12.51.144:55584/UDP|IP4:10.12.51.144:3478/UDP)): Timed out
10:57:02     INFO -  STUN-CLIENT(srflx(IP4:10.12.51.144:64882/UDP|IP4:10.12.51.144:3478/UDP)): Timed out
10:57:02     INFO -  STUN-CLIENT(srflx(IP4:10.12.51.144:51589/UDP|IP4:10.12.51.144:3478/UDP)): Timed out
10:57:02     INFO -  agent: Close
10:57:02     INFO -  agent: Signaling State: SignalingClosed
10:57:02     INFO -  [  FAILED  ] SignalingAgentTest.CreateOfferSetLocalTrickleTestServer (4534 ms)
The test is here - 
http://mxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/test/signaling_unittests.cpp#3020

I suspect the PR_Sleep(1000) that comes right before this assert.
Isn't the problem just the lack of a STUN server?
>Isn't the problem just the lack of a STUN server?

Would that be intermittent though? It succeeded many times on M-I as well.  I see a least a dozen green 'cpp'.  We could disable this one test or change how it waits.  Your thoughts?
You're right. I was just not thinking.

I don't know what the problem is, since it looks like we weren't supposed
to get these candidates and we didn't.
I just ran the signaling_unittests locally without network connectivity (but with interfaces from VMWare up so ICE would at least find some interfaces to try) and all the tests are passing. So it is pretty sure these tests don't depend on a STUN server being present.
Bug 1140584 is where the action is now.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1140584
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.