Remove Android 9-specific annotations from test manifests

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gbrown, Assigned: gbrown)

Tracking

unspecified
Firefox 48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(4 attachments)

(Assignee)

Description

3 years ago
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

3 years ago
See Also: → 979921
(Assignee)

Comment 1

3 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 3

3 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

3 years ago
Attachment #8731540 - Flags: review?(jmaher)
(Assignee)

Comment 5

3 years ago
Attachment #8731541 - Flags: review?(jmaher)
(Assignee)

Comment 6

3 years ago
Attachment #8731542 - Flags: review?(jmaher)
(Assignee)

Comment 7

3 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 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 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 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 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

3 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

3 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

3 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

3 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

3 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).
You need to log in before you can comment on or make changes to this bug.