messaging-app

Merge lp:~canonical-platform-qa/messaging-app/test_helper_to_send_sms_from_suggested_contact_list into lp:messaging-app

test_helper_to_send_sms_from_suggested_contact_list
Merge into trunk

Proposed by Omer Akram on 2014-07-22

Status:	Needs review
Proposed branch:	lp:~canonical-platform-qa/messaging-app/test_helper_to_send_sms_from_suggested_contact_list
Merge into:	lp:messaging-app
Diff against target:	192 lines (+56/-25) 5 files modified debian/control (+1/-0) src/qml/Messages.qml (+1/-0) tests/autopilot/messaging_app/emulators.py (+36/-21) tests/autopilot/messaging_app/tests/__init__.py (+0/-4) tests/autopilot/messaging_app/tests/test_messaging.py (+18/-0)
To merge this branch:	bzr merge lp:~canonical-platform-qa/messaging-app/test_helper_to_send_sms_from_suggested_contact_list
Related bugs:	Link a bug report

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

Commit message

Autopilot: add test to send sms by selecting contact from the suggested contact list

Description of the change

Autopilot: add test to send sms by selecting contact from the suggested contact list

Note this branch depends on an new package address-book-service-testability to land. I just need a code review for now.

prereq: https://code.launchpad.net/~canonical-platform-qa/ubuntu-ui-toolkit/add_helpers_for_TextArea/+merge/228385

Revision history for this message

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

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/299/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

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

19 text_entry = self.get_newmessage_textarea()
20 - self.pointing_device.click_object(text_entry)
21 - text_entry.focus.wait_for(True)
22 - time.sleep(.3)
23 + self._focus_field(text_entry)
24 self.keyboard.type(str(message), delay=0.2)

Replace this with something like
text_field = self.select_single(ubuntuuitoolkit.TextField, objectName='...')
text_field.write('...')

It will take care of clicking and waiting for it to be focused. It won't sleep the extra .3. Why is that needed? If it's not, just remove it. If it is, you can overwrite the write method, but please comment with the reason.

29 + def type_contact_phone_num(self, num_or_contact, select=False):

You need to explain that select argument in a docstring.

45 + self.keyboard.press_and_release("Enter")

Pressing Enter is something that you can't do from the phone, as a real user. Is there a button to click instead?

50 + def select_contact_from_list(self, name):
51 + """Select the given contact from the list
52 +
53 + :parameter name: name of the contact
54 + """
55 +
56 + contact_list = self.get_root_instance().select_single(
57 + 'ContactSimpleListView')
58 + list_item = contact_list.wait_select_single(
59 + 'Label', text='{}'.format(name))
60 + self.pointing_device.click_object(list_item)

This needs to be improved to follow closely the page object pattern.
First, the method shouldn't be in main_view. It should be on the component that executes the action. That's the new chat page, right?

Then, the you can make an object for ContactSimpleListView and add a method select_contact there.

Let me know if you need any kind of help with this.

The test you added is nice, thanks for that.

review: Needs Fixing

Revision history for this message

Omer Akram (om26er) wrote on 2014-07-24:

> 19 text_entry = self.get_newmessage_textarea()
> 20 - self.pointing_device.click_object(text_entry)
> 21 - text_entry.focus.wait_for(True)
> 22 - time.sleep(.3)
> 23 + self._focus_field(text_entry)
> 24 self.keyboard.type(str(message), delay=0.2)
>
> Replace this with something like
> text_field = self.select_single(ubuntuuitoolkit.TextField, objectName='...')
> text_field.write('...')

Hi! I didn't know TextField emulator existed, thanks for that. The component I am trying to interact with, is not a TextField, its a custom component used only in messaging-app I believe.

>
> It will take care of clicking and waiting for it to be focused. It won't sleep
> the extra .3. Why is that needed? If it's not, just remove it. If it is, you
> can overwrite the write method, but please comment with the reason.

I am not sure about the sleep, I did not add it, it was already there and is in a few other places as well, I will not remove since there was probably a reason for that.

>
> 29 + def type_contact_phone_num(self, num_or_contact, select=False):
>
> You need to explain that select argument in a docstring.

Added.

>
> 45 + self.keyboard.press_and_release("Enter")
>
> Pressing Enter is something that you can't do from the phone, as a real user.
> Is there a button to click instead?

It was already there. Now that I poked through the app, the only way to lose focus in that field is by focusing the 'message' box, there is no other button in this page. We can go with that but I am not sure.
>
> 50 + def select_contact_from_list(self, name):
> 51 + """Select the given contact from the list
> 52 +
> 53 + :parameter name: name of the contact
> 54 + """
> 55 +
> 56 + contact_list = self.get_root_instance().select_single(
> 57 + 'ContactSimpleListView')
> 58 + list_item = contact_list.wait_select_single(
> 59 + 'Label', text='{}'.format(name))
> 60 + self.pointing_device.click_object(list_item)
>
> This needs to be improved to follow closely the page object pattern.
> First, the method shouldn't be in main_view. It should be on the component
> that executes the action. That's the new chat page, right?
>
> Then, the you can make an object for ContactSimpleListView and add a method
> select_contact there.
>
> Let me know if you need any kind of help with this.

Done, thanks.
>
> The test you added is nice, thanks for that.

Thanks!

> 19      text_entry = self.get_newmessage_textarea()
> 20      - self.pointing_device.click_object(text_entry)
> 21      - text_entry.focus.wait_for(True)
> 22      - time.sleep(.3)
> 23      + self._focus_field(text_entry)
> 24      self.keyboard.type(str(message), delay=0.2)
> 
> Replace this with something like
> text_field = self.select_single(ubuntuuitoolkit.TextField, objectName='...')
> text_field.write('...')

Hi! I didn't know TextField emulator existed, thanks for that. The component I am trying to interact with, is not a TextField, its a custom component used only in messaging-app I believe.

> 
> It will take care of clicking and waiting for it to be focused. It won't sleep
> the extra .3. Why is that needed? If it's not, just remove it. If it is, you
> can overwrite the write method, but please comment with the reason.

I am not sure about the sleep, I did not add it, it was already there and is in a few other places as well, I will not remove since there was probably a reason for that.

> 
> 29      + def type_contact_phone_num(self, num_or_contact, select=False):
> 
> You need to explain that select argument in a docstring.

Added.

> 
> 45      + self.keyboard.press_and_release("Enter")
> 
> Pressing Enter is something that you can't do from the phone, as a real user.
> Is there a button to click instead?

It was already there. Now that I poked through the app, the only way to lose focus in that field is by focusing the 'message' box, there is no other button in this page. We can go with that but I am not sure.
> 
> 50 + def select_contact_from_list(self, name):
> 51 + """Select the given contact from the list
> 52 +
> 53 + :parameter name: name of the contact
> 54 + """
> 55 +
> 56 + contact_list = self.get_root_instance().select_single(
> 57 + 'ContactSimpleListView')
> 58 + list_item = contact_list.wait_select_single(
> 59 + 'Label', text='{}'.format(name))
> 60 + self.pointing_device.click_object(list_item)
> 
> This needs to be improved to follow closely the page object pattern.
> First, the method shouldn't be in main_view. It should be on the component
> that executes the action. That's the new chat page, right?
> 
> Then, the you can make an object for ContactSimpleListView and add a method
> select_contact there.
> 
> Let me know if you need any kind of help with this.

Done, thanks.
> 
> The test you added is nice, thanks for that.

Thanks!

Revision history for this message

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

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/307/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Leo Arias (elopio) on 2014-07-25:

review: Needs Fixing

Revision history for this message

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

Omer, it would be nice if you try to improve all the code you are touching, even if you didn't write it originally. That's the only way we can get the tests to a better shape.

34 + to select it by tapping from the suggeted list.

You have a typo there ^. It's suggeSted.

> > 19 text_entry = self.get_newmessage_textarea()
> > 20 - self.pointing_device.click_object(text_entry)
> > 21 - text_entry.focus.wait_for(True)
> > 22 - time.sleep(.3)
> > 23 + self._focus_field(text_entry)
> > 24 self.keyboard.type(str(message), delay=0.2)
> >
> Hi! I didn't know TextField emulator existed, thanks for that. The component I
> am trying to interact with, is not a TextField, its a custom component used
> only in messaging-app I believe.

It's a text area, not a text field.
We have an open bug to make a helper for text area. https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1327354
It's really simple, as it should just inherit from TextField. Now that you need it, it will be the perfect moment for you to contribute to the toolkit :D

> I am not sure about the sleep, I did not add it, it was already there and is
> in a few other places as well, I will not remove since there was probably a
> reason for that.

Try removing the sleep and if all the tests pass, that's good enough. If we need to add it in the future, then we will understand the reason for it and put a comment. If you get new errors because of the removed sleep, you will be able to find the reason.
Any thing that's not clear is broken.

> It was already there. Now that I poked through the app, the only way to lose
> focus in that field is by focusing the 'message' box, there is no other button
> in this page. We can go with that but I am not sure.

