Bug 1059245 - Update tests and framework to use the new Browser app #23467
Conversation
|
|
chirarobert
| @@ -215,7 +215,7 @@ def enable_caps_lock(self): | |||
| self.apps.switch_to_displayed_app() | |||
|
|
|||
| # this would go through fastest way to tap/click through a string | |||
| def send(self, string): | |||
| def send(self, string, tap_key_at_end=None): | |||
There was a problem hiding this comment.
Let's not combine two steps like this, there's no point.
In the test we can always call self.keyboard._enter_key directly.
There was a problem hiding this comment.
The issue here is that the send() method closes the keyboard after typing. To reopen it I would have to tap on the input field which is in the Homescreen frame.
Would that be preferred over this method?
There was a problem hiding this comment.
If the keyboard hide, I suggest to maintain the focus of input field instead of doing a work-around. Also, What's wrong with hide the keyboard after typing it? (No offense, just want to know more)
There was a problem hiding this comment.
the problem with the keyboard being dismissed after typing is that we can no longer tap the enter key which opens the browser.
There was a problem hiding this comment.
@chirarobert I don't see where in the code the send method is closing the keyboard. Is switching back to the displayed app causing it to lose focus?
I would create a new public method called tap_search_button in the Keyboard object, that represents the button on the keyboard and call that in the test.
I'd prefer not to overload this method with more options.
There was a problem hiding this comment.
So I've tested this and I tihnk it's a bug that the Marionette command is causing the keyboard to close. Marionette should be able to switch frame without triggering events like this. So we'll need to file a bug against Marionette, probably.
I think it would be better not to change the signature of this method so let's put a hack in search_panel.py to tap the url bar again (to open the keyboard) and then tap enter key. We'll add a comment citing the Marionette bug as the reason for the hack and to remind us to remove it later.
chirarobert
force-pushed
the
update_browser_stuff
branch
from
d4747df
to
16f8b6d
Compare
| self.apps.kill(browser.app) | ||
| self.apps.kill(search.app) | ||
|
|
||
| # since the browser windows do not have an app origin they need to be closed manually |
There was a problem hiding this comment.
The kill here is not acutally a requirement of the test case so to avoid the cards view thing, let's just omit this step and jump straight to disable_wifi
I tihnk we used this to stop hitting OOM on Hamachi, but we have better RAM on flame now!
There was a problem hiding this comment.
Removed the steps for closing the search and browser apps.
|
|
chirarobert
force-pushed
the
update_browser_stuff
branch
from
16f8b6d
to
c1aa2da
Compare
|
|
| @@ -1,147 +0,0 @@ | |||
| # This Source Code Form is subject to the terms of the Mozilla Public | |||
There was a problem hiding this comment.
Is this a good way? Removing the whole browser app and using the search app which doesn't make much sense to me somehow...
There was a problem hiding this comment.
This page object will not be used anymore after upgrading to the new browser app.
There was a problem hiding this comment.
I know that, but there is still a browser right? instead of a searching app?!
There was a problem hiding this comment.
Those windows are represented by a new page object.
chirarobert
force-pushed
the
update_browser_stuff
branch
from
c1aa2da
to
707255b
Compare
|
|
chirarobert
force-pushed
the
update_browser_stuff
branch
from
707255b
to
650b578
Compare
|
|
chirarobert
force-pushed
the
update_browser_stuff
branch
from
650b578
to
b15ab99
Compare
|
|
chirarobert
force-pushed
the
update_browser_stuff
branch
from
b15ab99
to
25bb142
Compare
|
|
| @@ -26,6 +26,9 @@ def type_into_search_box(self, search_term): | |||
| # The search results frame is not findable with AppWindowManager | |||
| self._switch_to_search_results_frame() | |||
|
|
|||
| def go_to_url(self, url): | |||
chirarobert
force-pushed
the
update_browser_stuff
branch
from
25bb142
to
96e0d87
Compare
|
|
chirarobert
force-pushed
the
update_browser_stuff
branch
from
96e0d87
to
d910874
Compare
|
|
|
test_browser_bookmark.py is failing with https://pastebin.mozilla.org/6295565. |
| def go_to_url(self, url): | ||
| self.keyboard.send(url) | ||
|
|
||
| #TODO Remove hack once Bug 1062309 is fixed |
There was a problem hiding this comment.
👍 nice, doesn't look too bad in the end :)
chirarobert
force-pushed
the
update_browser_stuff
branch
from
d910874
to
7310932
Compare
|
|
chirarobert
force-pushed
the
update_browser_stuff
branch
from
7310932
to
5317c1f
Compare
|
|
chirarobert
force-pushed
the
update_browser_stuff
branch
from
5317c1f
to
19a3ea9
Compare
|
Lgtm, tests are working as expected. |
|
|
Bug 1059245 - Update tests and framework to use the new Browser app
No description provided.