Add sandbox.Androidx86 and skip some reftests for Android x86

RESOLVED DUPLICATE of bug 917361

Status

RESOLVED DUPLICATE of bug 917361
5 years ago
5 years ago

People

(Reporter: dminor, Assigned: dminor)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Reftests run on the emulators with a small number of unexpected results.

Adding the ability to detect the android x86 platform in the reftest sandbox and then adding skip-ifs to the reftest lists will allow reftests to run green for Android x86.
you will actually want fails-if(androidX86) to denote a know failure.  skip-if() should only be used in situations where you have a crash or horrible consequences.  random-if() is used if we have a test case that acts random.
(Assignee)

Updated

5 years ago
Blocks: 877266
(Assignee)

Updated

5 years ago
No longer blocks: 877266
(Assignee)

Comment 2

5 years ago
Created attachment 756996 [details] [diff] [review]
Patch to add AndroidX86 flag and associated changes to manifests

Try run for this patch:
https://tbpl.mozilla.org/?tree=Try&rev=0ad0c992b9af
Attachment #756996 - Flags: review?(jmaher)
Comment on attachment 756996 [details] [diff] [review]
Patch to add AndroidX86 flag and associated changes to manifests

Review of attachment 756996 [details] [diff] [review]:
-----------------------------------------------------------------

A bunch of nits and what seems to be some potential logic problems and one manifest with addition of Android instead of AndroidX86.  

In general, It seems that we have the same results for many of the android and androidx86 tests, could we have these conditions:
* Android
** AndroidX86
** Android2.2
** Android4.0

Then we could leave a lot of the Android conditions in there, but change things for Android2.2 or AndroidX86 specifically.  Thoughts?

::: content/canvas/test/reftest/reftest.list
@@ +145,5 @@
>  fuzzy-if(B2G,256,83)  == webgl-color-alpha-test.html?colorVal=1.0&alphaVal=0.0&alpha&nogl  wrapper.html?white.png
>  fails-if(B2G)         == webgl-color-alpha-test.html?colorVal=1.0&alphaVal=0.0&alpha       wrapper.html?white.png
>  
> +fuzzy(1,65536) fuzzy-if(B2G,256,83) fuzzy-if(Android||B2G,9,65536) fails-if(AndroidX86) == webgl-color-alpha-test.html?colorVal=0.5&alphaVal=1.0&nogl  wrapper.html?half-colors.png
> +fuzzy(1,65536) fuzzy-if(B2G,256,83) fuzzy-if(Android||B2G,9,65536) fails-if(AndroidX86) == webgl-color-alpha-test.html?colorVal=0.5&alphaVal=1.0       wrapper.html?half-colors.png

why are these Android and not Androidx86?

@@ +153,5 @@
> +fuzzy(1,65536) fails-if(B2G||AndroidX86)        fuzzy-if(Android,9,65536)                                      == webgl-color-alpha-test.html?colorVal=1.0&alphaVal=0.5&alpha               wrapper.html?colors-half-alpha.png
> +fuzzy(1,65536) fuzzy-if(B2G,256,83) fuzzy-if(Android||B2G,9,65536) fails-if(AndroidX86)                                == webgl-color-alpha-test.html?colorVal=0.5&alphaVal=0.5&alpha&nogl          wrapper.html?half-colors-half-alpha.png
> +fuzzy(1,65536) fails-if(B2G)        fuzzy-if(Android,9,65536) fails-if(cocoaWidget||Android||AndroidX86)       == webgl-color-alpha-test.html?colorVal=0.5&alphaVal=0.5&alpha               wrapper.html?half-colors-half-alpha.png
> +fuzzy(1,65536) fuzzy-if(B2G,256,83) fuzzy-if(Android||B2G,9,65536) fails-if(AndroidX86)                                == webgl-color-alpha-test.html?colorVal=0.5&alphaVal=0.5&alpha&premult&nogl  wrapper.html?colors-half-alpha.png
> +fuzzy(1,65536) fails-if(B2G)        fuzzy-if(Android,9,65536) fails-if(AndroidX86)                                     == webgl-color-alpha-test.html?colorVal=0.5&alphaVal=0.5&alpha&premult       wrapper.html?colors-half-alpha.png

more Android

