889201 - Add tests for VideoPlaybackQuality

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Matthew Gregan [:kinetik]]

VideoPlaybackQuality was added in bug 855130.  It needs tests.

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: Add tests for VideoPlaybackQuality. r=roc

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 770647 [details] [diff] [review]
Add tests for VideoPlaybackQuality.  r=roc

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

r+ with those fixed

::: content/media/test/test_VideoPlaybackQuality.html
@@ +13,5 @@
> +
> +addLoadEvent(function() {
> +  SpecialPowers.setBoolPref("media.mediasource.enabled", true);
> +  var video = document.createElement("video");
> +  ok(video.getVideoPlaybackQuality, "getVideoPlaybackQuality should be exposed with pref set");

Use SpecialPowers.pushPrefEnv

@@ +41,5 @@
> +    ok(vpq.corruptedVideoFrames >= 0, "corruptedVideoFrames should be >= 0");
> +    ok(vpq.playbackJitter >= 0, "playbackJitter should be >= 0");
> +
> +    SpecialPowers.setBoolPref("media.video_stats.enabled", false);
> +    vpq = video.getVideoPlaybackQuality();

here too.

::: content/media/test/test_VideoPlaybackQuality_disabled.html
@@ +12,5 @@
> +SimpleTest.waitForExplicitFinish();
> +
> +addLoadEvent(function() {
> +  var video = document.createElement("video");
> +  ok(!video.getVideoPlaybackQuality, "getVideoPlaybackQuality should be hidden behind a pref");

pushPrefEnv a false value for the preference so this test doesn't fail when we flip the pref

Comment 3

[Matthew Gregan [:kinetik]]

Attachment: Add tests for VideoPlaybackQuality. r=roc

Comment 4

[Matthew Gregan [:kinetik]]

https://tbpl.mozilla.org/?tree=Try&rev=877b6500773f

Comment 5

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ea64b6ba89b

Comment 6

[Phil Ringnalda (:philor)]

Sorry, backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/7d0eece628c3. I tried to convince my self it was just intermittently failing, but

https://tbpl.mozilla.org/php/getParsedLog.php?id=24957996&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24960362&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24962098&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24969421&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24968940&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=24972267&tree=Mozilla-Inbound

is just too much.

Comment 7

[Matthew Gregan [:kinetik]]

Thanks, sorry you had to back out.

Extra logging and try pushes reveal this, so far:

00:00:22     INFO -  0x11a11b400 GetVideoPlayback sVideoStatsEnabled=1
00:00:22     INFO -  145986 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_VideoPlaybackQuality.html | creationTime should be 0 - got 5159.374753, expected 0
00:00:22     INFO -  145987 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_VideoPlaybackQuality.html | totalVideoFrames should be 0 - got 120, expected 0

i.e. that sVideoStatsEnabled is (racily) still true when the test tries to check that disabling video stats results in a VideoPlaybackQuality with all attributes set to zero.

Comment 8

[Matthew Gregan [:kinetik]]

Duh, stupid mistake.  SimpleTest.finish() called outside of the last pushPrefEnv callback.

Relanded with simple test fix:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d800f929d54

Green try push:
https://tbpl.mozilla.org/?tree=Try&rev=cb7bf119133d

Comment 9

[Ed Morley [:emorley]]

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