761125 - switch from android.json to android-failures.json

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: initial stab of android-failures.json

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

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: list of test cases which fail on android native (1.0)

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.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

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.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: list of test cases which fail on android native (1.1)

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.

Comment 4

[Jonathan Griffin (: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.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd8e40687a8d

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

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) ?

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

actually that sounds wrong as we will have different failures on each branch, we really need a solution for doing this on m-c

Comment 8

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

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.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/cd8e40687a8d

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

I should have mentioned that this has more than just a m-c patch.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: updated android.json to support runonly and excluded tests (1.0)

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.

Comment 12

[Jonathan Griffin (: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.

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: updated android.json to support runonly and excluded tests (2.0)

addressed review feedback.  added a runOnly param to the commandline and that will act as a flag when using the old json format.

Comment 14

[Jonathan Griffin (: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.

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

on inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7a1d56f7ccf

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e7a1d56f7ccf