Closed Bug 1153883 Opened 5 years ago Closed 5 years ago

[Messages] Use sinon to fix mozContacts.find results directly in tests

Categories

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

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
FxOS-S3 (24Jul)

People

(Reporter: julienw, Assigned: anubhav.worklinux, Mentored)

References

Details

(Whiteboard: [good first bug][lang=js][sms-papercuts])

Attachments

(1 file)

In apps/sms/test/unit/contacts_test.js we define results of mozContacts.find according to the input value. This is done in a quite "manual" way and it's difficult to relate the result value and the test that is using it.

We should use sinon's capabilities to return the values we want, and this should be closer to the tests using these values.

This will need the patch of bug 1142540 before working on this.

Sinon documentation is at [1].

[1] sinonjs.org/docs/
Whiteboard: [good first bug][lang=js]
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][sms-papercuts]
hi!
would like to work on this bug.
but contacts_test.js is in apps/sms/views/shared/test/unit/contacts_test.js
Hey, sure !

Yes you're right, we moved this file recently :)

Thanks a lot !
Assignee: nobody → anubhav.worklinux
Attachment #8629594 - Flags: review?(felash)
Comment on attachment 8629594 [details] [review]
[gaia] anu7495:sinon-fix > mozilla-b2g:master

Thanks for the patch !

I left some comments and questions on github.

Please fix the remaining comments in a separate commit and answer the questions if you can, and request review again once you're ready !

Thanks a lot again :)
Attachment #8629594 - Flags: review?(felash)
Attachment #8629594 - Flags: review?(felash)
Comment on attachment 8629594 [details] [review]
[gaia] anu7495:sinon-fix > mozilla-b2g:master

This is really more readable than before, thanks a lot for this !

2 more nits:
* you can have a top level "setup" function that would register the stub -- this way you don't need to repeat it everywhere.
* you can remove the line that restores mozContacts in the teardown function.

And then I think we're good :)

Please do these changes in a separate commit and then ask review again when you're ready :)
Attachment #8629594 - Flags: review?(felash)
Attachment #8629594 - Flags: review?(felash)
Comment on attachment 8629594 [details] [review]
[gaia] anu7495:sinon-fix > mozilla-b2g:master

This looks good, r=me!

Please squash the commit together, then force push to your branch, and put "checkin-needed" in the "keywords" section in the bug.

Thanks again for the work :)
Attachment #8629594 - Flags: review?(felash) → review+
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/commit/96bbd70db0b106ee29169597bdc2c3b4309a3b19
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S3 (24Jul)
You need to log in before you can comment on or make changes to this bug.