950317 - [emulator][mochitest] Setup eth0 IP address to NetworkManager for running PeerConnection mochitest cases

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Jason Smith [:jsmith]]

See https://tbpl.mozilla.org/?tree=Try&rev=b0d0d508d054.

The emulator mochitests are failing right now across the board because setting a local description on a remote peer connection is failing to fire a success callback. As such, all of the mochitests are timing out on emulators.

Comment 1

[Jason Smith [:jsmith]]

ekr - this is a blocker for getting the Peer Connection mochitests running on emulators. Can find someone to look into this?

Comment 2

[Maire Reavy [:mreavy]]

Randell and I are looking at this now.

Comment 3

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

We need to setup the eth0 IP address in NetworkManager on emulator while running mochitest. Gecko cannot generate a correct SDP with candidate address without have IP address setup in b2g NetworkManager.

Comment 4

[Jason Smith [:jsmith]]

(In reply to Shih-Chiang Chien [:schien] from comment #3)
> We need to setup the eth0 IP address in NetworkManager on emulator while
> running mochitest. Gecko cannot generate a correct SDP with candidate
> address without have IP address setup in b2g NetworkManager.

Note - there still is a bug here in the P2P code. We should eventually fire a callback for any handshake-based function, even if the function call will cause an error. We shouldn't timeout by having no success or error callback fire.

Comment 5

[Eric Rescorla (:ekr)]

(In reply to Jason Smith [:jsmith] from comment #4)
> (In reply to Shih-Chiang Chien [:schien] from comment #3)
> > We need to setup the eth0 IP address in NetworkManager on emulator while
> > running mochitest. Gecko cannot generate a correct SDP with candidate
> > address without have IP address setup in b2g NetworkManager.
> 
> Note - there still is a bug here in the P2P code. We should eventually fire
> a callback for any handshake-based function, even if the function call will
> cause an error. We shouldn't timeout by having no success or error callback
> fire.

1. This is how the API is specified, so if you think it's wrong, you need
to raise an issue with W3C, not us.
2. AFAIK all the WebRTC code behaves identically here.

Comment 6

[Eric Rescorla (:ekr)]

As you can see from the trace below, it appears we are firing a callback.
We are firing success (look for EKR in the trace).

The reason you are seeing a timeout rather than an error is that the
tests should be but aren't watching the ICE state, so they just wait
for media to start flowing and that takes a long time to time out.


3:39:35     INFO -  12-13 11:06:05.427   708   708 I GeckoDump: 12638 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Got offer: {"type":"offer","sdp":"v=0\r\no=Mozilla-SIPUA-29.0a1 1034 0 IN IP4 0.0.0.0\r\ns=SIP Call\r\nt=0 0\r\na=ice-ufrag:96f40307\r\na=ice-pwd:9e567771c9b80919cc096a9a7d616d47\r\na=fingerprint:sha-256 D1:B1:1A:2F:B7:B5:5C:90:E9:6B:49:EE:E2:64:96:B1:45:62:B2:DF:B5:27:F5:2D:5E:8C:0B:16:0C:95:9B:8F\r\nm=audio 9 RTP/SAVPF 109 0 8 101\r\nc=IN IP4 0.0.0.0\r\na=rtpmap:109 opus/48000/2\r\na=ptime:20\r\na=rtpmap:0 PCMU/8000\r\na=rtpmap:8 PCMA/8000\r\na=rtpmap:101 telephone-event/8000\r\na=fmtp:101 0-15\r\na=sendrecv\r\na=setup:actpass\r\na=rtcp-mux\r\nm=application 9 DTLS/SCTP 5000 \r\nc=IN IP4 0.0.0.0\r\na=sctpmap:5000 webrtc-datachannel 16\r\na=setup:actpass\r\n"}
03:39:35     INFO -  12-13 11:06:05.457   708   708 I GeckoDump: 12639 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Local create offer does not change signaling state
03:39:35     INFO -  12-13 11:06:05.477   708   708 I GeckoDump: 12640 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | m=application is contained in the SDP
03:39:35     INFO -  12-13 11:06:05.497   708   708 I GeckoDump: 12641 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Run step: PC_LOCAL_SET_LOCAL_DESCRIPTION
03:39:36     INFO -  12-13 11:06:05.567   708   708 I GeckoDump: 12642 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcLocal): 'onsignalingstatechange' event fired
03:39:36     INFO -  12-13 11:06:05.567   708   708 I GeckoDump: 12643 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcLocal): 'onsignalingstatechange' event registered for async check
03:39:36     INFO -  12-13 11:06:05.627   708   708 I GeckoDump: 12644 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcLocal): Successfully set the local description
03:39:36     INFO -  12-13 11:06:05.637   708   708 I GeckoDump: 12645 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | signalingState after local setLocalDescription is 'have-local-offer'
03:39:36     INFO -  12-13 11:06:05.667   708   708 I GeckoDump: 12646 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Run step: PC_REMOTE_SET_REMOTE_DESCRIPTION
03:39:36     INFO -  12-13 11:06:05.777   708   708 I GeckoDump: 12647 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): 'onsignalingstatechange' event fired
03:39:36     INFO -  12-13 11:06:05.797   708   708 I GeckoDump: 12648 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): 'onsignalingstatechange' event registered for async check
03:39:36     INFO -  12-13 11:06:05.817   708   708 I GeckoDump: 12649 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): Successfully set remote description
03:39:36     INFO -  12-13 11:06:05.827   708   708 I GeckoDump: 12650 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | signalingState after remote setRemoteDescription is 'have-remote-offer'
03:39:36     INFO -  12-13 11:06:05.847   708   708 I GeckoDump: 12651 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Run step: PC_REMOTE_CREATE_ANSWER
03:39:36     INFO -  12-13 11:06:05.967   708   708 I GeckoDump: 12652 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): Got answer: {"type":"answer","sdp":"v=0\r\no=Mozilla-SIPUA-29.0a1 273 0 IN IP4 0.0.0.0\r\ns=SIP Call\r\nt=0 0\r\na=ice-ufrag:dceaa766\r\na=ice-pwd:3b68a81d40e38f1e2059cbbd243ec9d5\r\na=fingerprint:sha-256 76:C1:AD:58:EB:CB:0F:B7:18:E6:A3:C2:72:5E:54:A1:44:01:6C:36:7A:78:AB:6C:DA:5F:BE:30:A4:60:01:A9\r\nm=audio 9 RTP/SAVPF 109 101\r\nc=IN IP4 0.0.0.0\r\na=rtpmap:109 opus/48000/2\r\na=ptime:20\r\na=rtpmap:101 telephone-event/8000\r\na=fmtp:101 0-15\r\na=sendrecv\r\na=setup:active\r\na=rtcp-mux\r\nm=application 9 DTLS/SCTP 5000 \r\nc=IN IP4 0.0.0.0\r\na=sctpmap:5000 webrtc-datachannel 16\r\na=setup:active\r\n"}
03:39:36     INFO -  12-13 11:06:05.977   708   708 I GeckoDump: 12653 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Remote create offer does not change signaling state
03:39:36     INFO -  12-13 11:06:05.997   708   708 I GeckoDump: 12654 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Run step: PC_LOCAL_SET_REMOTE_DESCRIPTION

