Code review comment for lp:~renatofilho/ubuntu-ui-toolkit/fix-1236464

Revision history for this message
Leo Arias (elopio) wrote :

<elopio> renato_: I think it would be enough to test one of the objects that inherit from SwipeToDeleteTestCase, and then to test that all the emulators you added inherit from SwipeToDeleteTestCase.
<elopio> and that second test would be a little dumb, so I would just leave either the empty or the standard item on your test.
<elopio> renato_: every time you do select_single, and continue on the next line, you have a pep8 error there because of the extra space.
<elopio> so jenkins is going to reject your branch for that.
<elopio> renato_: you are missing some tests to cover all the possible flows:
<elopio> test_delete_item -> with default direction value
<elopio> test_delete_item_swiping_to_the_left
<elopio> test_delete_non_removable_item
<elopio> test_confirm_removal_when_item_was_not_swiped
<elopio> renato_: and I don't like this: 109␉+ self.implicitHeight.wait_for(0)
<elopio> if the dialog doesn't have a visible property that works, or an opened property or something like that, I think we should fix it to be
<elopio> self.visible.wait_for(False)
<elopio> I say "we" as in somebody who know how to do it, not me :)

Thank you for working on this!

review: Needs Fixing (code review)

« Back to merge proposal