976986 - SettingsListener should have unobserve() method

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Some classes of system app 'observe' some setting values but unable to 'unobserve' them, this make unit tests of these class difficult to read and to write. And 'unobserve()' method should be existed to be pair of 'observe()' from design point of view.

Comment 1

[Tzu-Lin Huang [:dwi2][:tzhuang]]

it's a follow-up of bug 971498

Comment 3

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: pull request

a WIP patch, I am still working on the unit test of it

Comment 4

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8388281 [details] [review]
pull request

Hi Alive,

Please help to review the patch, Thanks.

Comment 5

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8388281 [details] [review]
pull request

I should ask for review from Tim because he is the original author.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8388281 [details] [review]
pull request

The code itself is fine.

I would like to see setTimeout() in mock_navigator_moz_settings.js in being removed, instead of being workaround'd with fake timers. We shouldn't simulate async operation with setTimeout(). I won't object if that's being done in the next bug though.

Comment 7

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Tim, 

I filed a follow up bug in order to remove all async method/DOM request simulated timers in unit test - bug 982544.
And bubble-tea is freezed now, I'll land this patch to master once bubble-tea merge into master.

Comment 8

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: pull request against master

Old PR to bubble-tea is obsolete because branch bubble-tea is freezed.
I made a new PR to master with exact the same patch, but I still need Tim's review+ before landing the patch

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8390974 [details] [review]
pull request against master

You should land on bubble-tea, don't worry about it.

Comment 10

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Travis green
https://travis-ci.org/mozilla-b2g/gaia/builds/20516745

Landed on bubble-tea
https://github.com/mozilla-b2g/gaia/commit/77405d208b6209a149e61802f4ba5aa099f33c9e

Comment 11

[[e-mail]]

Attachment: mozilla-b2g:master PR#17495

Comment 12

[[e-mail]]

travis all green, merged https://github.com/mozilla-b2g/gaia/pull/17495