Permaorange in test_animation_observers_sync.html when Gecko 55 merges to mozilla-beta on 2017-06-12

RESOLVED FIXED in Firefox 55

Status

()

Core
DOM: Animation
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: philor, Assigned: boris)

Tracking

({regression})

55 Branch
mozilla55
regression
Points:
---

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55+ fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

9 months ago
Apparently https://hg.mozilla.org/mozilla-central/rev/2a0949cc856f added the first use of anything behind dom.animations-api.core.enabled to test_animation_observers_sync.html, since it was passing in simulations of trunk as though it was already merged to beta until that landed, and now it fails like https://treeherder.mozilla.org/logviewer.html#?job_id=100611558&repo=try. Looks like now it needs to set it like the other tests in that directory do.

[Tracking Requested - why for this release]: merge bustage which would result in a closed tree and a delayed beta 1.
(Assignee)

Comment 1

9 months ago
Thanks for catching this. We should enable dom.animations-api.core.enabled in this test case.
Comment hidden (mozreview-request)

Comment 3

9 months ago
mozreview-review
Comment on attachment 8869690 [details]
Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

https://reviewboard.mozilla.org/r/141260/#review144856

I don't think we need to open a sub window in this test case, since it runs on chrom privilege.
We can just call pushPrefEnv just like other tests (e.g. test_animation_properties.html).
Attachment #8869690 - Flags: review?(hikezoe)
Comment hidden (mozreview-request)

Comment 5

9 months ago
mozreview-review
Comment on attachment 8869690 [details]
Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

https://reviewboard.mozilla.org/r/141260/#review144860

Thank you for the quick fix!

::: dom/animation/test/chrome/test_animation_observers_sync.html:1000
(Diff revision 2)
> -  assert_equals_records(observer.takeRecords(),
> +    assert_equals_records(observer.takeRecords(),
> -    [{ added: [], changed: [], removed: [anim] }],
> +      [{ added: [], changed: [], removed: [anim] }],
> -    "records after animation is finished");
> +      "records after animation is finished");
> -}, "exclude_animations_targeting_pseudo_elements");
> +  }, "exclude_animations_targeting_pseudo_elements");
> +}
> +

We need to call setup({explicit_done: true}) here since pushPrefEnv() is asynchronous call.  This is the reason why done() is called after runTest()?
Oh, I just realized some tests in dom/animation/test/chrome/ don't use explicit_done at all..
Attachment #8869690 - Flags: review?(hikezoe) → review+
(Assignee)

Comment 6

9 months ago
mozreview-review-reply
Comment on attachment 8869690 [details]
Bug 1366441 - Enable dom.animations-api.core.enabled for test_animation_observers_sync.html.

https://reviewboard.mozilla.org/r/141260/#review144860

> We need to call setup({explicit_done: true}) here since pushPrefEnv() is asynchronous call.  This is the reason why done() is called after runTest()?
> Oh, I just realized some tests in dom/animation/test/chrome/ don't use explicit_done at all..

Yes. That's why I didn't add it because I just copy from them. Anyway, I will add setup({explicit_done: true}). Thanks a lot.
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Assignee: nobody → boris.chiou

Comment 8

9 months ago
Pushed by bchiou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df9c4ed9ec08
Enable dom.animations-api.core.enabled for test_animation_observers_sync.html. r=hiro
(Reporter)

Comment 9

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/df9c4ed9ec08
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
tracking-firefox55: ? → +
You need to log in before you can comment on or make changes to this bug.