Intermittent TEST-UNEXPECTED-TIMEOUT | dom/media/tests/mochitest/test_peerConnection_noTrickleOffer.html | application timed out after 330 seconds with no output

RESOLVED DUPLICATE of bug 1304519

Status

()

defect
P3
normal
Rank:
35
RESOLVED DUPLICATE of bug 1304519
3 years ago
3 years ago

People

(Reporter: intermittent-bug-filer, Assigned: drno)

Tracking

({intermittent-failure})

unspecified
mozilla52
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox50 unaffected, firefox51 fixed, firefox52 fixed)

Details

Attachments

(1 attachment)

Rank: 35
Priority: -- → P3
Hmm really bizarre. ICE logging claims both PC's successfully connected within the first second of the test running. But the mochitest logging claims the ICE connection state never progressed from 'new' on one side and 'checking' on the second PC to any other state.
I'm wondering if it would help to cleanup the waitForIceConnection() mess.
Comment on attachment 8793562 [details]
Bug 1304269: lets promise ice from now on.

https://reviewboard.mozilla.org/r/80286/#review79070

Seeing stuff that try barfed on, will re-review once that's cleared up.
Attachment #8793562 - Flags: review?(docfaraday)
Comment on attachment 8793562 [details]
Bug 1304269: lets promise ice from now on.

https://reviewboard.mozilla.org/r/80286/#review79152

With nits.

::: dom/media/tests/mochitest/pc.js:767
(Diff revision 2)
> +  this.iceConnectedReject;
> +  this.iceConnected = new Promise((resolve, reject) => {
> +    this.iceConnectedResolve = resolve;
> +    this.iceConnectedReject = reject;
> +  });
>    this.iceCheckingRestartExpected = false;

We can get rid of this, right?

::: dom/media/tests/mochitest/pc.js:787
(Diff revision 2)
> +    if (iceState === "connected") {
> +      this.iceConnectedResolve();
> +    } else if (iceState == "failed") {
> +      this.iceConnectedReject();
> +    }

Why === on one, but == on the other?
Attachment #8793562 - Flags: review?(docfaraday) → review+
Comment on attachment 8793562 [details]
Bug 1304269: lets promise ice from now on.

https://reviewboard.mozilla.org/r/80286/#review79152

> We can get rid of this, right?

No we can't because it is still used to decide when it is okay to have the ICE connection state to go backwards.
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/552477d11cd0
lets promise ice from now on. r=bwc
https://hg.mozilla.org/mozilla-central/rev/552477d11cd0
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
What bug caused this?
Assignee: nobody → drno
Flags: needinfo?(drno)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> What bug caused this?

There is no specific bug I point at, just my observation from comment #1. It's mostly that the test code was written less then ideal back in the days before we had Promises. This is simply an attempt to handle waiting for ICE to connect in a way more clean way. My hope is that this could actually solve some intermittents around ICE failing to connect. But we should wait and see.
Flags: needinfo?(drno)
Yeah, I'm mainly asking to assess which other branches might be affected.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)
> Yeah, I'm mainly asking to assess which other branches might be affected.

Sure. The not ideal test code has in our repo's since ages (way before 49). I was initially thinking to simply let this ride the train.
But I can take an action item to look at how our ICE orange numbers are looking next week. And then we decide if it's worth uplifting.
See Also: → 1304940
Sorry to disappoint, this was bug 1304519.
Blocks: 1218576
Resolution: FIXED → DUPLICATE
Duplicate of bug: 1304519
You need to log in before you can comment on or make changes to this bug.