Convert image/test/mochitest/chrome.ini mochitests to something other than mochitest-chrome where appropriate

RESOLVED FIXED in Firefox 47

Status

()

Core
Layout: Images
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

(Blocks: 1 bug)

48 Branch
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 wontfix, firefox47 fixed, firefox48 fixed)

Details

Attachments

(16 attachments, 16 obsolete attachments)

3.89 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
5.34 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.12 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.16 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
2.87 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.83 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
5.86 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
4.16 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
4.88 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
5.23 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.83 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.48 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.48 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
5.83 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
3.09 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
6.31 KB, patch
tnikkel
: review+
Details | Diff | Splinter Review
The tests in image/test/mochitest/chrome.ini generally look like they should run in both the parent and child process in an e10s environment. However, they are currently chrome mochitests, and so the never really run in the child process. We should convert them to mochitest-plain or browser-chrome tests instead.
Created attachment 8736913 [details] [diff] [review]
Part 1 - Convert test_animSVGImage.html

This one was relatively easy
Created attachment 8736914 [details] [diff] [review]
Part 2 - Convert test_animSVGImage2.html

Little bit harder, but SpecialPowers.wrap to the rescue!
test_animation.html and test_animation2.html are disabled, so I left those alone. test_background_image_anim.html and test_bullet_animation.html were easy to convert. test_bug415761.html seems a bit harder, since it does some stuff with file copying, and I'm not sure how that will work with http:// URIs instead of chrome:// URIs. It's probably easier to convert it to a reftest instead.
Created attachment 8736924 [details] [diff] [review]
Part 3 - Convert test_background_image_anim.html
Created attachment 8736925 [details] [diff] [review]
Part 4 - Convert test_bullet_animation.html
Created attachment 8736941 [details] [diff] [review]
Part 5 - Convert test_changeOfSource.html
Created attachment 8736942 [details] [diff] [review]
Part 6 - Convert test_changeOfSource2.html
Created attachment 8736943 [details] [diff] [review]
Part 7 - Convert test_has_transparency.html
Created attachment 8736944 [details] [diff] [review]
Part 8 - Convert test_net_failedtoprocess.html
status-firefox46: --- → wontfix
status-firefox47: --- → affected
Created attachment 8737175 [details] [diff] [review]
Part 9 - Convert test_removal_ondecode.html
Attachment #8737175 - Attachment is patch: true
Created attachment 8737176 [details] [diff] [review]
Part 10 - Convert test_removal_onload.html
Created attachment 8737177 [details] [diff] [review]
Part 11 - Convert test_staticClone.html
Created attachment 8737178 [details] [diff] [review]
Part 12 - Convert test_svg_animatedGIF.html
Created attachment 8737179 [details] [diff] [review]
Part 13 - Convert test_svg_filter_animation.html
Created attachment 8737180 [details] [diff] [review]
Part 14 - Convert test_synchronized_animation.html
Created attachment 8737181 [details] [diff] [review]
Part 15 - Convert test_xultree_animation.html

Surprisingly, embedding XUL into these tests works just fine.
Created attachment 8737182 [details] [diff] [review]
Part 16 - Convert test_bug1132427.html

