[v2.2] go_to_url method times out on longer text

RESOLVED FIXED

Status

Firefox OS
Gaia::UI Tests
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: njpark, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
when it takes input of words that are longer than dozen characters, it times out.  For example, test_browser_search.py in functional/browser will cause following failure when search_text = 'Mozilla Web QA'

Initially thought it is the same issue as Bug 1064299, but failure seems to have a different cause in this case.  It could be the browser App performance issue, since the keyboard response lags noticeably.

    no-jun-mbp:gaia-ui-tests mozilla$ gaiatest --address=localhost:2828 --testvars=gaiatest/b2g-7.json --restart  --type=b2g --fuzz_factor 5  --get-reference-image gaiatest/tests/graphics/test_browser_search_with_Imagecapture.py
    Results will not be posted to Treeherder. Please set the following environment variables to enable Treeherder reports: TREEHERDER_KEY, TREEHERDER_SECRET
    starting httpd
    running webserver on http://192.168.1.162:51755/
    SUITE-START | Running 1 tests
    TEST-START | test_browser_search_with_Imagecapture.py TestBrowserSearch.test_browser_search
    TEST-UNEXPECTED-ERROR | test_browser_search_with_Imagecapture.py TestBrowserSearch.test_browser_search | TimeoutException: TimeoutException: Timed out after 10.2 seconds
     
     
    Traceback (most recent call last):
      File "/usr/local/Cellar/python/2.7.6/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.8.3-py2.7.egg/marionette/marionette_test.py", line 171, in run
        testMethod()
      File "/Users/mozilla/GitRepo/gaia/tests/python/gaia-ui-tests/gaiatest/tests/graphics/test_browser_search_with_Imagecapture.py", line 26, in test_browser_search
        browser = search.go_to_url(search_text)
      File "/Users/mozilla/GitRepo/gaia/tests/python/gaia-ui-tests/gaiatest/apps/search/app.py", line 26, in go_to_url
        return search_panel.go_to_url(url)
      File "/Users/mozilla/GitRepo/gaia/tests/python/gaia-ui-tests/gaiatest/apps/homescreen/regions/search_panel.py", line 35, in go_to_url
        self.wait_for_condition(lambda m: url in self.apps.displayed_app.name)
      File "/Users/mozilla/GitRepo/gaia/tests/python/gaia-ui-tests/gaiatest/apps/base.py", line 56, in wait_for_condition
        Wait(self.marionette, timeout).until(method, message=message)
      File "/usr/local/Cellar/python/2.7.6/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/marionette_client-0.8.3-py2.7.egg/marionette/wait.py", line 143, in until
        cause=last_exc) 

Device version info:
Gaia      e2d70bee03b5380ac327a145e5d694fb2443f018
Gecko     https://hg.mozilla.org/mozilla-central/rev/0e581e529fd6
BuildID   20140915201445
Version   35.0a1
Device Name 	 flame
FW-Release 	 4.4.2
FW-Incremental 	 eng.cltbld.20140915.232418
FW-Date 	 Mon Sep 15 23:24:28 EDT 2014
Bootloader 	 L1TC10011800
(Reporter)

Comment 1

4 years ago
It seems that go_to_url cannot handle search words with spaces.  if search text is 'Mozilla Web QA,' then in https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/apps/homescreen/regions/search_panel.py#L35 the value of self.apps.displayed_app.name will contain Mozilla%20Web%20QA as part of the text, while url is 'Mozilla Web QA'  Thus the comparison fails and times out.

When the search text does not contain any spaces, like 'MozillaWebQA', then the function completes successfully.

Not sure what's the rule with sanitizing the output for spaces.  Zac, could you comment?
Flags: needinfo?(zcampbell)

Comment 2

