Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Schedule Android 4.0 Debug M4,M5,M6,M7,J1,J2,J3 (only) on all trunk trees and make them ride the trains

RESOLVED FIXED

Status

Release Engineering
General Automation
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gbrown, Assigned: kmoir)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

Some Android 4.0 Debug test suites are now running reliably:

https://tbpl.mozilla.org/?tree=Cedar&jobname=Android.*Debug

We should run what we can on trunk trees to avoid introducing new problems while we sort out the remaining failures (bug 940068).

At this time I am proposing that we start running M4,M5,M6,M7,J1,J2,J3.
(Assignee)

Updated

3 years ago
Assignee: nobody → kmoir
I'm working on testing this on my staging master, but having problems getting it to work.  Just wanted to let you know the status.
Mmm, Cedar has https://hg.mozilla.org/projects/cedar/rev/4b8ce16653e1 https://hg.mozilla.org/projects/cedar/rev/f2bf90cf80af https://hg.mozilla.org/projects/cedar/rev/e7b181f39d92 on it, which appears to me to be extremely temporary patches just to see for a day or two what the assertions on Android might look like. Doesn't look to me like that's a tree which is capable of cleanly demonstrating that anything is actually ready to run elsewhere.
Flags: needinfo?(jgilbert)
Created attachment 8396651 [details] [diff] [review]
bug982799.patch

So this is the patch I have so far.  I don't like the way the tests are excluded on a per branch basis but this is the least messy way I found how to do it. Also, I wasn't sure what the right version of gecko would be to ride the trains, so I guessed, this may have to be modified.
Attachment #8396651 - Flags: feedback?(bugspam.Callek)
Comment on attachment 8396651 [details] [diff] [review]
bug982799.patch

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

wow thats _ugly_ not sure if there's a better way here though.... I'm open to other ideas, or to continue with this way if you can't come up with something.
Attachment #8396651 - Flags: feedback?(bugspam.Callek)
Yes it's not elegant but it is only way I could filter out the tests to run on those branches without redefining a list for android debug and that would make it more confusing.