In this one I removed a load event listener from the main test file because the helper file (the one that is window.open'd) already has a onload=".." that does the same thing. This was causing the test to get run twice, and fail with a "finish() was called twice" error.
I can repro the test_xultree_animation failure when I run that test in isolation - somehow there is a JS error in textbox.xml which causes the code at [1] to call finish() again. This happens during shutdown *after* the test is done running. I'm investigating.

[1] http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js?rev=b78ed68af2fb#1587
Depends on: 1261509
With the patch on bug 1261509 (which is needed to make test_xultree_animation.xhtml pass) the try push is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=170818a31237&group_state=expanded. I also updated the patches to remove some of my requestFlakyTimeout additions since they turned out to be unneccessary.
Created attachment 8737754 [details] [diff] [review]
Part 1 - Convert test_animSVGImage.html
Attachment #8736913 - Attachment is obsolete: true
Attachment #8737754 - Flags: review?(tnikkel)
Created attachment 8737755 [details] [diff] [review]
Part 2 - Convert test_animSVGImage2.html
Attachment #8736914 - Attachment is obsolete: true
Attachment #8737755 - Flags: review?(tnikkel)
Created attachment 8737757 [details] [diff] [review]
Part 3 - Convert test_background_image_anim.html
Attachment #8736924 - Attachment is obsolete: true
Attachment #8737757 - Flags: review?(tnikkel)
Created attachment 8737758 [details] [diff] [review]
Part 4 - Convert test_bullet_animation.html
Attachment #8736925 - Attachment is obsolete: true
Attachment #8737758 - Flags: review?(tnikkel)
Created attachment 8737759 [details] [diff] [review]
Part 5 - Convert test_changeOfSource.html
Attachment #8736941 - Attachment is obsolete: true
Attachment #8737759 - Flags: review?(tnikkel)
Created attachment 8737760 [details] [diff] [review]
Part 6 - Convert test_changeOfSource2.html
Attachment #8736942 - Attachment is obsolete: true
Attachment #8737760 - Flags: review?(tnikkel)
Created attachment 8737761 [details] [diff] [review]
Part 7 - Convert test_has_transparency.html
Attachment #8736943 - Attachment is obsolete: true
Attachment #8737761 - Flags: review?(tnikkel)
Created attachment 8737762 [details] [diff] [review]
Part 8 - Convert test_net_failedtoprocess.html
Attachment #8736944 - Attachment is obsolete: true
Attachment #8737762 - Flags: review?(tnikkel)
Created attachment 8737763 [details] [diff] [review]
Part 9 - Convert test_removal_ondecode.html
Attachment #8737175 - Attachment is obsolete: true
Attachment #8737763 - Flags: review?(tnikkel)
Created attachment 8737764 [details] [diff] [review]
Part 10 - Convert test_removal_onload.html
Attachment #8737176 - Attachment is obsolete: true
Attachment #8737764 - Flags: review?(tnikkel)
Created attachment 8737766 [details] [diff] [review]
Part 11 - Convert test_staticClone.html
Attachment #8737177 - Attachment is obsolete: true
Attachment #8737766 - Flags: review?(tnikkel)
Created attachment 8737767 [details] [diff] [review]
Part 12 - Convert test_svg_animatedGIF.html
Attachment #8737178 - Attachment is obsolete: true
Attachment #8737767 - Flags: review?(tnikkel)
Created attachment 8737768 [details] [diff] [review]
Part 13 - Convert test_svg_filter_animation.html
Attachment #8737179 - Attachment is obsolete: true
Attachment #8737768 - Flags: review?(tnikkel)
Created attachment 8737769 [details] [diff] [review]
Part 14 - Convert test_synchronized_animation.html
Attachment #8737180 - Attachment is obsolete: true
Attachment #8737769 - Flags: review?(tnikkel)
Created attachment 8737770 [details] [diff] [review]
Part 15 - Convert test_xultree_animation.html
Attachment #8737181 - Attachment is obsolete: true
Attachment #8737770 - Flags: review?(tnikkel)
Created attachment 8737771 [details] [diff] [review]
Part 16 - Convert test_bug1132427.html
Attachment #8737182 - Attachment is obsolete: true
Attachment #8737771 - Flags: review?(tnikkel)
Attachment #8737754 - Flags: review?(tnikkel) → review+
Comment on attachment 8737755 [details] [diff] [review]
Part 2 - Convert test_animSVGImage2.html

>+[test_animSVGImage2.html]
>+skip-if = buildapp == 'b2g' || os == 'android'

I'm curious, why do we need to skip on b2g and android?
Attachment #8737755 - Flags: review?(tnikkel) → review+
Attachment #8737757 - Flags: review?(tnikkel) → review+
Attachment #8737758 - Flags: review?(tnikkel) → review+
Attachment #8737759 - Flags: review?(tnikkel) → review+
Attachment #8737760 - Flags: review?(tnikkel) → review+
(In reply to Timothy Nikkel (:tnikkel) from comment #37)
> I'm curious, why do we need to skip on b2g and android?

Well in the chrome.ini it has the skip-if in the DEFAULT section at the top of the file, so I carried it over into mochitest.ini. I suspect chrome mochitests are generally disabled on Fennec/B2G because their architecture is so different. We could probably enable these tests for Fennec at least as plain mochitests, but I didn't want to do that as part of this bug because it's expanding the scope. We can do that as a follow-up if you want.
Attachment #8737761 - Flags: review?(tnikkel) → review+
Attachment #8737762 - Flags: review?(tnikkel) → review+
Attachment #8737763 - Flags: review?(tnikkel) → review+
Attachment #8737764 - Flags: review?(tnikkel) → review+
Attachment #8737766 - Flags: review?(tnikkel) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #38)
> Well in the chrome.ini it has the skip-if in the DEFAULT section at the top
> of the file, so I carried it over into mochitest.ini.

Oh, I didn't see that.
Attachment #8737767 - Flags: review?(tnikkel) → review+
Attachment #8737768 - Flags: review?(tnikkel) → review+
Attachment #8737769 - Flags: review?(tnikkel) → review+
Attachment #8737770 - Flags: review?(tnikkel) → review+
Attachment #8737771 - Flags: review?(tnikkel) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3)
> test_animation.html and test_animation2.html are disabled, so I left those
> alone.

I did a try push with all disabled imagelib tests re-enabled. These two didn't fail, same with test_undisplayed_iframe.html. I'm triggering a bunch more jobs to see if they fail intermittently.
Re-enabled the three disabled mochitest-chrome tests in bug 1262269. They will need to be converted too, I'll do that in another bug.
Thanks! I also uplifted these patches to aurora since the e10s folks prefer that any e10s-related test changes be uplifted to 47 as well.

https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=f5f4bbaf8f02
status-firefox47: affected → fixed
Depends on: 1263474
Depends on: 1267003
No longer depends on: 1267003
You need to log in before you can comment on or make changes to this bug.