Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Description • 10 years ago

As ZaC reminded, the nasty workaround should be fixed:

https://bugzilla.mozilla.org/show_bug.cgi?id=937442#c36

So I need to fight with a old devil that I temporarily sealed once, and suffered from the inconsistent result of Travis and local console again...

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 1 • 10 years ago

UI test failed as I said:

https://travis-ci.org/mozilla-b2g/gaia/jobs/22446246

And I don't think there is something wrong with my code, at least it passed local tests as before:

https://github.com/mozilla-b2g/gaia/pull/18035/files#diff-1c89300cc68e8b8cf66db00946358182R36

To see if Zac has any comments or know someone can help.

	Zac C (:zac)
	Comment 2 • 10 years ago

If you repeat it 5 times and it works then this is almost certainly some kind of timing issue, for example the lockscreen is not fully initialised and then the swipe action is not having an effect.

I am not endorsing it as a solution but putting sleep(1) before the swipe, it will probably make this work. This will let you narrow down where the problem is (ie the step before the swipe) and then you can make a better wait there.

In UI testing like this when one step fails you generally have to look 1 or 2 steps before the failing step to find the actual step that's not working.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 3 • 10 years ago

The test passed after I tried to re-run it. So the timing issue may be confirmed.

https://travis-ci.org/mozilla-b2g/gaia/jobs/22446246

Need to fix the test...

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 4 • 10 years ago

Updated: I feel it's still strange. The line #35:

    self.wait_for_element_displayed(*self._lockscreen_handle_locator)

Should wait until the slide handler displayed. And since the LockScreen has no transition while it was opened, the display condition should means it is fully displayed, not been hide partly.

So now we have a fully operate-able handle. Now we

    self._slide_to_unlock('homescreen')

Drag the handle to the 'homescreen' side to show the passcode panel. The implementation of this function is simply execute an action as I said. If the handle suddenly got disappeared or something is really broken, the test should fail at this point, but it never do this.

So we can now assume that the dragging is success. Thus, the lockscreen would switch to the passcode panel. And the next line

    self.wait_for_element_displayed(*self._lockscreen_passcode_panel_locator)

would guarantee it. Even though the transition of showing passcode panel may fail the test because the panel may not display fully to let the next instruction to manipulate it, but it was failed *before* this possibility. Because Travis reported that the test was failed, just because of the failed waiting timeout of the |wait_for_element_displayed| line. So it seems that the dragging didn't trigger the switching panel, or it actually executed the switching but something was wrong cause the panel had never displayed.

However, since it never happened on my local console, and even on Travis it would pass sometimes, I suspect the switching was success at every test, but some mysterious issue cause the panel just didn't show itself. I'll print out some log to show the panel related CSS to confirm this.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 5 • 10 years ago

UPDATE: It seems the |self._slide_to_unlock('homescreen')| is totally broken. I capture the screenshot of the unlocking moment, there is no passcode pad and the main panel just show as before. Maybe we can fix this with forcibly focusing on the frame.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 6 • 10 years ago

UPDATE: |switch_to_frame| didn't help. I wonder why the action chain would success even it apparently didn't manipulate the device correctly.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 7 • 10 years ago

UPDATE: change the |flick| to manually compose press-move-release chain, but it still not works. According to the screenshot, the slide seems not been pressed even after the command successfully executed. And the |release| would complain that the element was not pressed.

	Zac C (:zac)
	Comment 8 • 10 years ago

Greg thanks for your work on this! It's correct that the Marionette action chain won't explicitly fail. The Action chain does not know what elements it is acting on, it does not know or care if they are displayed or not, it uses the element to derive the pixel locations only. So it will send the events to Gecko and Gaia needs to be ready to handle them.

I think it's safe to assume that the action chain worked, but something else was wrong like the lockscreen not initialised, or the event handlers are not applied to the slider in time. (which might explain why the current loop works, it just keeps trying until the lockscreen is ready).

I will have a try at this later today (if I get time) too.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 9 • 10 years ago

Thanks. since I'm not familiar the detail of Marionette client, this is really helpful!

	Zac C (:zac)
	Comment 10 • 10 years ago

Greg, I replicated this locally too.

I found that if I put sleep(2) before slide_to_unlock('homescreen') then it would pass. The sleep is not a proper fix, but it at least confirms my suspicions that the lockscreen is not fully initialised when the slide Action is performed.

Maybe when there is a passcode it takes longer to initialise. I say that because the non-passcode tests work fine, the lockscreen initialises fast.

