[Merge] lp:~om26er/address-book-app/test_collapse into lp:address-book-app

[Leo Arias]

Review: Needs Fixing

I left some comments inline.
This makes me wonder if it should be a QML test for the list component. It would be ok to make it an autopilot test only if the QML test is too complex.

Diff comments:

> === modified file 'tests/autopilot/address_book_app/emulators/contact_list_page.py'
> --- tests/autopilot/address_book_app/emulators/contact_list_page.py	2014-03-18 12:37:36 +0000
> +++ tests/autopilot/address_book_app/emulators/contact_list_page.py	2014-03-18 12:37:36 +0000
> @@ -214,3 +214,30 @@
>          except StateNotFoundError:
>              logger.error("Favorite contact {} not found.".format(name))
>              return False
> +
> +    def get_header_by_letter(self, header_letter):

This should be internal. Please rename it to _get_header_by_letter.

> +        """Get the index header by its letter string (text), e.g. 'B'
> +
> +        :param header_letter: Index letter text, e.g. 'B'
> +        :raises StateNotFoundError: if header letter not found
> +        """
> +        return self.wait_select_single('Header',
> +                                       text=header_letter,
> +                                       visible=True)
> +        
> +    def click_header_by_letter(self, header_letter):
> +        """Click the index header by its letter string (text), e.g. 'B'
> +
> +        :param header_letter: Index letter text, e.g. 'B'
> +        :raises StateNotFoundError: if header letter not found
> +        """
> +        header = self.get_header_by_letter(header_letter)
> +        self.pointing_device.click_object(header)
> +
> +    def is_expanded(self):
> +        """Is the ContactListView expanded?
> +
> +        :raises StateNotFoundError: if the ContactListView is not found
> +        """
> +        contact_list_view = self.wait_select_single('ContactListView')
> +        return contact_list_view.expanded == True
> 
> === added file 'tests/autopilot/address_book_app/tests/test_collapse.py'
> --- tests/autopilot/address_book_app/tests/test_collapse.py	1970-01-01 00:00:00 +0000
> +++ tests/autopilot/address_book_app/tests/test_collapse.py	2014-03-18 12:37:36 +0000
> @@ -0,0 +1,48 @@
> +# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
> +# Copyright 2014 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.
> +
> +"""Tests of collapsing/expanding address-book-app contacts list"""
> +
> +from __future__ import absolute_import
> +
> +from autopilot.introspection.dbus import StateNotFoundError
> +from autopilot.matchers import Eventually
> +from testtools.matchers import Equals
> +
> +from address_book_app.tests import DummyServiceBackendSetup
> +from address_book_app.emulators.contact_list_page import ContactListPage
> +
> +import time
> +
> +
> +class TestCollapse(DummyServiceBackendSetup):
> +    """Test for collapsing and expanding the contacts list"""
> +
> +    def setUp(self):
> +        super(TestCollapse, self).setUp()
> +
> +    def test_contract_and_expand_list(self):
> +        """Ensure contacts list can be expanded and collapsed."""
> +        contact_list_page = self.main_window.get_contact_list_page()
> +        contact_list_page.get_contact_by_name('Zack Constantine')

It's not clear what is this step for. You get the contact but never use it.

> +        contact_list_page.click_header_by_letter('T')
> +        # bit of a wait here

I think it would be better to move the wait inside the click method.

> +        self.assertThat(lambda: contact_list_page.is_expanded(),
> +                        Eventually(Equals(False)))
> +        # just also verify that contacts are visible (one is enough)

I think it would be better to make a method: are_contacts_visible()

> +        self.assertRaises(
> +            StateNotFoundError,
> +            lambda : contact_list_page.get_contact_by_name('Zack Constantine'))
> +        contact_list_page.click_header_by_letter('Z')
> +        self.assertThat(lambda: contact_list_page.is_expanded(),

You don't need the lambda. Just pass the callable:
contact_list_page.is_expanded, without the parenthesis.

> +                        Eventually(Equals(True)))
> +
> +        # Using a 1 second sleep here because currently the .expanded
> +        # property is not reliable
> +        time.sleep(1)

Please file a bug about this.

> +        contact_list_page.get_contact_by_name('teste teste2')
> +        contact_list_page.get_contact_by_name('Zack Constantine')

These two statements are not clear. If what you want is to check that the two contacts are present, it would be better to:

contacts = contact_list_page.get_contacts()
self.assertIn('teste teste2', contacts)
self.assertIn('Zack Constantine', contacts)

> 
> === modified file 'tests/data/vcard.vcf'
> --- tests/data/vcard.vcf	2013-12-12 15:55:57 +0000
> +++ tests/data/vcard.vcf	2014-03-18 12:37:36 +0000
> @@ -28,3 +28,13 @@
>  TEL:111111
>  CATEGORIES:T
>  END:VCARD
> +BEGIN:VCARD
> +VERSION:3.0
> +UID:0d753ce1005dde92f69e4ddb62222240639113a3
> +X-QTPROJECT-EXTENDED-DETAIL:CLIENTPIDMAP;[\n    "1"\n]\n
> +N:Constantine;Zack;;;
> +FN:Zack Constantine
> +X-QTPROJECT-FAVORITE:false;0
> +TEL:98878
> +CATEGORIES:T
> +END:VCARD
> 


-- 
https://code.launchpad.net/~om26er/address-book-app/test_collapse/+merge/211505
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~om26er/address-book-app/test_collapse into lp:address-book-app.