Closed Bug 1252586 Opened 8 years ago Closed 8 years ago

|mach firefox-ui-test| ignores --binary option

Categories

(Testing :: Firefox UI Tests, defect)

47 Branch
defect
Not set
normal

Tracking

(firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: sambuddhabasu1, Assigned: sydpolk)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160210153822

Steps to reproduce:

Ran ./mach firefox-ui-test --binary /Applications/Firefox.app/Contents/MacOS/firefox


Actual results:

Mach fails with an error,
Exception: Binary expected at path_to_obj-x86_64-apple-darwin15.3.0/dist/Nightly.app/Contents/MacOS/firefox does not exist.


Expected results:

The tests should run with the binary specified with the --binary option.
Confirmed. Working on it.
Blocks: 1252608
Blocks: 1252609
Has this been tested with no local objdir available?
Flags: needinfo?(spolk)
Indeed, David is correct. This is not feasible:

Syd-Polks-Mozilla-MBP:mozilla-central spolk$ rm -rf obj*
Syd-Polks-Mozilla-MBP:mozilla-central spolk$ ./mach firefox-ui-test --binary /Applications/Firefox.app/Contents/MacOS/firefox
It looks like you tried to run a mach command from an invalid context. The firefox-ui-test
command failed to meet the following conditions: 

  is_firefox - Must have a Firefox build.

Run |mach help| to show a list of all commands available to the current context.

Syd-Polks-Mozilla-MBP:mozilla-central spolk$ 

If you are running out of tree, you should use the harness that is in the tree.
Flags: needinfo?(spolk)
Comment on attachment 8725402 [details]
MozReview Request: Bug 1252586 - Handle |mach firefox-ui-test --binary| correctly - r=whimboo, r=gps

https://reviewboard.mozilla.org/r/37461/#review34043

::: testing/firefox-ui/mach_commands.py:63
(Diff revision 1)
> +        if kwargs['binary'] is None:
> -        kwargs['binary'] = self.get_binary_path('app')
> +            kwargs['binary'] = self.get_binary_path('app')

If it works and get_binary_path isn't expensive, you may want to write this as:

kwargs['binary'] = kwargs.get('binary', self.get_binary_path('app'))

or if the "binary" key is always present:

kwargs['binary'] = kwargs['binary'] or self.get_binary_path('app')

Also, it is pretty customary to skip the expicit type checking in Python. 95% of the time you'll see the conditional written as "if not kwargs['binary']"
Attachment #8725402 - Flags: review?(gps) → review+
Assignee: nobody → spolk
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Version: 44 Branch → 47 Branch
Comment on attachment 8725402 [details]
MozReview Request: Bug 1252586 - Handle |mach firefox-ui-test --binary| correctly - r=whimboo, r=gps

https://reviewboard.mozilla.org/r/37461/#review34095

Looks fine to me with the changes as proposed by gps having implemented before landing.
Attachment #8725402 - Flags: review?(hskupin) → review+
Are you happy with having this when the person will have had to build Firefox? If you are then great but this needs to be a conscious decision.
Flags: needinfo?(hskupin)
If commands under mach offer a binary option I think we should respect this. By default we still use the self-built Firefox under obj/dist/bin/firefox on Linux and similar on other platforms. So this is IMHO a very helpful option to not force the users to build Firefox. Also given that gps is happy with this I assume this behavior is in compliance with mach. Or where is it written that those commands can only run the self-built binary?
Flags: needinfo?(hskupin)
Comment on attachment 8725402 [details]
MozReview Request: Bug 1252586 - Handle |mach firefox-ui-test --binary| correctly - r=whimboo, r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37461/diff/1-2/
Attachment #8725402 - Attachment description: MozReview Request: Bug 1252586 - Handle |mach firefox-ui-test --binary| correctly - r?whimboo, r?gps → MozReview Request: Bug 1252586 - Handle |mach firefox-ui-test --binary| correctly - r=whimboo, r=gps
https://hg.mozilla.org/integration/mozilla-inbound/rev/09bb9469a14d4587e44027e648438a5f23526cd7
Bug 1252586 - Handle |mach firefox-ui-test --binary| correctly - r=whimboo, r=gps
https://hg.mozilla.org/mozilla-central/rev/09bb9469a14d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: