Investigate switching Thunderbird comm-central xpcshell-tests to mozharness

RESOLVED FIXED

Status

Release Engineering
Mozharness
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jcranmer, Assigned: Callek)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
This may or may not require ccrework to be completed first.
(Reporter)

Comment 1

3 years ago
Hmmm...

Would it be possible to test copying the xpcshell config at <http://hg.mozilla.org/build/buildbot-configs/annotate/ac7f8b0a69fd/mozilla-tests/config.py#l518> to <http://mxr.mozilla.org/build/source/buildbot-configs/mozilla-tests/thunderbird_config.py#131> and friends?

(Although I fear this may conflict with the efforts of bug 680203, particularly attachment 8390175 [details] [diff] [review]).
(Reporter)

Updated

3 years ago
Assignee: nobody → bugspam.Callek

Updated

3 years ago
Component: General Automation → Mozharness
Duplicate of this bug: 1052381
Depends on: 1052389
Summary: Investigate switching comm-central xpcshell-tests to mozharness → Investigate switching Thunderbird comm-central xpcshell-tests to mozharness
(Assignee)

Comment 3

3 years ago
Created attachment 8472024 [details] [diff] [review]
[configs] v0 - untested

This patch is completely and utterly untested, and is was not even parsed for syntax errors.

*however* I am relatively confident it does what I think it does, and that is adds xpcshell mozharness to all "trunk" trees and leaves non-mozharness on others.

The specifics of passing *any* different script params beyond what Moco passes is *not* accomplished with this patch however, so if anything needs passing thats not automatically picked up by the build properties I'd like to know and I can add it to this before I ready a real deploy.
Attachment #8472024 - Flags: feedback?(standard8)
Attachment #8472024 - Flags: feedback?(Pidgeot18)
(Reporter)

Comment 4

3 years ago
Comment on attachment 8472024 [details] [diff] [review]
[configs] v0 - untested

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

I think we may eventually need extra arguments for handling the extensions business for calendar tests, but this should suffice for the time being.

Although, really, it's Standard8 who knows this far better than I.
Attachment #8472024 - Flags: feedback?(Pidgeot18) → feedback+
Comment on attachment 8472024 [details] [diff] [review]
[configs] v0 - untested

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

Yes, as Joshua mentioned we may need extra arguments for calendar extensions, but for now, this should get us something running again.
Attachment #8472024 - Flags: feedback?(standard8) → feedback+
(Assignee)

Comment 6

3 years ago
Created attachment 8473770 [details] [diff] [review]
[configs] v1

This passes checkconfig, and the builder-list changes look accurate!

This has a chance of failing but the TB team is itching to see this deployed and wishes to fix up in production, since their xpcshell tests are already failing without this.
Attachment #8472024 - Attachment is obsolete: true
Attachment #8473770 - Flags: review?(jlund)
(Assignee)

Comment 7

3 years ago
Created attachment 8473771 [details] [diff] [review]
builder-list-diff

Comment 8

3 years ago
Comment on attachment 8473770 [details] [diff] [review]
[configs] v1

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

I *think* this will accomplish what you want. :)

r+ if you remove the talos_config.py references and put the append(XPCSHELL_OLD) outside of loop (both explained in line).

However, I should point out that I have no idea how thunderbird works. I would imagine that desktop_unittests.py will need some thunderbird love. As you have it, this will basically run the exact same mozharness script and args as the Firefox equivalent. The only difference will be the buildprops.json for things like the installer binary url and tests url.

Has this script been locally tested on a machine outside of automation or else with dev-master? Has thunderbird used this script before for other unittests? I feel like we shouldn't land this on trunk without at least one test run. Ignore me if you have already looked into that.

::: mozilla-tests/thunderbird_config.py
@@ -54,4 @@
>  PLATFORMS['macosx64']['snowleopard'] = {'name': builder_prefix + "Rev4 MacOSX Snow Leopard 10.6"}
>  PLATFORMS['macosx64']['mountainlion'] = {'name': builder_prefix + "Rev5 MacOSX Mountain Lion 10.8"}
>  PLATFORMS['macosx64']['stage_product'] = 'thunderbird'
> -PLATFORMS['macosx64']['mozharness_python'] = '/tools/buildbot/bin/python'

