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)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: automatedtester, Assigned: automatedtester)
References
Details
Attachments
(2 files, 1 obsolete file)
10.45 KB,
patch
|
jgriffin
:
review+
|
Details | Diff | Splinter Review |
717 bytes,
patch
|
cmtalbert
:
review+
|
Details | Diff | Splinter Review |
the current option (--b2gbin) isn't obvious that it can be used with desktop so would be great to have that simplified
Assignee | ||
Comment 1•12 years ago
|
||
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 2•12 years ago
|
||
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 3•12 years ago
|
||
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+
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #625099 -
Attachment is obsolete: true
Attachment #626383 -
Flags: review?(jgriffin)
Assignee | ||
Comment 5•12 years ago
|
||
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 6•12 years ago
|
||
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 | ||
Updated•12 years ago
|
Assignee: nobody → dburns
Assignee | ||
Comment 7•12 years ago
|
||
Landed on HG http://hg.mozilla.org/mozilla-central/rev/f80c35afea4a
Comment 8•12 years ago
|
||
geckoinstance.py needs an MPL header.
Assignee | ||
Comment 9•12 years ago
|
||
https://github.com/mozilla/marionette_client/commit/5787fbcf2a11165637b237376117af7c253caf34
Assignee | ||
Comment 10•12 years ago
|
||
Attachment #626757 -
Flags: review?(ctalbert)
Attachment #626757 -
Flags: review?(ctalbert) → review+
Assignee | ||
Comment 11•12 years ago
|
||
Landed in http://hg.mozilla.org/mozilla-central/rev/749d641d5ac7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•