EKR: This is the caller setting the remote description. At this point he thinks
the call is set up and will start doing ICE.


03:39:36     INFO -  12-13 11:06:06.577   708   708 I GeckoDump: 12655 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcLocal): 'onsignalingstatechange' event fired
03:39:36     INFO -  12-13 11:06:06.607   708   708 I GeckoDump: 12656 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcLocal): 'onsignalingstatechange' event registered for async check
03:39:36     INFO -  12-13 11:06:06.648   708   708 I GeckoDump: 12657 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcLocal): Successfully set remote description

EKR: This is the callback indicating that the caller setRemote succeeded.




03:39:36     INFO -  12-13 11:06:06.677   708   708 I GeckoDump: 12658 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | signalingState after local setRemoteDescription is 'stable'
03:39:36     INFO -  12-13 11:06:06.767   708   708 I GeckoDump: 12659 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcLocal): 'onaddstream' event fired for [object MediaStream]
03:39:36     INFO -  12-13 11:06:06.777   708   708 I GeckoDump: 12660 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Got media stream: video (remote)
03:39:36     INFO -  12-13 11:06:06.957   708   708 I GeckoDump: 12661 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Run step: PC_REMOTE_SET_LOCAL_DESCRIPTION

EKR: This is the callee setting the local description. At this point he thinks
the call is set up and will start doing ICE.


03:39:36     INFO -  12-13 11:06:07.027   708   708 I GeckoDump: 12662 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): Register callbacks for 'ondatachannel' and 'onopen'
03:39:36     INFO -  12-13 11:06:07.517   708   708 I GeckoDump: 12663 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | canplaythrough fired for media element pcLocal_remote_video
03:39:36     INFO -  12-13 11:06:07.717   708   708 I GeckoDump: 12664 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | timeupdate fired for media element pcLocal_remote_video
03:39:36     INFO -  12-13 11:06:07.727   708   708 I GeckoDump: 12665 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | time passed for media element pcLocal_remote_video
03:39:36     INFO -  12-13 11:06:08.517   708   708 I GeckoDump: 12666 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): 'onsignalingstatechange' event fired
03:39:36     INFO -  12-13 11:06:08.537   708   708 I GeckoDump: 12667 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): 'onsignalingstatechange' event registered for async check
03:39:36     INFO -  12-13 11:06:08.577   708   708 I GeckoDump: 12668 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): Successfully set the local description

EKR: This is the callback indicating that the callee setLocal succeeded.



03:39:36     INFO -  12-13 11:06:08.647   708   708 I GeckoDump: 12669 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): 'onaddstream' event fired for [object MediaStream]
03:39:36     INFO -  12-13 11:06:08.657   708   708 I GeckoDump: 12670 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Got media stream: video (remote)
03:39:36     INFO -  12-13 11:06:09.058   708   708 I GeckoDump: 12671 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | canplaythrough fired for media element pcRemote_remote_video
03:39:36     INFO -  12-13 11:06:09.067   708   708 I GeckoDump: 12672 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | timeupdate fired for media element pcRemote_remote_video
03:39:36     INFO -  12-13 11:06:09.107   708   708 I GeckoDump: 12673 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | time passed for media element pcRemote_remote_video


EKR: The errors.





03:39:36     INFO -  12-13 11:06:10.617   665   764 E Profiler: BPUnw: [1 total] thread_unregister_for_profiling(me=0x22ee0)  (NOT REGISTERED)
03:39:36     INFO -  12-13 11:06:11.077   665   770 E Profiler: BPUnw: [1 total] thread_unregister_for_profiling(me=0x22fa0)  (NOT REGISTERED)
03:39:36     INFO -  12-13 11:06:11.227   665   772 E Profiler: BPUnw: [1 total] thread_unregister_for_profiling(me=0x22fe0)  (NOT REGISTERED)
03:39:36     INFO -  12-13 11:08:27.797   665   791 E Profiler: BPUnw: [1 total] thread_unregister_for_profiling(me=0x22c58)  (NOT REGISTERED)

Comment 7

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: gUM_enable_3g_data.patch