4 years ago
(In reply to No-Jun Park [:njpark] from comment #1)
> Not sure what's the rule with sanitizing the output for spaces.  Zac, could
> you comment?

There is no rule, it only matters that the code works properly and doesn't break. 

This is a use case that we haven't hit yet. So can you please file whatever patch you like and we'll test it and make sure it all works.
Flags: needinfo?(zcampbell)
(Reporter)

Comment 3

4 years ago
Created attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

patch to fix the space issue
Attachment #8493325 - Flags: review?(zcampbell)
Attachment #8493325 - Flags: review?(bob.silverberg)
(Reporter)

Comment 4

4 years ago
(In reply to No-Jun Park [:njpark] from comment #3)
> Created attachment 8493325 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304
> 
> patch to fix the space issue

wait, this has bugs, lemme work on the fix
(Reporter)

Comment 5

4 years ago
Code is updated, same pull request as Comment 3, and change is here:

https://github.com/npark-mozilla/gaia/commit/e3760cc44b552fe0ddfccbb1e2135f35d3e7d247

Comment 7

4 years ago
Comment on attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

redirect to Bebe as Bob doesn't work on the project any longer.
Attachment #8493325 - Flags: review?(bob.silverberg) → review?(florin.strugariu)
Comment on attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

As I understand for every string we type in the Search app and we tap the search button on the keyboard (return) a match is done to check is it's a URL

If it's not then it's considered a search term and a browser window will open with the default search provider search for that term.

Because of this I think we can do a urllib.quote() for every time we do a search (tap return/search)

urllib.quote - Replace special characters in string using the %xx escape. Letters, digits, and the characters '_.-' are never quoted. By default, this function is intended for quoting the path section of the URL. The optional safe parameter specifies additional characters that should not be quoted — its default value is '/'.

So basically if we have any special characters in the search term this will convert it to a URL friendly string that we can wait for.
Attachment #8493325 - Flags: review?(florin.strugariu) → review-
(Reporter)

Comment 9

4 years ago
(In reply to Florin Strugariu [:Bebe] from comment #8)
> Comment on attachment 8493325 [details] [review]
> Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304
> 
> As I understand for every string we type in the Search app and we tap the
> search button on the keyboard (return) a match is done to check is it's a URL
> 
> If it's not then it's considered a search term and a browser window will
> open with the default search provider search for that term.
> 
> Because of this I think we can do a urllib.quote() for every time we do a
> search (tap return/search)
> 
> urllib.quote - Replace special characters in string using the %xx escape.
> Letters, digits, and the characters '_.-' are never quoted. By default, this
> function is intended for quoting the path section of the URL. The optional
> safe parameter specifies additional characters that should not be quoted —
> its default value is '/'.
> 
> So basically if we have any special characters in the search term this will
> convert it to a URL friendly string that we can wait for.

Aha, I didn't know about the optional safe parameter.  it was converting http:// text to % chars.  I'll make the fix.  Thanks!
(Reporter)

Updated

4 years ago
Flags: needinfo?(florin.strugariu)
(Reporter)

Updated

4 years ago
Attachment #8493325 - Flags: review?(zcampbell) → review?(florin.strugariu)
(Reporter)

Updated

4 years ago
Attachment #8493325 - Flags: review?(zcampbell)
Comment on attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

LGTM

R+ when we squash the commits
Attachment #8493325 - Flags: review?(florin.strugariu)
Attachment #8493325 - Flags: review-
Attachment #8493325 - Flags: review+
(Reporter)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Comment 13

4 years ago
Comment on attachment 8493325 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24304

r+, njpark can you squash the commits into 1 with the correct commit message before you merge it?
Attachment #8493325 - Flags: review?(zcampbell) → review+

Comment 14

4 years ago
This hasn't been merged?

njpark if you merged this somewhere else can you post the link to the commit?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 15

4 years ago
(In reply to Zac C (:zac) from comment #14)
> This hasn't been merged?
> 
> njpark if you merged this somewhere else can you post the link to the commit?

Sorry, pretty new with this. I think I squashed all commits here with the rebase command (I checked it contains all of my changes, and updated latest master branch into the test branch):

https://github.com/npark-mozilla/gaia/commit/292780c12a1e6d10719a5594af611462dbdb2ef1

If I didn't do it right, let me know and I'll ping you tomorrow morning for help.

Comment 16

4 years ago
No-Jun, that's merged in the tip but what we wnat to do is squash it into one single commit. then you may need to force push it to overwrite the old commits.
(Reporter)

Comment 17

4 years ago
Created attachment 8498885 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/24708

new pull request for this bug
(Reporter)

Comment 18

4 years ago
try server test failed in Gij - create new ringtone test, but it looks to me this test has been failing intermittently before.  How can I re-trigger the try server test?
(Reporter)

Comment 19

4 years ago
Hi Zac, when you have time, could you let me know what I need to do with this patch at this point?  thanks!
Flags: needinfo?(zcampbell)

Comment 20

4 years ago
No-jun thanks for ni? I didn't notice you'd updated it.

Now we merge! I will do it when Gaia re-opens.
Flags: needinfo?(zcampbell)

Comment 21

4 years ago
Merge this
Flags: needinfo?(zcampbell)

Comment 22

4 years ago
Merged:
https://github.com/mozilla-b2g/gaia/commit/8b7f9b19234c51011452a12e16969232b92fbcb9
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Flags: needinfo?(zcampbell)
Resolution: --- → FIXED
Depends on: 1082759
You need to log in before you can comment on or make changes to this bug.