address-book-app

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

ux-calling-helpers
Merge into trunk

Proposed by Richard Huddie on 2014-07-11

Status:	Superseded
Proposed branch:	lp:~rhuddie/address-book-app/ux-calling-helpers
Merge into:	lp: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:~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.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:~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:~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:~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:~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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Richard Huddie

Rufus

Ubuntu Phablet Team

 === modified file 'debian/control'
 --- debian/control	2014-07-09 17:22:14 +0000
 +++ debian/control	2014-07-22 08:46:30 +0000
@@ -81,6 +81,8 @@
           libqt5test5,
           libqt5widgets5,
           python-testscenarios,
++         python3-dbusmock,
++         python3-fixtures,
           qtdeclarative5-ubuntu-ui-toolkit-plugin (>= 0.1.49+14.10.20140707),
           ubuntu-ui-toolkit-autopilot (>= 0.1.46+14.10.20140527),
           address-book-app (>= ${binary:Version}),
 === added file 'tests/autopilot/address_book_app/fake_url_dispatcher.py'
 --- tests/autopilot/address_book_app/fake_url_dispatcher.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/address_book_app/fake_url_dispatcher.py	2014-07-22 08:46:30 +0000
@@ -0,0 +1,74 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2013 Canonical
++#
++# This program is free software: you can redistribute it and/or modify it
++# under the terms of the GNU General Public License version 3, as published
++# by the Free Software Foundation.
++
++import subprocess
++import dbus
++import dbusmock
++import fixtures
++
++
++class ContactEditorException(Exception):
++    """Exception raised from ContactEditor"""
++
++
++class FakeURLDispatcher(fixtures.Fixture):
++    """Fake URL Dispatcher interface object"""
++
++    def setUp(self):
++        """Start Fake dispatcher service"""
++        super(FakeURLDispatcher, self).setUp()
++        self.fake_service = FakeURLDispatcherService()
++        self.addCleanup(self.fake_service.stop)
++        self.fake_service.start()
++
++    def get_last_dispatch_url_call_parameter(self):
++        """Get the last call url from the fake dispatcher service"""
++        return self.fake_service.get_last_dispatch_url_call_parameter()
++
++
++class FakeURLDispatcherService(object):
++    """Fake URL Dispatcher service using a dbusmock interface."""
++
++    def __init__(self):
++        super(FakeURLDispatcherService, self).__init__()
++        self.dbus_connection = dbusmock.DBusTestCase.get_dbus(system_bus=False)
++
++    def start(self):
++        """Start the fake URL Dispatcher service."""
++        # Stop the real url-dispatcher.
++        subprocess.call(['/sbin/initctl', 'stop', 'url-dispatcher'])
++        self.dbus_mock_server = dbusmock.DBusTestCase.spawn_server(
++            'com.canonical.URLDispatcher',
++            '/com/canonical/URLDispatcher',
++            'com.canonical.URLDispatcher',
++            system_bus=False,
++            stdout=subprocess.PIPE)
++        self.mock = self._get_mock_interface()
++        self.mock.AddMethod(
++            'com.canonical.URLDispatcher', 'DispatchURL', 's', '', '')
++
++    def _get_mock_interface(self):
++        return dbus.Interface(
++            self.dbus_connection.get_object(
++                'com.canonical.URLDispatcher', '/com/canonical/URLDispatcher'),
++            dbusmock.MOCK_IFACE)
++
++    def stop(self):
++        """Stop the fake URL Dispatcher service."""
++        self.dbus_mock_server.terminate()
++        self.dbus_mock_server.wait()
++        # re-start the real service
++        subprocess.call(['/sbin/initctl', 'start', 'url-dispatcher'])
++
++    def get_last_dispatch_url_call_parameter(self):
++        """Return the parameter used in the last call to dispatch URL."""
++        calls = self.mock.GetCalls()
++        if len(calls) == 0:
++            raise ContactEditorException(
++                'URL dispatcher has not been called.')
++        last_call = self.mock.GetCalls()[-1]
++        return last_call[2][0]
 === modified file 'tests/autopilot/address_book_app/pages/_contact_list_page.py'
 --- tests/autopilot/address_book_app/pages/_contact_list_page.py	2014-06-06 22:40:23 +0000
 +++ tests/autopilot/address_book_app/pages/_contact_list_page.py	2014-07-22 08:46:30 +0000
