Merge lp:~sil2100/unity/autopilot_ibus_improve into lp:unity

Proposed by Łukasz Zemczak on 2013-02-12
Status: Merged
Approved by: Łukasz Zemczak on 2013-02-25
Approved revision: 3148
Merged at revision: 3169
Proposed branch: lp:~sil2100/unity/autopilot_ibus_improve
Merge into: lp:unity
Diff against target: 164 lines (+96/-10)
1 file modified
tests/autopilot/unity/tests/test_ibus.py (+96/-10)
To merge this branch: bzr merge lp:~sil2100/unity/autopilot_ibus_improve
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing on 2013-02-25
Thomi Richards (community) 2013-02-12 Approve on 2013-02-23
Review via email: mp+147951@code.launchpad.net

Commit Message

Improve IBus autopilot tests.
With these improvements, asian characters do not need to be hardcoded in the tests anymore. Now, the 'expected' values are taken directly from IBus before each test. We're doing it by using a separate input context and storing it for comparison. Using the existing input context was buggy, so this seemed like the best way to go otherwise.
To do this, I had to work-around the new IBus bindings from GIR, since create_input_context() - which was needed, was not introspectable.

Description of the Change

- Problem:

IBus tests are failing frequently. The reason is that we have hard-coded result values for IBus testing, where the results are not always the same - depending on search history and the ibus version. We also cannot easily clear the history.

- Fix:

My proposed fix is: let's poll (query) IBus with the input string, fetch the resulting string, and then start the standard IBus test and check the search entry text against it. We don't want to test IBus accuracy in our integration tests. We want to test if IBus integration works - if when IBus is enabled, that the input fields will have characters coming from IBus. That's why, we just compare if what we get on the search field is what IBus should return.

We do this by creating a new (separate) input context, feed it with the same input and fetch the resulting, committed strings as results. Best way of course would be to simply hook up to the existing input context of nux and the dash and just 'listen in' - this would be the *perfect* solution. Sadly, that didn't seem to be possible, as hooking up on the existing input context's signals did not work. Too bad...

So, this solution seems sufficient. It has some backsides, but in overall it should *finally* fix IBus tests.

Also, well, there are some hacks here too. The problem was - I could not use the old IBus bindings because of the glib version mismatching. The new bindings from GIR, well, those didn't allow introspection of create_input_context method in IBus.Bus(). So, to workaround that, I had to directly do all the things that normally create_input_context() would do. But it works!

- Tests:

N/A

To post a comment you must log in.
Thomi Richards (thomir) wrote :

Hi,

First, when running the tests, I get:

/tmp/autopilot_ibus_improve/tests/autopilot/unity/tests/test_ibus.py:48: PyGIDeprecationWarning: MainLoop is deprecated; use GLib.MainLoop instead
  self._glibloop = GObject.MainLoop()

(autopilot:15226): IBUS-WARNING **: org.freedesktop.IBus.InputContext.GetEngine: GDBus.Error:org.freedesktop.IBus.Error.NoEngine: Input context does not have engine.
/tmp/autopilot_ibus_improve/tests/autopilot/unity/tests/test_ibus.py:79: PyGIDeprecationWarning: timeout_add_seconds is deprecated; use GLib.timeout_add_seconds instead
  GObject.timeout_add_seconds(10, lambda *args: self._glibloop.quit())

So we should fix that. Seconf, they enter an endless loop where they constantly print:

(autopilot:15226): IBUS-WARNING **: org.freedesktop.IBus.InputContext.GetEngine: GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such interface `org.freedesktop.IBus.InputContext' on object at path /org/freedesktop/IBus/InputContext_58

so I can't actually run the tests to verify that they work as expected.

review: Needs Fixing
Łukasz Zemczak (sil2100) wrote :

Thanks for checking it out! I will look on what's wrong tomorrow, maybe it has something to do that I'm using quantal for testing - maybe raring has different things going on.

Łukasz Zemczak (sil2100) wrote :

Actually! I even came up now with a better idea of dealing with this, I'll try it out tomorrow in the morning (I just ran out of my bed because of this idea ;) ). Maybe we'll be able to deal it without having to create a new input context!

Łukasz Zemczak (sil2100) wrote :

Fixed the GObject thing. Sorry about that!

Ok, so, I looked at the existing code...

Thomi, are you sure you have a clean, working ibus environment? All tests work correctly for me and Francis, both on quantal and raring. I also checked the differences between the ibus in quantal and raring and nothing changed regarding this matter. So I have no idea why you get the errors. Is your ibus installation correct? Are the engines installed?

Anyway, even in the working case one (autopilot:15226): IBUS-WARNING **: org.freedesktop.IBus.InputContext.GetEngine warning will be noticable most of the time. This is because I didn't want to do any time.sleep() waits until the engine is set - and IBus prints a warning when a get_engine() is done with no engine set. We use that to make sure that the engine finally gets set.

Can you re-try on a clean raring or quantal system with ibus installed?

Łukasz Zemczak (sil2100) wrote :

Ok, so, after the fixes I made now, anthy tests pass properly every time. With hangul, well, it's still broken. I think we'll have to use hard-coded values for hangul still... :( That's because hangul seems to be written in a strange way that I don't get any text (the argument is empty) during update-preedit-text signals. Still asking the IBus guys about it.

Thomi Richards (thomir) :
review: Approve
Francis Ginther (fginther) wrote :

Build failed due to a launchpad service issue. Re-approving.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/autopilot/unity/tests/test_ibus.py'
2--- tests/autopilot/unity/tests/test_ibus.py 2013-02-04 19:54:47 +0000
3+++ tests/autopilot/unity/tests/test_ibus.py 2013-02-22 19:04:23 +0000
4@@ -1,6 +1,6 @@
5 # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
6 # Copyright 2012 Canonical
7-# Author: Thomi Richards, Martin Mrazik
8+# Authors: Thomi Richards, Martin Mrazik, Łukasz 'sil2100' Zemczak
9 #
10 # This program is free software: you can redistribute it and/or modify it
11 # under the terms of the GNU General Public License version 3, as published
12@@ -23,6 +23,84 @@
13
14 from unity.tests import UnityTestCase
15
16+from gi.repository import GLib
17+from gi.repository import IBus
18+import time
19+import dbus
20+import threading
21+
22+
23+# See lp:ibus-query
24+class IBusQuery:
25+ """A simple class allowing string queries to the IBus engine."""
26+
27+ def __init__(self):
28+ self._bus = IBus.Bus()
29+ self._dbusconn = dbus.connection.Connection(IBus.get_address())
30+
31+ # XXX: the new IBus bindings do not export create_input_context for
32+ # introspection. This is troublesome - so, to workaround this problem
33+ # we're directly fetching a new input context manually
34+ ibus_obj = self._dbusconn.get_object(IBus.SERVICE_IBUS, IBus.PATH_IBUS)
35+ self._test = dbus.Interface(ibus_obj, dbus_interface="org.freedesktop.IBus")
36+ path = self._test.CreateInputContext("IBusQuery")
37+ self._context = IBus.InputContext.new(path, self._bus.get_connection(), None)
38+
39+ self._glibloop = GLib.MainLoop()
40+
41+ self._context.connect("commit-text", self.__commit_text_cb)
42+ self._context.connect("update-preedit-text", self.__update_preedit_cb)
43+ self._context.connect("disabled", self.__disabled_cb)
44+
45+ self._context.set_capabilities (9)
46+
47+ def __commit_text_cb(self, context, text):
48+ self.result += text.text
49+ self._preedit = ''
50+
51+ def __update_preedit_cb(self, context, text, cursor_pos, visible):
52+ if visible:
53+ self._preedit = text.text
54+
55+ def __disabled_cb(self, a):
56+ self.result += self._preedit
57+ self._glibloop.quit()
58+
59+ def __abort(self):
60+ self._abort = True
61+
62+ def poll(self, engine, ibus_input):
63+ if len(ibus_input) <= 0:
64+ return None
65+
66+ self.result = ''
67+ self._preedit = ''
68+ self._context.focus_in()
69+ self._context.set_engine(engine)
70+
71+ # Timeout in case of the engine not being installed
72+ self._abort = False
73+ timeout = threading.Timer(4.0, self.__abort)
74+ timeout.start()
75+ while self._context.get_engine() is None:
76+ if self._abort is True:
77+ print "Error! Could not set the engine correctly."
78+ return None
79+ continue
80+ timeout.cancel()
81+
82+ for c in ibus_input:
83+ self._context.process_key_event(ord(c), 0, 0)
84+
85+ self._context.set_engine('')
86+ self._context.focus_out()
87+
88+ GLib.timeout_add_seconds(5, lambda *args: self._glibloop.quit())
89+ self._glibloop.run()
90+
91+ return unicode(self.result, "UTF-8")
92+
93+
94
95 class IBusTests(UnityTestCase):
96 """Base class for IBus tests."""
97@@ -30,6 +108,7 @@
98 def setUp(self):
99 super(IBusTests, self).setUp()
100 self.set_correct_ibus_trigger_keys()
101+ self._ibus_query = None
102
103 def set_correct_ibus_trigger_keys(self):
104 """Set the correct keys to trigger IBus.
105@@ -68,6 +147,8 @@
106
107 """
108 available_engines = get_available_input_engines()
109+ if self._ibus_query is None:
110+ self._ibus_query = IBusQuery()
111 if engine_name in available_engines:
112 if get_active_input_engines() != [engine_name]:
113 IBusTests._old_engines = set_active_engines([engine_name])
114@@ -98,6 +179,11 @@
115
116 def do_ibus_test(self):
117 """Do the basic IBus test on self.widget using self.input and self.result."""
118+ try:
119+ result = self.result
120+ except:
121+ result = self._ibus_query.poll(self.engine_name, self.input)
122+
123 widget = getattr(self.unity, self.widget)
124 widget.ensure_visible()
125 self.addCleanup(widget.ensure_hidden)
126@@ -107,7 +193,7 @@
127 if commit_key:
128 self.keyboard.press_and_release(commit_key)
129 self.deactivate_ibus(widget.searchbar)
130- self.assertThat(widget.search_string, Eventually(Equals(self.result)))
131+ self.assertThat(widget.search_string, Eventually(Equals(result)))
132
133
134
135@@ -119,11 +205,11 @@
136 scenarios = multiply_scenarios(
137 IBusWidgetScenariodTests.scenarios,
138 [
139- ('basic', {'input': 'abc1', 'result': u'\u963f\u5e03\u4ece'}),
140- ('photo', {'input': 'zhaopian ', 'result': u'\u7167\u7247'}),
141- ('internet', {'input': 'hulianwang ', 'result': u'\u4e92\u8054\u7f51'}),
142- ('disk', {'input': 'cipan ', 'result': u'\u78c1\u76d8'}),
143- ('disk_management', {'input': 'cipan guanli ', 'result': u'\u78c1\u76d8\u7ba1\u7406'}),
144+ ('basic', {'input': 'abc1'}),
145+ ('photo', {'input': 'zhaopian '}),
146+ ('internet', {'input': 'hulianwang '}),
147+ ('disk', {'input': 'cipan '}),
148+ ('disk_management', {'input': 'cipan guanli '}),
149 ]
150 )
151
152@@ -165,9 +251,9 @@
153 scenarios = multiply_scenarios(
154 IBusWidgetScenariodTests.scenarios,
155 [
156- ('system', {'input': 'shisutemu ', 'result': u'\u30b7\u30b9\u30c6\u30e0'}),
157- ('game', {'input': 'ge-mu ', 'result': u'\u30b2\u30fc\u30e0'}),
158- ('user', {'input': 'yu-za- ', 'result': u'\u30e6\u30fc\u30b6\u30fc'}),
159+ ('system', {'input': 'shisutemu '}),
160+ ('game', {'input': 'ge-mu '}),
161+ ('user', {'input': 'yu-za- '}),
162 ],
163 [
164 ('commit_j', {'commit_key': 'Ctrl+j'}),