999569 - [NFC] Initialization test cases for NfcManager

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Krzysztof Mioduszewski[:tauzen]]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/34.0.1847.116 Safari/537.36

Steps to reproduce:

Test cases which would test if NfcManager is being properly initialized  are required for proper testing. This should also lay some groundwork for future NfcManager tests.

Comment 1

[Krzysztof Mioduszewski[:tauzen]]

Attachment: 0001-Bug-999569-NFC-Initialization-test-cases-for-NfcMana.patch

Added init test suit with two test cases:
1. NfcManager listens on screenchange, lock, unlock events
2. SettingsListner callback nfc.enabled fired 

All tests can now use the following mocks 
- window.navigator.mozSetMessageHandler - lack of this was resulting in an error during NfcManager.init() which prevented the event listners from being set
- window.SettingsListner
- window.lockScreen

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, Krzysztof
For Gaia patches, we use Github.
You could send a PullRequest when your patch is ready,
and when you submit a PR, it would run some travis test, once the travis is green, you can send r? here (Bugzilla) to request for review.

Thanks

Comment 3

[Krzysztof Mioduszewski[:tauzen]]

Ok, this is still work in progress. Latest version is available on my github https://github.com/tauzen/gaia/tree/Bug999569.

Comment 4

[Krzysztof Mioduszewski[:tauzen]]

Attachment: Pull Request

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8411774 [details] [review]
Pull Request

Forward r? to Alive, as he's the right guy to review changes in System app.

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8411774 [details] [review]
Pull Request

r+ if the change is made per discussions.
Thank you for making this better.

Comment 7

[Krzysztof Mioduszewski[:tauzen]]

I've made the discussed changes and added two new test cases to check if events are handled properly in handleEvent. I've modified previous tests to use this.sinon and removed calls to stub.restore(). Travis CI build passes. It would be good if you could take a look (at least at the new test cases).

We should consider refactoring NfcManage.handleEvent and NfcManager.isScreenUnlockAndEnabled, but I assume this should be done once locksreen.js is removed.

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

Basically OK. Thanks!

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://github.com/mozilla-b2g/gaia/commit/efd8019af2367fe14160cda252b9fc9fe239519a