1007196 - Re-enable WebRTC unit-tests

Status: RESOLVED DUPLICATE of bug 1140584

(Core :: WebRTC: Signaling, defect)

Description

[Nils Ohlmeier [:drno]]

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.

Comment 1

[(not currently active) Ted Mielczarek]

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

Comment 2

[Ethan Hugg [:ehugg]]

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.

Comment 3

[Ethan Hugg [:ehugg]]

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 4

[Ethan Hugg [:ehugg]]

Attachment: Re-enable the Signaling unittests for Linux and OSX

Comment 5

[Ethan Hugg [:ehugg]]

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.

Comment 6

[Ethan Hugg [:ehugg]]

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 7

[(not currently active) Ted Mielczarek]

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?

Comment 8

[Nils Ohlmeier [:drno]]

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?

Comment 9

[(not currently active) Ted Mielczarek]

We discussed this briefly on IRC, comment 1 contains pretty much everything you need.

Comment 10

[Ethan Hugg [:ehugg]]

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?

Comment 11

[(not currently active) Ted Mielczarek]

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 12

[(not currently active) Ted Mielczarek]

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!

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/047f98eef5cf

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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

Comment 15

[Ethan Hugg [:ehugg]]

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)

Comment 16

[Ethan Hugg [:ehugg]]

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.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://tbpl.mozilla.org/php/getParsedLog.php?id=39376211&tree=Mozilla-Inbound

Comment 18

[Eric Rescorla (:ekr)]

Isn't the problem just the lack of a STUN server?

Comment 19

[Ethan Hugg [:ehugg]]

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

Comment 20

[Eric Rescorla (:ekr)]

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.

Comment 21

[Nils Ohlmeier [:drno]]

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.

Comment 22

[Byron Campen [:bwc]]

Bug 1140584 is where the action is now.

Comment 23

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.