Code review comment for lp://staging/~tpeeters/ubuntu-ui-toolkit/optIn-tabsDrawer

42 + // FIXME: Currently autopilot can only get visual items, but once it is
43 + // updated to support non-visual items, a QtObject may be used.

Please add a link to http://pad.lv/1273956 so we know what you are referring to. Also, I find it useful to sign the FIXME comments, with something like --timp - 2014-03-19

637 + current_tab = tabs.get_current_tab()
638 + number_of_switches = 0
639 + while not tabs.selectedTabIndex == index:
640 + logger.debug(
641 + 'Current tab index: {0}.'.format(tabs.selectedTabIndex))
642 + if number_of_switches >= number_of_tabs - 1:
643 + # This prevents a loop. But if this error is ever raised, it's
644 + # likely there's a bug on the emulator or on the QML Tab.
645 + raise ToolkitEmulatorException(
646 + 'The tab with index {0} was not selected.'.format(index))
647 + current_tab = self.switch_to_next_tab()
648 + number_of_switches += 1
649 + return current_tab

You should move this into a method. You can call it something like _switch_to_deprecated_tab_by_index

651 + if (index != tabs.selectedTabIndex):
652 + self.get_header().switch_to_tab_by_index(index)
653 + current_tab = tabs.get_current_tab()
654 + return current_tab

This is not a big deal, but it would also make it clearer putting those statements on _switch_to_tab_in_drawer_by_index

668 + return false

Typo. It's False.

677 + try:
678 + tab_bar = self.select_single(TabBar)
679 + except dbus.StateNotFoundError:
680 + raise ToolkitEmulatorException(_NO_TABS_ERROR)
681 + tab_bar.switch_to_next_tab()
682 + self._get_animating().wait_for(False)

Same thing here ^

684 + try:
685 + tabs_drawer_button = self.select_single('AbstractButton', objectName='tabsButton')
686 + except dbus.StateNotFoundError:
687 + raise ToolkitEmulatorException(_NO_TABS_ERROR)
688 + self.pointing_device.click_object(tabs_drawer_button)
689 +
690 + tabs_model_properties = self.select_single('QQuickItem', objectName='tabsModelProperties')
691 + next_tab_index = (tabs_model_properties.selectedIndex + 1) % tabs_model_properties.count
692 +
693 + try:
694 + tab_button = self.get_root_instance().select_single('Standard',
695 + objectName='tabButton'+str(next_tab_index))
696 + except dbus.StateNotFoundError:
697 + raise ToolkitEmulatorException("Tab button {0} not found.".format(next_tab_index))
698 +
699 + self.pointing_device.click_object(tab_button)

and here ^. Please wrap them in something like _switch_to_next_deprecated_tab, and _switch_to_next_tab_in_drawer

4 + try:
685 + tabs_drawer_button = self.select_single('AbstractButton', objectName='tabsButton')
686 + except dbus.StateNotFoundError:
687 + raise ToolkitEmulatorException(_NO_TABS_ERROR)

You can also move this ^

724 + try:
725 + tab_button = self.get_root_instance().select_single('Standard',
726 + objectName='tabButton'+str(index))
727 + except dbus.StateNotFoundError:
728 + raise ToolkitEmulatorException("Tab button {0} not found.".format(index))

and this ^ into a private methods, as you have the same statements twice.

755 +class DeprecatedTabsTestCase(TabsTestCase):

I would prefer to do it with scenarios. Take a look at http://paste.ubuntu.com/7121301/
I haven't tried it, but something like that should work.

702 + def switch_to_tab_by_index(self, index):

You are missing a test for this new method.

And some of your lines are too long. They will fail pep8.