address-book-app

Merge lp://qastaging/~rhuddie/address-book-app/ux-calling-helpers into lp://qastaging/address-book-app

ux-calling-helpers
Merge into trunk

Proposed by Richard Huddie on 2014-07-11

Status:	Superseded
Proposed branch:	lp://qastaging/~rhuddie/address-book-app/ux-calling-helpers
Merge into:	lp://qastaging/address-book-app
Diff against target:	322 lines (+246/-5) 5 files modified debian/control (+2/-0) tests/autopilot/address_book_app/fake_url_dispatcher.py (+74/-0) tests/autopilot/address_book_app/pages/_contact_list_page.py (+37/-5) tests/autopilot/address_book_app/pages/_contact_view.py (+69/-0) tests/autopilot/address_book_app/tests/test_contactlist.py (+64/-0)
To merge this branch:	bzr merge lp://qastaging/~rhuddie/address-book-app/ux-calling-helpers
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2014-07-22
Leo Arias (community)	code review	2014-07-14	Needs Fixing on 2014-07-14
Ubuntu Phablet Team		2014-07-11	Pending
Review via email: mp+226473@code.qastaging.launchpad.net

This proposal has been superseded by a proposal from 2014-07-22.

Commit message

New autopilot helper methods to press the telephone and message icons for a specific phone number in contacts view

Description of the change

New autopilot helper methods to press the telephone and message icons for a specific phone number in contacts view

lp://qastaging/~rhuddie/address-book-app/ux-calling-helpers updated on 2014-07-11

239. By Richard Huddie on 2014-07-11: flake8

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/601/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/602/rebuild

review: Needs Fixing (continuous-integration)

lp://qastaging/~rhuddie/address-book-app/ux-calling-helpers updated on 2014-07-14

240. By Richard Huddie on 2014-07-14: add dependencies

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-14:

FAILED: Continuous integration, rev:240
http://jenkins.qa.ubuntu.com/job/address-book-app-ci/603/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-utopic-amd64-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-utopic-armhf-ci/56
        deb: http://jenkins.qa.ubuntu.com/job/address-book-app-utopic-armhf-ci/56/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-utopic-i386-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1928
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1606
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2179
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3036
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3036/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9769
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1344
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1809
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1809/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/603/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Leo Arias (elopio) wrote on 2014-07-14:

Thanks Richard. This is nice.

125 + def get_contact_index_by_name(self, display_name):

This needs some love, but feel free to leave it for the future because it's not your fault. Many things would be a lot easier if we had a method called get_contacts_info on the contacts list page. That method should return a list of contact tuples composed of all the visible information, ordered by the y coordinate. Reminders has a good example of this.
Then you could look for the contact index on that list, instead of in the QML tree.
I'm just saying it would be nice :)

174 + def _get_contact(self, index, phone_number):
This would be clearer as two methods: _get_contact_by_index and _get_contact_by_phone_number

203 + def call_contact(self, index=0, phone_number=None):
215 + def message_contact(self, index=0, phone_number=None):
Just like this, I would make two methods. And for a high level helper, I think index might not be a good parameter. I would be inclined to make a call_contact_by_number and call_contact_by_name. But in our tests we will generally have only one contact, so if you want to call him, index=0 sounds like the easiest thing to do. Feel free to keep it if you prefer that way.

231 + def get_call_button(self):
234 + def get_message_button(self):

Please make these two private method. And make two additional methods click_call_button and click_message_button. That helps encapsulating the responsibilities in the right custom proxy objects.

273 + self.add_contact("Fulano", "de Tal", [self.PHONE_NUMBER])

This is the old helper, it needs to be removed but I didn't have time to refactor all the tests.
This is the new way:

test_contact = data.Contact(
first_name='Fulano', last_name='de Tal',
phones=[self.PHONE_NUMBER])

contact_editor = self.app.main_window.go_to_add_contact()
contact_editor.fill_form(test_contact)

self.app.main_window.save()

You can wrap that in an add_contact method. Please don't use the other one, so it's easier to remove it once we have time.

277 + def convert_phone_number_to_url(self, phone_number):

There's a phone object on the data.py module. This sounds like a good method to have in that object.

Thanks Richard. This is nice.

125	+ def get_contact_index_by_name(self, display_name):

174	+ def _get_contact(self, index, phone_number):
This would be clearer as two methods: _get_contact_by_index and _get_contact_by_phone_number

203	+ def call_contact(self, index=0, phone_number=None):
215	+ def message_contact(self, index=0, phone_number=None):
Just like this, I would make two methods. And for a high level helper, I think index might not be a good parameter. I would be inclined to make a call_contact_by_number and call_contact_by_name. But in our tests we will generally have only one contact, so if you want to call him, index=0 sounds like the easiest thing to do. Feel free to keep it if you prefer that way.

231	+ def get_call_button(self):
234	+ def get_message_button(self):

Please make these two private method. And make two additional methods click_call_button and click_message_button. That helps encapsulating the responsibilities in the right custom proxy objects.

273	+ self.add_contact("Fulano", "de Tal", [self.PHONE_NUMBER])

This is the old helper, it needs to be removed but I didn't have time to refactor all the tests.
This is the new way:

test_contact = data.Contact(
    first_name='Fulano', last_name='de Tal',
    phones=[self.PHONE_NUMBER])

contact_editor = self.app.main_window.go_to_add_contact()
contact_editor.fill_form(test_contact)

self.app.main_window.save()

You can wrap that in an add_contact method. Please don't use the other one, so it's easier to remove it once we have time.

277	+ def convert_phone_number_to_url(self, phone_number):

There's a phone object on the data.py  module. This sounds like a good method to have in that object.

review: Needs Fixing (code review)

lp://qastaging/~rhuddie/address-book-app/ux-calling-helpers updated on 2014-07-21

241. By Richard Huddie on 2014-07-21: review updates

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-21:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/621/rebuild

review: Needs Fixing (continuous-integration)

lp://qastaging/~rhuddie/address-book-app/ux-calling-helpers updated on 2014-07-22

242. By Richard Huddie on 2014-07-21: merge trunk
243. By Richard Huddie on 2014-07-22: fix review comments

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-22:

FAILED: Continuous integration, rev:243
http://jenkins.qa.ubuntu.com/job/address-book-app-ci/624/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-utopic-amd64-ci/77
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-utopic-armhf-ci/77
        deb: http://jenkins.qa.ubuntu.com/job/address-book-app-utopic-armhf-ci/77/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-utopic-i386-ci/77
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2292
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1893
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2482
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3497
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/3497/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10198
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1589
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2123
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2123/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/624/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

243. By Richard Huddie on 2014-07-22: fix review comments
242. By Richard Huddie on 2014-07-21: merge trunk
241. By Richard Huddie on 2014-07-21: review updates
240. By Richard Huddie on 2014-07-14: add dependencies
239. By Richard Huddie on 2014-07-11: flake8
238. By Richard Huddie on 2014-07-11: added fake url dispatcher and updated tests
237. By Richard Huddie on 2014-07-10: Added contact list tests for call and message contact
236. By Richard Huddie on 2014-07-10: add methods to call/message a specified contact
235. By Richard Huddie on 2014-07-09: add message_contact method
234. By Richard Huddie on 2014-07-09: Add new helpers for selecting a contact and calling them