Hi Vicamo, here is the patch for enabling 3G data inside the mochitest. To run this test case, you need to execute |./mach mochitest-remote dom/media/tests/mochitest/| under b2g root folder.

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Shih-Chiang Chien [:schien] from comment #7)
> Hi Vicamo, here is the patch for enabling 3G data inside the mochitest. To
> run this test case, you need to execute |./mach mochitest-remote
> dom/media/tests/mochitest/| under b2g root folder.

I got:

I/Gecko   (  310): -*- RadioInterface[0]: 'ril.data.enabled' is now true
I/Gecko   (  310): -*- RadioInterface[0]: RIL is not ready for data connection: radio's not ready

But that doesn't meet my expectation :(

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

When started with `./run-emulator.sh` or `./mach marionette-webapi`:

  $ telnet localhost 5554
  > gsm status
  gsm voice state: home
  gsm data state:  home
  OK

When started with `./mach mochitest-remote`:

  $ telnet localhost 5554
  > gsm status
  gsm voice state: unregistered
  gsm data state:  unregistered
  OK

That's really weird.

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

Somehow b2g process is restarted, and when it does, nobody sends 'setRadioEnabled' down to RIL again.

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

Two things:
1) is it normal that b2g is killed/restarted during mochitest tests?
2) who's responsible for the second 'setRadioEnabled' request to RIL?

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

And, no one is sending "system-message-listener-ready", so all unsolicited messages are queued up and never reach content side.

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Attachment: gUM_enable_3g_data.patch : v2

Still don't work because we need "system-message-listener-ready" being fired, or no attribute updates will ever occur on content side, so there is no way to determine whether we have already data connection set up. :(

Comment 14

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

@ahal, can you help giving us some clue about the question in comment 11 and comment 13? Vicamo and I are trying to enable 3G data connection in emulator mochitest.

Comment 15

[Andrew Halberstadt [:ahal]]

(In reply to Vicamo Yang [:vicamo][:vyang] (PTO Dec. 21 ~ Jan. 5) from comment #11)
> Two things:
> 1) is it normal that b2g is killed/restarted during mochitest tests?
> 2) who's responsible for the second 'setRadioEnabled' request to RIL?

Yes it's normal. We need to restart the b2g process to use the specially generated profile for the tests. Are we not restarting it properly? Is there a call we can make to do it manually? You might be able to drop it in here: http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/b2g_start_script.js

Comment 16

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

In emulator mochitest, we replace the system app with test-container. Therefore, no system-message-listener-ready event is sent to gecko under this configuration. I'm trying to manually send this event in b2g_start_script.js

Comment 17

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: Part 1 - send system-message-listener-ready event in emulator mochitest

Emulator mochitest will replace system app with test-container. We need to simulator a system-message-listener-ready event like system app.

Comment 18

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: WIP - Part 2, setup network interface for emulator

For emulator, we need to setup ip address if none existed. I can see a valid candidate ip address in SDP and see test finish message. However, there seems some JS scope issue in this patch so that TestRunner is still consider test case timeout.

Here is the error message:
>164 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcRemote): Closed connection.
>165 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Test finished
>166 INFO TEST-INFO | MEMORY STAT vsize after test: 110063616
>167 INFO TEST-INFO | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
>168 INFO TEST-INFO | MEMORY STAT residentFast after test: 49799168
>169 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 16561488
>170 INFO TEST-END | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | finished in 17174ms
>[Child 332] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file /home/hellfire/workspace/hg/mozilla-central/docshell/base/nsDocShell.cpp, line 8588
>[Child 332] WARNING: NS_ENSURE_TRUE(ParseTypeAttribute(type, &version)) failed: file /home/hellfire/workspace/hg/mozilla-central/content/base/src/nsScriptLoader.cpp, line 508
>[Child 332] WARNING: NS_ENSURE_TRUE(ParseTypeAttribute(type, &version)) failed: file /home/hellfire/workspace/hg/mozilla-central/content/base/src/nsScriptLoader.cpp, line 508
>[Child 332] WARNING: NS_ENSURE_TRUE(ParseTypeAttribute(type, &version)) failed: file /home/hellfire/workspace/hg/mozilla-central/content/base/src/nsScriptLoader.cpp, line 508
>[Child 332] WARNING: NS_ENSURE_TRUE(ParseTypeAttribute(type, &version)) failed: file /home/hellfire/workspace/hg/mozilla-central/content/base/src/nsScriptLoader.cpp, line 508
>[Parent 289] WARNING: Failed to terminate!: file /home/hellfire/workspace/hg/mozilla-central/dom/workers/WorkerPrivate.cpp, line 2534

Comment 19

[Andrew Halberstadt [:ahal]]

Comment on attachment 8349350 [details] [diff] [review]
Part 1 - send system-message-listener-ready event in emulator mochitest

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

::: testing/mochitest/b2g_start_script.js
@@ +81,5 @@
> +//RIL DOM events will be pending until RIL receiveing system-message-listener-ready event.
> +homescreen.contentWindow.dispatchEvent(new CustomEvent('mozContentEvent', {
> +  bubbles: true, cancelable: false,
> +  detail: { type: 'system-message-listener-ready' }
> +}));

We want to make sure content has received this event before we start mochitest, right? In that case we might want to send it earlier since the container.src = mochitestUrl; line is what kicks the tests off. Though even then there will still be a race condition. We might want to make sure content has received the message and data is enabled before executing container.src = mochitestURL. I don't know if there is an easy way to do this, if not I could be convinced to take this patch (though maybe send the event at the beginning of the script).

Comment 20

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: WIP - all-in-one diff