::: editor/reftests/reftest.list
@@ +43,3 @@
>  == spellcheck-textarea-attr.html spellcheck-textarea-nofocus-ref.html
>  #the random-if(Android) tests pass on android native, but fail on android-xul, see bug 728942
> +skip-if(B2G) random-if(Android) fails-if(AndroidX86) needs-focus != spellcheck-textarea-attr.html spellcheck-textarea-ref.html

oh ugliness

::: image/test/reftest/pngsuite-palettes/reftest.list
@@ +13,2 @@
>  # ps2n2c16 - six-cube suggested palette (2 bytes) in true-color image 
> +fails-if(Android||AndroidX86) == ps2n2c16.png ps2n2c16.html

seems like we should just ignore this reftest.list file for android.

::: image/test/reftest/pngsuite-transparency/reftest.list
@@ +22,5 @@
>  # tp0n2c08 - not transparent for reference (logo on gray)
>  # tp0n3p08 - not transparent for reference (logo on gray)
>  # ...these 3 not tested because they're not transparent.
>  # tp1n3p08 - transparent, but no background chunk 
> +fails-if(Android||AndroidX86) == wrapper.html?tp1n3p08.png tp1n3p08.html

seems like we should just ignore this reftest.list file for android.

::: layout/reftests/backgrounds/reftest.list
@@ +67,5 @@
>  random-if(bug685516) == background-size-body-percent-percent-no-repeat.html background-size-body-percent-percent-ref.html
>  random-if(bug685516) == background-size-body-percent-percent-not-fixed.html background-size-body-percent-percent-ref.html
>  random-if(bug685516) == background-size-body-cover.html background-size-body-cover-ref.html
>  random-if(bug685516) == background-size-body-cover-no-repeat.html background-size-body-cover-ref.html
> +fails-if(smallScreen&&Android||AndroidX86) random-if(bug685516) != background-size-body-cover-not-fixed.html background-size-body-cover-ref.html

do we need the smallScreen clause for x86 as well?

::: layout/reftests/first-letter/reftest.list
@@ +58,5 @@
>  == 441418-1.html 441418-1-ref.html
>  == 469227-1.html 469227-1-ref.html
>  == 484400-1.html 484400-1-ref.html
>  == 594303-1.html 594303-1-ref.html
> +fails-if(!gtk2Widget&&!Android||AndroidX86) == 617869-1.html 617869-1-ref.html 

not sure about the &&,|| logic here.

::: layout/reftests/font-matching/reftest.list
@@ +79,5 @@
>  # **NOTE** we skip these on Linux because of bug 769659.
>  # test 1 uses Cherokee; expected to pass on OS X and Win7
>  random-if(!(cocoaWidget||/^Windows\x20NT\x206\.1/.test(http.oscpu))) skip-if(gtk2Widget) != bold-system-fallback-1.html bold-system-fallback-1-notref.html
> +# test 2 uses Chess symbols; expected to pass on Android||AndroidX86
> +random-if(!Android||AndroidX86) skip-if(gtk2Widget) != bold-system-fallback-2.html bold-system-fallback-2-notref.html

odd, this is random on all OS's (Android x86) except for Android

::: layout/reftests/layers/reftest.list
@@ +1,2 @@
>  == move-to-background-1.html move-to-background-1-ref.html
> +fuzzy-if(cocoaWidget,2,6) random-if(Android||AndroidX86&&!browserIsRemote) == component-alpha-exit-1.html component-alpha-exit-1-ref.html # bug 760275

androidx86 && !browserisRemote?

::: layout/reftests/mathml/reftest.list
@@ +7,5 @@
>  == dir-7.html dir-7-ref.html
>  fails == dir-8.html dir-8-ref.html
>  fails == dir-9.html dir-9-ref.html # Bug 787215
> +random-if(AndroidX86) == dir-10.html dir-10-ref.html
> +skip-if(B2G) fails-if(smallScreen&&Android||AndroidX86) fuzzy(255,200) == mirror-op-1.html mirror-op-1-ref.html

do we need smallScreen for x86 as well?

