Closed Bug 875801 Opened 12 years ago Closed 11 years ago

Add sandbox.Androidx86 and skip some reftests for Android x86

Categories

(Testing :: Reftest, defect)

Product:
Testing
Component:
Reftest
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 917361

People

(Reporter: dminor, Assigned: dminor)

Details

Attachments

(1 file, 1 obsolete file)

Patch to add AndroidX86 flag.
1.07 KB, patch
jmaher
: review-
dbaron
: feedback-
Details | Diff | Splinter Review
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.
Blocks: 877266
No longer blocks: 877266
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.
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.
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-
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.
Resolving this as a duplicate of bug 917361 as that is where ongoing work with Android x86 is being tracked.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: