973440 - [settings] refactor Screen lock panel with AMD pattern

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[[e-mail]]

Overview Description:

Refactor  Screen lock panel with AMD pattern referring to
https://github.com/crh0716/gaia/tree/settings2_iterative

to make it modularize and more easier to maintain

Steps to Reproduce:
1) run make test-perf APP=settings
2) run make test-integration APP=settings


Expected Results:

pass all settings test and act the same as original implementation

Additional Information:

Comment 1

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: patch on master

Comment 2

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8402448 [details] [review]
patch on master

Arthur, need your feedback about this patch ! 

THanks :)

Comment 3

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8402448 [details] [review]
patch on master

Well done, EJ! Please check my comments in github, thanks!

Comment 4

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8402448 [details] [review]
patch on master

Hi Zac, 

I made some changes on python gaia-ui-tests, can you give me some feedbacks about this ? :)

Comment 5

[Zac C (:zac)]

Comment on attachment 8402448 [details] [review]
patch on master

The code syntax and naming is all great, BUT a test is failing on Travis!

I retriggered it and it's still failing :(

Comment 6

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to Zac C (:zac) from comment #5)
> Comment on attachment 8402448 [details] [review]
> patch on master
> 
> The code syntax and naming is all great, BUT a test is failing on Travis!
> 
> I retriggered it and it's still failing :(

Thanks for your quick f+, Zac.

The problem you are saying is what I want to ask. I tried to ask Travis gurus to get a permission to access its VM so that I can test it on the real environment. After doing the same tests, the specific test always got passed no matter I tested it alone or with all tests. 

Because I always get passed on its VM and from my local, I am not sure why it would get failed when switching keyboards in that test case(you can check the error message). But the most weird stuff is that we can definitely switch keyboards successfully on different tests. 

Based on so many clues, I want to ask is there any similar failure like this happened before or am I doing anything wrong !?

Any feedback or hint would be helpful :)

Thanks Zac !!

Comment 7

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to EJ Chen [:eragonj][:小龍哥] from comment #6)
> (In reply to Zac C (:zac) from comment #5)
Ok I got the root cause because I changed the timing to focus the input. Although the problem is fixed, I still can't understand why it works on my local with tests running on b2g desktop while got failed on travis ..

Comment 8

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8402448 [details] [review]
patch on master

Arthur & Zac, 

I think this big patch is ready for your review now. 

Please help me review ! Thank :)

Comment 9

[Zac C (:zac)]

Comment on attachment 8402448 [details] [review]
patch on master

r+ and looks good! thanks for updating our test code.

Comment 10

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8402448 [details] [review]
patch on master

Thank you for the great work, EJ! Please check my comments in github.

Comment 11

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8402448 [details] [review]
patch on master

Arthur,

please help me review this again, I think we are almost there !! 

I just rebase to master and everything looks good now !

Comment 12

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8402448 [details] [review]
patch on master

r=me with the last nit addressed and conflicts resolved. Thanks!

Comment 13

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks all, 

this patch got merged into Gaia/master : afcd647bb13598c1a101ac900207f4b479d26734

:)

Comment 14

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Because there is one test failure in TBPL, 

I have to revert my patch here fbf71791da87ec2c8d9776507d2a7ca6ec327f4f.

Comment 15

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: patch on master (Fix TBPL)

This patch is taken from original one (with r+) and is focusing on fix failures on TBPL.

Comment 16

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks all, merged into Gaia/master: 007e4d0a090a2b93bee5accc64617246092933ab

related tests all got passed on try server here : https://tbpl.mozilla.org/?tree=Try&rev=7cfd79a084a1