It sounds like type_contact_phone_num should leave the focus on that textfield. The next helper will click the other text area, so I think it's not needed to press enter.

Revision history for this message

Omer Akram (om26er) wrote on 2014-07-26:

> Omer, it would be nice if you try to improve all the code you are touching,
> even if you didn't write it originally. That's the only way we can get the
> tests to a better shape.
>
> 34 + to select it by tapping from the suggeted list.
>
> You have a typo there ^. It's suggeSted.

Fixed.

>
>
> > > 19 text_entry = self.get_newmessage_textarea()
> > > 20 - self.pointing_device.click_object(text_entry)
> > > 21 - text_entry.focus.wait_for(True)
> > > 22 - time.sleep(.3)
> > > 23 + self._focus_field(text_entry)
> > > 24 self.keyboard.type(str(message), delay=0.2)
> > >
> > Hi! I didn't know TextField emulator existed, thanks for that. The component
> I
> > am trying to interact with, is not a TextField, its a custom component used
> > only in messaging-app I believe.
>
> It's a text area, not a text field.
> We have an open bug to make a helper for text area. https://bugs.launchpad.net
> /ubuntu-ui-toolkit/+bug/1327354
> It's really simple, as it should just inherit from TextField. Now that you
> need it, it will be the perfect moment for you to contribute to the toolkit :D
>

Ok, proposed a branch for uitoolkit, please review: https://code.launchpad.net/~canonical-platform-qa/ubuntu-ui-toolkit/add_helpers_for_TextArea/+merge/228385

> > I am not sure about the sleep, I did not add it, it was already there and is
> > in a few other places as well, I will not remove since there was probably a
> > reason for that.
>
> Try removing the sleep and if all the tests pass, that's good enough. If we
> need to add it in the future, then we will understand the reason for it and
> put a comment. If you get new errors because of the removed sleep, you will be
> able to find the reason.
> Any thing that's not clear is broken.

Done, no random sleep.

>
> > It was already there. Now that I poked through the app, the only way to lose
> > focus in that field is by focusing the 'message' box, there is no other
> button
> > in this page. We can go with that but I am not sure.
>
> It sounds like type_contact_phone_num should leave the focus on that
> textfield. The next helper will click the other text area, so I think it's not
> needed to press enter.

Yeah, didn't think of that. Removed the Enter hack.

> Omer, it would be nice if you try to improve all the code you are touching,
> even if you didn't write it originally. That's the only way we can get the
> tests to a better shape.
> 
> 34      + to select it by tapping from the suggeted list.
> 
> You have a typo there ^. It's suggeSted.

Fixed.

> 
> 
> > > 19      text_entry = self.get_newmessage_textarea()
> > > 20      - self.pointing_device.click_object(text_entry)
> > > 21      - text_entry.focus.wait_for(True)
> > > 22      - time.sleep(.3)
> > > 23      + self._focus_field(text_entry)
> > > 24      self.keyboard.type(str(message), delay=0.2)
> > >
> > Hi! I didn't know TextField emulator existed, thanks for that. The component
> I
> > am trying to interact with, is not a TextField, its a custom component used
> > only in messaging-app I believe.
> 
> It's a text area, not a text field.
> We have an open bug to make a helper for text area. https://bugs.launchpad.net
> /ubuntu-ui-toolkit/+bug/1327354
> It's really simple, as it should just inherit from TextField. Now that you
> need it, it will be the perfect moment for you to contribute to the toolkit :D
>

Ok, proposed a branch for uitoolkit, please review: https://code.launchpad.net/~canonical-platform-qa/ubuntu-ui-toolkit/add_helpers_for_TextArea/+merge/228385
 
> > I am not sure about the sleep, I did not add it, it was already there and is
> > in a few other places as well, I will not remove since there was probably a
> > reason for that.
> 
> Try removing the sleep and if all the tests pass, that's good enough. If we
> need to add it in the future, then we will understand the reason for it and
> put a comment. If you get new errors because of the removed sleep, you will be
> able to find the reason.
> Any thing that's not clear is broken.

Done, no random sleep.

> 
> > It was already there. Now that I poked through the app, the only way to lose
> > focus in that field is by focusing the 'message' box, there is no other
> button
> > in this page. We can go with that but I am not sure.
> 
> It sounds like type_contact_phone_num should leave the focus on that
> textfield. The next helper will click the other text area, so I think it's not
> needed to press enter.

Yeah, didn't think of that. Removed the Enter hack.

Revision history for this message

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