::: layout/reftests/percent-overflow-sizing/reftest.list
@@ +4,5 @@
> +skip-if(B2G) fails-if(AndroidX86||Android&&browserIsRemote) == hScrollSimpleHeightQuirks-1.html greenboxhbar.html # bug 650591, 732565 # bug 773482
> +skip-if(B2G) fails-if(AndroidX86||Android&&browserIsRemote) == hScrollSimpleHeightQuirks-2.html greenboxhbar.html # bug 650591, 732565 # bug 773482
> +skip-if(B2G) fails-if(AndroidX86||Android&&browserIsRemote) == hScrollSimpleHeightQuirks-3.html greenboxhbar.html # bug 650591, 732565 # bug 773482
> +skip-if(B2G) fails-if(AndroidX86||Android&&browserIsRemote) == hScrollAbsHeight.html greenboxhbar.html # bug 650591, 732565 # bug 773482
> +skip-if(B2G) fails-if(AndroidX86||Android&&browserIsRemote) == hScrollAbsHeightQuirks.html greenboxhbar.html # bug 650591, 732565 # bug 773482

so this seems to work on Android if !browserIsRemote...?

::: layout/reftests/reftest-sanity/reftest.list
@@ +1,3 @@
> +skip-if(AndroidX86) == data:text/html,<body> about:blank
> +skip-if(AndroidX86) == data:text/plain, about:blank
> +skip-if(AndroidX86) != data:text/plain,HELLO about:blank

this isn't good of these are failing on AndroidX86, we should have a deeper look into this.

@@ +70,5 @@
>  
>  # test that all corners are visible
>  != corners-1.html corners-1-ref.html
> +fails-if(AndroidX86) != corners-2.html corners-2-ref.html
> +fails-if(AndroidX86) != corners-3.html corners-3-ref.html

probably an issue with screen resolution

::: layout/reftests/scrolling/reftest.list
@@ +4,5 @@
>  skip-if(B2G) random-if(gtk2Widget) random-if(bug685516) HTTP == fixed-text-1.html fixed-text-1.html?ref
>  random-if(bug685516) HTTP == fixed-text-2.html fixed-text-2.html?ref
> +random-if(Android||AndroidX86&&!browserIsRemote) == iframe-border-radius.html iframe-border-radius-ref.html # bug 760269
> +random-if(Android||AndroidX86) HTTP == image-1.html image-1.html?ref
> +random-if(Android||AndroidX86&&!browserIsRemote) HTTP == opacity-mixed-scrolling-1.html opacity-mixed-scrolling-1.html?ref # bug 760269

I think you messed up the ||,&& logic here.

::: layout/reftests/svg/image/reftest.list
@@ +2,5 @@
>  
>  == image-fill-01.svg          ../pass.svg
>  skip-if(B2G) == image-filter-01.svg        image-filter-01-ref.svg # bug 773482
>  == image-load-01.svg          ../pass.svg
> +fuzzy-if(Android||AndroidX86&&!browserIsRemote,4,32) == image-opacity-01.svg image-opacity-01-ref.svg # Bug 779514 for Android

likewise ||,&& logic

::: layout/reftests/svg/sizing/reftest.list
@@ +10,5 @@
>  #   http://www.w3.org/TR/SVGMobile12/coords.html#InitialViewport
>  #   http://www.w3.org/TR/SVGMobile12/coords.html#IntrinsicSizing
>  #   http://www.w3.org/TR/CSS21/visudet.html
>  
> +skip-if(B2G) fails-if(smallScreen&&Android||AndroidX86) != scrollbars-01.svg scrollbars-01-anti-ref.svg

&&,|| logic here

::: layout/reftests/svg/smil/transform/reftest.list
@@ +11,5 @@
>  fuzzy(32,120) == rotate-angle-5.svg rotate-angle-ref.svg # bug 839865
>  fuzzy(16,6237) == scale-1.svg scale-1-ref.svg            # bug 839865
>  == set-transform-1.svg lime.svg
>  fuzzy(132,13656) == skew-1.svg skew-1-ref.svg            # bug 839865
> +random-if(Android||AndroidX86&&!browserIsRemote) == translate-clipPath-1.svg lime.svg # bug 760266

not sure about this logic.

::: layout/reftests/text/reftest.list
@@ +49,5 @@
>  skip-if(!(d2d||cocoaWidget)) random-if(d2d) != subpixel-glyphs-x-2a.html subpixel-glyphs-x-2b.html
>  skip-if(B2G) HTTP(..) == subpixel-glyphs-x-3a.html subpixel-glyphs-x-3b.html
>  # No platforms do subpixel positioning vertically
>  == subpixel-glyphs-y-1a.html subpixel-glyphs-y-1b.html
> +fuzzy-if((Android||AndroidX86||B2G),231,603) == subpixel-lineheight-1a.html subpixel-lineheight-1b.html

