Code review comment for lp:~canonical-platform-qa/messaging-app/test_helper_to_send_sms_from_suggested_contact_list

Revision history for this message
Omer Akram (om26er) wrote :

> 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='<b>{}</b>'.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!

« Back to merge proposal