Closed Bug 747200 Opened 12 years ago Closed 12 years ago

change --b2gbin to be --executable or --binary so that we can pass in the Firefox executable

Categories

(Remote Protocol :: Marionette, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: automatedtester, Assigned: automatedtester)

References

Details

Attachments

(2 files, 1 obsolete file)

the current option (--b2gbin) isn't obvious that it can be used with desktop so would be great to have that simplified
I did very little to get this working. The only tests that were failing were find element tests in Chrome but Malini is working on that already
Attachment #625099 - Flags: review?(mdas)
Comment on attachment 625099 [details] [diff] [review]
Adds binary and profile and launches and waits for everything

I'll take this review since mdas is on PTO for a while
Attachment #625099 - Flags: review?(mdas) → review?(jgriffin)
Comment on attachment 625099 [details] [diff] [review]
Adds binary and profile and launches and waits for everything

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

Looks good; I've noticed a few nits below.  Has anyone tested this on a B2G desktop build?  I don't expect it should be any different, but I'd like to verify it, so if no one has, I'll make a build to check it out.

::: testing/marionette/client/marionette/geckoinstance.py
@@ +2,5 @@
> +import os
> +import socket
> +import subprocess
> +import time
> +''' from mozprofile import Profile'''

Remove this line.

@@ +34,5 @@
> +        args = [self.bin, '-profile', self.profile, '-no-remote']
> +        self.proc = subprocess.Popen(args,
> +                                     stdout=subprocess.PIPE,
> +                                     stderr=subprocess.STDOUT)
> +        """

Let's remove this commented code before committing.

::: testing/marionette/client/marionette/marionette.py
@@ +118,5 @@
>          self.noWindow = noWindow
>          self.logcat_dir = logcat_dir
>  
> +        if bin:
> +            self.instance = GeckoInstance(host=self.host, port=self.port, bin=self.bin, profile=self.profile)

Let's wrap the arguments here to be consistent with other calls.

::: testing/marionette/client/marionette/runtests.py
@@ +151,4 @@
>          self.baseurl = None
>          self.marionette = None
>          self.logcat_dir = logcat_dir
> +        

This (empty) line has an accidental insertion of several spaces.

@@ +162,4 @@
>          if self.logcat_dir:
>              if not os.access(self.logcat_dir, os.F_OK):
>                  os.mkdir(self.logcat_dir)
> +    

This (empty) line has an accidental insertion of several spaces.

@@ +189,5 @@
> +                host, port = self.address.split(':')
> +            else:
> +                host = 'localhost'
> +                port = 2828
> +            self.marionette = Marionette(host=host, port=int(port), bin=self.bin, profile=self.profile, baseurl=self.baseurl)

Let's wrap the arguments here to be consistent with the other calls.

@@ +270,2 @@
>              self.start_httpd()
> +            print "done httpd"

Accidental debug prints?
Attachment #625099 - Flags: review?(jgriffin) → review+
Attached patch updated patchSplinter Review
Attachment #625099 - Attachment is obsolete: true
Attachment #626383 - Flags: review?(jgriffin)
Updated the patch after nits. I havent tried this on a desktop B2G yet so haven't landed in case there was anything that needs changing
Comment on attachment 626383 [details] [diff] [review]
updated patch

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

Looks good.  This change will require changes in the Gaia Makefile here:

https://github.com/andreasgal/gaia/blob/master/Makefile#L305

There's no way to synchronize these changes unfortunately; but go ahead and land it, and I'll update the Gaia repo when you do.

::: testing/marionette/client/marionette/runtests.py
@@ +194,5 @@
> +            else:
> +                host = 'localhost'
> +                port = 2828
> +            self.marionette = Marionette(host=host, port=int(port),\
> +                    bin=self.bin, profile=self.profile, baseurl=self.baseurl)

should be formatted something like:

self.marionette = Marionette(host=host, port=int(port),
		             bin=self.bin, profile=self.profile, 
                             baseurl=self.baseurl)

See http://www.python.org/dev/peps/pep-0008/#indentation
Attachment #626383 - Flags: review?(jgriffin) → review+
Assignee: nobody → dburns
geckoinstance.py needs an MPL header.
Attachment #626757 - Flags: review?(ctalbert)
Attachment #626757 - Flags: review?(ctalbert) → review+
Landed in http://hg.mozilla.org/mozilla-central/rev/749d641d5ac7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: