1157326 - test_lockscreen_unlock_to_homescreen_with_passcode.py in Imagecompare needs to be updated

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[No-Jun Park [:njpark]]

Now the switch to root frame must be called every time the screen wakes up in order to call unlock_to_passcode_pad() method

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] npark-mozilla:1157326 > mozilla-b2g:master

Comment 2

[Martijn Wargers (dead)]

Comment on attachment 8596052 [details] [review]
[gaia] npark-mozilla:1157326 > mozilla-b2g:master

It's pretty obvious that this is needed, as the non-graphics version of this test also has it:
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/lockscreen/test_lockscreen_unlock_to_homescreen_with_passcode.py#28
I wonder if you even have to ask review on things like this.

Comment 3

[No-Jun Park [:njpark]]

Merged:

https://github.com/mozilla-b2g/gaia/commit/1a056f7080609ee742a749f5cef4a193131a4f2a

Comment 4

[Johan Lorenzo [:jlorenzo]]

This was due to bug 1147731. We changed the way turn_screen_on() worked to make sure we're in the system app frame. Then we switch to the displayed app. That's why we need to switch back to the system frame.

I wonder if a smarter way to solve this issue was to change the signature of
> def turn_screen_on(self)
to
> def turn_screen_on(self, switch_back_to_displayed_app=True)

So we could call in this particular test case, we could call it with:
> lock_screen.turn_screen_on(switch_back_to_displayed_app=False)

What do you guys think?

Side note: I'm not in favor of merging even a small change without a review. If the quality team doesn't do it, why wouldn't the developers skip it for a 1-line-CSS-change which doesn't break the tests? It also helps the team to know what's going on in the code base (like this fallout of 1147731).

Comment 5

[No-Jun Park [:njpark]]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #4)
That was why, I see.  yes I do think changing turn_screen_on() method is a good idea, although since the only time it shouldn't switch to the displayed app is when we need to enter the passcode, so we could create something like lock_screen.turn_screen_on_to_passcode() or something like that.

this would also involve updating existing test case (which hopefully won't be too much work).  

pinging davehunt for feedback as well.

Comment 6

[Martijn Wargers (dead)]

The test_lockscreen_unlock_to_homescreen_with_passcode.py in imagecompare is really the same as the one in the orginal testcase:
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/functional/lockscreen/test_lockscreen_unlock_to_homescreen_with_passcode.py
http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/tests/graphics/test_lockscreen_unlock_to_homescreen_with_passcode.py

I really prefer if they would stay as much the same as possible.

I haven't really thought about comment 4, that might be a good idea, but then, that should be happening in the imagecompare test as well as the normal test, in my opinion.

Comment 7

[Johan Lorenzo [:jlorenzo]]

(In reply to No-Jun Park [:njpark] from comment #5)
> so we could create something like lock_screen.turn_screen_on_to_passcode(
> or something like that.

You don't need to create a new method if you add an optional argument like switch_back_to_displayed_app=True. For every app calling the method without this argument would behave like before.

(In reply to Martijn Wargers [:mwargers] (QA) from comment #6)
> I really prefer if they would stay as much the same as possible.

I agree, we could wrap each step into a explanatory functions and we just add the "take_screenshot()" between some steps. However, this'll be a bit too much work, especially know that Gip may be dropped.

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I don't think adding a switch_back_to_displayed_app argument is a good idea. At the moment some methods that switch to system frame to do what they need, and then try to switch back, and others that leave us in the system frame. Having an additional variable without making it apply to all methods just adds complexity. If we want to make sure all methods switch to the appropriate frame, then I would instead look at making unlock_to_passcode_pad switch to the system frame. It would be at least useful to document each method to state when the frame focus is changed and may need resolving by the test author.