If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[steeplechase] tests are broken in multiple places

RESOLVED FIXED in Firefox 38

Status

()

Core
WebRTC
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: drno, Assigned: drno)

Tracking

Trunk
mozilla39
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Steeplechase tests in the QA lab are currently broken in multiple places.
(Assignee)

Updated

3 years ago
Assignee: nobody → drno
(Assignee)

Comment 1

3 years ago
Created attachment 8572432 [details] [diff] [review]
bug_1139094_steeplechase_tests.patch

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

3 years ago
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1083d9d007c3

Comment 3

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

3 years ago
Created attachment 8573601 [details] [diff] [review]
bug_1139094_steeplechase_tests.patch
Attachment #8572432 - Attachment is obsolete: true
Attachment #8573601 - Flags: review?(martin.thomson)
(Assignee)

Comment 6

3 years ago
New try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=610f27a50d9f

Updated

3 years ago
Attachment #8573601 - Flags: review?(martin.thomson) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d92a01ab497
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3d92a01ab497
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
(Assignee)

Updated

3 years ago
status-firefox38: --- → affected
(Assignee)

Comment 9

3 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

3 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/6338c7ac59a1
Flags: needinfo?(drno)
status-firefox38: affected → fixed
You need to log in before you can comment on or make changes to this bug.