1170653 - [Music] [Front-end] Add unit tests for ui/tab_bar.js

Status: RESOLVED FIXED

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

Description

[Dominic Kuo [:dkuo]]

Currently ui/tab_bar.js lacks for the unit tests so let's add them.

Comment 1

[Hubert Figuiere [:hub]]

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

This should be the first one. While this are really "unit" the are run by the marionette tests because I need the UI to be set up.

Comment 2

[Hubert Figuiere [:hub]]

Bug 1161492 will break this. So the order of landing will impact.

Comment 3

[Dominic Kuo [:dkuo]]

Hub,

In fact, I don't know what's a suitable way to write unit tests that html involved, so I tried to look what's existing in the gaia repository then found a similar case in clock app [1], do you think we can also use this approach or keep what you have in your patch? thanks.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/clock/test/unit/tabs_test.js

Comment 4

[Hubert Figuiere [:hub]]

I missed that example. That's a valid approach, but I don't think, that for us, it provides any more coverage than what I did using our actual HTML. My approach test also the fact we still match the document model with the code.

I'm not opposed to use the other approach. Let's see if that can be done,

Comment 5

[Dominic Kuo [:dkuo]]

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

Let's have Jim to also comment on this :)

Comment 6

[Hubert Figuiere [:hub]]

I have updated the pull request to provide an alternative test done as suggested in comment 3. Only minor caveat: I had to change the widget to actually listen to the 'click' events as well.

Comment 7

[Hubert Figuiere [:hub]]

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

Dominic,

This is the alternative testing method you suggested. I think it is better too.

Thanks.

Comment 8

[Hubert Figuiere [:hub]]

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

Cancelling review. I found out I have added duplicated mocks.... I'll rework it to use the existing ones.

Comment 9

[Hubert Figuiere [:hub]]

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

This time we should be alright.

Comment 10

[Dominic Kuo [:dkuo]]

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

Hub, thanks for working on this. The patch looks good to me with some minor issues, I have left the comments on github and please address them before landing it.

Comment 11

[Hubert Figuiere [:hub]]

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