annotate reftest manifests to note tests that fail on Android

RESOLVED FIXED

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mobile_unittests])

Attachments

(2 attachments, 2 obsolete attachments)

currently on mobile we have a lot of tests failing.  As we fix bugs and tests we can turn more on, but hundreds of failed tests are not useful for anybody to see.

I have a patch here that adds a reftest_mobile.list which includes directories that are currently green on android tegra.
Whiteboard: [mobile_unittests]
Attachment #515099 - Flags: review?(mark.finkle)
Attachment #515099 - Flags: feedback?(ted.mielczarek)
Comment on attachment 515099 [details] [diff] [review]
add reftest_mobile.patch (1.0)

I agree with the intent of the patch
Attachment #515099 - Flags: review?(mark.finkle) → review+
Comment on attachment 515099 [details] [diff] [review]
add reftest_mobile.patch (1.0)

I think you should get signoff from dbaron on this.
Attachment #515099 - Flags: feedback?(ted.mielczarek) → review?(dbaron)
Assignee: nobody → jmaher
Seems like this disables 90% of our reftest coverage.

What's wrong with just annotating all the failing tests with fails-if(Android)?
For example:  a large portion of the tests are in layout/reftests/bugs/.  It seems like the wrong tradeoff to disable *all* the tests in that directory just because *some* of them are failing now.
Attachment #515099 - Flags: review?(dbaron) → review-
point well taken.  One thing to consider is a large portion of these failures will probably get fixed as time goes on.  Some will probably never be supported and those I could see us doing a permanent fails-if(android) clause.

Let me see how many failures we have.  The intention of this was to do something with minimal impact to get a basic set of green tests up.  But it would be nice to get a lot more coverage that is green at the same time.
this patch is half of the tests.  We are planning on running in 2 chunks, and this is chunk 2 of 2.  I verified with 3 runs that we have zero failures, so I would expect this to be green.
Attachment #515099 - Attachment is obsolete: true
Attachment #516480 - Flags: review?(dbaron)
Summary: create a reftest_mobile.list which allows for turning on/off tests for mobile only (1.0) → annotate reftest manifests to note tests that fail on Android
this is the second patch in the series here.  With both of these patches we are only left with some random failures that we will need to  address in another patch.
Attachment #517441 - Flags: review?(dbaron)
this is a small modifcation to reftest.list which will allow for green runs.  We hit about 70% of the total reftests after this patch ignores the common random orange culprits.  Unfortunately this removes 'bugs'.  I am open to any other suggestions about how to get green on android.  For reference to the random oranges see bug 638815.
Attachment #517447 - Flags: review?(dbaron)
Attachment #516480 - Attachment description: fix reftest.list files to fails-if or skip-if reftests for android (1.0) → fix reftest.list files to fails-if or skip-if reftests for android chunks 2 of 2 (1.0)
Comment on attachment 517441 [details] [diff] [review]
fix reftest.list files to fails-if or skip-if reftests for android chunks 1 of 2 (1.0)

Marking this patch obsolete because it is identical to the patch following it.  (Was the following patch supposed to be different?)
Attachment #517441 - Attachment is obsolete: true
Attachment #517441 - Flags: review?(dbaron)
Comment on attachment 516480 [details] [diff] [review]
fix reftest.list files to fails-if or skip-if reftests for android chunks 2 of 2 (1.0)

in layout/reftests/ogg-video/reftest.list:
>+skip-if(Android) random-if(!Android) HTTP(..) == object-aspect-ratio-1a.xhtml aspect-ratio-1-ref.html #this is skipped because we hang on the htmlparser tests when this is ran
>+skip-if(Android) random-if(!Android) HTTP(..) == object-aspect-ratio-1b.xhtml aspect-ratio-1-ref.html
>+skip-if(Android) HTTP(..) == offset-1.xhtml offset-1-ref.html
>+skip-if(Android) random-if(!Android) HTTP(..) == object-aspect-ratio-2a.xhtml aspect-ratio-2-ref.html
>+skip-if(Android) random-if(!Android) HTTP(..) == object-aspect-ratio-2b.xhtml aspect-ratio-2-ref.html

if you put the skip-if(Android) after the random, you won't need to change the random to random-if.

Also, please put the comment on its own line above the test, since it applies to four of the next five tests (I assume).


modules/plugin/test/reftest/reftest.list

>+skip-if(Android) random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)&&!Android) fails-if(!haveTestPlugin&&!Android) == border-padding-3.html border-padding-3-ref.html # bug 629430

Same here, for both the random-if and fails-if.
Comment on attachment 516480 [details] [diff] [review]
fix reftest.list files to fails-if or skip-if reftests for android chunks 2 of 2 (1.0)

r=dbaron with the changes in comment 10
Attachment #516480 - Flags: review?(dbaron) → review+
Comment on attachment 517447 [details] [diff] [review]
fix reftest.list files to fails-if or skip-if reftests for android chunks 1 of 2 (1.0)

r=dbaron
Attachment #517447 - Flags: review?(dbaron) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.