Closed Bug 1007196 Opened 5 years ago Closed 4 years ago
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.
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.
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.
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?
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?
We discussed this briefly on IRC, comment 1 contains pretty much everything you need.
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.
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+
Backed out for intermittent Linux failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/e9cfc11a722d https://tbpl.mozilla.org/php/getParsedLog.php?id=39373956&tree=Mozilla-Inbound
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.
You need to log in before you can comment on or make changes to this bug.