I cannot run gaiatests when I'm offline

RESOLVED FIXED in mozilla31

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dmarcos, Assigned: wlach)

Tracking

unspecified
mozilla31
x86
macOS
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Steps to reproduce:

I'm on OSX 10.9.1

1. Go offline and run gaiatests:

gaiatest --type=b2g --address=127.0.0.1:2828 --profile=$B2G_HOME/gaia/profile --testvars=testvars_template.json gaia/tests/python/gaia-ui-tests/gaiatest/tests/functional/camera/test_camera_capture_photo.py
This is what I'm getting:

starting httpd
Traceback (most recent call last):
  File "/usr/local/bin/gaiatest", line 9, in <module>
    load_entry_point('gaiatest==0.21.7', 'console_scripts', 'gaiatest')()
  File "/Users/dmarcos/Development/Mozilla/gaia/tests/python/gaia-ui-tests/gaiatest/runtests.py", line 49, in main
    cli(runner_class=GaiaTestRunner, parser_class=GaiaTestOptions)
  File "/usr/local/Cellar/python/2.7.5/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.7.3-py2.7.egg/marionette/runtests.py", line 28, in cli
    runner = startTestRunner(runner_class, options, tests)
  File "/usr/local/Cellar/python/2.7.5/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.7.3-py2.7.egg/marionette/runtests.py", line 19, in startTestRunner
    runner.run_tests(tests)
  File "/usr/local/Cellar/python/2.7.5/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.7.3-py2.7.egg/marionette/runner/base.py", line 724, in run_tests
    self.run_test(test)
  File "/usr/local/Cellar/python/2.7.5/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.7.3-py2.7.egg/marionette/runner/base.py", line 764, in run_test
    self.start_httpd()
  File "/usr/local/Cellar/python/2.7.5/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.7.3-py2.7.egg/marionette/runner/base.py", line 600, in start_httpd
    host = moznetwork.get_ip()
  File "/usr/local/Cellar/python/2.7.5/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/moznetwork-0.24-py2.7.egg/moznetwork/moznetwork.py", line 106, in get_ip
    raise NetworkError('Unable to obtain network address')
moznetwork.moznetwork.NetworkError: Unable to obtain network address
def get_ip():
    return "127.0.0.1"

