979550 - Investigate switching Thunderbird comm-central xpcshell-tests to mozharness

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Joshua Cranmer [:jcranmer]]

This may or may not require ccrework to be completed first.

Comment 1

[Joshua Cranmer [:jcranmer]]

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]).

Comment 3

[Justin Wood (:Callek)]

Attachment: [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.

Comment 4

[Joshua Cranmer [:jcranmer]]

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.

Comment 5

[Mark Banner (:standard8)]

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.

Comment 6

[Justin Wood (:Callek)]

Attachment: [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.

Comment 7

[Justin Wood (:Callek)]

Attachment: builder-list-diff

Comment 8

[Jordan Lund (:jlund)]

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?

Comment 9

[Mark Banner (:standard8)]

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.

Comment 10

[Justin Wood (:Callek)]

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

Comment 11

[Simone Bruno [:simone]]

In production with reconfig on 2014-08-19 10:47 PT

Comment 12

[Joshua Cranmer [:jcranmer]]

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.

Comment 13

[Justin Wood (:Callek)]

Now running xpcshell jobs in production, followups to make them green will be in new bug(s)