All users were logged out of Bugzilla on October 13th, 2018

Imagecompare: go_back_and_exit() method in header.py does not work in RTL

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: njpark, Assigned: njpark)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Because it works by the coordinates, it won't work in RTL since the coordinate is reversed.  It looks like we no longer need this method, since we can locate elements under shadow DOM now.
(Assignee)

Updated

3 years ago
Assignee: nobody → npark
Created attachment 8678428 [details] [review]
[gaia] npark-mozilla:1218063 > mozilla-b2g:master
(Assignee)

Comment 2

3 years ago
Comment on attachment 8678428 [details] [review]
[gaia] npark-mozilla:1218063 > mozilla-b2g:master

This was failing the RTL test, since they recently reverse the coordinate system.  Also, i realized I can combine two methods into one.
Attachment #8678428 - Flags: review?(martijn.martijn)
(Assignee)

Updated

3 years ago
Blocks: 1218064
(Assignee)

Updated

3 years ago
No longer blocks: 1218064
(Assignee)

Updated

3 years ago
No longer depends on: 1218001
Comment on attachment 8678428 [details] [review]
[gaia] npark-mozilla:1218063 > mozilla-b2g:master

What happens if you add time.sleep(1) between "if exit_app:" and "app.wait_to_not_be_displayed"?
I think you get the errors as mentioned at: http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/base.py#100

For the rest, this looks like a good idea to me, we should make sure that the back button is tapped upon from the system app, though, I think.
Attachment #8678428 - Flags: review?(martijn.martijn)
(Assignee)

Comment 4

3 years ago
Actually, I did check whether it causes NoSuchWindowException, and in this case it does not happen.  Probably because everything I do afterwards does not involve root_elements.  If I exit the app, the context is switched, so running Wait(self.marionette).until(expected.element_not_displayed(self.root_element)) will cause NoSuchWindowException, but not when I check the app itself.
(Assignee)

Comment 5

3 years ago
Comment on attachment 8678428 [details] [review]
[gaia] npark-mozilla:1218063 > mozilla-b2g:master

I just verified that adding sleep calls do not cause failures
Attachment #8678428 - Flags: review?(martijn.martijn)
Created attachment 8678478 [details]
failure.txt

I get the failure in the attachment with this pull request applied and adding this to the code:

--- a/tests/python/gaia-ui-tests/gaiatest/form_controls/header.py
+++ b/tests/python/gaia-ui-tests/gaiatest/form_controls/header.py
@@ -23,7 +23,10 @@ class GaiaHeader(Widget):
             self.marionette.switch_to_shadow_root()
             self.root_element.find_element(*self._close_button_locator).tap()
         if exit_app:
+            import time
+            time.sleep(2)
             app.wait_to_not_be_displayed()
+            print(self.apps.displayed_app.manifest_url)
             self.apps.switch_to_displayed_app()

That failure is exactly what happens in bug 1109213.
(Assignee)

Comment 7

3 years ago
I just updated my pull request, I double checked that entire script passes on both LTR and RTL.
Comment on attachment 8678428 [details] [review]
[gaia] npark-mozilla:1218063 > mozilla-b2g:master

You also need to change this instance:
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/gallery/regions/view_image.py#40
I guess the following self.apps.switch_to_displayed_app() line can be removed.

Also, I think this line:
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/graphics/RTL/test_settings_personalization_RTL.py#30
settings.switch_to_settings_app() can be removed, since that's already done in the go_back call.
Also on line 40 and line 54, I think it can be removed.
Attachment #8678428 - Flags: review?(martijn.martijn) → review+
(Assignee)

Comment 9

3 years ago
I made above suggestions, plus the check for the shadow root so it doesn't do unnecessary self.marionette.switch_to_shadow_root()call.


Merged:
https://github.com/mozilla-b2g/gaia/commit/b564b21e08ffc3e4962f08850843c7482932ee7b
(Assignee)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Great!
You need to log in before you can comment on or make changes to this bug.