Thanks Omer. Now you can either wait for the toolkit to land with your TextArea helper, or duplicate the code of the TextArea helper in the messaging app branch.
If duplicate, please make sure that you report a bug assigned to you, and that you comment on the duplicate code with a link to the bug.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-06:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/359/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-06:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/362/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

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

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/374/rebuild

review: Needs Fixing (continuous-integration)

lp:~canonical-platform-qa/messaging-app/test_helper_to_send_sms_from_suggested_contact_list updated on 2014-08-18

169. By Omer Akram on 2014-08-18: merge trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-18:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/398/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Omer Akram (om26er) wrote on 2014-08-18:

Are there any related MPs required for this MP to build/function as expected? Please list.

None. manual test run of your code change and any related functionality on device or emulator?

Did you successfully run all tests found in your component's Test Plan on device or emulator?

Yes

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-25:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/411/rebuild

review: Needs Fixing (continuous-integration)

lp:~canonical-platform-qa/messaging-app/test_helper_to_send_sms_from_suggested_contact_list updated on 2014-09-03

170. By Omer Akram on 2014-09-03: merge trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-03:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/messaging-app-ci/439/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

170. By Omer Akram on 2014-09-03: merge trunk
169. By Omer Akram on 2014-08-18: merge trunk
168. By Omer Akram on 2014-08-11: fix import
167. By Omer Akram on 2014-08-11: merge trunk
166. By Omer Akram on 2014-08-05: merge trunk
165. By Omer Akram on 2014-07-26: mallit-server does not work on desktop so don't try to stop it
164. By Omer Akram on 2014-07-26: remove hack to lose focus from the input field
163. By Omer Akram on 2014-07-26: fix docstring typo
162. By Omer Akram on 2014-07-26: make right use of ubuntuuitoolkit module
161. By Omer Akram on 2014-07-26: make use of uitoolkit helpers to write into input boxes

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:

Canonical Platform QA Team

Sam Short

Ubuntu Phablet Team

 === modified file 'debian/control'
 --- debian/control	2014-08-19 15:24:14 +0000
 +++ debian/control	2014-09-03 13:53:56 +0000
@@ -51,6 +51,7 @@
           python3-autopilot,
           ubuntu-ui-toolkit-autopilot,
           ofono-phonesim-autostart,
++         address-book-service-testability,
  Description: autopilot tests for messaging-app
   This package contains the autopilot tests for messaging-app
+  .
 === modified file 'src/qml/Messages.qml'
 --- src/qml/Messages.qml	2014-08-28 21:59:17 +0000
 +++ src/qml/Messages.qml	2014-09-03 13:53:56 +0000
