Closed Bug 914129 Opened 7 years ago Closed 7 years ago

Moznetwork has no fall back if the machine its running on has no network

Categories

(Testing :: Mozbase, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: automatedtester, Assigned: automatedtester)

Details

Attachments

(2 files)

Today when I didnt have internet I tried to run Marionette tests but couldnt. Moznetwork should probably fall back to localhost in situations like this otherwise we can't do any development in situations with no internet (planes, trains and automobiles)


I got the following error

(marionette)☁  marionette  python runtests.py tests/unit/test_getattr.py --binary ../../../../obj-ff-dbg/dist/Nightly.app/Contents/MacOS/firefox
starting httpd
Traceback (most recent call last):
  File "runtests.py", line 794, in <module>
    cli()
  File "runtests.py", line 789, in cli
    runner = startTestRunner(runner_class, options, tests)
  File "runtests.py", line 781, in startTestRunner
    runner.run_tests(tests)
  File "runtests.py", line 421, in run_tests
    self.run_test(test)
  File "runtests.py", line 457, in run_test
    self.start_httpd()
  File "runtests.py", line 308, in start_httpd
    host = moznetwork.get_ip()
  File "/Users/dburns/.virtualenvs/marionette/lib/python2.7/site-packages/moznetwork-0.23-py2.7.egg/moznetwork/moznetwork.py", line 107, in get_ip
    raise NetworkError('Unable to obtain network address')
moznetwork.moznetwork.NetworkError: Unable to obtain network address

My Virtuanenv has the following

(marionette)☁  marionette  pip freeze
ManifestDestiny==0.5.7
marionette-client==0.5.36
mozcrash==0.8
mozdevice==0.29
mozfile==0.11
mozhttpd==0.6
mozinfo==0.6
mozlog==1.3
moznetwork==0.23
mozprocess==0.11
mozprofile==0.15
mozrunner==5.24
wsgiref==0.1.2
Attached patch bug914129.patchSplinter Review
Assignee: nobody → dburns
Attachment #813672 - Flags: review?(jhammel)
Ok, I wasn't flagged for review, but I'd have to say my feeling here is that the fallback for this case should in marionette, not moznetwork. There are cases (e.g. eideticker) where we absolutely need an external network to work. I'd prefer for those cases that we be as noisy as possible about the failure, not silently fall back to something that won't work.

(see also the rule of repair, here: http://www.catb.org/esr/writings/taoup/html/ch01s06.html#id2878538)
(In reply to William Lachance (:wlach) from comment #2)
> Ok, I wasn't flagged for review, but I'd have to say my feeling here is that
> the fallback for this case should in marionette, not moznetwork. There are
> cases (e.g. eideticker) where we absolutely need an external network to
> work. I'd prefer for those cases that we be as noisy as possible about the
> failure, not silently fall back to something that won't work.


How often are we going to be doing this? If you don't need a network using MozNetwork to me, as someone who doesn't use mozbase often, seems counter intuitive. The way that systems work is that there is _always_ a valid IP so get_ip() should always should return something. Either one from a DHCP server or localhost in my opinion
(In reply to David Burns :automatedtester from comment #3)
> (In reply to William Lachance (:wlach) from comment #2)
> > Ok, I wasn't flagged for review, but I'd have to say my feeling here is that
> > the fallback for this case should in marionette, not moznetwork. There are
> > cases (e.g. eideticker) where we absolutely need an external network to
> > work. I'd prefer for those cases that we be as noisy as possible about the
> > failure, not silently fall back to something that won't work.
> 
> 
> How often are we going to be doing this? If you don't need a network using
> MozNetwork to me, as someone who doesn't use mozbase often, seems counter
> intuitive. The way that systems work is that there is _always_ a valid IP so
> get_ip() should always should return something. Either one from a DHCP
> server or localhost in my opinion

I think it's quite possible we'd want that behaviour in other cases. If you want an address of the local machine that others can reach, localhost just won't do.

What we could perhaps do is add a "get_external_ip" method which replaces "get_ip", and then make "get_ip" do what you suggest. The user of the moznetwork could then select the variant that best fits their intention.
Comment on attachment 813672 [details] [diff] [review]
bug914129.patch

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

Moving r? to wlach since jhammel is pretty under the gun with other issues.
Attachment #813672 - Flags: review?(jhammel) → review?(wlachance)
(In reply to Jonathan Griffin (:jgriffin) from comment #5)
> Comment on attachment 813672 [details] [diff] [review]
> bug914129.patch
> 
> Review of attachment 813672 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Moving r? to wlach since jhammel is pretty under the gun with other issues.

Sorry, I meant to do this myself since :wlach also seems to have the best understanding of the issues.
Comment on attachment 813672 [details] [diff] [review]
bug914129.patch

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

r- for now, see comments above.
Attachment #813672 - Flags: review?(wlachance) → review-
Flags: needinfo?(dburns)
I still feel like this is the wrong solution. I know gaiatest can work for desktop b2g with 127.0.0.1. We should try and detect that scenario and just use 127.0.0.1 for the host ip in that case. Changing moznetwork in this way will break software (e.g. eideticker, gaiatest with a device) in confusing ways when a device is on a different network than the host.
(In reply to William Lachance (:wlach) from comment #10)
> I still feel like this is the wrong solution. I know gaiatest can work for
> desktop b2g with 127.0.0.1. We should try and detect that scenario and just
> use 127.0.0.1 for the host ip in that case. Changing moznetwork in this way
> will break software (e.g. eideticker, gaiatest with a device) in confusing
> ways when a device is on a different network than the host.

Er, I should have said "Changing moznetwork in this way will break software in confusing ways if used with a device and the host is not connected to a wireless network". The device being on a different network than the host is another confusing case, different from this one. ;)
(In reply to William Lachance (:wlach) from comment #10)
> I still feel like this is the wrong solution. I know gaiatest can work for
> desktop b2g with 127.0.0.1. We should try and detect that scenario and just
> use 127.0.0.1 for the host ip in that case. Changing moznetwork in this way
> will break software (e.g. eideticker, gaiatest with a device) in confusing
> ways when a device is on a different network than the host.

How do we get the details of the device in Moznetwork to check if something is on a different host? Looking at the code it appears to only check details on the host.
Mobile is shit* and we can't have ponies so closing

* since devices dont have a reverse proxy we can't update moznetwork. Emulators and desktop work fine with the patch above.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.