1176349 - /lockscreen/test_lockscreen_unlock_to_camera_with_passcode.py requires proper check for unlock

Status: RESOLVED FIXED

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

Description

[No-Jun Park [:njpark]]

Currently, remains of the Bug 1123325 is causing /lockscreen/test_lockscreen_unlock_to_camera_with_passcode.py to fail unlocking lockscreen. 

Yet the script is reported to be passing.  It needs lock_screen.wait_for_lockscreen_not_visible() as well as self.wait_for_condition(lambda m: self.apps.displayed_app.name == homescreen.name) at the end.

Comment 1

[No-Jun Park [:njpark]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30678

Comment 2

[No-Jun Park [:njpark]]

Ideally, this should be merged when 1123325 is fixed.

Comment 3

[Martijn Wargers (dead)]

Comment on attachment 8624873 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30678

If this works, then that's great.
I'm just wondering, wouldn't it be better to put these Waits in the app object's functions? http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/gaiatest/apps/lockscreen/app.py

Comment 4

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8624873 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30678

Agreed with Martijn, we might use lockscreen.wait_for_lockscreen_not_visible() which would likely do the wait. As it's a method within LockScreen, we can add these waits in all the unlock methods. What do you think No-Jun?

Comment 5

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8624873 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30678

One small nit. Apart from that, LGTM!

Comment 6

[Martijn Wargers (dead)]

I get merge conflicts when trying to import this pull request, could you perhaps merge it to the latest master?

Comment 7

[Martijn Wargers (dead)]

Oh wait, this depends on the bug 1123325 fix? Shouldn't this be part as the updated pull request for bug 1123325 then, since that original pull request also tried to fix these tests.

Comment 8

[No-Jun Park [:njpark]]

(In reply to Martijn Wargers [:mwargers] (QA) from comment #7)
> Oh wait, this depends on the bug 1123325 fix? Shouldn't this be part as the
> updated pull request for bug 1123325 then, since that original pull request
> also tried to fix these tests.

Yes per comment 2, ideally this should be merged to master after Bug 1123325 is fixed.  So I'm planning to merge it once Bug 1123325 is resolved. they are technically different bugs, since the problem fixed by this pull request have existed before bug 1123325.

Comment 9

[Martijn Wargers (dead)]

Comment on attachment 8624873 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/30678

Ok, fine by me then. The changes look fine by me. I would like to have run the tests myself, but I guess I would have to apply the pull request from 1123325, but that doesn't apply for me locally well, at all, so I guess I'm just giving an r+ like this.

Comment 10

[No-Jun Park [:njpark]]

Merged:

https://github.com/mozilla-b2g/gaia/commit/8394b4788d91d181e6b144e9cf044ed1e9065270