Still cannot make mochitest continue executing next test script after SimpleTest.finish(). This problem only happens when the emulator try setting both ril.data.apnSettings and ril.data.enabled, the setting order/place doesn't make any difference.
I've confirmed that SimpleTest.finish() / TestRunner._makeIframe() are both invoked and finished without error.
I also tried to print some log at 2nd test case but nothing shown in console. Looks like the html of the test case doesn't even be loaded.

Comment 21

[Andrew Halberstadt [:ahal]]

Comment on attachment 8350027 [details] [diff] [review]
WIP - all-in-one diff

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

This approach seems good to me. I mentioned to :schien on irc that another possible approach to fixing this is to not replace the homescreen app, and instead load the test-container app and then use marionette to switch into its context.

Comment 22

[Andrew Halberstadt [:ahal]]

(In reply to Shih-Chiang Chien [:schien] from comment #17)
> Emulator mochitest will replace system app with test-container. We need to
> simulator a system-message-listener-ready event like system app.

Though I just re-read this, and I don't think this is true. They'll replace the homescreen app with test-container, but as I understand it, the system app remains as is.

Comment 23

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8350027 [details] [diff] [review]
WIP - all-in-one diff

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

::: dom/media/tests/mochitest/head.js
@@ +117,5 @@
> +        info("Network interface is ready");
> +        return true;
> +      }
> +    }
> +    // ip address is not available

I'd suggest moving the info() here, and removing it from the success case

Comment 24

[Randell Jesup [:jesup] (needinfo me)]

Attachment: backtrace

Looks like a DNS issue, as I guessed.

