583142 - mozrunner does not accept full path to binary on Mac OS X

Status: VERIFIED FIXED

(Testing :: Mozbase, defect)

Description

[Jeff Hammel]

On OSX, if I run

mozmill -b /Volumes/Minefield/Minefield.app/Contents/MacOS/firefox-bin

I get an error:

/Volumes/Minefield/Minefield.app/Contents/MacOS/firefox-bin/Contents/MacOS/firefox-bin does not exist

it appends the Contents/MacOS/firefox-bin, trying to be clever.  However, in this case it should be too clever.  If the user specifies the full path to the binary, it shouldn't be that clever.

Comment 1

[Heather Arthur [:harth]]

I forgot about this issue, we should really fix this one. If not for 1.4.2 then for a separate mozrunner release right after. Too many people run into this.

Comment 2

[Heather Arthur [:harth]]

Attachment: don't add 'Contents/MacOS/x-bin' to binary if they already specify it

quick patch to fix this problem.

Comment 3

[Heather Arthur [:harth]]

tested on Mac. http://github.com/harthur/mozmill/commit/3d9e5732d361abac846514a5a67ed40947b66f8a

Comment 4

[cmtalbert]

As much as I hate this behavior and want to see this bug fixed, I can't take it for 1.4.2.  I think that ship has sailed.  If we do a dot release after 1.4.2 we'll take this as a ride along. (I envision us doing that for the expected event stuff).

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Such a change will also break our automation scripts, which will have to be updated. So the impact is a bit broader and I'm not sure how much it will affect other consumers of Mozrunner/Mozmill out there. Sounds good for the next dot release.

Comment 6

[Heather Arthur [:harth]]

Okay.

(In reply to comment #5)
> Such a change will also break our automation scripts, which will have to be
> updated. So the impact is a bit broader and I'm not sure how much it will
> affect other consumers of Mozrunner/Mozmill out there. Sounds good for the next
> dot release.

I'm interested in how this breaks current scripts? just specifiying /Firefox.app still works, but /Firefox.app/Contents/MacOS/firefox-bin also works now.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #6)
> I'm interested in how this breaks current scripts? just specifiying
> /Firefox.app still works, but /Firefox.app/Contents/MacOS/firefox-bin also
> works now.

Ah ok. So our scripts aren't affected then. Haven't had such a closer look to your patch.

Comment 8

[cmtalbert]

Comment on attachment 467323 [details] [diff] [review]
don't add 'Contents/MacOS/x-bin' to binary if they already specify it

Hooray for this patch.  r=ctalbert

Comment 9

[Jeff Hammel]

It looks like this needs to be landed on master too:

http://github.com/mozautomation/mozmill/blob/master/mozrunner/mozrunner/runner.py

Comment 10

[Heather Arthur [:harth]]

master:
http://github.com/mozautomation/mozmill/commit/40e24ea3a192ac0d21f93697ede926c0a5578ab9

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

It had been landed on hotfix-1.5.1 as:
http://github.com/mozautomation/mozmill/commit/f3f6829552dead9d7935c841e82b5141a00963e9

Checked and I can verify the expected behavior. Thanks!