Closed
Bug 1251013
Opened 9 years ago
Closed 9 years ago
Remove Android 9-specific annotations from test manifests
Categories
(Firefox for Android Graveyard :: Testing, defect)
Firefox for Android Graveyard
Testing
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: gbrown, Assigned: gbrown)
References
Details
Attachments
(4 files)
(deleted),
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
Some tests are disabled specifically on Android 2.3. Once 2.3 is retired, we can remove those skip-if, fail-if annotations from manifests.
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 1•9 years ago
|
||
While I'm at it...
- "AndroidVersion == 15" likely means Android 4.0 tests on Pandaboards, no longer run -- these can be eliminated.
- "AndroidVersion >= 15" is redundant, in that Android 15 is the minimum supported version now -- these can be replaced with "Android".
- "AndroidVersion == 17" likely means Android 4.2 on x86, where we only run xpcshell now -- these can be eliminated.
Really, since most tests only run on Android 4.3, any AndroidVersion/android_version annotation can probably be simplified.
Assignee | ||
Comment 2•9 years ago
|
||
Assignee | ||
Comment 3•9 years ago
|
||
The scope of this bug expanded to update test manifests for Android - remove/simplify anything related to tegras, pandas, android 2.3, or android x86 (except xpcshell). I think that's good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c0ba017537a
I'll update all the affected bugs once this lands.
Attachment #8731538 -
Flags: review?(jmaher)
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8731540 -
Flags: review?(jmaher)
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8731541 -
Flags: review?(jmaher)
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8731542 -
Flags: review?(jmaher)
Assignee | ||
Comment 7•9 years ago
|
||
I notice there's a failure in the try run on dom/canvas/test/reftest/drawFocusIfNeeded.html on OS X 10.10. This had the curious annotation "skip-if(Android&&AndroidVersion<15,8,500)" before...I wonder how that's interpretted.
Comment 8•9 years ago
|
||
Comment on attachment 8731538 [details] [diff] [review]
update manifests - robocop
Review of attachment 8731538 [details] [diff] [review]:
-----------------------------------------------------------------
out of curiosity, are we testing these disabled tests on non 4.3? It seems most of them were disabled on 2.3 and 4.3, have they ever worked? can we just get rid of them?
Attachment #8731538 -
Flags: review?(jmaher) → review+
Comment 9•9 years ago
|
||
Comment on attachment 8731540 [details] [diff] [review]
update manifests - xpcshell
Review of attachment 8731540 [details] [diff] [review]:
-----------------------------------------------------------------
::: image/test/unit/xpcshell.ini
@@ +40,5 @@
> [test_async_notification_animated.js]
> [test_encoder_apng.js]
> [test_encoder_png.js]
> [test_imgtools.js]
> # Bug 676968: test fails consistently on Android; crashes on 4.3 (bug 1156452)
can we clean this comment up?
Attachment #8731540 -
Flags: review?(jmaher) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8731541 [details] [diff] [review]
update manifests - reftests
Review of attachment 8731541 [details] [diff] [review]:
-----------------------------------------------------------------
lots of nits, mostly around bugs in the comments.
::: dom/canvas/test/reftest/reftest.list
@@ +153,5 @@
> == stroketext-shadow.html stroketext-shadow-ref.html
>
> # focus rings
> +pref(canvas.focusring.enabled,true) skip-if(B2G) skip-if(winWidget) needs-focus == drawFocusIfNeeded.html drawFocusIfNeeded-ref.html
> +pref(canvas.customfocusring.enabled,true) skip-if(B2G) skip-if(Android) skip-if(winWidget) needs-focus == drawCustomFocusRing.html drawCustomFocusRing-ref.html
for drawFocusIfNeeded you removed all android bits, but for drawCustomFocusring you have a simplified skip-if(Android)
I think based on your try run, we should have skip-if(Android) on both.
::: gfx/tests/reftest/reftest.list
@@ +1,4 @@
> # 468496-1 will also detect bugs in video drivers.
> == 468496-1.html 468496-1-ref.html
> fuzzy-if(winWidget,175,443) == 611498-1.html 611498-ref.html
> +skip-if(B2G) fuzzy-if(Android,8,1000) == 709477-1.html 709477-1-ref.html # bug 773482
is this a bug that we plan to fix?
::: layout/reftests/first-letter/reftest.list
@@ +34,5 @@
> == 23605-6.html 23605-6-ref.html
> != 229764-1.html 229764-ref.html
> == 229764-2.html 229764-ref.html
> == 329069-1.html 329069-1-ref.html
> +fails-if(Android) == 329069-2.html 329069-2-ref.html # Bug 999139
does this referenced bug have work items associated with it?
::: layout/reftests/font-face/reftest.list
@@ +6,5 @@
> HTTP(..) == download-2.html download-2-ref.html
> HTTP(..) != download-2.html about:blank
> random-if(winWidget) HTTP(..) == download-2-big.html download-2-big-otf.html # bug 470713
> HTTP(..) != download-2-big-otf.html about:blank
> +asserts-if(Android&&!asyncPan,4-8) HTTP(..) != download-3-notref.html download-3.html # bug 1019192, bug 936226
I think one of these bugs need to be removed from the comment.
Attachment #8731541 -
Flags: review?(jmaher) → review+
Comment 11•9 years ago
|
||
Comment on attachment 8731542 [details] [diff] [review]
update manifests - mochitests
Review of attachment 8731542 [details] [diff] [review]:
-----------------------------------------------------------------
looking good, a few nits.
::: dom/html/test/mochitest.ini
@@ +403,5 @@
> skip-if = (buildapp == 'b2g' && toolkit != 'gonk') #Bug 931116, b2g desktop specific, initial triage
> [test_bug658746.html]
> [test_bug659596.html]
> [test_bug659743.xml]
> +skip-if = (buildapp == 'b2g' && toolkit != 'gonk') #Bug 931116, b2g desktop specific, initial triage
nit: trailing whitespace is added here.
::: dom/media/tests/mochitest/identity/mochitest.ini
@@ +2,4 @@
> # won't run on b2g desktop tests - bug 1119993
> # broken HTTPS on b2g emulator - bug 1135339
> subsuite = media
> +skip-if = android_version == '18' || (buildapp == 'b2g' && toolkit != 'gonk') || (buildapp == 'b2g' && toolkit == 'gonk') || buildapp == 'mulet'
do we not have a bug for android version 18?
Attachment #8731542 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 12•9 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/10cf6a630944 (bug 540456) introduced drawFocusIfNeeded.html (originally named drawSystemFocusRing.html) and drawCustomFocusRing.html and used "skip-if(Android&&AndroidVersion<15,8,500)", which appears to evaluate to skip-if(true): I think those 2 tests have never run on any platform. I'll update the manifest to skip on failing platforms and report a new bug.
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #8)
> out of curiosity, are we testing these disabled tests on non 4.3? It seems
> most of them were disabled on 2.3 and 4.3, have they ever worked? can we
> just get rid of them?
The mobile team does some local testing, so it's possible, but otherwise, no. :margaret is trying to determine if those tests can be removed.
Assignee | ||
Comment 14•9 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #9)
> Comment on attachment 8731540 [details] [diff] [review]
> update manifests - xpcshell
>
> Review of attachment 8731540 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: image/test/unit/xpcshell.ini
> @@ +40,5 @@
> > [test_async_notification_animated.js]
> > [test_encoder_apng.js]
> > [test_encoder_png.js]
> > [test_imgtools.js]
> > # Bug 676968: test fails consistently on Android; crashes on 4.3 (bug 1156452)
>
> can we clean this comment up?
Yes - thanks! After reviewing both bugs, I'm going with simply:
# Bug 676968
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #10)
> ::: gfx/tests/reftest/reftest.list
> @@ +1,4 @@
> > # 468496-1 will also detect bugs in video drivers.
> > == 468496-1.html 468496-1-ref.html
> > fuzzy-if(winWidget,175,443) == 611498-1.html 611498-ref.html
> > +skip-if(B2G) fuzzy-if(Android,8,1000) == 709477-1.html 709477-1-ref.html # bug 773482
>
> is this a bug that we plan to fix?
Bug 773482 is "enable reftests on B2G". I question if it will be fixed, but I am reluctant to make changes unrelated to Android in this bug.
> ::: layout/reftests/first-letter/reftest.list
> @@ +34,5 @@
> > == 23605-6.html 23605-6-ref.html
> > != 229764-1.html 229764-ref.html
> > == 229764-2.html 229764-ref.html
> > == 329069-1.html 329069-1-ref.html
> > +fails-if(Android) == 329069-2.html 329069-2-ref.html # Bug 999139
>
> does this referenced bug have work items associated with it?
Bug 999139 was opened for Android 2.3 and not updated for 4.3 -- until now.
> ::: layout/reftests/font-face/reftest.list
> @@ +6,5 @@
> > HTTP(..) == download-2.html download-2-ref.html
> > HTTP(..) != download-2.html about:blank
> > random-if(winWidget) HTTP(..) == download-2-big.html download-2-big-otf.html # bug 470713
> > HTTP(..) != download-2-big-otf.html about:blank
> > +asserts-if(Android&&!asyncPan,4-8) HTTP(..) != download-3-notref.html download-3.html # bug 1019192, bug 936226
>
> I think one of these bugs need to be removed from the comment.
936226 (x86) removed.
Assignee | ||
Comment 16•9 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #11)
> ::: dom/media/tests/mochitest/identity/mochitest.ini
> @@ +2,4 @@
> > # won't run on b2g desktop tests - bug 1119993
> > # broken HTTPS on b2g emulator - bug 1135339
> > subsuite = media
> > +skip-if = android_version == '18' || (buildapp == 'b2g' && toolkit != 'gonk') || (buildapp == 'b2g' && toolkit == 'gonk') || buildapp == 'mulet'
>
> do we not have a bug for android version 18?
Good catch -- that should still be bug 981881 (bug needs updating to make that clearer).
Comment 17•9 years ago
|
||
Comment 18•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/059094bbd0b8
https://hg.mozilla.org/mozilla-central/rev/7202434767c0
https://hg.mozilla.org/mozilla-central/rev/40cc2419d4f7
https://hg.mozilla.org/mozilla-central/rev/cd6d52ddb575
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•