996417 - [Settings] Make sure we would render current level of battery on root panel

Status: RESOLVED FIXED

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

Description

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

Currently we are refactoring Settings app with AMD modules, we missed something when refactoring and gaia-ui-tests didn't catch it. 

I think it would be nice to add one more simple test to make sure we show the information on the root panel. 

Not sure this bug makes sense or not, if it is ok and someone can mentor me, I can take it as my good first bug in gaia-ui-test :)

Comment 1

[Zac C (:zac)]

I'll put it up for community mentoring EJ.

Comment 2

[Florin Strugariu [:Bebe]]

EJ do you need any help to start on this?

Comment 3

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

Thanks for reminding me of this bug xD

I forgot to take this bug when reporting it. Let me take this ;)

Comment 4

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

Attachment: patch on master

Hi Zac,

I just made a patch for it, can you give me some advices about the patch ?! 

Thanks :]

(To be honest, I can't setup my environment successfully, so Travis helped me to verify my patch xD)

Comment 5

[Zac C (:zac)]

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

(Pardon being so late reviewing this)

r-, you need to add the test into settings/manifest.ini and enable or disable for which environment (desktop, device or both) you want to run it on.

Comment 6

[Zac C (:zac)]

ejchen, I'm not really sure this is a valuable test for us to run. Has this code regressed often? 

I'd rather this test validate some functionality of battery rather than just validate strings. That way it is valuable to running on device.

Comment 7

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

(In reply to Zac C (:zac) from comment #6)
> ejchen, I'm not really sure this is a valuable test for us to run. Has this
> code regressed often? 
> 

Not really, I didn't see these tests failed. (I am not familiar with gaia-try, still get used to it, if I am wrong on this, please point it out to let me know.)

> I'd rather this test validate some functionality of battery rather than just
> validate strings. That way it is valuable to running on device.

I think this may not be the concept of uitest, and what you mentioned here is all about unit tests. For these ui tests, all they have to test is to make sure whether our javascript code would do the right thing and reflect related status on UI.

If you check settings/elements/root.html, you may notice that at first, all related elements are without any string and our js would put status on them later when it is ready.

Hope this makes senses to you and any feedback / idea is welcome ! 

Thanks, Zac.

Comment 8

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

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

Zac, I updated the patch and it looks better on TBPL/Travis.

Not sure this patch is ok for you or not, and also, please check my previous comment first.

Thanks !!

Comment 9

[Zac C (:zac)]

I'm OK, it was just in my backlog :)

Merged:
https://github.com/mozilla-b2g/gaia/commit/25b509c422a47b5c4cbf3ced249c4cd9e3a95b1e

Comment 10

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

Thanks my mentor (lord), Zac !

It's cool to have this bug as my good-first-bug in python :P

Comment 11

[Zac C (:zac)]

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

r+

Comment 12

[Zac C (:zac)]

No problem EJ, sorry it took some time :)