Is there a DOM object like "loaded" class on the lockscreen that we could wait for in the `device.lock()` action?

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 11 • 10 years ago

I would try this today. The LockScreen now initialize with 2 parts:

1. The LockScreen window
2. The LockScreen instance

This is because we need to make LockScreen as an app in the future (so we need the window). But while there are still lots of works need to do before it's an app, the legacy instance is still necessary.

So I may detect if there is a |.lockScrenWindow| inside the |#windows| element to see if the LockScreen window got initialized, and meanwhile to put a code to detect if there is a |window.lockScreen| to see if the instance got initialized. Although the instance should be only instantiated by the window when it get opened.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 12 • 10 years ago

UPDATE: It seems something wrong in the 'atoms/gaia_lock_screen.js'. I'll try to fix it.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 13 • 10 years ago

UPDATE: I've found a way to let the LockScreen show itself but the handle is unable to be manipulated. This seems because of the asynchronous state of the LockScreen between the window and the instance.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 14 • 10 years ago

UPDATE: Still have no clue. After make the atom more reasonable according to the new LockScreenWindowManager mechanism, it passed the local test perfectly, but it changed nothing on the Travis.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 15 • 10 years ago

UPDATE: It passed with the old loop-trick:

https://travis-ci.org/mozilla-b2g/gaia/jobs/22670343#L2486

According to the first message, the first |_slide_to_unlock('homescreen')| apparently failed because the panel still keep as '.main', which should be '.passcode'. But after it tried again, the panel now was the corrected one, and the test of course successfully passed.

I may take some time to see how to add some condition to force the unlocking action execute only after the window and panel is ready, or repeat it until it success (like the loop...)

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 16 • 10 years ago

UPDATE: It seems the Python doesn't have a |wait_for_condition| with do something repeatedly as JS version's |waitFor|. So may need to emulate it by adding the function inside the predict lambda.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 17 • 10 years ago

UPDATE: Finally! It passed the test with my new predict function. I would refine the patch and send it again. If it can still pass the tests, I can set the review.

https://travis-ci.org/mozilla-b2g/gaia/jobs/22674135

	Zac C (:zac)
	Comment 18 • 10 years ago

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #16)
> UPDATE: It seems the Python doesn't have a |wait_for_condition| with do
> something repeatedly as JS version's |waitFor|. So may need to emulate it by
> adding the function inside the predict lambda.

What is it you need to do? We have various Waits in python framework, I'm sure we have something you can use already.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 19 • 10 years ago

Travis is green with the new patch:

https://travis-ci.org/mozilla-b2g/gaia/builds/22743518

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 20 • 10 years ago

Patch with the new predict function.

Because I don't know who should review this so I only set feedback.
And Python is allow to use method as arguments (it's ordinary object). See:

http://stackoverflow.com/questions/706721/how-do-i-pass-a-method-as-a-parameter-in-python

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 21 • 10 years ago

Oh, I forgot to remove the commented out lines. I'll update the patch again.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 22 • 10 years ago

The test passed again:

https://travis-ci.org/mozilla-b2g/gaia/builds/22750351

	Zac C (:zac)
	Comment 23 • 10 years ago

Comment on attachment 8405200 [details]
Patch

f-, see comment in git pr.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 24 • 10 years ago

Replied on the PR.

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 25 • 10 years ago

Zac: Except try it on a slower local VM, I can't find any way to find out the root cause. It even continually unreproducible on my local machine.

And about the problem you worry about with the new patch, I think I can test it with 100+ on Travis, to see if there would be any serious problem. We may find something during this mass tests.

	Zac C (:zac)
	Comment 26 • 10 years ago

I can replicate the problem locally so let me try and fix it.

	Zac C (:zac)
	Comment 27 • 10 years ago

Greg, to debug this I dumped the DOM source immediately after locking and then again after a small sleep. Then by diff on the two source codes I could see that the lockscreen was taking some time to transition in and that told me what to wait for :)

	Greg Weng [:snowmantw][:gweng][:λ] Assignee
	Comment 28 • 10 years ago

Oh great! I dumped before (before the workaround), but never notice that. Thank you!

	[:AndreiH]
	Comment 29 • 10 years ago

Comment on attachment 8405332 [details]
github pr

Looks good!

	Zac C (:zac)
	Comment 30 • 10 years ago

Thanks!
Merged:
https://github.com/mozilla-b2g/gaia/commit/c26661c054978c022a8cc2bbd8849eec3bb8ef5b