Closed Bug 761125 Opened 12 years ago Closed 12 years ago

switch from android.json to android-failures.json


(Firefox for Android Graveyard :: General, defect)

Not set


(Not tracked)

Firefox 15


(Reporter: jmaher, Assigned: jmaher)


(Depends on 1 open bug)



(1 file, 4 obsolete files)

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:

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

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

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

Then change 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
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 15 → Firefox 16
I should have mentioned that this has more than just a m-c patch.
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+
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Depends on: 765193, 765182
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.