Closed
Bug 792048
Opened 12 years ago
Closed 12 years ago
add support for mDNS to mozdevice
Categories
(Testing :: Mozbase, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: vlad, Unassigned)
Details
Attachments
(1 file)
63.34 KB,
patch
|
wlach
:
review+
|
Details | Diff | Splinter Review |
[Note: I know pull requests aren't great, but in this case I've got 9 patches in as separate commits -- 8 of them are just largely trivial misc fixes, and 1 adds mDNS support. I'd rather keep them separate for easier reviewing than mashing everything together (even mashing them into 2 patches), so I'll post a pull request here shortly (chicken-and-egg problem -- I need the bug # to reference in the pull request, and I need the pull request to reference here!)
If whoever ends up reviewing would really, really rather see things squashed into two patches, I can do that as well.]
This adds Droid.DroidConnectByHWID to mozdevice, which will first try to connect via SUT on the local network via mDNS, and then will try adb locally for the given hwid.
It also makes a couple other misc changes, largely around making things less verbose, making 'adb root' optional (because it causes adbd to restart even if it can't get root, which can screw up a usb connection), and adds DeviceManager.installLocalApp for both adb and sut (install local apk file to remote device -- the existing installApp functionality disagreed where the apk was located between adb and sut implementations!)
Reporter | ||
Comment 1•12 years ago
|
||
Pull request: https://github.com/mozilla/mozbase/pull/33
Comment 2•12 years ago
|
||
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #1)
> Pull request: https://github.com/mozilla/mozbase/pull/33
I will review this. The pull request is strictly speaking against policy but I don't think it's that big a deal and I agree it would be a lot of work to manually attach this many patches to a bug (at least not without a tool).
Comment 3•12 years ago
|
||
Status on this?
Reporter | ||
Comment 4•12 years ago
|
||
I got distracted by some of the surrounding stuff that happened as part of my pull request; will fix things up and land the mdns part tomorrow.
Reporter | ||
Comment 5•12 years ago
|
||
Let's make this just about mDNS. Down to two patches, one just a minor quieting cleanup.
Can test via sutcli.py:
$ python sutcli.py -d 015d2bc2825ff206 info screen
X:800 Y:1205
Two commits: https://github.com/vvuk/mozbase/compare/mdns
Looks good?
Summary: add support for mDNS to mozdevice, and some other misc mozdevice fixes → add support for mDNS to mozdevice
Comment 6•12 years ago
|
||
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #5)
> Let's make this just about mDNS. Down to two patches, one just a minor
> quieting cleanup.
>
> Can test via sutcli.py:
>
> $ python sutcli.py -d 015d2bc2825ff206 info screen
> X:800 Y:1205
>
> Two commits: https://github.com/vvuk/mozbase/compare/mdns
>
> Looks good?
Yup, LGTM.
I'd be happy to merge this.
Reporter | ||
Comment 7•12 years ago
|
||
https://github.com/mozilla/mozbase/pull/35 for your one-click merging pleasure.
Comment 8•12 years ago
|
||
We *don't* merge pull requests directly, but ... ;)
Reporter | ||
Comment 9•12 years ago
|
||
I know, but one click! Give in to the ease and convenience! Give iiiiinnnnnnnnn....
Comment 10•12 years ago
|
||
Attachment #670040 -
Flags: review?(wlachance)
Reporter | ||
Comment 11•12 years ago
|
||
Hopefully the diff doesn't get manually committed.. (loses separate commits, loses author, etc.)
Comment 12•12 years ago
|
||
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #9)
> I know, but one click! Give in to the ease and convenience! Give
> iiiiinnnnnnnnn....
There are reasons we have the workflow that we have. If our workflow was entirely in github, that would at least be consistent and there would be one place to look at an issue. But mozbase's issue tracker is bugzilla, which is not only a much more thorough issue tracker but also is used for the rest of mozilla's efforts, so when other mozilla software including test harnesses on mozilla-central need pieces of mozbase, they can depend on and cross-reference bugs in a meaningful way.
Having worked on a number of projects that split issue tracking across bugzilla and github pull requests, I can safely say that it is anything but convenient for me. Some comments go in the bug. Some comments go in the pull request. It is at best difficult to follow both conversations and many of the comments need to be cross-posted.
Comment 13•12 years ago
|
||
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #11)
> Hopefully the diff doesn't get manually committed.. (loses separate commits,
> loses author, etc.)
We general want a single commit per bug; otherwise you are free to generate separate commits and post here separately for review (`git show <rev>`). The author is done via --author 'name <email>'
Updated•12 years ago
|
Attachment #670040 -
Flags: review?(wlachance) → review+
Comment 14•12 years ago
|
||
I pulled in vlad's branch and rebased the commits to our standard format (preserving the author information). Thanks for the patch! This looks sweet.
https://github.com/mozilla/mozbase/compare/e9ab5da488...bd895aeb46
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•