Closed Bug 1484238 Opened Last year Closed Last year
Add an 'adb
_reverse' command to mozdevice .ADBAndroid
Raptor uses 'adb reverse' so it's webext can talk to the control server, and so the geckoview app on the device can access the benchmarks running on localhost on the host machine. Instead of using 'adb reverse' from within raptor directly, an 'adb_reverse' command should be added to mozdevice.ADBAndroid.
Bikeshedding for a moment.... adb_reverse seems like a redundant name considering where it will live. I would go with just 'reverse' but I'm ok with adb_reverse if you think that is more understandable as to its use. adb reverse doesn't require any explicit android functionality on the device does it? I would think we could put it in ADBDevice where it could be reused by anyone still using the adb_b2g stuff. It doesn't really matter if we don't care about the b2g stuff anymore and I'm sure you would like to put that in the past as well. ;-)
possibly a "good first bug" ?
egao: I think this will be a good bug to work on. the forward command is very similar and you can see how it is implemented in https://searchfox.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/adb.py#907
I've committed proposed additions of the 'reverse' method to adb.py. Method is heavily inspired and taken from the 'forward' method. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7e979612b672c614c69c0ad9ecc11d67145a90f
Upon closer inspection, I realized I was not iterating through a list of tuples signifying if the port is local or remote. Try run for the fixed segment of code: https://treeherder.mozilla.org/#/jobs?repo=try&revision=603180c3f3faf206b911aed272bb4a7e7681a3d3
Patch attached, includes correction to the for loop compared to the first submission.
Assignee: nobody → egao
Comment on attachment 9005040 [details] [diff] [review] reverse_mozdevice_android.patch Review of attachment 9005040 [details] [diff] [review]: ----------------------------------------------------------------- Looks good as far as it goes. Let's complete the methods to fully implement adb reverse. ::: testing/mozbase/mozdevice/mozdevice/adb.py @@ +1050,3 @@ > > self.command_output(cmd, timeout=timeout) > add a comment to show this is distinct from the forwarding methods... # Port reversing methods @@ +1071,5 @@ > + cmd = ["reverse", local, remote] > + if not allow_rebind: > + cmd.insert(1, "--no-rebind") > + self.command_output(cmd, timeout=timeout) > + complete the methods by adding list_reverses and remove_reverses.
(In reply to Bob Clary [:bc:] from comment #7) > add a comment to show this is distinct from the forwarding methods... > > # Port reversing methods > Comment added. Noted in an one-liner comment for both forward and reverse what they do. > complete the methods by adding list_reverses and remove_reverses. Implemented request: - list_reverses - remove_reverses Code is essentially identical to the list_forwards and remove_forwards methods; I believe there is room to refactor this into one method in the near future, something akin to remove_ports and list_ports (naming TBD). ----------------------------------------------------------------- In addition, the latest patch includes corrections to the docstring that referenced the methods where it was copied from (forward). These instances have been fixed to refer to 'reverse'.
Heavily inspired by the forward() method present in same file.
Fixed a mistake where for loop was iterating through a list of strings, not a tuple as forward does. Depends on D4716
Added list_reverses and remove_reverses commands. Corrected mistakes in the docstring. Depends on D4717
Comment on attachment 9005656 [details] Bug 1484238 - Add an 'adb_reverse' command to mozdevice.ADBAndroid r?bc Bob Clary [:bc:] has approved the revision.
Comment on attachment 9005656 [details] Bug 1484238 - Add an 'adb_reverse' command to mozdevice.ADBAndroid r?bc Geoff Brown [:gbrown] has approved the revision.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/1ea388fd649a Add an 'adb_reverse' command to mozdevice.ADBAndroid r=gbrown,bc
Try run from the final revision: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa1b38f6b75dbf1067015cebcde4c516b7866708
You need to log in before you can comment on or make changes to this bug.