Closed
Bug 950317
Opened 11 years ago
Closed 11 years ago
[emulator][mochitest] Setup eth0 IP address to NetworkManager for running PeerConnection mochitest cases
Categories
(Core :: WebRTC, defect)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: jsmith, Assigned: schien)
References
Details
Attachments
(2 files, 15 obsolete files)
1.75 MB,
text/plain
|
Details | |
18.76 KB,
patch
|
schien
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•11 years ago
|
||
ekr - this is a blocker for getting the Peer Connection mochitests running on emulators. Can find someone to look into this?
Flags: needinfo?(ekr)
Assignee | ||
Comment 3•11 years ago
|
||
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.
Summary: Setting a local description in a remote Peer Connection in mochitests is failing to fire a success callback in emulator → [emulator][mochitest] Setup eth0 IP address to NetworkManager for running PeerConnection mochitest cases
Reporter | ||
Comment 4•11 years ago
|
||
(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•11 years ago
|
||
(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•11 years ago
|
||
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)
Assignee | ||
Comment 7•11 years ago
|
||
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.
Attachment #8348579 -
Flags: feedback?(vyang)
Comment 8•11 years ago
|
||
(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 :(
Assignee: nobody → vyang
Comment 9•11 years ago
|
||
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•11 years ago
|
||
Somehow b2g process is restarted, and when it does, nobody sends 'setRadioEnabled' down to RIL again.
Comment 11•11 years ago
|
||
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•11 years ago
|
||
And, no one is sending "system-message-listener-ready", so all unsolicited messages are queued up and never reach content side.
Comment 13•11 years ago
|
||
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. :(
Attachment #8348579 -
Attachment is obsolete: true
Attachment #8348579 -
Flags: feedback?(vyang)
Assignee | ||
Comment 14•11 years ago
|
||
@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.
Flags: needinfo?(ahalberstadt)
Comment 15•11 years ago
|
||
(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
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 16•11 years ago
|
||
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
Assignee | ||
Comment 17•11 years ago
|
||
Emulator mochitest will replace system app with test-container. We need to simulator a system-message-listener-ready event like system app.
Attachment #8349350 -
Flags: review?(ahalberstadt)
Attachment #8349350 -
Flags: feedback?(vyang)
Assignee | ||
Comment 18•11 years ago
|
||
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
Attachment #8348699 -
Attachment is obsolete: true
Comment 19•11 years ago
|
||
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).
Attachment #8349350 -
Flags: review?(ahalberstadt) → review-
Assignee | ||
Comment 20•11 years ago
|
||
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.
Attachment #8349350 -
Attachment is obsolete: true
Attachment #8349355 -
Attachment is obsolete: true
Attachment #8349350 -
Flags: feedback?(vyang)
Attachment #8350027 -
Flags: feedback?(vyang)
Attachment #8350027 -
Flags: feedback?(rjesup)
Attachment #8350027 -
Flags: feedback?(ahalberstadt)
Comment 21•11 years ago
|
||
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.
Attachment #8350027 -
Flags: feedback?(ahalberstadt) → feedback+
Comment 22•11 years ago
|
||
(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•11 years ago
|
||
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
Attachment #8350027 -
Flags: feedback?(rjesup) → feedback+
Assignee | ||
Comment 25•11 years ago
|
||
(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 ).
Assignee | ||
Comment 26•11 years ago
|
||
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.
Flags: needinfo?(ahalberstadt)
Comment 27•11 years ago
|
||
(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
Flags: needinfo?(ahalberstadt)
Comment 28•11 years ago
|
||
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•11 years ago
|
||
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.
Attachment #8350027 -
Flags: feedback?(vyang) → feedback+
Assignee | ||
Comment 30•11 years ago
|
||
Modification based on bug 952464 comment #14.
Let's see if it works on TBPL.
https://tbpl.mozilla.org/?tree=Try&rev=35adaba643ce
Assignee: vyang → schien
Attachment #8350027 -
Attachment is obsolete: true
Attachment #8350345 -
Attachment is obsolete: true
Flags: needinfo?(mrbkap)
Assignee | ||
Comment 31•11 years ago
|
||
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.
Flags: needinfo?(rjesup)
Assignee | ||
Comment 32•11 years ago
|
||
Turn on WebRTC logging for retrieving more information.
https://tbpl.mozilla.org/?tree=Try&rev=c193a2024acf
Comment 33•11 years ago
|
||
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".
Flags: needinfo?(rjesup)
Assignee | ||
Comment 34•11 years ago
|
||
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
Attachment #8358354 -
Attachment is obsolete: true
Attachment #8360175 -
Flags: review?(rjesup)
Attachment #8360175 -
Flags: review?(jsmith)
Assignee | ||
Comment 35•11 years ago
|
||
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
Attachment #8360175 -
Attachment is obsolete: true
Attachment #8360175 -
Flags: review?(rjesup)
Attachment #8360175 -
Flags: review?(jsmith)
Attachment #8360356 -
Flags: review?(rjesup)
Attachment #8360356 -
Flags: review?(jsmith)
Attachment #8360356 -
Flags: feedback?(ekr)
Assignee | ||
Comment 36•11 years ago
|
||
Here is the logcat of executing all test cases under dom/media/tests/mochitest/ on my PC (ubuntu 12.04).
Comment 37•11 years ago
|
||
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•11 years ago
|
||
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?
Attachment #8360356 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 39•11 years ago
|
||
(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•11 years ago
|
||
(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?
Assignee | ||
Comment 41•11 years ago
|
||
test_switch_dataConnection.html is updated and here is the try result for emulator.
https://tbpl.mozilla.org/?tree=Try&rev=c71da9db09d0
Reporter | ||
Comment 42•11 years ago
|
||
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).
Attachment #8360356 -
Flags: review?(vyang)
Attachment #8360356 -
Flags: review?(jsmith)
Attachment #8360356 -
Flags: feedback+
Assignee | ||
Comment 43•11 years ago
|
||
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
Attachment #8360356 -
Attachment is obsolete: true
Attachment #8360356 -
Flags: review?(vyang)
Attachment #8360356 -
Flags: feedback?(ekr)
Attachment #8360961 -
Flags: review?(vyang)
Assignee | ||
Comment 44•11 years ago
|
||
@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.
Assignee | ||
Comment 45•11 years ago
|
||
no longer depend on bug 952464 (see comment #44).
No longer depends on: 952464
Assignee | ||
Comment 46•11 years ago
|
||
[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.
Attachment #8360974 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(fabrice)
Comment 47•11 years ago
|
||
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•11 years ago
|
||
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.
Attachment #8360974 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Flags: needinfo?(fabrice)
Comment 49•11 years ago
|
||
(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!
Severity: normal → critical
Flags: needinfo?(vyang)
Flags: needinfo?(schien)
Flags: needinfo?(ahalberstadt)
Comment 50•11 years ago
|
||
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.
Flags: needinfo?(ahalberstadt) → needinfo?(bobbyholley+bmo)
Comment 51•11 years ago
|
||
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
Assignee | ||
Comment 52•11 years ago
|
||
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
Attachment #8361516 -
Flags: feedback?(jsmith)
Flags: needinfo?(vyang)
Flags: needinfo?(schien)
Flags: needinfo?(bobbyholley+bmo)
Comment 53•11 years ago
|
||
(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).
Reporter | ||
Comment 54•11 years ago
|
||
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.
Attachment #8361516 -
Flags: feedback?(jsmith) → feedback?(vyang)
Updated•11 years ago
|
Attachment #8360961 -
Flags: review?(vyang) → review+
Reporter | ||
Comment 55•11 years ago
|
||
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•11 years ago
|
||
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.
Attachment #8361516 -
Flags: feedback?(vyang)
Assignee | ||
Comment 57•11 years ago
|
||
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
Attachment #8360961 -
Attachment is obsolete: true
Attachment #8361516 -
Attachment is obsolete: true
Attachment #8361604 -
Flags: review?(vyang)
Attachment #8361604 -
Flags: review?(rjesup)
Attachment #8361604 -
Flags: review?(jsmith)
Assignee | ||
Comment 58•11 years ago
|
||
patch for b2g28
Attachment #8360974 -
Attachment is obsolete: true
Updated•11 years ago
|
Attachment #8361604 -
Flags: review?(rjesup) → review+
Reporter | ||
Comment 59•11 years ago
|
||
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.
Attachment #8361604 -
Flags: review?(jsmith) → review-
Assignee | ||
Comment 60•11 years ago
|
||
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
Attachment #8361759 -
Flags: feedback?(jsmith)
Reporter | ||
Comment 61•11 years ago
|
||
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.
Attachment #8361759 -
Flags: feedback?(jsmith) → feedback-
Reporter | ||
Comment 62•11 years ago
|
||
Andrew - See the review comments above. What do you think we should do here?
Flags: needinfo?(ahalberstadt)
Reporter | ||
Comment 63•11 years ago
|
||
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•11 years ago
|
||
(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.
Flags: needinfo?(ahalberstadt)
Reporter | ||
Comment 65•11 years ago
|
||
(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•11 years ago
|
||
(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.
Reporter | ||
Comment 67•11 years ago
|
||
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.
Reporter | ||
Updated•11 years ago
|
Attachment #8361604 -
Flags: review- → review+
Reporter | ||
Comment 68•11 years ago
|
||
(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•11 years ago
|
||
(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.
Reporter | ||
Comment 70•11 years ago
|
||
(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?
Assignee | ||
Comment 71•11 years ago
|
||
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.
Assignee | ||
Comment 72•11 years ago
|
||
try result for v5: https://tbpl.mozilla.org/?tree=Try&rev=2112f4b22183
Comment 73•11 years ago
|
||
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.
Assignee | ||
Comment 74•11 years ago
|
||
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.
Reporter | ||
Comment 75•11 years ago
|
||
(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.
Reporter | ||
Comment 76•11 years ago
|
||
(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•11 years ago
|
||
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!
Attachment #8361604 -
Flags: review?(vyang) → review+
Assignee | ||
Comment 78•11 years ago
|
||
disable test_dataChannel_basicAudio.html in b2g-debug.json.
try: https://tbpl.mozilla.org/?tree=Try&rev=289413789c6a
Attachment #8361604 -
Attachment is obsolete: true
Attachment #8361759 -
Attachment is obsolete: true
Assignee | ||
Comment 79•11 years ago
|
||
Comment on attachment 8364238 [details] [diff] [review]
emulator-mochitest-system-event.patch, v5.1
forget to carry r+.
Attachment #8364238 -
Flags: review+
Assignee | ||
Comment 80•11 years ago
|
||
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.
Attachment #8361606 -
Attachment is obsolete: true
Comment 81•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/3822e729fc7f
Try is busted for all B2G Emulator builds currently (bug 963148)
Target Milestone: --- → mozilla29
Comment 82•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment 83•11 years ago
|
||
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.
Blocks: 963524
Comment 84•11 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•