Closed Bug 996862 Opened 11 years ago Closed 11 years ago

devicemanager getLanIp relies on a list of expected network interface names

Categories

(Testing :: Mozbase, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla31

People

(Reporter: gbrown, Assigned: wlach)

References

Details

Attachments

(3 files, 1 obsolete file)

bjacob had trouble running remote reftests today. He was getting this error: bjacob:/hack/mozilla-central$ MOZ_HOST_BIN=/hack/mozilla-central/obj-firefox-debug/dist/bin/ TEST_PATH=layout/reftests/bugs/reftest.list make -C obj-mobile-debug/ reftest-remote make: Entering directory `/hack/mozilla-central/obj-mobile-debug' ERROR: Either you specified the loopback for the remote webserver or your local IP cannot be detected. Please provide the local ip in --remote-webserver ERROR: Invalid options specified, use --help for a list of valid options /bin/sh: line 11: @errors=: command not found make: Leaving directory `/hack/mozilla-central/obj-mobile-debug' which seems to be caused by getLanIp failing because none of bjacob's network interface names matches the list at: http://hg.mozilla.org/mozilla-central/annotate/1f932e462b84/testing/mozbase/mozdevice/mozdevice/devicemanager.py#l601 if (ip is None or ip.startswith("127.")) and os.name != "nt": interfaces = ["eth0","eth1","eth2","wlan0","wlan1","wifi0","ath0","ath1","ppp0"] There must be a better way to do this.
Wait, don't we have code to do this already? http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/moznetwork/moznetwork/moznetwork.py I'm guessing devicemanager was written before moznetwork and was never updated to use the new library.
(In reply to Andrew Halberstadt [:ahal] from comment #2) > Wait, don't we have code to do this already? > http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/moznetwork/ > moznetwork/moznetwork.py > > I'm guessing devicemanager was written before moznetwork and was never > updated to use the new library. Yes, correct. We should probably turn getLanIp() into a compatibility shim around moznetwork.
(In reply to William Lachance (:wlach) from comment #3) > (In reply to Andrew Halberstadt [:ahal] from comment #2) > > Wait, don't we have code to do this already? > > http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/moznetwork/ > > moznetwork/moznetwork.py > > > > I'm guessing devicemanager was written before moznetwork and was never > > updated to use the new library. > > Yes, correct. We should probably turn getLanIp() into a compatibility shim > around moznetwork. And I guess in this case, just replace calls to getLanIp() (and any abstractions around it) with calls to moznetwork.
I'm going to take this one. I think the right way forward is to kill any usage of getLanIp(). As far as I know there are two users: the in-tree code (subject of this bug) and talos... I thought sut_tools might as well, but apparently not: https://hg.mozilla.org/build/tools/file/c290db3dd537/sut_tools/ (1) Fix in-tree code to use moznetwork (2) Fix talos to use moznetwork (3) Remove this functionality from mozdevice
Assignee: nobody → wlachance
Attached patch Remove getLanIp usage, replace with moznetwork (obsolete) (deleted) — Splinter Review
Attachment #8408472 - Flags: review?(ahalberstadt)
Comment on attachment 8408472 [details] [diff] [review] Remove getLanIp usage, replace with moznetwork Review of attachment 8408472 [details] [diff] [review]: ----------------------------------------------------------------- Is something still using NetworkTools? Any reason not to remove that as well? ::: build/mobile/b2gautomation.py @@ -129,5 @@ > > return app, args > > - def getLanIp(self): > - nettools = NetworkTools() You can get rid of the import too. ::: build/mobile/remoteautomation.py @@ -175,5 @@ > # return app, ['--environ:NO_EM_RESTART=1'] + args > return app, args > > - def getLanIp(self): > - nettools = NetworkTools() Same.
Attachment #8408472 - Flags: review?(ahalberstadt) → review+
Attachment #8408490 - Flags: review?(jmaher)
(In reply to Andrew Halberstadt [:ahal] from comment #8) > Is something still using NetworkTools? Any reason not to remove that as well? Just talos AFAIK (just wrote up a patch for that). I was planning to do up a seperate patch to remove from mozdevice.
Comment on attachment 8408490 [details] [diff] [review] Remove use of mozdevice's networktools Review of attachment 8408490 [details] [diff] [review]: ----------------------------------------------------------------- r- for the need of error handling. ::: create_talos_zip.py @@ +44,4 @@ > ('mozdevice/mozdevice/droid.py', 'mozdevice/droid.py')] > mozdevice = [(mozdevice_src + src, destination) for src, destination in mozdevice_files] > > +moznetwork_src = 'https://raw.github.com/mozilla/mozbase/moznetwork-0.24/' does this really live on github? I thought we were in tree ::: setup.py @@ +18,4 @@ > 'mozhttpd == 0.5', > 'mozinfo == 0.7', > 'datazilla == 1.4', > + 'moznetwork == 0.24', does this live on pypi.mozilla.org (or whatever that server is for internal python packages) ::: talos/PerfConfigurator.py @@ +436,5 @@ > if self.config.get('develop'): > if not self.config.get('webserver'): > + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) > + s.bind(("127.0.0.1",0)) > + self.config['webserver'] = 'localhost:%s' % s.getsockname()[1] what if there is an error here? can we try/except and terminate if we fail?
Attachment #8408490 - Flags: review?(jmaher) → review-
Comment on attachment 8408490 [details] [diff] [review] Remove use of mozdevice's networktools I was beaten into submission on IRC and now have a r+.
Attachment #8408490 - Flags: review- → review+
Removing now-unnecessary imports per review suggestion, carrying forward r+.
Attachment #8408472 - Attachment is obsolete: true
Attachment #8409677 - Flags: review+
Try run looks good (https://tbpl.mozilla.org/?tree=Try&rev=65744b48160c), please checkin patch 8409677 when there is a chance. https://bug996862.bugzilla.mozilla.org/attachment.cgi?id=8409677 (leave this open for followup work)
Attachment #8408490 - Attachment is obsolete: true
Comment on attachment 8408490 [details] [diff] [review] Remove use of mozdevice's networktools This one isn't obsolete, though it shouldn't be checked into m-c.
Attachment #8408490 - Attachment is obsolete: false
And here's a patch to remove NetworkTools from mozdevice altogether. I don't think anything uses it anymore. In the unlikely event that there is, pretty trivial to update it to use moznetwork.
Attachment #8409793 - Flags: review?(ahalberstadt)
Comment on attachment 8409793 [details] [diff] [review] Remove NetworkTools from mozdevice Review of attachment 8409793 [details] [diff] [review]: ----------------------------------------------------------------- \o/ awesome, thanks for doing this!
Attachment #8409793 - Flags: review?(ahalberstadt) → review+
Ok, let's land attachment 8409793 [details] [diff] [review] as well! I didn't do a try run, but I did run the mozbase unit tests and all seems to be good. Also a quick mxrs shows that we're no longer importing/using NetworkTools anywhere aside from inside mozdevice itself (which this patch addresses): http://mxr.mozilla.org/mozilla-central/search?string=NetworkTools http://mxr.mozilla.org/mozilla-central/search?string=getLanIp
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: