Closed
Bug 1139094
Opened 10 years ago
Closed 10 years ago
[steeplechase] tests are broken in multiple places
Categories
(Core :: WebRTC, defect)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla39
People
(Reporter: drno, Assigned: drno)
Details
Attachments
(1 file, 1 obsolete file)
6.50 KB,
patch
|
mt
:
review+
|
Details | Diff | Splinter Review |
Steeplechase tests in the QA lab are currently broken in multiple places.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → drno
Assignee | ||
Comment 1•10 years ago
|
||
This fixes:
- steeplechase hangs because local/remote tracks IDs don't get properly transferred to the far end
- steeplechase hangs because pc.js executes code from network.js before test.js got loaded, which results in 'info' not being defined
- steeplechase timeouts throw errors about undefined teardown() method
- steeplechase tests sometimes fail because of RTCP timestamps differences
Attachment #8572432 -
Flags: review?(martin.thomson)
Attachment #8572432 -
Flags: review?(docfaraday)
Assignee | ||
Comment 2•10 years ago
|
||
Comment 3•10 years ago
|
||
Comment on attachment 8572432 [details] [diff] [review]
bug_1139094_steeplechase_tests.patch
Review of attachment 8572432 [details] [diff] [review]:
-----------------------------------------------------------------
Just a nit.
::: dom/media/tests/mochitest/pc.js
@@ +1778,5 @@
> }
>
> function runNetworkTest(testFunction) {
> return scriptsReady
> + .then(() => runTestWhenReady(options => {startNetworkAndTest(); testFunction(options);}));
Let's throw some newlines in here.
Attachment #8572432 -
Flags: review?(docfaraday) → review+
Comment 4•10 years ago
|
||
Comment on attachment 8572432 [details] [diff] [review]
bug_1139094_steeplechase_tests.patch
Review of attachment 8572432 [details] [diff] [review]:
-----------------------------------------------------------------
I trust that you can handle the comment here.
::: dom/media/tests/mochitest/templates.js
@@ +143,5 @@
> function PC_SETUP_SIGNALING_CLIENT(test) {
> if (test.steeplechase) {
> setTimeout(() => {
> ok(false, "PeerConnectionTest timed out");
> + throw new Error('PeerConnectionTest timed out after 30s');
Sure this triggers onerror, but this isn't going to get caught properly. You need to roll this failure into the Promise that this step returns.
var timeout = new Promise((res, rej) => setTimeout(rej, 30000));
return Promise.race([theRealOutcome, timeout]);
With the timeout promise only being added to the race if things are under steeplechase.
Attachment #8572432 -
Flags: review?(martin.thomson) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8572432 -
Attachment is obsolete: true
Attachment #8573601 -
Flags: review?(martin.thomson)
Assignee | ||
Comment 6•10 years ago
|
||
Updated•10 years ago
|
Attachment #8573601 -
Flags: review?(martin.thomson) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 7•10 years ago
|
||
Flags: in-testsuite+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Assignee | ||
Updated•10 years ago
|
status-firefox38:
--- → affected
Assignee | ||
Comment 9•10 years ago
|
||
Try run on Aurora: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f503e28ff39e
Reminder to myself to check for the results
Flags: needinfo?(drno)
Assignee | ||
Comment 10•10 years ago
|
||
Flags: needinfo?(drno)
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•