I ran sendchanges against these for tests on m-i, m-c and m-a and they seem fine.  I'm always unsure of what the set of "all trunk trees" entails.  In this case where I filter for gecko 29, this includes m-i, m-c and m-a but also includes all the projects branches and b2g-inbound.  Callek is this correct?
Flags: needinfo?(bugspam.Callek)
(In reply to Kim Moir [:kmoir] from comment #5)
> Yes it's not elegant but it is only way I could filter out the tests to run
> on those branches without redefining a list for android debug and that would
> make it more confusing.

Yea, and is why I'm cringing openly, but not blocking.

> I ran sendchanges against these for tests on m-i, m-c and m-a and they seem
> fine.  I'm always unsure of what the set of "all trunk trees" entails.  In
> this case where I filter for gecko 29, this includes m-i, m-c and m-a but
> also includes all the projects branches and b2g-inbound.  Callek is this
> correct?

Indeed correct, it will also include "try", etc.
Flags: needinfo?(bugspam.Callek)
(In reply to Phil Ringnalda (:philor) from comment #2)
> Mmm, Cedar has https://hg.mozilla.org/projects/cedar/rev/4b8ce16653e1
> https://hg.mozilla.org/projects/cedar/rev/f2bf90cf80af
> https://hg.mozilla.org/projects/cedar/rev/e7b181f39d92 on it, which appears
> to me to be extremely temporary patches just to see for a day or two what
> the assertions on Android might look like. Doesn't look to me like that's a
> tree which is capable of cleanly demonstrating that anything is actually
> ready to run elsewhere.

I have backed out e7b181f39d92 and verified that nsDebugImpl.cpp is the same on Cedar as on mozilla-central now -- will check to see if this changes any of our results or logging for the worse.
(Assignee)

Updated

3 years ago
Attachment #8396651 - Flags: review?(bugspam.Callek)
(In reply to Geoff Brown [:gbrown] from comment #7)
> (In reply to Phil Ringnalda (:philor) from comment #2)
> I have backed out e7b181f39d92 and verified that nsDebugImpl.cpp is the same
> on Cedar as on mozilla-central now -- will check to see if this changes any
> of our results or logging for the worse.

Post-backout, I see no changes to test results, or logging -- I think it is all good still.

Regarding assertion logging in particular, consider https://tbpl.mozilla.org/php/getParsedLog.php?id=36749135&tree=Cedar&full=1 and its logcat (currently compressed due to a blobber bug) http://mozilla-releng-blobs.s3.amazonaws.com/blobs/cedar/sha512/f34d5b819ce6e2f92927b3495aa6a4c5e765d77c387c74bc4510ef876eac3df4d76d06c519edd157cb8019312ddac058c42a7ac0ca03ebcfa30ca1bad16a39d6. The assertion failure in test_badMimeType.html has logcat:

03-26 11:52:43.257 I/GeckoDump( 2232): 1697 INFO TEST-START | /tests/uriloader/exthandler/tests/mochitest/test_badMimeType.html
03-26 11:52:43.304 I/Gecko   ( 2232): ++DOMWINDOW == 61 (0x6ab826a0) [pid = 2232] [serial = 1520] [outer = 0x6c03a060]
03-26 11:52:43.406 I/Gecko   ( 2232): [2232] ###!!! ASSERTION: Not a UTF-8 string. This code should only be used for converting from known UTF-8 strings.: 'Error', file ../../../dist/include/nsUTF8Utils.h, line 421
03-26 11:52:43.406 I/Gecko   ( 2232): [2232] ###!!! ASSERTION: Not a UTF-8 string. This code should only be used for converting from known UTF-8 strings.: 'Error', file ../../../dist/include/nsUTF8Utils.h, line 68
03-26 11:52:43.406 I/Gecko   ( 2232): [2232] ###!!! ASSERTION: Input wasn't UTF8 or incorrect length was calculated: 'Error', file /builds/slave/ced-and-d-00000000000000000000/build/xpcom/string/src/nsReadableUtils.cpp, line 222
03-26 11:52:43.406 I/Gecko   ( 2232): AndroidBridge::GetHandlersForMimeType
03-26 11:52:43.414 I/Gecko   ( 2232): AndroidBridge::GetExtensionFromMimeType
03-26 11:52:43.414 I/Gecko   ( 2232): [2232] ###!!! ASSERTION: Not a UTF-8 string. This code should only be used for converting from known UTF-8 strings.: 'Error', file ../../../dist/include/nsUTF8Utils.h, line 421
03-26 11:52:43.414 I/Gecko   ( 2232): [2232] ###!!! ASSERTION: Not a UTF-8 string. This code should only be used for converting from known UTF-8 strings.: 'Error', file ../../../dist/include/nsUTF8Utils.h, line 68
03-26 11:52:43.414 I/Gecko   ( 2232): [2232] ###!!! ASSERTION: Input wasn't UTF8 or incorrect length was calculated: 'Error', file /builds/slave/ced-and-d-00000000000000000000/build/xpcom/string/src/nsReadableUtils.cpp, line 222
03-26 11:52:43.421 I/Gecko   ( 2232): AndroidBridge::GetMimeTypeFromExtensions

This looks essentially identical to logcat for previous builds.

:jgilbert -- Hope you don't mind the backout. Feel free to re-push your logging changes if they are still of interest.

:philor -- Thanks for pointing that out. Any other concerns?
Flags: needinfo?(philringnalda)
(In reply to Phil Ringnalda (:philor) from comment #2)
> Mmm, Cedar has https://hg.mozilla.org/projects/cedar/rev/4b8ce16653e1
> https://hg.mozilla.org/projects/cedar/rev/f2bf90cf80af
> https://hg.mozilla.org/projects/cedar/rev/e7b181f39d92 on it, which appears
> to me to be extremely temporary patches just to see for a day or two what
> the assertions on Android might look like. Doesn't look to me like that's a
> tree which is capable of cleanly demonstrating that anything is actually
> ready to run elsewhere.

Yep, they're supposed to be temporary. I tracked down the bug I needed for this. I thought they were removed already, but I can back them out.
Not to worry, if I thought of something else, I wouldn't be shy about mentioning it.
Flags: needinfo?(philringnalda)
Flags: needinfo?(jgilbert)
Comment on attachment 8396651 [details] [diff] [review]
bug982799.patch

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

I'd be interested in a "builder name" diff of before and after with this patch too, to have an extra check that this adds only what we want, where we want.

That said I still despise this code, but it "does what we want" (based on code inspection)

::: mozilla-tests/mobile_config.py
@@ +1399,5 @@
> +    for platform in branch['platforms']:
> +        if not platform in PLATFORMS:
> +            continue
> +        if not platform.startswith('android'):
> +            continue

looks like this would enable debug testing on android-armv6, android-x86 etc. we should hard == 'android' here

@@ +1435,5 @@
> +                for type in branch['platforms'][platform][slave_plat]:
> +                    print "type " + str(type)
> +                    if 'debug_unittest_suite' in type:
> +                        for suite in branch['platforms'][platform][slave_plat][type][:]:
> +                            if suite[0] in d:

actually I'd rather do the opposite, "if suite[0] not in d" and have d list the *good* suites. That way if we add a new suite above we don't accidentally enable everywhere

@@ +1439,5 @@
> +                            if suite[0] in d:
> +                               branch['platforms'][platform][slave_plat][type].remove(suite)
> +
> +#have to disable debug tests on ash manually or it fails checkconfig in misc.py
> +BRANCHES['ash']['platforms']['android']['enable_debug_unittests'] = False

I'm curious why this is, fwiw.
Attachment #8396651 - Flags: review?(bugspam.Callek) → review+
Created attachment 8398526 [details] [diff] [review]
bug982799.patch

Fixed the first two issues you mentioned.  I don't know why the issue on ash is happening.  I spent many hours debugging it and couldn't find the root cause.  I decided to move ahead and get them working on other branches since ash isn't a critical branch.  I'll attach the builder comparison which looks good.
Attachment #8396651 - Attachment is obsolete: true
Attachment #8398526 - Flags: review?(bugspam.Callek)
Created attachment 8398528 [details]
builder.diff

diff of builders
Comment on attachment 8398528 [details]
builder.diff

>211a226,232
>> Android 4.0 Panda mozilla-beta debug test mochitest-4 ScriptFactory
>> Android 4.0 Panda mozilla-beta debug test mochitest-5 ScriptFactory
>> Android 4.0 Panda mozilla-beta debug test mochitest-6 ScriptFactory
>> Android 4.0 Panda mozilla-beta debug test mochitest-7 ScriptFactory
>> Android 4.0 Panda mozilla-beta debug test jsreftest-1 ScriptFactory
>> Android 4.0 Panda mozilla-beta debug test jsreftest-2 ScriptFactory
>> Android 4.0 Panda mozilla-beta debug test jsreftest-3 ScriptFactory
>290a312,318
>> Android 4.0 Panda release-mozilla-beta debug test mochitest-4 ScriptFactory
>> Android 4.0 Panda release-mozilla-beta debug test mochitest-5 ScriptFactory
>> Android 4.0 Panda release-mozilla-beta debug test mochitest-6 ScriptFactory
>> Android 4.0 Panda release-mozilla-beta debug test mochitest-7 ScriptFactory
>> Android 4.0 Panda release-mozilla-beta debug test jsreftest-1 ScriptFactory
>> Android 4.0 Panda release-mozilla-beta debug test jsreftest-2 ScriptFactory
>> Android 4.0 Panda release-mozilla-beta debug test jsreftest-3 ScriptFactory
>440a469,475
>> Android 4.0 Panda mozilla-aurora debug test mochitest-4 ScriptFactory
>> Android 4.0 Panda mozilla-aurora debug test mochitest-5 ScriptFactory
>> Android 4.0 Panda mozilla-aurora debug test mochitest-6 ScriptFactory
>> Android 4.0 Panda mozilla-aurora debug test mochitest-7 ScriptFactory
>> Android 4.0 Panda mozilla-aurora debug test jsreftest-1 ScriptFactory
>> Android 4.0 Panda mozilla-aurora debug test jsreftest-2 ScriptFactory
>> Android 4.0 Panda mozilla-aurora debug test jsreftest-3 ScriptFactory

.... This doesn't look like riding the trains....
Attachment #8398528 - Flags: review-
Comment on attachment 8398526 [details] [diff] [review]
bug982799.patch

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

r- per builder diff
Attachment #8398526 - Flags: review?(bugspam.Callek) → review-
Created attachment 8398553 [details] [diff] [review]
bug982799.patch

fixed
Attachment #8398526 - Attachment is obsolete: true
Attachment #8398553 - Flags: review?(bugspam.Callek)
Created attachment 8398554 [details]
builder.diff

new builder.diff - note no m-b or m-r
(Assignee)

Updated

3 years ago
Attachment #8398528 - Attachment is obsolete: true
Created attachment 8398566 [details] [diff] [review]
bug982799.patch
Attachment #8398553 - Attachment is obsolete: true
Attachment #8398553 - Flags: review?(bugspam.Callek)
Attachment #8398566 - Flags: review?(bugspam.Callek)
Created attachment 8398567 [details]
builder.diff

no m-a, m-b, m-r since we check for gecko 31
(Assignee)

Updated

3 years ago
Attachment #8398554 - Attachment is obsolete: true

Updated

3 years ago
Attachment #8398566 - Flags: review?(bugspam.Callek) → review+
(Assignee)

Updated

3 years ago
Attachment #8398566 - Flags: checked-in+
in production.
verifed on tbpl
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.