switch from android.json to android-failures.json

RESOLVED FIXED in Firefox 15

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

(Depends on 1 bug)

Trunk
Firefox 15
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Currently we have android.json which specifies which files/directories to run.  this ignore the majority of the tests.  

Inside of mochitest we have --run-only-tests and --exclude-tests.  Using the --exclude-tests, we can specify the exact files that fail and run everything else, including new tests!

I found that if we run in 10 chunks (instead of the current 8), we finish the majority of the tests in <25 minutes.  Chunk 3 and 9 are at 50 minutes each and chunk 10 is having problems running.

I suspect I can get this solid this week and checked in.  Then we need to work with releng to update the buildbot scripts.

I need feedback on this idea, the increased number of chunks, tests and total tegra time per push.  Doing this will increase the random oranges we encounter, but somebody could either fix the tests or manifest them out:)
lets get this checked in.  This requires us to run in 10 chunks on m-c.

Regarding times, Here are some offenders:
test_bug684671.html
test_sessionhistory.html
test_bug369814.html 
test_overlapping_transactions.html
test_fallback.html
test_redirectUpdateItem.html
test_devicemotion_multiple_listeners.html
test_xhr_timeout.html
test_singlebyte_overconsumption.html
test_bug399951.html
test_media_queries_dynamic.html
test_parse_rule.html
test_selectors_on_anonymous_content.html
test_bug467672-5c.html

I think we should be aware of the time issue here and either disable these tests for now, or get them looked at right away.

There are others, I bet I could find 100 other tests that take .>10 seconds longer on android instead of linux.
Assignee: nobody → jmaher
Attachment #629751 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #629850 - Flags: review?(ted.mielczarek)
this doesn't require us to run in 10 chunks, but we really should for tegra load and turnaround issues, I think once this is landed we can move to 10 chunks as a followup.
Target Milestone: --- → Firefox 15
updated with a few cases that I found on try server.

Once this is landed, I would like to merge in the work :bjacob is doing to clean up content/media tests so this is in sync.
Attachment #629850 - Attachment is obsolete: true
Attachment #629850 - Flags: review?(ted.mielczarek)
Attachment #630305 - Flags: review?(jgriffin)
Comment on attachment 630305 [details] [diff] [review]
list of test cases which fail on android native (1.1)

Cool, this is awesome. I'll try this list later on B2G; I suspect B2G will need its own manifest, but this is a great place to start.
Attachment #630305 - Flags: review?(jgriffin) → review+
Now I need to figure out how to change stuff to run on android native mochitests for mozilla-central only:
http://mxr.mozilla.org/build/source/buildbotcustom/steps/unittest.py#921

            self.command.extend(['--run-only-tests', testManifest])

would change to:
            self.command.extend(['--exclude-tests', testManifest])


Then change config.py in buildbot-custom to send in the new android-failures.json.

Maybe I need to land android-failures.json on all branches (aurora, beta, release) ?
actually that sounds wrong as we will have different failures on each branch, we really need a solution for doing this on m-c
per releng/ateam/mobile mtg just now: 

1) Having more tests like this is cool. 

2) Before turning these on in production, we'll break the long-running suites (3,9,10?) in smaller chunks. The idea is to get each runnable chunk into ~25mins. This matters so we can keep job-timeout thresholds as low as possible, and to avoid gumming up our queuing/scheduling plumbing when we start run this at scale in production.
Depends on: 762248
Depends on: 664029
Depends on: 762217
https://hg.mozilla.org/mozilla-central/rev/cd8e40687a8d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 15 → Firefox 16
I should have mentioned that this has more than just a m-c patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 16 → Firefox 15
ok, this is sort of round 2.  Now android.json has the master list of runtests, excludedtests instead of just a list of tests.  setup.js will support both the old and new formats of the .json file.  

Also I pulled out some directories as they are eating up the majority of our runtime.  When we adjust to 10 chunks on buildbot, then we can look at turning on some of these slower directories.
Attachment #630305 - Attachment is obsolete: true
Attachment #632666 - Flags: review?(jgriffin)
Comment on attachment 632666 [details] [diff] [review]
updated android.json to support runonly and excluded tests (1.0)

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

Thanks, this looks great, but see the note about --run-only-tests below, which I think this patch breaks.

::: testing/mochitest/tests/SimpleTest/setup.js
@@ +217,5 @@
> +    runtests = filter.runtests;    
> +  if ('excludetests' in filter)
> +    excludetests = filter.excludetests;
> +  if (!('runtests' in filter) && !('excludetests' in filter)) {
> +    if (runOnly)

runOnly is no longer defined (since it was removed from the list of parameters for filterTests), so this will always evaluate to false.  It seems like we still need a runOnly url parameter, in cases where someone has specified --run-only-tests.

@@ +226,2 @@
>    
> +  //This will run if excludetests = {}

This comment is inaccurate, or else I don't understand it.  ;)  It seems like this block of code determines what tests will run after filtering out any tests that are defined in excludedtests.

@@ +240,5 @@
> +        break;
> +      }
> +    }
> +    if (!found)
> +        excludedTests.push(test_path);

I was totally confused by this code the first time I read it because of this variable name.  I think this would be more accurately called notExcludedTests, or testsNotExcluded, or something like that.
Attachment #632666 - Flags: review?(jgriffin) → review-
addressed review feedback.  added a runOnly param to the commandline and that will act as a flag when using the old json format.
Attachment #632666 - Attachment is obsolete: true
Attachment #632824 - Flags: review?(jgriffin)
Comment on attachment 632824 [details] [diff] [review]
updated android.json to support runonly and excluded tests (2.0)

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

Looks great, with one nit.  Thanks for writing this!

::: testing/mochitest/tests/SimpleTest/setup.js
@@ +193,4 @@
>  
>  // Open the file referenced by runOnly|exclude and use that to compare against
>  // gTestList.  Return a modified version of gTestList
> +function filterTests(filterFile, runOnly='false') {

Can't provide default argument values in JS like this.  We probably don't need a default value at all.
Attachment #632824 - Flags: review?(jgriffin) → review+
https://hg.mozilla.org/mozilla-central/rev/e7a1d56f7ccf
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Depends on: 765193, 765182
You need to log in before you can comment on or make changes to this bug.