Merge lp:~sbaldassin/autopilot/fix_1671155 into lp:autopilot

Proposed by Santiago Baldassin
Status: Merged
Approved by: Richard Huddie
Approved revision: 600
Merged at revision: 593
Proposed branch: lp:~sbaldassin/autopilot/fix_1671155
Merge into: lp:autopilot
Diff against target: 129 lines (+61/-6)
4 files modified
autopilot/display/__init__.py (+1/-1)
autopilot/input/__init__.py (+12/-4)
autopilot/tests/unit/test_display.py (+33/-0)
autopilot/tests/unit/test_input.py (+15/-1)
To merge this branch: bzr merge lp:~sbaldassin/autopilot/fix_1671155
Reviewer Review Type Date Requested Status
Richard Huddie (community) Approve
platform-qa-bot continuous-integration Approve
Heber Parrucci (community) Approve
Review via email: mp+319367@code.launchpad.net

Commit message

Restoring backends order

Description of the change

Restoring the backends order so that existing applications running autopilot test cases in u7 can still work
Fixes: https://bugs.launchpad.net/ubuntu/+source/webbrowser-app/+bug/1671155

To post a comment you must log in.
Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Heber Parrucci (heber013) wrote :

Code LGTM

review: Approve
Revision history for this message
Richard Huddie (rhuddie) wrote :

Confirm the fix is working. But what I have found is that when trying to create a keyboard on unity8, there is about 10 second delay in creating it whilst there is a check to try and get proxy object for maliit process for OSK. If OSK is below UInput in list this doesn't happen. But the OSK would then only be selected if specifically selected by preferred_backend parameter. Perhaps best fix would be to remove the timeout for getting the maliit proxy object so it would fail immediately?

You can reproduce this by doing following on unity8 with this fix installed:
$ python3
>>> from autopilot.input import Keyboard
>>> k = Keyboard().create()

review: Needs Information
Revision history for this message
Santiago Baldassin (sbaldassin) wrote :

Good catch Richard. I haven't been able to reproduce it though. I've added more logging to autopilot and this is what I get

>>> from autopilot.input import Keyboard
>>>
>>>
>>> k = Keyboard().create()
11:41:56.823493: Creating X11
11:41:56.914873: Creating OSK
11:41:56.988823: Creating UInput

The risk that I see is the same I've been pointing out when we decided to change the list order, there are people out there using autopilot and if we changed the default list order, we might impact them.

If someone, either ust, the web browser app or anyone else wants to avoid any failed attempt, they can use the preferred_backend option as you pointed out. In our case...

>>> k = Keyboard().create(preferred_backend='UInput')
11:44:14.934115: Creating UInput

Revision history for this message
Richard Huddie (rhuddie) wrote :

That is strange, you're on unity8? This is the output I get with similar logging, which shows the big gap between OSK and UInput:

>>> from autopilot.input import Keyboard
>>> k = Keyboard().create()
12:15:45.329685 Creating X11
12:15:45.331326 Creating OSK
12:15:56.319388 Creating UInput
>>> type(k)
<class 'autopilot.input._uinput.Keyboard'>

lp:~sbaldassin/autopilot/fix_1671155 updated
594. By Santiago Baldassin

Adding unit tests

Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~sbaldassin/autopilot/fix_1671155 updated
595. By Santiago Baldassin

Fixing flake8

Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~sbaldassin/autopilot/fix_1671155 updated
596. By Santiago Baldassin

Using unittest.mock.patch

Revision history for this message
Richard Huddie (rhuddie) wrote :

The 10 sec delay when creating the keyboard is triggered if you have the ubuntu-keyboard-autopilot helpers installed. If this is the case it will try to get the proxy object for maliit process, which has a 10 sec retry period. With ubuntu-keyboard-autopilot removed it doesn't attempt to create osk, in which case there is no delay.

So we would either need to move osk to bottom of list, in which case you would only get it if you asked for it with preferred_backend parameter, or to remove the timeout period on getting the maliit proxy object to make it fail with no delay.

Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~sbaldassin/autopilot/fix_1671155 updated
597. By Santiago Baldassin

Fixing flake8 errors

Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Approve (continuous-integration)
lp:~sbaldassin/autopilot/fix_1671155 updated
598. By Santiago Baldassin

Adding workaround to avoid delays

Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Richard Huddie (rhuddie) wrote :

Comment below.

review: Needs Fixing
lp:~sbaldassin/autopilot/fix_1671155 updated
599. By Santiago Baldassin

Minor change

Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Richard Huddie (rhuddie) wrote :

Minor fix needed

review: Needs Fixing
lp:~sbaldassin/autopilot/fix_1671155 updated
600. By Santiago Baldassin

