Closed Bug 889201 Opened 7 years ago Closed 7 years ago

Add tests for VideoPlaybackQuality

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: kinetik, Assigned: kinetik)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 1 obsolete file)

VideoPlaybackQuality was added in bug 855130.  It needs tests.
Attachment #770647 - Flags: review?(roc)
Assignee: nobody → kinetik
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
Attachment #770647 - Flags: review?(roc) → review+
Attachment #770657 - Flags: review? → review+
Attachment #770647 - Attachment is obsolete: true
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.
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
https://hg.mozilla.org/mozilla-central/rev/3d800f929d54
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.