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)

(Reporter)

Description

3 years ago
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
(Assignee)

Comment 1

3 years ago
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)
(Assignee)

Comment 3

3 years ago
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)
(Assignee)

Comment 5

3 years ago
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)
(Reporter)

Comment 7

3 years ago
(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)
(Reporter)

Comment 8

3 years ago
(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+
(Assignee)

Comment 12

3 years ago
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)
(Assignee)

Comment 13

3 years ago
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-
(Assignee)

Comment 16

3 years ago
Created attachment 8398553 [details] [diff] [review]
bug982799.patch

fixed
Attachment #8398526 - Attachment is obsolete: true
Attachment #8398553 - Flags: review?(bugspam.Callek)
(Assignee)

Comment 17

3 years ago
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
(Assignee)

Comment 18

3 years ago
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)
(Assignee)

Comment 19

3 years ago
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.
(Assignee)

Comment 21

3 years ago
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.