Minor fix

Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Richard Huddie (rhuddie) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'autopilot/display/__init__.py'
--- autopilot/display/__init__.py 2016-11-22 13:12:14 +0000
+++ autopilot/display/__init__.py 2017-03-13 14:06:25 +0000
@@ -107,8 +107,8 @@
107 return Display()107 return Display()
108108
109 backends = OrderedDict()109 backends = OrderedDict()
110 backends['X11'] = get_x11_display
110 backends['UPA'] = get_upa_display111 backends['UPA'] = get_upa_display
111 backends['X11'] = get_x11_display
112 return _pick_backend(backends, preferred_backend)112 return _pick_backend(backends, preferred_backend)
113113
114 class BlacklistedDriverError(RuntimeError):114 class BlacklistedDriverError(RuntimeError):
115115
=== modified file 'autopilot/input/__init__.py'
--- autopilot/input/__init__.py 2016-11-22 12:42:52 +0000
+++ autopilot/input/__init__.py 2017-03-13 14:06:25 +0000
@@ -57,6 +57,9 @@
5757
58from collections import OrderedDict58from collections import OrderedDict
59from contextlib import contextmanager59from contextlib import contextmanager
60
61import psutil
62
60from autopilot.input._common import get_center_point63from autopilot.input._common import get_center_point
61from autopilot.utilities import _pick_backend, CleanupRegistered64from autopilot.utilities import _pick_backend, CleanupRegistered
6265
@@ -119,16 +122,21 @@
119122
120 def get_osk_kb():123 def get_osk_kb():
121 try:124 try:
122 from autopilot.input._osk import Keyboard125 maliit = [p for p in
123 return Keyboard()126 psutil.process_iter() if p.name() == 'maliit-server']
127 if maliit:
128 from autopilot.input._osk import Keyboard
129 return Keyboard()
130 else:
131 raise RuntimeError('maliit-server is not running')
124 except ImportError as e:132 except ImportError as e:
125 e.args += ("Unable to import the OSK backend",)133 e.args += ("Unable to import the OSK backend",)
126 raise134 raise
127135
128 backends = OrderedDict()136 backends = OrderedDict()
137 backends['X11'] = get_x11_kb
138 backends['OSK'] = get_osk_kb
129 backends['UInput'] = get_uinput_kb139 backends['UInput'] = get_uinput_kb
130 backends['OSK'] = get_osk_kb
131 backends['X11'] = get_x11_kb
132 return _pick_backend(backends, preferred_backend)140 return _pick_backend(backends, preferred_backend)
133141
134 @contextmanager142 @contextmanager
135143
=== added file 'autopilot/tests/unit/test_display.py'
--- autopilot/tests/unit/test_display.py 1970-01-01 00:00:00 +0000
+++ autopilot/tests/unit/test_display.py 2017-03-13 14:06:25 +0000
@@ -0,0 +1,33 @@
1# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
2#
3# Autopilot Functional Test Tool
4# Copyright (C) 2016 Canonical
5#
6# This program is free software: you can redistribute it and/or modify
7# it under the terms of the GNU General Public License as published by
8# the Free Software Foundation, either version 3 of the License, or
9# (at your option) any later version.
10#
11# This program is distributed in the hope that it will be useful,
12# but WITHOUT ANY WARRANTY; without even the implied warranty of
13# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14# GNU General Public License for more details.
15#
16# You should have received a copy of the GNU General Public License
17# along with this program. If not, see <http://www.gnu.org/licenses/>.
18#
19import unittest
20from unittest.mock import patch
21from autopilot.display import Display
22
23
24class DisplayTestCase(unittest.TestCase):
25
26 @patch('autopilot.display._pick_backend')
27 def test_input_backends_default_order(self, pick_backend):
28 d = Display()
29 d.create()
30
31 backends = list(pick_backend.call_args[0][0].items())
32 self.assertTrue(backends[0][0] == 'X11')
33 self.assertTrue(backends[1][0] == 'UPA')
034
=== modified file 'autopilot/tests/unit/test_input.py'
--- autopilot/tests/unit/test_input.py 2016-06-24 11:13:25 +0000
+++ autopilot/tests/unit/test_input.py 2017-03-13 14:06:25 +0000
@@ -18,6 +18,7 @@
18#18#
1919
20import logging20import logging
21import unittest
2122
22import testscenarios23import testscenarios
23from evdev import ecodes, uinput24from evdev import ecodes, uinput
@@ -30,7 +31,7 @@
30 tests,31 tests,
31 utilities32 utilities
32)33)
33from autopilot.input import _uinput, get_center_point34from autopilot.input import _uinput, get_center_point, Keyboard
3435
3536
36class Empty(object):37class Empty(object):
@@ -1101,3 +1102,16 @@
1101 self.assert_power_button_press_release_emitted_write_and_sync(1102 self.assert_power_button_press_release_emitted_write_and_sync(
1102 device._device.mock_calls1103 device._device.mock_calls
1103 )1104 )
1105
1106
1107class KeyboardTestCase(unittest.TestCase):
1108
1109 @patch('autopilot.input._pick_backend')
1110 def test_input_backends_default_order(self, pick_backend):
1111 k = Keyboard()
1112 k.create()
1113
1114 backends = list(pick_backend.call_args[0][0].items())
1115 self.assertTrue(backends[0][0] == 'X11')
1116 self.assertTrue(backends[1][0] == 'OSK')
1117 self.assertTrue(backends[2][0] == 'UInput')

Subscribers

People subscribed via source and target branches