mozrunner should allow user to specify processhandler

RESOLVED FIXED

Status

Testing
Mozbase
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozbase])

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
At the moment mozrunner forces the user to use mozprocess.ProcessHandler, which goes against the whole mozbase inheritance philosophy. There are two ways to fix this:

1) Make mozrunner inherit from mozprocess
2) Add an @class method or ctor argument to pass in a ProcessHandler

In my mind, 1) makes the most sense since mozrunner is a specific process runner designed to run firefox processes. The downside here is that we'd have to change the start/stop methods into run/kill which would break backwards compatibility (do we care?).

Solution 2) has the advantage of being really simple and straight forward while keeping backwards compatibility. One could argue that it is also the proper way to do it.

Any comments or preferences?
(Assignee)

Updated

7 years ago
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Whiteboard: [mozbase]

Comment 1

7 years ago
I would prefer 2, though its hard for me to see how well inheritence would fit without seeing a mockup.  In addition, you could set

class MozRunner:
   processhandler = MozProcess

which is basically the same thing.

I like 1. less since mozrunner is really mozprocess + mozprofile and a front end.  I think of mozrunner as an interface pattern:

http://www.mindspring.com/~mgrand/pattern_synopses.htm#Interface

In addition, to give some perspective which may not directly affect this bug, I envision all of the abstractions over Firefox and Thunderbird, etc, going into FirefoxRunner, ThunderbirdRunner, etc, rather than having them split between that and MozProfile
(Assignee)

Comment 2

7 years ago
The only thing mozrunner does related to profiles is pass in the -P arg to mozprocess and some cleanup related stuff. To me mozrunner has always been a more specific version of mozprocess designed to run Firefox, which happens to require manipulation of profiles.

That being said.. I can see your point of view. I also realize that 1) doesn't really offer many benefits other than "it feels like the right thing to do" plus it would break backwards compatibility and force applications to update their code.

So I'll just go with 2) for now.
(Assignee)

Comment 3

7 years ago
Created attachment 562773 [details] [diff] [review]
Allow mozrunner to receive process handler through ctor
Attachment #562773 - Flags: review?(jhammel)

Comment 4

7 years ago
Comment on attachment 562773 [details] [diff] [review]
Allow mozrunner to receive process handler through ctor

wfm
Attachment #562773 - Flags: review?(jhammel) → review+
(Assignee)

Comment 5

7 years ago
master: https://github.com/mozautomation/mozmill/commit/ba025b9f41066a9e8b474f4315a58412d2fc54d8
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.