689295 - mozrunner should allow user to specify processhandler

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Andrew Halberstadt [:ahal]]

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?

Comment 1

[Jeff Hammel]

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

Comment 2

[Andrew Halberstadt [:ahal]]

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.

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Allow mozrunner to receive process handler through ctor

Comment 4

[Jeff Hammel]

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

wfm

Comment 5

[Andrew Halberstadt [:ahal]]

master: https://github.com/mozautomation/mozmill/commit/ba025b9f41066a9e8b474f4315a58412d2fc54d8