1055759 - Thunderbird xpcshell tests need extra arguments in mozharness

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Joshua Cranmer [:jcranmer]]

At one point in the mozharness process, mozinstall is called. Since we were using Firefox's desktop config for unittests, mozinstall defaults to processing an application called firefox which doesn't exist. (This doesn't affect OS X, since that instead looks up a bundle name).

Thunderbird somehow needs to get a -a thunderbird passed into mozinstall, which appears to use the application parameter in the config to get this working. I'm not entirely certain of the best way to get the changes into the tree, as forking the config just to add a simple theoretically easily-configurable change seems drastic to me.

Comment 1

[Joshua Cranmer [:jcranmer]]

Attachment: [mozharness] Add an extra TB config file for tests.

I have no idea who should review this patch, or even if this is sufficient changes for mozharness.

The basic solution is to use this to override the application name when installing. I'm sticking it where it is because:
a) I expect this override would be prospectively needed for future unit test suites should we run them.
b) I don't expect this file would be a useful override for running builds from mozharness.

Although, admittedly, both of those are very wild, prospective guesses.

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8475654 [details] [diff] [review]
[mozharness] Add an extra TB config file for tests.

This looks good to me, but I'd rather someone from releng review where this goes.

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 8475654 [details] [diff] [review]
[mozharness] Add an extra TB config file for tests.

302 jlund :)

Comment 4

[Jordan Lund (:jlund)]

Comment on attachment 8475654 [details] [diff] [review]
[mozharness] Add an extra TB config file for tests.

this should achieve what you want. another option would be to add cli item here:
http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/testing/testbase.py#25

    [["--application"],
     {"action": "store",
     "dest": "application",
     "default": None,
     "help": "bla bla",
      }],

I prefer the above if this 'application' is going to be the only item in thunderbird_extra.py. If you think there are going to be more differences for the thunderbird scenario, it might be better to have a separate config as you have.

Either way, we will need a buildbot patch to pass the additional arg: '--cfg unittests/thunderbird_extra.py' in this patch's case.

I wonder what other issues we will hit after this? I guess this is fine to have: http://mxr.mozilla.org/build/source/mozharness/configs/unittests/linux_unittest.py#6 since we overwrite self.binary_path in install() later on anyway. If we keep hitting new issues though, it might be better to get a loaned machine and test locally: https://wiki.mozilla.org/ReleaseEngineering/Mozharness/How_to_run_tests_as_a_developer

hopefully this process becomes easier once https://bugzil.la/1019962 is resolved.

Comment 5

[Joshua Cranmer [:jcranmer]]

(In reply to Jordan Lund (:jlund) from comment #4)
> Comment on attachment 8475654 [details] [diff] [review]
> [mozharness] Add an extra TB config file for tests.
> 
> this should achieve what you want. another option would be to add cli item
> here:
> http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/testing/
> testbase.py#25
> 
>     [["--application"],
>      {"action": "store",
>      "dest": "application",
>      "default": None,
>      "help": "bla bla",
>       }],
> 
> I prefer the above if this 'application' is going to be the only item in
> thunderbird_extra.py. If you think there are going to be more differences
> for the thunderbird scenario, it might be better to have a separate config
> as you have.

We're also considering moving Mozmill tests to mozharness, which would necessitate extra lines somewhere, and Lightning requires further modification that could potentially necessitate changes to the config as well.

Comment 6

[Justin Wood (:Callek)]

self-needinfo to remind me to do the buildbot patch

Comment 7

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8475654 [details] [diff] [review]
[mozharness] Add an extra TB config file for tests.

Checked in to mozharness:
https://hg.mozilla.org/build/mozharness/rev/a42db17de1c7

Comment 8

[Simone Bruno [:simone]]

In production with reconfig on 2014-08-25 08:49 PT

Comment 9

[Mark Banner (:standard8)]

Attachment: Add Thunderbird config file to arguments for Thunderbird tests

This adds the extra argument for the xpcshell tests. Hopefully this should get us working now.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8480160 [details] [diff] [review]
Add Thunderbird config file to arguments for Thunderbird tests

https://hg.mozilla.org/build/buildbot-configs/rev/ab58ef69eb30

Comment 11

[Jordan Lund (:jlund)]

patch(es) in production :)

Comment 12

[Joshua Cranmer [:jcranmer]]

Confirmed working.