white space issues cause devicemanagerADB not to load URI

RESOLVED FIXED in mozilla11

Status

()

Core
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: blassey, Assigned: gbrown)

Tracking

unspecified
mozilla11
x86
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 573070 [details] [diff] [review]
patch
Attachment #573070 - Flags: review?(jmaher)
Comment on attachment 573070 [details] [diff] [review]
patch

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

this patch doesn't work for me, also appears to mess up adb automation on the aurora branch.
Attachment #573070 - Flags: review?(jmaher) → review-
(Assignee)

Updated

6 years ago
Assignee: nobody → gbrown
(Assignee)

Comment 2

6 years ago
When I run mochitest-remote with adb on birch:

MOZ_HOST_BIN=~/src/objdir-firefox/dist/bin TEST_PATH=content/smil/test
make mochitest-remote

devicemanagerADB reports:

...
INFO | runtests.py | Running tests: start.

['shell', 'am', 'start', '-n', 'org.mozilla.fennec_mozdev/.App', '--es', 'args', ' -no-remote -profile /data/local/tests/profile/ http://mochi.test:8888/tests/content/smil/test?autorun=1&closeWhenDone=1&logFile=%2Fdata%2Flocal%2Ftests%2Flogs%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO']
Starting: Intent { cmp=org.mozilla.fennec_mozdev/.App (has extras) }
...

Fennec starts and displays the start page -- it does not fetch the requested URL.

Command line tests:

FAILS: shell am start -n org.mozilla.fennec_mozdev/.App --es args -no-remote http://www.google.com

SUCCEEDS: shell am start -n org.mozilla.fennec_mozdev/.App --es args -no-remote -d http://www.google.com

With Brad's patch, devicemanagerADB reports:

...
['shell', 'am', 'start', '-n', 'org.mozilla.fennec_mozdev/.App', '--es', 'args', "' -no-remote -profile /data/local/tests/profile/'", '-d', "'http://mochi.test:8888/tests/content/smil/test?autorun=1&closeWhenDone=1&logFile=%2Fdata%2Flocal%2Ftests%2Flogs%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO'"]
Starting: Intent { dat=http://mochi.test:8888/tests/content/smil/test?autorun=1&closeWhenDone=1&logFile=%2Fdata%2Flocal%2Ftests%2Flogs%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO cmp=org.mozilla.fennec_mozdev/.App (has extras) }
...
and Fennec starts and tries to open the requested url (but the request fails).

I think I would prefer to see something like:

['shell', 'am', 'start', '-n', 'org.mozilla.fennec_mozdev/.App', '--es', 'args', "' -no-remote -profile /data/local/tests/profile/ -d http://mochi.test:8888/tests/content/smil/test?autorun=1&closeWhenDone=1&logFile=%2Fdata%2Flocal%2Ftests%2Flogs%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO'"]
(Assignee)

Comment 3

6 years ago
I see now: -d is an argument to "am start" -- it sets the "data URI", which GeckoApp picks up from the Intent. In that case, Brad's patch looks right to me, at least for this case.
(Assignee)

Comment 4

6 years ago
Although it looked right, the extra quoting of the "args" argument was preventing the -profile argument from being interpreted correctly. Without the correct profile, there was no user.js proxy re-directing http://mochi.test to the local web-server, and tests would time out waiting for a connection.
(Assignee)

Comment 5

6 years ago
Created attachment 574783 [details] [diff] [review]
minor update to Brad's patch

This works for me, on birch, with ADB:
TEST_PATH=dom/tests/mochitest/dom-level1-core
MOZ_HOST_BIN=/home/mozdev/src/objdir-firefox/dist/bin
DM_TRANS=adb
make mochitest-remote
...
2044 INFO Passed: 755
2045 INFO Failed: 0
2046 INFO Todo:   233
2047 INFO SimpleTest FINISHED
Attachment #573070 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Created attachment 575604 [details] [diff] [review]
minor update to Brad's patch

This works well for me now -- mostly tested with mochitest-remote.
Attachment #574783 - Attachment is obsolete: true
Attachment #575604 - Flags: review?(jmaher)
Comment on attachment 575604 [details] [diff] [review]
minor update to Brad's patch

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

interesting!  I think this is a good approach and similar code is working in our other harnesses (we have some in talos as well).  I would like to test this with talos...try server won't get that though, let me work on that for double checking.
Attachment #575604 - Flags: review?(jmaher) → review+
(Assignee)

Updated

6 years ago
OS: Mac OS X → Android
(Assignee)

Comment 8

6 years ago
jmaher asserted on irc that this looks to be fine on Talos -- ready for check-in!
Keywords: checkin-needed
inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8a2d4389e8f
https://hg.mozilla.org/mozilla-central/rev/e8a2d4389e8f
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.