Closed
Bug 943556
Opened 7 years ago
Closed 7 years ago
Intermittent test_videocontrols_standalone.html | Height of audio element should be 28, which is equal to the controls bar. - got 150, expected 28
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla28
Tracking | Status | |
---|---|---|
firefox26 | --- | unaffected |
firefox27 | --- | unaffected |
firefox28 | --- | fixed |
firefox-esr24 | --- | unaffected |
People
(Reporter: RyanVM, Assigned: jaws)
Details
(Keywords: intermittent-failure)
Attachments
(1 file, 2 obsolete files)
4.69 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
jaws, weren't you playing with this test recently? https://tbpl.mozilla.org/php/getParsedLog.php?id=31087377&tree=B2g-Inbound Rev5 MacOSX Mountain Lion 10.8 b2g-inbound opt test mochitest-5 on 2013-11-26 02:34:28 PST for push f66d18acab45 slave: talos-mtnlion-r5-082 02:41:57 INFO - 134626 INFO TEST-START | /tests/toolkit/content/tests/widgets/test_videocontrols_standalone.html 02:41:57 INFO - 134627 INFO TEST-PASS | /tests/toolkit/content/tests/widgets/test_videocontrols_standalone.html | Width of the video should match expectation 02:41:57 INFO - 134628 INFO TEST-PASS | /tests/toolkit/content/tests/widgets/test_videocontrols_standalone.html | Height of video should match expectation 02:41:57 INFO - 134629 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols_standalone.html | Height of audio element should be 28, which is equal to the controls bar. - got 150, expected 28 02:41:57 INFO - 134630 INFO TEST-INFO | MEMORY STAT vsize after test: 3929051136 02:41:57 INFO - 134631 INFO TEST-INFO | MEMORY STAT residentFast after test: 325296128 02:41:57 INFO - 134632 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 104918272 02:41:57 INFO - 134633 INFO TEST-END | /tests/toolkit/content/tests/widgets/test_videocontrols_standalone.html | finished in 368ms
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(jaws)
Assignee | ||
Comment 1•7 years ago
|
||
Yes, this test is new. I'll investigate it.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(jaws)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 4•7 years ago
|
||
I can't explain comment #2 or #3. The failures make no sense to me since the test is saying that it expects the height to be 28px, but that only happens if the user agent doesn't include "Android" in it, and these test failures are on Android 4.0. Notice too that the actual height is 123, which is the height that the test expects controls on Android to have. However, this patch does adjust the event that is listened to for the measurement to be slightly later which should fix the initial orange.
Attachment #8339068 -
Flags: review?(gijskruitbosch+bugs)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 6•7 years ago
|
||
Comment on attachment 8339068 [details] [diff] [review] Patch Review of attachment 8339068 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Jared Wein [:jaws] from comment #4) > Created attachment 8339068 [details] [diff] [review] > Patch > > I can't explain comment #2 or #3. The failures make no sense to me since the > test is saying that it expects the height to be 28px, but that only happens > if the user agent doesn't include "Android" in it, and these test failures > are on Android 4.0. Notice too that the actual height is 123, which is the > height that the test expects controls on Android to have. > > However, this patch does adjust the event that is listened to for the > measurement to be slightly later which should fix the initial orange. Is it possible another test messed with the userAgent here as well? I watched as philor figured out another intermittent orange caused by something like that recently... Might be worth info()ing the userAgent when you check it. See bug 942470. ::: toolkit/content/tests/widgets/test_videocontrols_standalone.html @@ +25,5 @@ > var video = getMediaElement(popup); > if (video.readyState >= video.HAVE_METADATA) > runTestVideo(video); > else { > + video.addEventListener("play", function onPlay() { This should match the readyState check (or an alternative) because now the two paths through this if/else statement diverge (further) which sets us up for more intermittent orange in the future.
Attachment #8339068 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 7•7 years ago
|
||
Good catch on the readyState change. Thanks!
Attachment #8339068 -
Attachment is obsolete: true
Attachment #8339106 -
Flags: review?(gijskruitbosch+bugs)
Comment 8•7 years ago
|
||
Comment on attachment 8339106 [details] [diff] [review] Patch v1.1 Review of attachment 8339106 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #8339106 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 11•7 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6cbce980e1c2
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 13•7 years ago
|
||
(In reply to TBPL Robot from comment #12) > philor > https://tbpl.mozilla.org/php/getParsedLog.php?id=31180834&tree=Fx-Team > Android 4.0 Panda fx-team opt test mochitest-8 on 2013-11-27 12:21:19 > revision: 7897e6dbbebf > slave: panda-0152 > > 77420 ERROR TEST-UNEXPECTED-FAIL | > /tests/toolkit/content/tests/widgets/test_videocontrols_standalone.html | > Height of audio element should be 28, which is equal to the controls bar. - > got 123, expected 28 > 11-27 12:48:34.437 I/GeckoDump( 2236): 77420 ERROR TEST-UNEXPECTED-FAIL | > /tests/toolkit/content/tests/widgets/test_videocontrols_standalone.html | > Height of audio element should be 28, which is equal to the controls bar. - > got 123, expected 28 > 11-27 12:48:34.437 I/GeckoDump( 2236): 77420 ERROR TEST-UNEXPECTED-FAIL | > /tests/toolkit/content/tests/widgets/test_videocontrols_standalone.html | > Height of audio element should be 28, which is equal to the controls bar. - > got 123, expected 28 The user agent in this failure is "DummyUserAgent". These are incarnations of bug 942470. The 150 number though should be fixed by this patch nonetheless.
Assignee | ||
Comment 14•7 years ago
|
||
Pushed a follow-up patch to switch from checking HAVE_ENOUGH_DATA to !element.paused since that will achieve the correct balance with waiting for the "play" event. https://hg.mozilla.org/integration/fx-team/rev/48c0dfb60fac
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 17•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6cbce980e1c2 https://hg.mozilla.org/mozilla-central/rev/48c0dfb60fac
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 19•7 years ago
|
||
This is probably at fault for turning Win8 bc debug tests semi-perma-orange, after the merge to inbound. I backed out both changesets landed here: https://hg.mozilla.org/integration/mozilla-inbound/rev/aef6901931d8 See https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=7e9a7bde5c2c for example.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 25•7 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #19) > This is probably at fault for turning Win8 bc debug tests semi-perma-orange, > after the merge to inbound. I backed out both changesets landed here: > > https://hg.mozilla.org/integration/mozilla-inbound/rev/aef6901931d8 > > See https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=7e9a7bde5c2c for > example. hm strange also after the backout this error still happens :(
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 63•7 years ago
|
||
This patch is a roll-up of the previous two patches from this bug. It also includes a waitForCondition that will spin until the audio/video reaches the desired size (timing out at 3 seconds). https://tbpl.mozilla.org/?tree=Try&rev=277dd64aa426
Attachment #8339106 -
Attachment is obsolete: true
Attachment #8341551 -
Flags: review?(gijskruitbosch+bugs)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 68•7 years ago
|
||
Comment on attachment 8341551 [details] [diff] [review] Patch v1.2 Review of attachment 8341551 [details] [diff] [review]: ----------------------------------------------------------------- r=me assuming the retriggers are green(ish) ::: toolkit/content/tests/widgets/test_videocontrols_standalone.html @@ +18,5 @@ > function getMediaElement(aWindow) { > return aWindow.document.getElementsByTagName("video")[0]; > } > > +function waitForCondition(condition, nextTest, errorMsg) { Nit: can this be in the appropriate head.js file so not all tests have to roll their own?
Attachment #8341551 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 78•7 years ago
|
||
I didn't move it to a head.js file originally because no other files were using it, but I guess that will happen eventually. https://hg.mozilla.org/integration/fx-team/rev/f096db1e9ddd
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Comment 80•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f096db1e9ddd
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Updated•7 years ago
|
status-firefox26:
--- → unaffected
status-firefox27:
--- → unaffected
status-firefox28:
--- → fixed
status-firefox-esr24:
--- → unaffected
Comment hidden (Legacy TBPL/Treeherder Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•