@@ -86,7 +86,6 @@
              self.selected_marks.append(self.selection_marks[idx])
              self.pointing_device.click_object(self.selection_marks[idx])
--
      def select_contacts_by_index(self, indices):
          """ Select contacts corresponding to the list of index in indices
@@ -121,8 +120,8 @@
          else:
              obj = self
          try:
--            buttons = obj.select_many("Button",
--                                       objectName=objectname)
++            buttons = obj.select_many(
++                "Button", objectName=objectname)
              for button in buttons:
                  if button.visible:
                      self.pointing_device.click_object(button)
@@ -134,6 +133,39 @@
      def delete(self, main_window):
          main_window.done_selection()
--        dialog = main_window.wait_select_single("RemoveContactsDialog",
--            objectName="removeContactsDialog")
++        main_window.wait_select_single(
++            "RemoveContactsDialog", objectName="removeContactsDialog")
          self.click_button(main_window, "removeContactsDialog.Yes")
++
++    def get_contact_index_by_name(self, display_name):
++        """
++        Return the index of the contact with the specified name
++        :param display_name: Display name of contact
++        :return: Index of the required contact within the list
++                 None if not found
++        """
++        counter = 0
++        match_index = None
++        contact_list = self.get_contacts()
++        for contact in contact_list:
++            try:
++                # try to find matching contact
++                contact.select_single(
++                    'ContactAvatar', displayName=display_name)
++            except StateNotFoundError:
++                # increase counter and carry on
++                counter += 1
++            else:
++                # found a match, record index and exit loop
++                match_index = counter
++                break
++        return match_index
++
++    def open_contact_by_name(self, display_name):
++        """
++        Open the contact specified by display_name
++        :param display_name: Name of contact to open
++        :return: Page with contact information
++        """
++        contact_index = self.get_contact_index_by_name(display_name)
++        return self.open_contact(contact_index)
 === modified file 'tests/autopilot/address_book_app/pages/_contact_view.py'
 --- tests/autopilot/address_book_app/pages/_contact_view.py	2014-06-13 15:05:40 +0000
 +++ tests/autopilot/address_book_app/pages/_contact_view.py	2014-07-22 08:46:30 +0000
@@ -15,6 +15,7 @@
  # along with this program. If not, see <http://www.gnu.org/licenses/>.
  from address_book_app.pages import _common, _contact_editor
++from autopilot.introspection.dbus import StateNotFoundError
  class ContactView(_common.PageWithHeader):
@@ -26,3 +27,71 @@
              _contact_editor.ContactEditor,
              objectName='contactEditorPage',
              active=True)
++
++    def _get_contact_by_phone_number(self, phone_number):
++        """
++        Find the contact using phone number
++        :param phone_number: Phone number for required contact
++        :return: ContactDetailPhoneNumberView matching search requirements
++                 None if match is not found
++        """
++        required_contact = None
++        contacts = self.select_many(ContactDetailPhoneNumberView)
++        if phone_number:
++            # search for a specific phone number
++            for contact in contacts:
++                try:
++                    # try to find matching phone number
++                    contact.select_single('Label', text=phone_number)
++                except StateNotFoundError:
++                    # move onto the next one
++                    pass
++                else:
++                    # found the required contact
++                    required_contact = contact
++                    break
++        return required_contact
++
++    def _get_contact_by_index(self, index):
++        """
++        Find the contact using index
++        :param index: Index offset for required contact
++        :return: ContactDetailPhoneNumberView matching search requirements
++        """
++        return self.select_many(ContactDetailPhoneNumberView)[index]
++
++    def call_contact(self, phone_number):
++        """
++        Press the call button for the required phone number
++        :param phone_number: The specified phone number to search for
++        """
++        contact_phone_number = self._get_contact_by_phone_number(phone_number)
++        contact_phone_number.click_call_button()
++
++    def message_contact(self, phone_number):
++        """
++        Press the message button for the required phone number
++        :param phone_number: The specified phone number to search for
++        """
++        contact_phone_number = self._get_contact_by_phone_number(phone_number)
++        contact_phone_number.click_message_button()
++
++
++class ContactDetailPhoneNumberView(ContactView):
++    """Class to represent a phone number view with call and message buttons"""
++
++    def _get_call_button(self):
++        """Return the call button ActionButton object"""
++        return self.select_single('ActionButton', iconName='call-start')
++
++    def _get_message_button(self):
++        """Return the message button ActionButton object"""
++        return self.select_single('ActionButton', iconName='message')
++
++    def click_call_button(self):
++        """Click on the call button for this phone number"""
++        self.pointing_device.click_object(self._get_call_button())
++
++    def click_message_button(self):
++        """Click on the message button for this phone number"""
++        self.pointing_device.click_object(self._get_message_button())
 === modified file 'tests/autopilot/address_book_app/tests/test_contactlist.py'
 --- tests/autopilot/address_book_app/tests/test_contactlist.py	2014-05-22 13:42:45 +0000
 +++ tests/autopilot/address_book_app/tests/test_contactlist.py	2014-07-22 08:46:30 +0000
@@ -10,13 +10,77 @@
  from autopilot.matchers import Eventually
  from testtools.matchers import Equals
++from address_book_app import data
  from address_book_app.tests import AddressBookAppTestCase
++from address_book_app.fake_url_dispatcher import (
++    FakeURLDispatcher,
++    ContactEditorException)
  class TestContactList(AddressBookAppTestCase):
      """Tests the contact list features"""
++    TEST_MOBILE_NUMBER = '073331234567'
++
++    def setUp(self):
++        super(TestContactList, self).setUp()
++        self.fake_url_dispatcher = FakeURLDispatcher()
++        self.useFixture(self.fake_url_dispatcher)
++        self.phone_number = data.Phone(
++            type_='Mobile', number=self.TEST_MOBILE_NUMBER)
++        self.message_url = 'message:///{}'.format(self.phone_number.number)
++        self.tel_url = 'tel:///{}'.format(self.phone_number.number)
++
++    def add_test_contact(self):
++        """
++        Create a contact for testing
++        """
++        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()
++
++    def create_and_open_test_contact(self):
++        """
++        Create a contact for testing, return detail page for new contact
++        :return: Contact detail page
++        """
++        self.add_test_contact()
++        contacts_list = self.app.main_window.get_contact_list_page()
++        return contacts_list.open_contact(0)
++
++    def assert_last_dispatch_url(self, expected_url):
++        """
++        Assert that the last dispatch url matches the expected url
++        :param expected_url: The expected url
++        """
++        url_dispatcher = self.fake_url_dispatcher
++
++        def get_last_dispatch_url_call_parameter():
++            # Workaround for http://pad.lv/1312384
++            try:
++                return url_dispatcher.get_last_dispatch_url_call_parameter()
++            except ContactEditorException:
++                return None
++        self.assertThat(
++            get_last_dispatch_url_call_parameter,
++            Eventually(Equals(expected_url)))
++
      def test_contact_list(self):
          contact_list = self.app.main_window.get_contact_list_page()
          self.assertThat(contact_list.visible, Eventually(Equals(True)))
          pass
++
++    def test_call_contact(self):
++        """Press the call button on a contact's phone number"""
++        contact_page = self.create_and_open_test_contact()
++        contact_page.call_contact(self.TEST_MOBILE_NUMBER)
++        self.assert_last_dispatch_url(self.tel_url)
++
++    def test_message_contact(self):
++        """Press the message button on a contact's phone number"""
++        contact_page = self.create_and_open_test_contact()
++        contact_page.message_contact(self.TEST_MOBILE_NUMBER)
++        self.assert_last_dispatch_url(self.message_url)