[Merge] lp:~canonical-platform-qa/address-book-app/remove-teardown-ap-tests into lp:address-book-app

Vincent Ladeuil vila at canonical.com
Tue May 12 07:01:28 UTC 2015


Review: Needs Information

See inline question.

Diff comments:

> === modified file 'tests/autopilot/address_book_app/tests/__init__.py'
> --- tests/autopilot/address_book_app/tests/__init__.py	2015-02-19 07:02:09 +0000
> +++ tests/autopilot/address_book_app/tests/__init__.py	2015-05-12 05:19:34 +0000
> @@ -16,6 +16,8 @@
>  
>  """address-book-app autopilot tests."""
>  
> +import fixtures
> +import logging
>  import os
>  import time
>  import subprocess
> @@ -28,6 +30,26 @@
>  import address_book_app
>  from ubuntuuitoolkit import emulators as toolkit_emulators
>  
> +logger = logging.getLogger(__name__)
> +
> +
> +class DisableOnScreenKeyboard(fixtures.Fixture):
> +    """Disable the OSK for the time of the test (Does nothing on desktop)."""
> +
> +    def setUp(self):
> +        super().setUp()
> +        if model() != 'Desktop':
> +            self.toggle_keyboard(state='stop')
> +            self.addCleanup(self.toggle_keyboard, state='start')
> +
> +    def toggle_keyboard(self, state):
> +        try:
> +            subprocess.check_call(['/sbin/initctl', state, 'maliit-server'])
> +        except subprocess.CalledProcessError:
> +            logger.error(
> +                'Failed to toggle state of keyboard to: {}'.format(state))

IIUC, this is the change introduced by this MP: the error is logged. Can you make it more verbose by grabbing details about why the call failed ?

> +            raise
> +
>  
>  class AddressBookAppTestCase(AutopilotTestCase):
>      """A common test case class that provides several useful methods for
> @@ -45,9 +67,7 @@
>          self.pointing_device = toolkit_emulators.get_pointing_device()
>          super(AddressBookAppTestCase, self).setUp()
>  
> -        # stop vkb
> -        if model() != "Desktop":
> -            subprocess.check_call(["/sbin/initctl", "stop", "maliit-server"])
> +        self.useFixture(DisableOnScreenKeyboard())
>  
>          if 'AUTOPILOT_APP' in os.environ:
>              self.app_bin = os.environ['AUTOPILOT_APP']
> @@ -86,13 +106,6 @@
>          AddressBookAppTestCase.ARGS = []
>          self.main_window.visible.wait_for(True)
>  
> -    def tearDown(self):
> -        super(AddressBookAppTestCase, self).tearDown()
> -
> -        # start the vkb
> -        if model() != "Desktop":
> -            subprocess.check_call(["/sbin/initctl", "start", "maliit-server"])
> -
>      def launch_test_local(self):
>          return self.launch_test_application(
>              self.app_bin,
> 


-- 
https://code.launchpad.net/~canonical-platform-qa/address-book-app/remove-teardown-ap-tests/+merge/258829
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~canonical-platform-qa/address-book-app/remove-teardown-ap-tests into lp:address-book-app.



More information about the Ubuntu-reviews mailing list