@@ -834,6 +834,7 @@
              TextArea {
                  id: messageTextArea
++                objectName: 'newMessageInput'
                  anchors {
                      top: attachments.count == 0 ? textEntry.top : attachmentThumbnails.bottom
                      left: parent.left
 === modified file 'tests/autopilot/messaging_app/emulators.py'
 --- tests/autopilot/messaging_app/emulators.py	2014-08-20 12:38:05 +0000
 +++ tests/autopilot/messaging_app/emulators.py	2014-09-03 13:53:56 +0000
@@ -15,10 +15,10 @@
  import time
  from autopilot import logging as autopilot_logging
--from autopilot.input import Keyboard
  from autopilot.platform import model
  from autopilot.introspection.dbus import StateNotFoundError
--from ubuntuuitoolkit import emulators as toolkit_emulators
++
++import ubuntuuitoolkit
  from ubuntuuitoolkit._custom_proxy_objects import _common
@@ -29,11 +29,11 @@
          """Exception raised when there is an error with the emulator."""
--class MainView(toolkit_emulators.MainView):
++class MainView(ubuntuuitoolkit.MainView):
      def __init__(self, *args):
          super(MainView, self).__init__(*args)
--        self.pointing_device = toolkit_emulators.get_pointing_device()
--        self.keyboard = Keyboard.create()
++        self.pointing_device = ubuntuuitoolkit.get_pointing_device()
++        self.keyboard = ubuntuuitoolkit.get_keyboard()
          self.logger = logging.getLogger(__name__)
      def get_pagestack(self):
@@ -241,26 +241,27 @@
          :parameter message: the message to type
          """
--        text_entry = self.get_newmessage_textarea()
--        self.pointing_device.click_object(text_entry)
--        text_entry.focus.wait_for(True)
--        time.sleep(.3)
--        self.keyboard.type(str(message), delay=0.2)
++        text_entry = self.select_single(
++            ubuntuuitoolkit.TextArea, objectName='newMessageInput')
++        text_entry.write(str(message))
          self.logger.info(
              'typed: "{}" expected: "{}"'.format(text_entry.text, message))
--    def type_contact_phone_num(self, num_or_contact):
++    def type_contact_phone_num(self, num_or_contact, select=False):
          """Select and type phone number or contact
          :parameter num_or_contact: number or contact to type
++        :parameter select: set True if a contact already exists and
++            you want to select it by tapping from the suggested list.
          """
--        text_entry = self.get_newmessage_multirecipientinput()
--        self.pointing_device.click_object(text_entry)
--        text_entry.focus.wait_for(True)
--        time.sleep(.3)
--        self.keyboard.type(str(num_or_contact), delay=0.2)
--        self.keyboard.press_and_release("Enter")
++        text_entry = self.select_single(
++            ubuntuuitoolkit.TextField, objectName='contactSearchInput')
++        text_entry.write(str(num_or_contact))
++        if select:
++            root = self.get_root_instance()
++            message_page = root.select_single(Messages)
++            message_page.select_contact_from_list(num_or_contact)
          self.logger.info(
              'typed "{}" expected "{}"'.format(
                  self.get_newmessage_textfield().text, num_or_contact))
@@ -391,15 +392,17 @@
          message.confirm_removal()
      @autopilot_logging.log_action(logger.info)
--    def send_message(self, number, message):
++    def send_message(self, num_or_name, message, select=False):
          """Write a new message and send it.
--        :param number: number of the contact to send message to.
++        :param num_or_name: number or name of the contact to send message to.
          :param message: the message to be sent.
++        :param select: whether to select the contact from the autocomplete
++                       list or just press 'Enter'.
          """
          self.start_new_message()
--        self.type_contact_phone_num(number)
++        self.type_contact_phone_num(num_or_name, select=select)
          self.type_message(message)
          old_message_count = self.get_multiple_selection_list_view().count
          self.click_send_button()
@@ -452,7 +455,7 @@
          return root.wait_select_single(Messages, participants=participants)
--class Messages(toolkit_emulators.UbuntuUIToolkitEmulatorBase):
++class Messages(ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase):
      """Autopilot helper for the Messages Page."""
      def get_messages_count(self):
@@ -506,6 +509,18 @@
              messages.append((date, text))
          return messages
++    def select_contact_from_list(self, name):
++        """Select the given contact from the list
++
++        :parameter name: name of the contact
++        """
++
++        contact_list = self.select_single(
++            'ContactSimpleListView')
++        list_item = contact_list.wait_select_single(
++            'Label', text='<b>{}</b>'.format(name))
++        self.pointing_device.click_object(list_item)
++
  class ListItemWithActions(_common.UbuntuUIToolkitCustomProxyObjectBase):
 === modified file 'tests/autopilot/messaging_app/tests/__init__.py'
 --- tests/autopilot/messaging_app/tests/__init__.py	2014-07-15 16:32:16 +0000
 +++ tests/autopilot/messaging_app/tests/__init__.py	2014-09-03 13:53:56 +0000
@@ -35,10 +35,6 @@
      """
--    # Don't use keyboard on desktop
--    if model() == 'Desktop':
--        subprocess.call(['/sbin/initctl', 'stop', 'maliit-server'])
--
      if model() == 'Desktop':
          scenarios = [
              ('with mouse', dict(input_device_class=Mouse)),
 === modified file 'tests/autopilot/messaging_app/tests/test_messaging.py'
 --- tests/autopilot/messaging_app/tests/test_messaging.py	2014-08-01 19:40:26 +0000
 +++ tests/autopilot/messaging_app/tests/test_messaging.py	2014-09-03 13:53:56 +0000
@@ -18,6 +18,7 @@
  from testtools.matchers import Equals, HasLength
  from testtools import skip
++from address_book_service_testability import fixture_setup as contact_fixture
  from messaging_app import emulators
  from messaging_app import fixture_setup
  from messaging_app import helpers
@@ -344,3 +345,20 @@
          _, remaining_message_text = remaining_messages[0]
          self.assertEqual(
              remaining_message_text, self.messages[0])
++
++
++class TestMessagingWithExistingContacts(BaseMessagingTestCase):
++
++    def setUp(self):
++        self.useFixture(contact_fixture.AddressBookServiceDummyBackend())
++        super(TestMessagingWithExistingContacts, self).setUp()
++
++    def test_send_message_to_existing_contact(self):
++        """Verify we can write and send a new text message
++        to an existing contact.
++        """
++        contact = 'UX User'
++        message = 'hi!'
++        bubble = self.main_view.send_message(contact, message, select=True)
++
++        self.assertProperty(bubble, visible=True)