The default bug view has changed. See this FAQ.

Thunderbird xpcshell tests need extra arguments in mozharness

RESOLVED FIXED

Status

Release Engineering
Mozharness
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jcranmer, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

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

Comment 1

3 years ago
Created attachment 8475654 [details] [diff] [review]
[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.
Attachment #8475654 - Flags: review?(ahalberstadt)
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.
Attachment #8475654 - Flags: review?(ahalberstadt) → review?(rail)
Comment on attachment 8475654 [details] [diff] [review]
[mozharness] Add an extra TB config file for tests.

302 jlund :)
Attachment #8475654 - Flags: review?(rail) → review?(jlund)

Comment 4

3 years ago
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.
Attachment #8475654 - Flags: review?(jlund) → review+
(Reporter)

Comment 5

3 years ago
(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.
self-needinfo to remind me to do the buildbot patch
Flags: needinfo?(bugspam.Callek)
(Reporter)

Comment 7

3 years ago
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
Attachment #8475654 - Flags: checked-in+
In production with reconfig on 2014-08-25 08:49 PT
Created attachment 8480160 [details] [diff] [review]
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.
Attachment #8480160 - Flags: review?(bugspam.Callek)
Flags: needinfo?(bugspam.Callek)
Attachment #8480160 - Attachment is patch: true

Updated

3 years ago
Attachment #8480160 - Flags: review?(bugspam.Callek) → review+
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
Attachment #8480160 - Flags: checked-in+
patch(es) in production :)
(Reporter)

Comment 12

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