The modification above fixes the issue in /usr/local/Cellar/python/2.7.5/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/moznetwork-0.24-py2.7.egg/moznetwork/moznetwork.py
I remember :automatedtester asked to modify moznetwork to return 127.0.0.1 if it couldn't find an external ip. That doesn't seem like the right solution to me, as there are many cases (e.g. where you're using a device) where the loopback address isn't working.

Is there some way we could check if we're trying to run tests on the local machine, and just use 127.0.0.1 (instead of an external ip) in that case?
Flags: needinfo?(mdas)
:jgriffin might know the answer to this question offhand as well.
Flags: needinfo?(jgriffin)
I wrote the patch for bug 914129 on my flight back from the US which would solve this.

I feel that we should always be returning at least 127.0.0.1 whenever we ask for an IP, as discussed in bug 914129 because relying on external IPs can lead to situations where contributors have situations like this. I will see what breaks on try for bug 914129 before putting it up for review
So here's a solution that I think is a bit more robust: use a local ip only if marionette says the device is "desktop", which indicates it's running on the local host.
Attachment #8384359 - Flags: feedback?(dburns)
(In reply to William Lachance (:wlach) from comment #6)
> Created attachment 8384359 [details] [diff] [review]
> Patch to use local ip if using a desktop build
> 
> So here's a solution that I think is a bit more robust: use a local ip only
> if marionette says the device is "desktop", which indicates it's running on
> the local host.

since I don't do mobile often, what happens in the case that you are forwarding traffic from the host to the device (via adb forward tcp:2828 tcp:2828) and the way the network works? In my head, and since moznetwork doesnt work directly on the device, this will rely on what is on the host if we are testing sites/apps that are coming from the host.
(In reply to David Burns :automatedtester from comment #7)
> (In reply to William Lachance (:wlach) from comment #6)
> > Created attachment 8384359 [details] [diff] [review]
> > Patch to use local ip if using a desktop build
> > 
> > So here's a solution that I think is a bit more robust: use a local ip only
> > if marionette says the device is "desktop", which indicates it's running on
> > the local host.
> 
> since I don't do mobile often, what happens in the case that you are
> forwarding traffic from the host to the device (via adb forward tcp:2828
> tcp:2828) and the way the network works? In my head, and since moznetwork
> doesnt work directly on the device, this will rely on what is on the host if
> we are testing sites/apps that are coming from the host.

Well really these are two different things:

1. We forward the marionette port from the device to the machine running the test, so the machine running the test can connect to the marionette server running on the device. Typically no network is required here, adb handles everything.
2. Some tests require access to resources over http. Unfortunately adb doesn't support reverse forwarding, so the device needs to communicate with the host over the wifi network. It's for this purpose that we need an external ip to run the http server.

Incidentally, we've talked in the past about creating a reverse proxy so that we could forward a port in the opposite direction, so we could run a server on the host, and the device could connect to it via a local ip. This would eliminate the need for any kind of wifi connection on either the device or the host. This hasn't happened yet though. :)
Comment on attachment 8384359 [details] [diff] [review]
Patch to use local ip if using a desktop build

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

::: testing/marionette/client/marionette/runner/base.py
@@ +749,5 @@
>              if not self._capabilities:
>                  self.capabilities
> +            # if we're working against a desktop version, we usually don't need
> +            # an external ip
> +            if self.marionette.session_capabilities['device'] == "desktop":

Should use self.capabilities here, to avoid an extra unnecessary Marionette request.
> Incidentally, we've talked in the past about creating a reverse proxy so that we could forward a port 
> in the opposite direction, so we could run a server on the host, and the device could connect to it 
> via a local ip. This would eliminate the need for any kind of wifi connection on either the device or 
> the host. This hasn't happened yet though. :)

We should think about this in the context of the b2g-certsuite; I imagine this would help alleviate one of the common problems people are going to encounter when running the suite.
Flags: needinfo?(jgriffin)
Don't need info from mdas anymore, I don't think. Unless she wants to comment.
Flags: needinfo?(mdas)
Assignee: nobody → wlachance
Attachment #8384359 - Attachment is obsolete: true
Attachment #8384359 - Flags: feedback?(dburns)
Attachment #8385698 - Flags: review?(jgriffin)
Minimal try run to make sure this doesn't break stuff: https://tbpl.mozilla.org/?tree=Try&rev=29bfe8c7e2d9
Attachment #8385698 - Flags: review?(jgriffin) → review+
(In reply to William Lachance (:wlach) from comment #13)
> Minimal try run to make sure this doesn't break stuff:
> https://tbpl.mozilla.org/?tree=Try&rev=29bfe8c7e2d9

This actually ran Gaia unit tests instead of Gaia UI tests.  Gaia unit tests don't use Marionette, so aren't a good test.
(In reply to Jonathan Griffin (:jgriffin) from comment #14)
> (In reply to William Lachance (:wlach) from comment #13)
> > Minimal try run to make sure this doesn't break stuff:
> > https://tbpl.mozilla.org/?tree=Try&rev=29bfe8c7e2d9
> 
> This actually ran Gaia unit tests instead of Gaia UI tests.  Gaia unit tests
> don't use Marionette, so aren't a good test.

Oops, trying again:

https://tbpl.mozilla.org/?tree=Try&rev=0ee5df340fb2
(In reply to William Lachance (:wlach) from comment #15)
> (In reply to Jonathan Griffin (:jgriffin) from comment #14)
> > (In reply to William Lachance (:wlach) from comment #13)
> > > Minimal try run to make sure this doesn't break stuff:
> > > https://tbpl.mozilla.org/?tree=Try&rev=29bfe8c7e2d9
> > 
> > This actually ran Gaia unit tests instead of Gaia UI tests.  Gaia unit tests
> > don't use Marionette, so aren't a good test.
> 
> Oops, trying again:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=0ee5df340fb2

That one errored because marionette's base runner thinks it needs the baseurl in advance: https://tbpl.mozilla.org/?tree=Try&rev=0ee5df340fb2&showall=1

Doing another try run with a patch that refactors things a bit more so I think it'll work:

https://tbpl.mozilla.org/?tree=Try&rev=976d2c0dbc75&showall=1
Posted patch Refactored patch (obsolete) — Splinter Review
Actually, may as well pre-emptively ask for a review in case the try run succeeds. :)
Attachment #8385698 - Attachment is obsolete: true
Attachment #8391412 - Flags: review?(jgriffin)
Attachment #8391412 - Flags: review?(jgriffin) → review+
Updated patch with commit message. Carrying forward r+.
Attachment #8391412 - Attachment is obsolete: true
Attachment #8392382 - Flags: review+
Try run looks good, but tree is currently closed. Adding checkin-needed.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8eac82c5e616
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.