why the extra parens here?

@@ +239,2 @@
>  == auto-hyphenation-xmllang-13b.xhtml auto-hyphenation-1-ref.html
> +random-if(AndroidX86) == auto-hyphenation-xmllang-14a.xhtml auto-hyphenation-4-ref.html

is there a reason so many of the auto-hyphenation tests are failing?  I would like a bugnumber for tracking.
Attachment #756996 - Flags: review?(jmaher) → review-
Comment on attachment 756996 [details] [diff] [review]
Patch to add AndroidX86 flag and associated changes to manifests

>     // Shortcuts for widget toolkits.
>     sandbox.B2G = xr.widgetToolkit == "gonk";
>-    sandbox.Android = xr.OS == "Android" && !sandbox.B2G;
>+    sandbox.AndroidX86 = xr.OS == "Android" && xr.XPCOMABI.contains("x86");
>+    sandbox.Android = xr.OS == "Android" && !sandbox.B2G && !sandbox.AndroidX86;
>     sandbox.cocoaWidget = xr.widgetToolkit == "cocoa";
>     sandbox.gtk2Widget = xr.widgetToolkit == "gtk2";
>     sandbox.qtWidget = xr.widgetToolkit == "qt";
>     sandbox.winWidget = xr.widgetToolkit == "windows";

Yeah, as jmaher said in comment 3, the existing Android condition seems to be what we want the Android condition to be.  And I'm suspicious of wanting to adjust reftest results based on a run in which some of the basic reftest-sanity tests are failing.
(Assignee)

Comment 5

5 years ago
Valid point with regard to adding AndroidX86 as a subcase of Android. I'll prepare a patch with just the revised condition check and no manifests. I'm not sure if it makes sense to spend more time on the manifests for X86 until we are closer to running AndroidX86 tests in production.

I think it makes sense to then look at the version check (Bug 877266), in case some of these results are a 2.x vs. 4.x difference and not something Android X86 specific.

For the isBrowserRemote and smallScreen flags, I only run X86 under one condition, so I currently have no way of testing whether these flags are required.
(Assignee)

Comment 6

5 years ago
Created attachment 757369 [details] [diff] [review]
Patch to add AndroidX86 flag.

Not sure if we necessarily want to land this without manifests which use it, but I thought it would be good to have this here in case it is a while before we look at reftests on Android X86 again.
Attachment #756996 - Attachment is obsolete: true
Attachment #757369 - Flags: review?(jmaher)
Attachment #757369 - Flags: feedback?(dbaron)
Comment on attachment 757369 [details] [diff] [review]
Patch to add AndroidX86 flag.

Review of attachment 757369 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/tools/reftest/reftest.js
@@ +574,5 @@
>      sandbox.Android = xr.OS == "Android" && !sandbox.B2G;
> +
> +    if (sandbox.Android) {
> +        sandbox.AndroidX86 = xr.XPCOMABI.contains("x86");
> +    }

in this case sandbox.AndroidX86 is not defined for non Android cases.  Can we define it as false and then set it to true conditionally?
Attachment #757369 - Flags: review?(jmaher) → review-
Comment on attachment 757369 [details] [diff] [review]
Patch to add AndroidX86 flag.

Instead of making a condition that's the && of two other conditions, could we perhaps make "x86" and "ARM" conditions that generally hold across platforms?  (I guess I'm not sure whether x86 should be true for x86_64/amd64, though...)
Attachment #757369 - Flags: feedback?(dbaron) → feedback-
(Assignee)

Comment 9

5 years ago
Thank you for the comments. So something like this:
+    sandbox.ARM = xr.XPCOMABI.contains("arm"); 
+    sandbox.X86 = xr.XPCOMABI.contains("x86"); 

Are there any reftests that would legitemately fail due to processor type?

To me, marking a result fails-if(sandbox.Android&&sandbox.X86) has a different implication from fails-if(sandbox.AndroidX86). The first one says the processor is the problem and the second one says that the "platform" is the problem, and maybe needs to be investigated further. 

Arguably, that investigation should happen prior to the manifests being modified, but I'm not sure if that is always practical.
(Assignee)

Comment 10

5 years ago
Resolving this as a duplicate of bug 917361 as that is where ongoing work with Android x86 is being tracked.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 917361
You need to log in before you can comment on or make changes to this bug.