Closed Bug 973440 Opened 6 years ago Closed 6 years ago

[settings] refactor Screen lock panel with AMD pattern

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.0)

RESOLVED FIXED
2.0 S1 (9may)
feature-b2g 2.0

People

(Reporter: gasolin, Assigned: eragonj)

References

Details

Attachments

(2 files)

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:
Assignee: nobody → ejchen
Status: NEW → ASSIGNED
Blocks: 956210
Target Milestone: --- → 1.4 S5 (11apr)
Comment on attachment 8402448 [details] [review]
patch on master

Arthur, need your feedback about this patch ! 

THanks :)
Attachment #8402448 - Flags: feedback?(arthur.chen)
Comment on attachment 8402448 [details] [review]
patch on master

Well done, EJ! Please check my comments in github, thanks!
Attachment #8402448 - Flags: feedback?(arthur.chen) → feedback+
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 ? :)
Attachment #8402448 - Flags: feedback?(zcampbell)
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 :(
Attachment #8402448 - Flags: feedback?(zcampbell) → feedback+
(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 !!
(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 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 :)
Attachment #8402448 - Flags: review?(zcampbell)
Attachment #8402448 - Flags: review?(arthur.chen)
Comment on attachment 8402448 [details] [review]
patch on master

r+ and looks good! thanks for updating our test code.
Attachment #8402448 - Flags: review?(zcampbell) → review+
Target Milestone: 1.4 S5 (11apr) → 1.4 S6 (25apr)
Comment on attachment 8402448 [details] [review]
patch on master

Thank you for the great work, EJ! Please check my comments in github.
Attachment #8402448 - Flags: review?(arthur.chen)
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 !
Attachment #8402448 - Flags: review?(arthur.chen)
Comment on attachment 8402448 [details] [review]
patch on master

r=me with the last nit addressed and conflicts resolved. Thanks!
Attachment #8402448 - Flags: review?(arthur.chen) → review+
Thanks all, 

this patch got merged into Gaia/master : afcd647bb13598c1a101ac900207f4b479d26734

:)
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Because there is one test failure in TBPL, 

I have to revert my patch here fbf71791da87ec2c8d9776507d2a7ca6ec327f4f.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 1.4 S6 (25apr) → 2.0 S1 (9may)
This patch is taken from original one (with r+) and is focusing on fix failures on TBPL.
Thanks all, merged into Gaia/master: 007e4d0a090a2b93bee5accc64617246092933ab

related tests all got passed on try server here : https://tbpl.mozilla.org/?tree=Try&rev=7cfd79a084a1
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
feature-b2g: --- → 2.0
You need to log in before you can comment on or make changes to this bug.