948415 - Make mock_contact.py pythonic, make insert_contact convert it to API-friendly format

Status: RESOLVED FIXED

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

Description

[Zac C (:zac)]

Some while back the mock_contact.py was changed to contain lists rather than nice Python-ness.

This task is to
1. Revert mock_contact.py back to Python [1]
2. Add to mock_contact.py a method to output to mozContact API friendly format [2]
3. In GaiaData.insert_contact , convert the contact to mozContact API friendly format before passing it into the javascript atom
4. Update all functional and endurance tests to use the revised mock_contact.py

Resources:
[1] https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/mocks/mock_contact.py
[2] https://wiki.mozilla.org/WebAPI/ContactsAPI

Comment 1

[Craig MacKenzie [:gentlefolk]]

I'd be willing to take this on if possible.

Comment 2

[Zac C (:zac)]

You're welcome Craig. I've assigned you. This is a fairly intermediate level task I'd say, taking into account you have to learn Firefox OS's idiosyncrasies too. Let me know if you would prefer a simpler task or if it's OK.

Keep in touch with us so we know how progress is going and whether we can help you! 

For any help you can email us directly or better to pop into #mozwebqa and #gaia channels on the IRC list.

Comment 3

[Craig MacKenzie [:gentlefolk]]

Great thanks!

Comment 4

[Craig MacKenzie [:gentlefolk]]

Attachment: Initial pull request for changes to MockContact

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8365584 [details] [review]
Initial pull request for changes to MockContact

Thanks for the patch, Craig! At the moment I feel like it's either over-complicated or under-documented. Could you add more comments to your code to explain what's happening, and please also add a unit test.

Comment 6

[Craig MacKenzie [:gentlefolk]]

Fair enough. I'll simplify/document and add a proper unit test.

Comment 7

[Craig MacKenzie [:gentlefolk]]

Attachment: Simplified creation of mozcontact api contact, added unit test

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8367058 [details] [review]
Simplified creation of mozcontact api contact, added unit test

Thanks Craig, that looks great! I'm going to request an additional review from Zac, but this gets my r+

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Oh, and the commits will need to be squashed before we can merge. You can either take care of this yourself, or we can do it before we merge. The commit message should be in the form of:

Bug [id] - [summary] r=[reviewer r=reviewer...]

So for this bug it might be:

Bug 948415 - Make mock_contact.py pythonic, make insert_contact convert it to API-friendly format. r=dhunt r=zcampbell

Comment 10

[Zac C (:zac)]

Whoa this looked to be a bit more work than I expected but it looks great! Thanks for your patience and hard work on this Craig, working out the API stuff. I'm reviewing/running the tests now

Comment 11

[Zac C (:zac)]

Comment on attachment 8367058 [details] [review]
Simplified creation of mozcontact api contact, added unit test

Some tests are missing migration in cases where we re-use attributes of the contact in the test. They are:

contacts/test_add_photo_to_contact.py
contacts/test_call_contact.py
contacts/test_sms_contact.py
dialer/test_dialer_find_contact.py
dialer/test_dialer_add_contact.py

Functionally it is working excellently :)

Comment 12

[Craig MacKenzie [:gentlefolk]]

Attachment: Migrated tests that were not converted to the new MockContact interface

Comment 13

[Craig MacKenzie [:gentlefolk]]

re Comment 9, I'll squash things once I've gotten review+'d by Zac. If you want to merge it in before I get to around to it, feel free to do it yourselves.

Comment 14

[Zac C (:zac)]

Thanks for the very through comment response, way more than was required!
Code looks good, just doing a test run and grep to check that all tests are migrated.

Then I can give you the all clear to squash.

Comment 15

[Zac C (:zac)]

Comment on attachment 8367767 [details] [review]
Migrated tests that were not converted to the new MockContact interface

r-, One syntax error in test_add_photo_to_contact.py

Comment 16

[Craig MacKenzie [:gentlefolk]]

Attachment: Fixed syntax error, migrated test to new MockContact interface

Comment 17

[Zac C (:zac)]

Comment on attachment 8368322 [details] [review]
Fixed syntax error, migrated test to new MockContact interface

r+!
Let's squash then merge!

Comment 18

[Craig MacKenzie [:gentlefolk]]

Commits have now been squashed.

Comment 19

[Florin Strugariu [:Bebe]]

Merged in https://github.com/mozilla-b2g/gaia/commit/15e4d356077a71bd2c9c3678e9d3bc4d136a58ee

Thanks for the hard work :gentlefolk