Merge lp:~renatofilho/address-book-app/fix-test_import_from_sim into lp:address-book-app

Proposed by Renato Araujo Oliveira Filho
Status: Superseded
Proposed branch: lp:~renatofilho/address-book-app/fix-test_import_from_sim
Merge into: lp:address-book-app
Prerequisite: lp:~renatofilho/address-book-app/split-app
Diff against target: 40 lines (+8/-7)
2 files modified
tests/autopilot/address_book_app/pages/_contact_list_page.py (+6/-0)
tests/autopilot/address_book_app/tests/test_import_from_sim.py (+2/-7)
To merge this branch: bzr merge lp:~renatofilho/address-book-app/fix-test_import_from_sim
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Omer Akram Pending
Leo Arias Pending
Review via email: mp+258932@code.launchpad.net

This proposal supersedes a proposal from 2015-04-14.

This proposal has been superseded by a proposal from 2015-05-12.

Commit message

Updated autopilot test:

Check if "import from sim button" is invisible when all SIM cards get removed.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Omer Akram (om26er) wrote : Posted in a previous version of this proposal

Now that the test behavior have been changed, please change the test name to something like:

test_import_item_disabled_without_sim_card()

review: Needs Fixing
Revision history for this message
Omer Akram (om26er) wrote : Posted in a previous version of this proposal

Other than that code looks good to me.

Revision history for this message
Omer Akram (om26er) wrote : Posted in a previous version of this proposal

Looks good now.

review: Approve
Revision history for this message
Leo Arias (elopio) wrote : Posted in a previous version of this proposal

Every failed test is an opportunity to clean up the test and make it more readable.
Take a look at:
https://code.launchpad.net/~canonical-platform-qa/address-book-app/fix-test_import_from_sim-cleanup/+merge/257931

In there, I removed the initial check because it has nothing to do with the name of the test. It should be tested elsewhere that when you open the app, no contacts are shown.

Also, I moved the selection to the page helper object. Every time you have a select single in the test body, there's something wrong. If we move them to the page object it will be easier to read and easier to maintain.

After the cleanup, it seems to me that this is a good candidate to be turned into a QML test. Here we are only checking that a button is not shown if certain conditions are not met. That's purely UI, it has nothing to do with a user story so autopilot seems like a tool too big and slow for it. Do you agree?

Finally, the name of the test still sounds wrong: test_import_item_disabled_without_sim_card
We are not checking that the item is disabled. We are checking that it is not visible.

review: Needs Fixing
Revision history for this message
Leo Arias (elopio) wrote : Posted in a previous version of this proposal

ah, and this might conflicts with federico's flake8 branch:
https://code.launchpad.net/~canonical-platform-qa/address-book-app/flake8-fixes/+merge/256684

It would be safer using that branch as a prerequisite.

Revision history for this message
Leo Arias (elopio) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/autopilot/address_book_app/pages/_contact_list_page.py'
2--- tests/autopilot/address_book_app/pages/_contact_list_page.py 2015-03-05 19:01:00 +0000
3+++ tests/autopilot/address_book_app/pages/_contact_list_page.py 2015-05-12 21:39:39 +0000
4@@ -132,6 +132,12 @@
5 except ubuntuuitoolkit.ToolkitException:
6 return None
7
8+ def is_import_from_sim_button_visible(self):
9+ import_from_sim_button = self.select_single(
10+ 'ContactListButtonDelegate',
11+ objectName='contactListView.importFromSimCardButton')
12+ return import_from_sim_button.visible
13+
14
15 class RemoveContactsDialog(
16 ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase):
17
18=== modified file 'tests/autopilot/address_book_app/tests/test_import_from_sim.py'
19--- tests/autopilot/address_book_app/tests/test_import_from_sim.py 2015-03-06 14:02:19 +0000
20+++ tests/autopilot/address_book_app/tests/test_import_from_sim.py 2015-05-12 21:39:39 +0000
21@@ -77,17 +77,12 @@
22 contacts.remove(contact)
23 self.assertThat(len(contacts), Equals(0))
24
25- def test_import_item_without_sim_card(self):
26+ def test_import_item_invisible_without_sim_card(self):
27 list_page = self.app.main_window.get_contact_list_page()
28
29- # contact list is empty
30- self.assertThat(len(list_page.get_contacts()), Equals(0))
31-
32 # remove all sim cards
33 helpers.remove_phonesim()
34
35- # check if the contact list is empty
36- import_page = self.app.main_window.start_import_contacts()
37 self.assertThat(
38- import_page.hasContacts,
39+ list_page.is_import_from_sim_button_visible,
40 Eventually(Equals(False), timeout=30))

Subscribers

People subscribed via source and target branches