Code review comment for lp:~chris.gagnon/gallery-app/autopilot-fix-flakyness-and-make-work-on-desktop

22 + self.pointing_device = ubuntuuitoolkit.emulators.get_pointing_device()

GalleryUtils already has a pointing_device attribute. You should just call the super __init__()
I would actually prefer not to inherit from something like GalleryUtils, but from an autopilot introspection object, but that probably should be in a later refactor.

35 + def _swipe_setup(self):

Here I don't like that you are adding attributes to the object on a method that's not __init__.
I think it could be clearer something like this:

def swipe_page_left(self, page_number):
    self._swipe_page(page_number, 'left')

def _swipe_page(self, page_number, direction):
    album = self.get_album_view()
    spread = self.get_spread_view()
    album.animationRunning.wait_for(False)

    spread_center_x = spread.globalRect.x + spread.globalRect.w // 2
    spread_center_y = spread.globalRect.y + spread.globalRect.h // 2

    start_x = spread.globalRect.x
    start_y = stop_y = spread_center_y

    if direction == 'left':
        stop_x = spread_center_x
        expected_page_matcher = LessThan
    elif direction == 'right':
        stop_x = spread.globalRect.x
        expected_page_matcher = GreatherThan
    else:
        raise Something.

    self.pointing_device.drag(
        start_x, start_y, stop_x, stop_y)

    album.animationRunning.wait_for(False)
    spread.viewingPage.wait_for(expected_page_matcher(page_number))

88 +class EventsViewException(Exception):

I would prefer if this inherits from a general GalleryAppException. Then you could ignore all the GalleryAppExceptions in some cases.

119 def number_of_photos_in_events(self):
[...]
123 + """Return the number of events"""

I think that docstring is wrong.

514 + def media_view(self):

If you do this as a property, then every time you call it the select_single will be executed. Is there a reason for not doing this on the __init__ like:

self.media_view = self.app.select_single(MediaViewer)

?

In general, this is a big step forward, so +1. I still don't like some things, but they don't come from before your branch.

Like this:

203 + def click_delete_dialog_cancel_button(self):

I don't like it being part of a utils module. There's probably a helper for dialogs on the toolkit, and we should return the dialog object from the method that opens it.

Anyway, thanks a lot for this. It's now a lot more understandable.