address-book-app

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

fix-test_import_from_sim
Merge into trunk

Proposed by Renato Araujo Oliveira Filho on 2015-05-12

Status:

Merged

Approved by:

Gustavo Pichorim Boiko on 2015-05-13

Approved revision:

427

Merged at revision:

435

Proposed branch:

lp://qastaging/~renatofilho/address-book-app/fix-test_import_from_sim

Merge into:

lp://qastaging/address-book-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://qastaging/~renatofilho/address-book-app/fix-test_import_from_sim

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Gustavo Pichorim Boiko (community)			Approve on 2015-05-13
PS Jenkins bot	continuous-integration	2015-05-12	Approve on 2015-05-13
Omer Akram		2015-05-12	Pending
Leo Arias		2015-05-12	Pending
Review via email: mp+258938@code.qastaging.launchpad.net

This proposal supersedes 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.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-14: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/792/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Omer Akram (om26er) wrote on 2015-04-30: 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 on 2015-04-30: 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 on 2015-04-30: Posted in a previous version of this proposal

Looks good now.

review: Approve

Revision history for this message

Leo Arias (elopio) wrote on 2015-04-30: 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 on 2015-04-30: 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) on 2015-04-30: Posted in a previous version of this proposal

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-30: Posted in a previous version of this proposal