Closed
Bug 1080338
Opened 10 years ago
Closed 7 years ago
preflight_cppunittest in desktop_unittest.py needs to be made aware of the new GreD on OSX
Categories
(Release Engineering :: Applications: MozharnessCore, defect)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: spohl, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jlund
:
feedback+
|
Details | Diff | Splinter Review |
The preflight_cppunittest in desktop_unittest.py is currently copying TestStartupCacheTelemetry.js and TestStartupCacheTelemetry.manifest to Contents/MacOS in the app dir. With the v2 signing changes however, the expected location is Contents/Resources. We need to adjust the directory accordingly before moving the files.
This blocks bug 1077282 from landing.
Reporter | ||
Comment 1•10 years ago
|
||
Something like this would do, but I'm not sure if the import of mozinfo works as expected. Ben, is there a way to test this? Or is there another (preferred) way to check whether we're running on OSX? Thanks!
Attachment #8502246 -
Flags: feedback?(bhearsum)
Reporter | ||
Updated•10 years ago
|
Summary: abs_app_dir in preflight_cppunittest (desktop_unittest.py) needs to be made aware of the new GreD on OSX → preflight_cppunittest in desktop_unittest.py needs to be made aware of the new GreD on OSX
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 4•10 years ago
|
||
Comment on attachment 8502246 [details] [diff] [review]
Patch (wip)
I don't think I'm a great reviewer for this one...Jordan, maybe you are (or know who is?)
Attachment #8502246 -
Flags: feedback?(bhearsum) → feedback?(jlund)
Comment 5•10 years ago
|
||
(In reply to Ben Hearsum [:bhearsum] from comment #4)
> Comment on attachment 8502246 [details] [diff] [review]
> Patch (wip)
>
> I don't think I'm a great reviewer for this one...Jordan, maybe you are (or
> know who is?)
oh cool, this *should* work as we can reach mozinfo from elsewhere in mozharness. I won't block if you feel strongly but I tend to lean on the side of making things configurable. Maybe we can add an item to the config used for osx cppunittests. This way, if we need to modify this for osx or any other platform in the future, it's just a change to a config not the script. Also, I suppose we should change the name of this var since it won't be the actual abs_app_dir if it's osx now.
i.e. this is the call we do for cppunitests osx: /tools/buildbot/bin/python scripts/scripts/desktop_unittest.py --cfg unittests/mac_unittest.py --cppunittest-suite cppunittest --blob-upload-branch mozilla-central --download-symbols ondemand
so we could add something like the following to unittests/mac_unittest.py:
'cppunittests_telemetry_dest': 's(app_dir_dirname)%/Resources' # or whatever key/name you want
then in the script:
telemetry_dest = self.query_abs_app_dir()
if self.config.get('cppunitests_telemetry_dest'):
telemetry_dest = self.config['cppunitests_telemetry_dest'] % {
'app_dir_dirname': os.path.dirname(abs_app_dir)
}
I am going to ping dminor here as he as recently worked on cppunittests, specifically within the same method as this patch.
dminor, do you see any issue changing the logic in preflight_cppunittest() regardless how we do it?
spohl, in the meantime you can test this patch or my suggestion anytime without review by landing on a ash-mozharness (somewhat of a mozharness try repo). Essentially, you will land this patch there, and then you can re-trigger individual jobs on the 'ash' branch (they will pick up the latest ash-mozharness rev everytime), or push to ash with a merge of m-c -> ash and that will trigger every job across every platform. backout the patch after if it gets an r- or breaks ash.
For more instructions, see: https://wiki.mozilla.org/ReleaseEngineering/SpecialBranches#Ash
does this help?
Flags: needinfo?(dminor)
Updated•10 years ago
|
Attachment #8502246 -
Flags: feedback?(jlund) → feedback+
Reporter | ||
Comment 6•10 years ago
|
||
I'm going to be out all of next week, so if someone else could drive this from here, I'd appreciate it.
Bug 1077282 landed with a workaround for this bug (see https://hg.mozilla.org/integration/mozilla-inbound/rev/d3a3560432b0). While testing a patch for mozharness here, we should revert the workaround. It can be reverted all together in the tree after a patch for this bug has landed.
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•