1139094 - [steeplechase] tests are broken in multiple places

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Nils Ohlmeier [:drno]]

Steeplechase tests in the QA lab are currently broken in multiple places.

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: 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

Comment 2

[Nils Ohlmeier [:drno]]

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

Comment 3

[Byron Campen [:bwc]]

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.

Comment 4

[Martin Thomson [:mt:]]

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.

Comment 5

[Nils Ohlmeier [:drno]]

Attachment: bug_1139094_steeplechase_tests.patch

Comment 6

[Nils Ohlmeier [:drno]]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d92a01ab497

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3d92a01ab497

Comment 9

[Nils Ohlmeier [:drno]]

Try run on Aurora: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f503e28ff39e

Reminder to myself to check for the results

Comment 10

[Nils Ohlmeier [:drno]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6338c7ac59a1