Closed Bug 826584 Opened 7 years ago Closed 7 years ago

Intermittent test_getUserMedia_basicAudio.html | canplaythrough event never fired, | Unexpected error callback with undefined

Categories

(Core :: WebRTC, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: philor, Assigned: jsmith)

References

Details

(Keywords: intermittent-failure, Whiteboard: [getUserMedia][blocking-gum+][qa-])

Attachments

(2 files, 1 obsolete file)

https://tbpl.mozilla.org/php/getParsedLog.php?id=18448574&tree=Mozilla-Inbound
Rev3 WINNT 6.1 mozilla-inbound debug test mochitest-2 on 2013-01-03 15:40:51 PST for push 29080e5ecaed
slave: talos-r3-w7-079

25767 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/media/tests/mochitest/test_getUserMedia_basicAudio.html | canplaythrough event never fired
25768 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/media/tests/mochitest/test_getUserMedia_basicAudio.html | Unexpected error callback with undefined
25769 INFO TEST-END | /tests/dom/media/tests/mochitest/test_getUserMedia_basicAudio.html | finished in 5506ms
Looks like the patch in bug 814718 didn't fully fix the canplaythrough event problem :(
Flags: needinfo?(roc)
Any ideas Roc?
Whiteboard: [getUserMedia][blocking-gum+]
I'm wondering if this related to the one issue I saw locally that only happened with real devices - the timeout I had was five seconds before, I occasionally got a failure like this. But I didn't get it when I increased it to ten seconds.
(In reply to Jason Smith [:jsmith] from comment #3)
> Any ideas Roc?

No.
Flags: needinfo?(roc)
Have we had the 'Unexpected error callback with undefined' before too?
(In reply to Henrik Skupin (:whimboo) from comment #6)
> Have we had the 'Unexpected error callback with undefined' before too?

Yeah. That fires anytime an onError callback is fired in my tests.

The only initial idea I have is assume its a test-oriented bug with the timeouts, so I'll put out a patch to increase the timeouts to 10 seconds. Doesn't hurt to do this, but if that fails, then I don't know what to do.
Actually, I think that might be the problem:

25766 ERROR TEST-UNEXPECTED-FAIL |
/tests/dom/media/tests/mochitest/test_getUserMedia_basicAudio.html |
canplaythrough event never fired
25767 ERROR TEST-UNEXPECTED-FAIL |
/tests/dom/media/tests/mochitest/test_getUserMedia_basicAudio.html | Unexpected
error callback with undefined
25779 ERROR TEST-UNEXPECTED-FAIL |
/tests/dom/media/tests/mochitest/test_getUserMedia_basicAudio.html |
[SimpleTest.finish()] this test already called finish!
25782 ERROR TEST-UNEXPECTED-FAIL |
/tests/dom/media/tests/mochitest/test_getUserMedia_basicVideo.html |
/tests/dom/media/tests/mochitest/test_getUserMedia_basicAudio.html finished in
a non-clean fashion, probably because it didn't call SimpleTest.finish()

SimpleTest.finish is getting called twice on basicAudio. That would likely happen if we hit a case where canplaythrough fires after the 5 second timeout, but before the test officially finishes.

So increasing the timeout might help here.
Assignee: nobody → jsmith
Status: NEW → ASSIGNED
Comment on attachment 697824 [details] [diff] [review]
Increase event callback timeouts for gum tests

Increased timeout from 5 to 10 seconds. If you think I should go higher on the timeouts, let me know.
Attachment #697824 - Flags: review?(rjesup)
(In reply to Jason Smith [:jsmith] from comment #9)
> SimpleTest.finish is getting called twice on basicAudio. That would likely
> happen if we hit a case where canplaythrough fires after the 5 second
> timeout, but before the test officially finishes.

In such a case it will not help to increase the timeout. Instead you will have to remove the event listener. There might still be a very minor chance that we hit that but it's very unlikely.

> So increasing the timeout might help here.

Timeouts are always fragile if you don't know the performance of the test machine. It might take longer to get the video started. 10s might help but I wonder what are the default timeouts used in mochitests, especially in other media ones.
Comment on attachment 697824 [details] [diff] [review]
Increase event callback timeouts for gum tests

Review of attachment 697824 [details] [diff] [review]:
-----------------------------------------------------------------

If this ends up not helping (i.e. it's not the timeout), we'll probably want to back this out.  However, it seems likely it's a timeout; too bad it doesn't flag itself so we know for sure then this happens.  The other way to handle issues like this is to somehow watch for "progress" and keep resetting a shorter timer.

We may want to file a follow-on bug to investigate (with roc) best ways to handle timeouts for these tests.
Attachment #697824 - Flags: review?(rjesup) → review+
Once the tree is open again the patch should be landed.
Keywords: checkin-needed
Attachment #697899 - Attachment is obsolete: true
Comment on attachment 697900 [details] [diff] [review]
Remove canplaythrough event listener if we timeout

Here's also a small fix for the double simpletest.finish issue so that we remove the event listener if we fail.
Attachment #697900 - Flags: review?(rjesup)
(In reply to Henrik Skupin (:whimboo) from comment #12)
> (In reply to Jason Smith [:jsmith] from comment #9)
> > SimpleTest.finish is getting called twice on basicAudio. That would likely
> > happen if we hit a case where canplaythrough fires after the 5 second
> > timeout, but before the test officially finishes.
> 
> In such a case it will not help to increase the timeout. Instead you will
> have to remove the event listener. There might still be a very minor chance
> that we hit that but it's very unlikely.

Actually it would be both in that case - one stops the case from happening entirely and the other prevents the double SimpleTest.finish issue. I attached a patch to remove event listener on failure.

> 
> > So increasing the timeout might help here.
> 
> Timeouts are always fragile if you don't know the performance of the test
> machine. It might take longer to get the video started. 10s might help but I
> wonder what are the default timeouts used in mochitests, especially in other
> media ones.

True, although I think that comes with the cost of writing automation that has indirection waiting for events to fire. The other choice I could go with is to use mochitest's timeout mechanism - which automatically fails the test if the test timeouts, although I thought that was considered to be a "not clean" way of doing it.
Attachment #697900 - Flags: review?(rjesup) → review+
Looks like it's only failing on m-c but not on inbound so it should be fixed by the next merge to m-c.
https://hg.mozilla.org/mozilla-central/rev/a37a77235f83
https://hg.mozilla.org/mozilla-central/rev/3355493a6964
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [getUserMedia][blocking-gum+] → [getUserMedia][blocking-gum+][qa-]
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.