Comment 25

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Andrew Halberstadt [:ahal] from comment #22)
> (In reply to Shih-Chiang Chien [:schien] from comment #17)
> > Emulator mochitest will replace system app with test-container. We need to
> > simulator a system-message-listener-ready event like system app.
> 
> Though I just re-read this, and I don't think this is true. They'll replace
> the homescreen app with test-container, but as I understand it, the system
> app remains as is.
I tried print out some message in console at onload event in system app, but this message doesn't show up after mochitest restart b2g service.
The system app manifest is specified in pref "browser.manifestURL" and it change to test-container while running mochitest (see http://dxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_b2g_unittest.js#4 ).

Comment 26

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

The default value of browser.manifestURL is generated in Gaia (see https://github.com/mozilla-b2g/gaia/blob/master/build/preferences.js#L18 ), which reference to the setting in Gaia Makefile (see https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L125 ). The "HOMESCREEN" variable is assigned to //system.gaiamobile.org/manifest.webapp, so replacing pref "browser.manifestURL" is definitely replacing the system app. However I'm not sure if anyone change the definition of this pref or it's just a confused variable naming / comment in our source code.

Comment 27

[Andrew Halberstadt [:ahal]]

(In reply to Shih-Chiang Chien [:schien] from comment #26)
> The default value of browser.manifestURL is generated in Gaia (see
> https://github.com/mozilla-b2g/gaia/blob/master/build/preferences.js#L18 ),
> which reference to the setting in Gaia Makefile (see
> https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L125 ). The
> "HOMESCREEN" variable is assigned to
> //system.gaiamobile.org/manifest.webapp, so replacing pref
> "browser.manifestURL" is definitely replacing the system app. However I'm
> not sure if anyone change the definition of this pref or it's just a
> confused variable naming / comment in our source code.

Weird, I thought manifestURL was referring to the homescreen app manifest. I'm not sure fixing this would have any impact on this bug, but I'm curious to see what would happen if we don't set that pref. Here's a try run for shits and giggles:
https://tbpl.mozilla.org/?tree=Try&rev=bb7a45133e02

Comment 28

[Andrew Halberstadt [:ahal]]

Bug 951895 may or may not be related to this problem. I don't think it is, but thought I'd mention it just in case.

Comment 29

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8350027 [details] [diff] [review]
WIP - all-in-one diff

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

::: dom/media/tests/mochitest/head.js
@@ +130,5 @@
> +  setLock.set({
> +    "ril.data.enabled": true,
> +  });
> +
> +  aCallback();

rmnet might still unavailable after enabling it via settings, so this might cause a racing condition.  But, maybe not all media mochitest scripts need networking, so I think having some more further checks in following steps is fine.

Comment 30

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: WIP - enable 3G data and manually set proxy type to PAC

Modification based on bug 952464 comment #14.

Let's see if it works on TBPL.
https://tbpl.mozilla.org/?tree=Try&rev=35adaba643ce

Comment 31

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Sending "system-message-listener-ready" event in b2g_start_script.js will cause some test failure in dom/contacts. I'm investigating the root cause now and we could probably workaround it by sending this event while running WebRTC test cases instead.

The WebRTC case are failed on allocating local video stream. I'm also investigating the root cause. @jesup do you have any insight on this failure?

I'll suggest that we can turn on audio-only test case for now because we are not shipping WebRTC with video on v1.3.

Comment 32

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Turn on WebRTC logging for retrieving more information.
https://tbpl.mozilla.org/?tree=Try&rev=c193a2024acf

Comment 33

[Randell Jesup [:jesup] (needinfo me)]

Yes, no need to enable video for the mochitests yet, and we're explicitly disabling video peerconnections for 1.3.  The failures all appear to be "I expected a video stream and didn't get one".

Comment 34

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: enabling WebRTC audio test on emulator

Disabling all the video test cases and avoid sending system-message-listener-ready event at the emulator startup.

Let's see if everything works fine on TBPL.
https://tbpl.mozilla.org/?tree=Try&rev=d670f62fe031

Comment 35

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: enabling WebRTC audio test on emulator. v2

Turning off all the test cases failed locally on my PC. Still seeing dom/media failures on try server and need someone who really understand these test cases to identify the reason.

Here is the exclude list:
dom/media/tests/mochitest/test_dataChannel_basicAudioVideo.html
dom/media/tests/mochitest/test_dataChannel_basicAudioVideoCombined.html
dom/media/tests/mochitest/test_dataChannel_basicVideo.html
dom/media/tests/mochitest/test_peerConnection_basicAudioVideo.html
dom/media/tests/mochitest/test_peerConnection_basicAudioVideoCombined.html
dom/media/tests/mochitest/test_peerConnection_basicVideo.html
dom/media/tests/mochitest/test_peerConnection_bug827843.html
dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveAudio.html
dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideo.html
dom/media/tests/mochitest/test_peerConnection_offerRequiresReceiveVideoAudio.html
dom/media/tests/mochitest/test_peerConnection_throwInCallbacks.html

Here is the try result: https://tbpl.mozilla.org/?tree=Try&rev=06bc6faa9850

Comment 36

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: emu-logcat.txt

Here is the logcat of executing all test cases under dom/media/tests/mochitest/ on my PC (ubuntu 12.04).

Comment 37

[Randell Jesup [:jesup] (needinfo me)]

dom/system/gonk/tests/mochitest/test_switch_dataConnection.html is timing out (perhaps because it assumed the data connection is not turned on, and our tests turn it on).  Perhaps changing the order of the tests; or revise the test to make it handle this case.

test_switch_dataConnection.html is in bug 952464 (part 2)

Comment 38

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8360356 [details] [diff] [review]
enabling WebRTC audio test on emulator. v2

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

::: dom/media/tests/mochitest/head.js
@@ +118,5 @@
> +        return true;
> +      }
> +    }
> +    // ip address is not available
> +    return false;

Should we log this?

Comment 39

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Randell Jesup [:jesup] from comment #37)
> dom/system/gonk/tests/mochitest/test_switch_dataConnection.html is timing
> out (perhaps because it assumed the data connection is not turned on, and
> our tests turn it on).  Perhaps changing the order of the tests; or revise
> the test to make it handle this case.
> 
> test_switch_dataConnection.html is in bug 952464 (part 2)
My bad, I'll modify this test case to allowing executing it after dom/media test cases.

Comment 40

[Andrew Halberstadt [:ahal]]

(In reply to Randell Jesup [:jesup] from comment #37)
> dom/system/gonk/tests/mochitest/test_switch_dataConnection.html is timing
> out (perhaps because it assumed the data connection is not turned on, and
> our tests turn it on).  Perhaps changing the order of the tests; or revise
> the test to make it handle this case.

Please don't change the order of the tests as a solution :). It will cause failures as soon as we add more tests and your test gets bumped to a different chunk. Is it not possible to just re-disable the data connection after the test is finished?

Comment 41

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

test_switch_dataConnection.html is updated and here is the try result for emulator.
https://tbpl.mozilla.org/?tree=Try&rev=c71da9db09d0

Comment 42

[Jason Smith [:jsmith]]

Comment on attachment 8360356 [details] [diff] [review]
enabling WebRTC audio test on emulator. v2

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

feedback+ - mainly because I know less about RIL setup code here, so I'd rather redirect this to Vicamo to do the review of the data connection setup code here. I like the approach though overall - it separates out the data connection code out cleanly & integrates in head.js in a simple manner.

We do need to get followup bug(s) filed for the tests remaining disabled along with including reasoning for why the tests are still disabled.

::: dom/media/tests/mochitest/head.js
@@ +102,5 @@
>    info("Call getUserMedia for " + JSON.stringify(constraints));
>    navigator.mozGetUserMedia(constraints, onSuccess, onError);
>  }
>  
> +function isNetworkReady() {

Nit - I'd add a comment before each function explaining what the function does.

@@ +118,5 @@
> +        return true;
> +      }
> +    }
> +    // ip address is not available
> +    return false;

General comment to build off this - we should add some info statements in each function to help with debugging in case we see failures in the future in these tests.

::: testing/mochitest/b2g-debug.json
@@ +313,1 @@
>      "dom/media/tests/mochitest/test_peerConnection_throwInCallbacks.html":"",

For all of these tests that are remaining disabled, we should two things for each test remaining disabled in the second string:

* The bug # tracking turning on the test
* The reason for why the test is disabled

See some examples in the json file (e.g. dom/network/tests/test_networkstats_basics.html is a good example).

Comment 43

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: emulator-mochitest-system-event.patch, v3

1. Revert network.proxy.type in WebRTC test scripts instead of depending on bug 952464.
2. Add comment and test log according to review comments.
3. Update reviewer info in hg commit log.

follow-up bug for the excluded test cases is Bug 960442.

try result: https://tbpl.mozilla.org/?tree=Try&rev=559d77524a39

Comment 44

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

@vicamo points out the patch in bug 952464 has a potential issue on using RIL/Wifi specific proxy as system initial value if the user pref is not reset before power-off, and we need another solution for reverting mochitest specific proxy setting. Therefore, I switch the implementation to manually revert proxy type in test script.

Comment 45

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

no longer depend on bug 952464 (see comment #44).

Comment 46

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: [b2g28] emulator-mochitest-system-event.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 923363
User impact if declined: unable to use WebRTC for audio call on Firefox OS
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): N/A
String or IDL/UUID changes made by this patch: N/A

This patch is a test case enabler for b2g emulator. Per discussion, we can only turn WebRTC default on after all the related test cases are passed on emulator.

Comment 47

[[:fabrice] Fabrice Desré]

Comment on attachment 8360974 [details] [diff] [review]
[b2g28] emulator-mochitest-system-event.patch

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

approval=me but I can't set the flag on approval-mozilla-aurora. Also, this is pending r+ from Vicamo of course.

Comment 48

[[:fabrice] Fabrice Desré]

Comment on attachment 8360974 [details] [diff] [review]
[b2g28] emulator-mochitest-system-event.patch

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

approval=me but I can't set the flag on approval-mozilla-aurora. Also, this is pending r+ from Vicamo of course.

Comment 49

[Randell Jesup [:jesup] (needinfo me)]

(In reply to Shih-Chiang Chien [:schien] from comment #43)
> Created attachment 8360961 [details] [diff] [review]
> emulator-mochitest-system-event.patch, v3
> 
> 1. Revert network.proxy.type in WebRTC test scripts instead of depending on
> bug 952464.
> 2. Add comment and test log according to review comments.
> 3. Update reviewer info in hg commit log.
> 
> follow-up bug for the excluded test cases is Bug 960442.
> 
> try result: https://tbpl.mozilla.org/?tree=Try&rev=559d77524a39

This is failing consistently after the webrtc tests have all run successfully with:
JavaScript error: http://mochi.test:8888/tests/SimpleTest/TestRunner.js, line 123: Permission denied to access property 'wrappedJSObject'

I didn't see this error when the tests were run with bug 952464 (though the testcase for that bug would fail after running the webrtc tests; likely because the data connection was already up).  I'm afraid this is outside my area and I'm stabbing in the dark to try to solve it (doubly so since I can't get emulator mochitests to run on my system anymore).  

We need to either solve this problem, or go back and solve the problem with the bug 952464 test I believe (unless that failure was hiding other errors behind it like this one).

Getting this resolved ASAP is critical.  And all the webrtc tests themselves are passing; it's all basically getting the framework happy now.

vicamo/ahal/schien (and anyone else who can help): please see if this can be resolved today.  Thanks!

Comment 50

[Andrew Halberstadt [:ahal]]

Unfortunately neither mdas, jgriffin or myself have seen that error before. And from a brief look at the code I'm not really sure what would be the causing it. Jgriffin suggested :bholley might be a good person to ask about it though.

Comment 51

[Andrew Halberstadt [:ahal]]

Jgriffin also pointed out there's some errors in the log that don't exist in passing logs, like:
INFO -  System JS : ERROR (null):0 - uncaught exception: Settings lock not open

Comment 52

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: WIP - reset-network-connection.patch

Test case timeout after dom/settings/tests/test_settings_basics.html modifys wifi settings. Turning off 3G data connection after dom/media will solve this problem.

In this patch, test_zmedia_cleanup.html will turning off 3G data connection after finishing all the other dom/media test cases. @jsmith, do you know a better way to register a clean-up procedure after test case finished?

try result: https://tbpl.mozilla.org/?tree=Try&rev=087e5724596d

Comment 53

[Bobby Holley (:bholley)]

(In reply to Randell Jesup [:jesup] from comment #49)
> 
> This is failing consistently after the webrtc tests have all run
> successfully with:
> JavaScript error: http://mochi.test:8888/tests/SimpleTest/TestRunner.js,
> line 123: Permission denied to access property 'wrappedJSObject'

Yes. The error is presumably because the frame has navigated to a domain that is not same-origin with that of the top-level test harness that's navigating it. The error looks like a cross-origin access rejection.

But more to the point - this code only runs in the case where the test has already timed out. So it's probably a red herring (though the test probably shouldn't be navigating itself to something cross-origin, since that might break other things).

Comment 54

[Jason Smith [:jsmith]]

Comment on attachment 8361516 [details] [diff] [review]
WIP - reset-network-connection.patch

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

I actually know less about the data connection here, so I'm going to redirect this to Vicamo.

Comment 55

[Jason Smith [:jsmith]]

Discussed this in person - we realized that placing the network dependency here in head.js has one problem - it places a data connection dependency on the getUserMedia tests, which shouldn't be necessary. Shih-Chiang is working on refactoring the solution here to only utilize the data connection in the PC tests.

Comment 56

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8361516 [details] [diff] [review]
WIP - reset-network-connection.patch

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

schien is going to provide another revision.

Comment 57

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: emulator-mochitest-system-event.patch, v4

1. trigger network setup for PeerConnectionTest and DataChannelTest.
2. append network teardown callback in CommandChain.

**NOTE: Putting network setup in CommandChain will not work because we need IP address before creating any PeerConnectionWrapper.

try result: https://tbpl.mozilla.org/?tree=Try&rev=090cfb966298

Comment 58

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: [b2g28] emulator-mochitest-system-event.patch

patch for b2g28

Comment 59

[Jason Smith [:jsmith]]

Comment on attachment 8361604 [details] [diff] [review]
emulator-mochitest-system-event.patch, v4

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

review- for the problem with the fact that the teardown command might not run if the test times out or crashes. We need an approach that ensures that the data connection is turned off even on a timeout or crash.

::: dom/media/tests/mochitest/pc.js
@@ +352,5 @@
> +  return true;
> +}
> +
> +/**
> + * Network setup utils for Gonk

Nit - Include an @return describing what's being returned

@@ +450,5 @@
>  
> +  // Insert network teardown after testcase execution.
> +  if (netTeardownCommand) {
> +    this.chain.append(netTeardownCommand);
> +  }

The problem with this approach is that this assumes the mochitest will run to completion to get to this command. However, that might not always be the case if we hit a test failure on a command before this test case (e.g. timeout, crash). What we really need to do here is ensure that this command always runs even on a test fault such as a timeout or crash. Otherwise, we'll run into the problems of what happens if the data connection remains active post end of the test.

Comment 60

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: emulator-mochitest-system-event.patch, v5

1. Add back test_zmedia_cleanup.html as a safe guard for resetting data connection after finishing all WebRTC test cases.
2. Update according to review comment for v4

try: https://tbpl.mozilla.org/?tree=Try&rev=5fa687377e9b

Comment 61

[Jason Smith [:jsmith]]

Comment on attachment 8361759 [details] [diff] [review]
emulator-mochitest-system-event.patch, v5

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

I think we need ahal's input on what to do here.

::: dom/media/tests/mochitest/mochitest.ini
@@ +53,5 @@
>  [test_peerConnection_setRemoteOfferInHaveLocalOffer.html]
>  [test_peerConnection_throwInCallbacks.html]
>  [test_peerConnection_toJSON.html]
> +# Bug950317: Hack for making a cleanup hook after finishing all WebRTC cases
> +[test_zmedia_cleanup.html]

I thought ahal clarified we couldn't do a global cleanup this way, as different mochitests within this directory can be chunked differently on TBPL.

Comment 62

[Jason Smith [:jsmith]]

Andrew - See the review comments above. What do you think we should do here?

Comment 63

[Jason Smith [:jsmith]]

The one idea I was thinking about that might solve this problem is to do at the mochitest framework level:

If after completing a mochitest & I detect that the data connection is still on, then turn off the data connection before I execute the next test.

Comment 64

[Andrew Halberstadt [:ahal]]

(In reply to Jason Smith [:jsmith] from comment #61)
> I thought ahal clarified we couldn't do a global cleanup this way, as
> different mochitests within this directory can be chunked differently on
> TBPL.

I said there wasn't a built-in global cleanup function we could call, this is what I suggested :schien do instead. You are right that it could cause problems if the test times out before the cleanup is called, but I don't know that I would let that hold up the whole feature.

Also to clarify, I'm not an expert at writing mochitests so I'm not really sure how other directories solve this problem. Ideally the harness would provide setup/teardown methods (bug 960972) but that's a big change that we don't want to block on.

Comment 65

[Jason Smith [:jsmith]]

(In reply to Andrew Halberstadt [:ahal] from comment #64)
> (In reply to Jason Smith [:jsmith] from comment #61)
> > I thought ahal clarified we couldn't do a global cleanup this way, as
> > different mochitests within this directory can be chunked differently on
> > TBPL.
> 
> I said there wasn't a built-in global cleanup function we could call, this
> is what I suggested :schien do instead. You are right that it could cause
> problems if the test times out before the cleanup is called, but I don't
> know that I would let that hold up the whole feature.

Well, the problem here is that these tests are known for having intermittent failures in TBPL. So if we risk not fixing this, then we'll likely have significant negative impact on other tests, which likely result in the tests being turned off not too long after they get preffed on.

> 
> Also to clarify, I'm not an expert at writing mochitests so I'm not really
> sure how other directories solve this problem. Ideally the harness would
> provide setup/teardown methods (bug 960972) but that's a big change that we
> don't want to block on.

Agree that we probably should avoid blocking on that large change. However, I think we need to take account my points about the intermittent failure rate problem above.

Comment 66

[Andrew Halberstadt [:ahal]]

(In reply to Jason Smith [:jsmith] from comment #65)
> Well, the problem here is that these tests are known for having intermittent
> failures in TBPL. So if we risk not fixing this, then we'll likely have
> significant negative impact on other tests, which likely result in the tests
> being turned off not too long after they get preffed on. 

If they have high intermittent failure rates, they'll get turned off whether or not they affect other tests :).. but yes if they affect other tests it'll make the situation worse.

I'm not really sure what to do here. I talked to a few people and I don't think other directories do solve this problem. So cleanup not being called after timeout isn't unique to these tests. With that in mind, I would be inclined to just land :schien's patch and wait until we can implement a general solution in the harness.

Otherwise another option would be to run them in their own special chunk (though this adds about 10min of setup/teardown time to our test runtime). I think this is just a matter of picking the lesser of evils.

Comment 67

[Jason Smith [:jsmith]]

In that case, let's land the patch as is. If we end up requiring the special chunk later because we're hitting too many problems with the current implemented approach, then we can always address this later when the problem arises.

Comment 68

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #67)
> In that case, let's land the patch as is. If we end up requiring the special
> chunk later because we're hitting too many problems with the current
> implemented approach, then we can always address this later when the problem
> arises.

Actually - we need to hold off on landing this. Nils mentioned in person that these tests are currently intermittently failing on local emulator runs. He mentioned that Randell also saw this happen with one of the mochitests enabled here as well.

We need to debug what's failing here, get dependent bugs filed, and resolve these to ensure that these tests can run consistently w/o timeouts & failures in buildbot.

Comment 69

[Randell Jesup [:jesup] (needinfo me)]

(In reply to Jason Smith [:jsmith] from comment #68)
> (In reply to Jason Smith [:jsmith] from comment #67)
> > In that case, let's land the patch as is. If we end up requiring the special
> > chunk later because we're hitting too many problems with the current
> > implemented approach, then we can always address this later when the problem
> > arises.
> 
> Actually - we need to hold off on landing this. Nils mentioned in person
> that these tests are currently intermittently failing on local emulator
> runs. He mentioned that Randell also saw this happen with one of the
> mochitests enabled here as well.

My latest mochitest-remote run was 527 passed, no failures.  Nils had accidentally disabled schien's patch which caused failures (which was what he had mentioned to you).  He re-applied the patch, and got 527 passed, no failures.  I seem to have an intermittent timeout on datachannel_basicAudio.html (see below).

> We need to debug what's failing here, get dependent bugs filed, and resolve
> these to ensure that these tests can run consistently w/o timeouts &
> failures in buildbot.

I suspect the problem I'm seeing here is a timing sensitivity in the basic mochitest framework (not the code) (and may be the cause of some low-freq desktop oranges as well).  So well worth figuring out.  The question is "are these happening in automation, and if so how often".  The older Try pushes (not the latest one, which didn't even start the WebRTC tests) seemed to run all the tests (including datachannel_basicAudio.html) without problems (and had a problem later).

We shouldn't land the automated tests until they'll run without a high orange count, so lets get that in place and then land.  In the meantime I'll see if I can figure out my local intermittent.

Comment 70

[Jason Smith [:jsmith]]

(In reply to Randell Jesup [:jesup] from comment #69)
> (In reply to Jason Smith [:jsmith] from comment #68)
> > (In reply to Jason Smith [:jsmith] from comment #67)
> > > In that case, let's land the patch as is. If we end up requiring the special
> > > chunk later because we're hitting too many problems with the current
> > > implemented approach, then we can always address this later when the problem
> > > arises.
> > 
> > Actually - we need to hold off on landing this. Nils mentioned in person
> > that these tests are currently intermittently failing on local emulator
> > runs. He mentioned that Randell also saw this happen with one of the
> > mochitests enabled here as well.
> 
> My latest mochitest-remote run was 527 passed, no failures.  Nils had
> accidentally disabled schien's patch which caused failures (which was what
> he had mentioned to you).  He re-applied the patch, and got 527 passed, no
> failures.  I seem to have an intermittent timeout on
> datachannel_basicAudio.html (see below).

Ah - I missed the part about the patch not being applied.

> 
> > We need to debug what's failing here, get dependent bugs filed, and resolve
> > these to ensure that these tests can run consistently w/o timeouts &
> > failures in buildbot.
> 
> I suspect the problem I'm seeing here is a timing sensitivity in the basic
> mochitest framework (not the code) (and may be the cause of some low-freq
> desktop oranges as well).  So well worth figuring out.  The question is "are
> these happening in automation, and if so how often".  The older Try pushes
> (not the latest one, which didn't even start the WebRTC tests) seemed to run
> all the tests (including datachannel_basicAudio.html) without problems (and
> had a problem later).
> 
> We shouldn't land the automated tests until they'll run without a high
> orange count, so lets get that in place and then land.  In the meantime I'll
> see if I can figure out my local intermittent.

Okay - can we kick off a final try run here?

Comment 71

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

I'll suggest that we use the v5 patch, which contains a clean-up testcase, because it showed a stable green on try result in my experience last week.

Comment 72

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

try result for v5: https://tbpl.mozilla.org/?tree=Try&rev=2112f4b22183

Comment 73

[Randell Jesup [:jesup] (needinfo me)]

I retriggered it like 8 times, all green :-)

Running the tests locally (with v5) I get better results, but still get some timeouts on datachannel_basicAudio.html (on inbound).  I'll look into those, but the acid test is tbpl: 8 for 8 green on tbpl seems good enough to land.

Comment 74

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

From the try result in comment #72, test_dataChannel_basicAudio.html consistently fail on timeout in emulator debug build. The test step stops before remote data channel receives |onopen| event. I'm investigating on it, however, we might disable this case in b2g-debug.json for now and file a follow-up bug.

Comment 75

[Jason Smith [:jsmith]]

(In reply to Shih-Chiang Chien [:schien] from comment #74)
> From the try result in comment #72, test_dataChannel_basicAudio.html
> consistently fail on timeout in emulator debug build. The test step stops
> before remote data channel receives |onopen| event. I'm investigating on it,
> however, we might disable this case in b2g-debug.json for now and file a
> follow-up bug.

I'm actually seeing a couple of more failures than that. The other failures are present in https://tbpl.mozilla.org/php/getParsedLog.php?id=33433647&tree=Try#error0, which shows two errors:

18:16:34     INFO -  7411 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | PeerConnectionWrapper (pcLocal) is in state: 'open' - got closed, expected open

18:20:26     INFO -  7432 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/media/tests/mochitest/test_dataChannel_basicAudio.html | Test timed out.

Comment 76

[Jason Smith [:jsmith]]

(In reply to Shih-Chiang Chien [:schien] from comment #74)
> From the try result in comment #72, test_dataChannel_basicAudio.html
> consistently fail on timeout in emulator debug build. The test step stops
> before remote data channel receives |onopen| event. I'm investigating on it,
> however, we might disable this case in b2g-debug.json for now and file a
> follow-up bug.

Let's land what we have anyways & disable the test in b2g-debug.json. We do need to fix this issue a followup.

Comment 77

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8361604 [details] [diff] [review]
emulator-mochitest-system-event.patch, v4

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

I really have no special suggestion about this.  The only thing I do have a little bit concern would be the place to etch these network enable code.  I'd like to move them into somewhere that all mochitest scripts can access, but I'm fine if your final decision is to postpone to that some time later.  Thank you!

Comment 78

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: emulator-mochitest-system-event.patch, v5.1

disable test_dataChannel_basicAudio.html in b2g-debug.json.

try: https://tbpl.mozilla.org/?tree=Try&rev=289413789c6a

Comment 79

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8364238 [details] [diff] [review]
emulator-mochitest-system-event.patch, v5.1

forget to carry r+.

Comment 80

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8361606 [details] [diff] [review]
[b2g28] emulator-mochitest-system-event.patch

patch v5.1 can be merge to aurora directly with no conflict.

Comment 81

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/b2g-inbound/rev/3822e729fc7f

Try is busted for all B2G Emulator builds currently (bug 963148)

Comment 82

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3822e729fc7f

Comment 83

[Randell Jesup [:jesup] (needinfo me)]

It appears the intermittent with basic_audio on emulator debug builds (not opt) applies to test_peerConnection_basicAudio.html as well (which isn't really surprising, I guess), so we'll need to disable that one as well until we solve the debug-only intermittent.

Comment 84

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cddd640cd774