1190371 - [mozrunner] should we make mozcrash an optional dependency of mozrunner ?

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Julien Pagès (:parkouss)]

mozrunner currently requires mozcrash, but in some cases we don't want/need it, e.g. in mozregression.

See bug 1185632 comment 3.

Should we make that dependency optional ?

Comment 1

[Julien Pagès (:parkouss)]

:ted, thoughts ? I *feel* that you are the right person to ask this for, but feel free to redirect the NI if it is not the case.

Comment 2

[(not currently active) Ted Mielczarek]

It seems like mozcrash is generally useful when running an app. The problems in that bug sound like we could work around them.

Comment 3

[Julien Pagès (:parkouss)]

Fixing this would resolve bug 1185602, (see bug 1185602 comment 7). I am not sure if we should fix mozcrash instead, but this is a workaround that would allow to not require mozcrash when installing mozrunner, and allowing to have a try/except around mozcrash import would fix the issue.

Comment 4

[Julien Pagès (:parkouss)]

Attachment: bug-1190371.patch

So, asking for review - we'll see what you think of it Ted :)

Basically this makes mozcrash an optional deps for mozrunner. So:

pip install mozrunner

would not install mozcrash, but:

pip install mozrunner[crash]

will.

For in-tree code, I don't think it will change anything, we always install every mozbase dependencies I think. But this may impact out-of-the-tree code that rely on mozcrash when they install mozrunner (I don't think there is any, but not sure) - in that case they should upgrade their dependency to "mozrunner[crash]".

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8664217 [details] [diff] [review]
bug-1190371.patch

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

I don't feel very strongly about this, so this is fine if it helps that use case. Sorry for the long review delay!

Comment 6

[Julien Pagès (:parkouss)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8df8c0ca820f

Comment 7

[Julien Pagès (:parkouss)]

Thanks Ted! Yep, that will help for mozregression - we will be able to make it work for MSYS2 on Windows 8.1 x64 (even if this version is not supported by test harnesses), and it will allow to remove the mozcrash dependency by the way.

Pushed to try, just to be sure.

Comment 8

[Julien Pagès (:parkouss)]

I can't push myself right now, the tree is closed.

Try is green - thanks for checkin this in!

Leave open so I can release a new mozrunner on pypi before closing the bug.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2bd9c7a075d

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b2bd9c7a075d

Comment 11

[Julien Pagès (:parkouss)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/03d53c79f7f5158587a21697ce5dfca9c3622800
Bug 1190371 - [mozrunner] bump release to 6.11. r=me

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/03d53c79f7f5

Comment 13

[Julien Pagès (:parkouss)]

Released on pypi:

Submitting dist/mozrunner-6.11.tar.gz to https://pypi.python.org/pypi
Server response (200): OK

Comment 14

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.