I think taking this out of pf is OK. AFAIK, misc.py will check both pf['mozharness_python'] and pf['mozharness_config']['mozharness_python']

@@ +56,5 @@
> +    'mozharness_python': '/tools/buildbot/bin/python',
> +    'hg_bin': 'hg',
> +    'reboot_command': ['/tools/buildbot/bin/python'] + MOZHARNESS_REBOOT_CMD,
> +    'system_bits': '64',
> +    'config_file': 'talos/mac_config.py',

this 'config_file' is a reference to our talos scripts. it isn't needed. I know, I just checked how we do things in mozilla-tests/config.py and it confused me too how we have unittests and talos intermingled.

@@ +369,5 @@
>      if 'linux64' in BRANCHES[branch]['platforms']:
>          BRANCHES[branch]['platforms']['linux64']['slave_platforms'] = ['ubuntu64_vm']
> +# xpcshell-on-mozharness should ride the trains
> +# Replace old trains with non-mozharness code.
> +# MERGE DAY (remove this code once Thunderbird no longer services Gecko 33 and lower)

I think this will work. It looks like we have a 'use_mozharness' and I think we normally pivot off that: http://mxr.mozilla.org/build/source/buildbotcustom/misc.py#2813

but this should do the job too

@@ +380,5 @@
> +            if slave_platform not in branch['platforms'][platform]:
> +                continue
> +
> +            for suite_type in ['opt_unittest_suites', 'debug_unittest_suites']:
> +                for xpcshell in XPCSHELL:

I'm not sure that XPCSHELL will ever get more than one job in the list, at least not for these trains, but you never know so this makes sense.

@@ +383,5 @@
> +            for suite_type in ['opt_unittest_suites', 'debug_unittest_suites']:
> +                for xpcshell in XPCSHELL:
> +                    try:
> +                        branch['platforms'][platform][slave_platform][suite_type].remove(xpcshell)
> +                        branch['platforms'][platform][slave_platform][suite_type].append(XPCSHELL_OLD)

should this append() be outside the `for xpcshell in XPCSHELL:` loop? this would add duplicate jobs if len(XPCSHELL) > 1 right?
Attachment #8473770 - Flags: review?(jlund) → review+
I have run the mozharness tests locally against Thunderbird. I did a mixture of following the a Firefox xpcshell run and adjusting parameters in the buildbot file to match a Thunderbird run. That's how I found the jsshell package issue that was blocking this bug (now fixed).

I think the main reason that we don't need to adjust mozharness files for xpcshell is that it doesn't rely on the executable name. Just the xpcshell binary - named the same for everyone.

The tests I ran locally completed fine with expected test failures. So I think we are good to land this. It should be better than what we have currently which doesn't run the tests at all.
(Assignee)

Comment 10

3 years ago
Comment on attachment 8473770 [details] [diff] [review]
[configs] v1

checked in with nits addressed (addtl r+ in irc for interdiff):

https://hg.mozilla.org/build/buildbot-configs/rev/6fd06296ba25
Attachment #8473770 - Flags: checked-in+
In production with reconfig on 2014-08-19 10:47 PT
(Reporter)

Comment 12

3 years ago
Okay, we have working tests on OS X. Linux fails due to the inability to find the binary. I don't know about Windows yet (not started), but I suspect it fails for the same reason.

It looks like mozinstall uses the bundle path on OS X (which is why that works), but it relies on an app name that defaults to firefox. Passing in a different -a to mozinstall ought to make it work, I don't know where the config to set that is, though.
(Reporter)

Updated

3 years ago
Depends on: 1055759
(Assignee)

Comment 13

3 years ago
Now running xpcshell jobs in production, followups to make them green will be in new bug(s)
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.