Merge lp:~charlesk/indicator-display/lp-1572545-prompt-in-lockscreen into lp:indicator-display/15.10
- lp-1572545-prompt-in-lockscreen
- Merge into trunk.15.10
Status: | Superseded |
---|---|
Proposed branch: | lp:~charlesk/indicator-display/lp-1572545-prompt-in-lockscreen |
Merge into: | lp:indicator-display/15.10 |
Diff against target: |
867 lines (+446/-155) 12 files modified
src/greeter.cpp (+130/-81) src/greeter.h (+8/-2) src/usb-manager.cpp (+59/-46) src/usb-snap.cpp (+1/-0) tests/integration/usb-manager-test.cpp (+11/-5) tests/unit/CMakeLists.txt (+5/-0) tests/unit/greeter-test.cpp (+159/-0) tests/unit/usb-snap-test.cpp (+3/-8) tests/utils/adbd-server.h (+9/-11) tests/utils/gtest-print-helpers.h (+18/-0) tests/utils/mock-greeter.h (+2/-2) tests/utils/mock-unity-greeter.py (+41/-0) |
To merge this branch: | bzr merge lp:~charlesk/indicator-display/lp-1572545-prompt-in-lockscreen |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Charles Kerr (community) | Approve | ||
Antti Kaijanmäki (community) | Approve | ||
PS Jenkins bot (community) | continuous-integration | Needs Fixing | |
Review via email: mp+292544@code.launchpad.net |
This proposal has been superseded by a proposal from 2016-09-22.
Commit message
Fix timing issue when talking to com.canonical.
Description of the change
Fix timing issue when talking to com.canonical.
Old behavior: watch for propertychanged events and ask it the value of the IsActive property.
New behavior: watch for propertychanged events and ask it the value of the IsActive property whenever we see the greeter appear on the bus. This way we'll pick up the property value even if the greeter appears on the bus after indicator-display running.
PS Jenkins bot (ps-jenkins) wrote : | # |
Antti Kaijanmäki (kaijanmaki) wrote : | # |
LGTM.
Antti Kaijanmäki (kaijanmaki) wrote : | # |
LGTM.
- 38. By Charles Kerr
-
increase timeout interval on usb manager integration test
Charles Kerr (charlesk) : | # |
- 39. By Charles Kerr
-
don't clear the snap decision when the greeter changes state
- 40. By Charles Kerr
-
always send a response to adb even in edge cases; e.g. send an auto-NO if the usb connection is broken while the prompt is up
- 41. By Charles Kerr
-
sync UsbManagerFixtu
re::USBDisconne ctedDuringPromp t to r40, to expect a 'no' each time there's a usb disconnect
Unmerged revisions
Preview Diff
1 | === modified file 'src/greeter.cpp' |
2 | --- src/greeter.cpp 2016-03-22 13:15:38 +0000 |
3 | +++ src/greeter.cpp 2016-05-04 00:04:00 +0000 |
4 | @@ -26,117 +26,166 @@ |
5 | { |
6 | public: |
7 | |
8 | - Impl(): |
9 | - m_cancellable{g_cancellable_new()} |
10 | - { |
11 | - g_bus_get(G_BUS_TYPE_SESSION, m_cancellable, on_bus_ready_static, this); |
12 | - } |
13 | - |
14 | - ~Impl() |
15 | - { |
16 | - g_cancellable_cancel(m_cancellable); |
17 | - g_clear_object(&m_cancellable); |
18 | - |
19 | - if (m_subscription_id != 0) |
20 | - g_dbus_connection_signal_unsubscribe (m_bus, m_subscription_id); |
21 | - |
22 | - g_clear_object(&m_bus); |
23 | - } |
24 | - |
25 | - core::Property<bool>& is_active() |
26 | - { |
27 | - return m_is_active; |
28 | + Impl() |
29 | + { |
30 | + m_cancellable.reset( |
31 | + g_cancellable_new(), |
32 | + [](GCancellable* o){ |
33 | + g_cancellable_cancel(o); |
34 | + g_clear_object(&o); |
35 | + } |
36 | + ); |
37 | + |
38 | + g_bus_get(G_BUS_TYPE_SESSION, m_cancellable.get(), on_bus_ready, this); |
39 | + } |
40 | + |
41 | + ~Impl() =default; |
42 | + |
43 | + core::Property<State>& state() |
44 | + { |
45 | + return m_state; |
46 | } |
47 | |
48 | private: |
49 | |
50 | - static void on_bus_ready_static(GObject* /*source*/, GAsyncResult* res, gpointer gself) |
51 | + void set_state(const State& state) |
52 | + { |
53 | + m_state.set(state); |
54 | + } |
55 | + |
56 | + static void on_bus_ready( |
57 | + GObject* /*source*/, |
58 | + GAsyncResult* res, |
59 | + gpointer gself) |
60 | { |
61 | GError* error {}; |
62 | - auto bus = g_bus_get_finish (res, &error); |
63 | - if (error != nullptr) { |
64 | + auto bus = g_bus_get_finish(res, &error); |
65 | + if (error != nullptr) |
66 | + { |
67 | if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) |
68 | - g_warning("UsbSnap: Error getting session bus: %s", error->message); |
69 | + g_warning("Greeter: Error getting bus: %s", error->message); |
70 | g_clear_error(&error); |
71 | - } else { |
72 | - static_cast<Impl*>(gself)->on_bus_ready(bus); |
73 | - } |
74 | - g_clear_object(&bus); |
75 | - } |
76 | - |
77 | - void on_bus_ready(GDBusConnection* bus) |
78 | - { |
79 | - m_bus = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(bus))); |
80 | - |
81 | - g_dbus_connection_call(m_bus, |
82 | - DBusNames::UnityGreeter::NAME, |
83 | - DBusNames::UnityGreeter::PATH, |
84 | - DBusNames::Properties::INTERFACE, |
85 | - "Get", |
86 | - g_variant_new("(ss)", DBusNames::UnityGreeter::INTERFACE, "IsActive"), |
87 | - G_VARIANT_TYPE("(v)"), |
88 | - G_DBUS_CALL_FLAGS_NONE, |
89 | - -1, |
90 | - m_cancellable, |
91 | - on_get_is_active_ready, |
92 | - this); |
93 | - |
94 | - m_subscription_id = g_dbus_connection_signal_subscribe(m_bus, |
95 | - DBusNames::UnityGreeter::NAME, |
96 | - DBusNames::Properties::INTERFACE, |
97 | - DBusNames::Properties::PropertiesChanged::NAME, |
98 | - DBusNames::UnityGreeter::PATH, |
99 | - DBusNames::UnityGreeter::INTERFACE, |
100 | - G_DBUS_SIGNAL_FLAGS_NONE, |
101 | - on_properties_changed_signal, |
102 | - this, |
103 | - nullptr); |
104 | - } |
105 | - |
106 | - static void on_get_is_active_ready(GObject* source, GAsyncResult* res, gpointer gself) |
107 | + } |
108 | + else |
109 | + { |
110 | + auto self = static_cast<Impl*>(gself); |
111 | + |
112 | + const auto watcher_id = g_bus_watch_name_on_connection( |
113 | + bus, |
114 | + DBusNames::UnityGreeter::NAME, |
115 | + G_BUS_NAME_WATCHER_FLAGS_AUTO_START, |
116 | + on_greeter_appeared, |
117 | + on_greeter_vanished, |
118 | + gself, |
119 | + nullptr); |
120 | + |
121 | + const auto subscription_id = g_dbus_connection_signal_subscribe( |
122 | + bus, |
123 | + DBusNames::UnityGreeter::NAME, |
124 | + DBusNames::Properties::INTERFACE, |
125 | + DBusNames::Properties::PropertiesChanged::NAME, |
126 | + DBusNames::UnityGreeter::PATH, |
127 | + DBusNames::UnityGreeter::INTERFACE, |
128 | + G_DBUS_SIGNAL_FLAGS_NONE, |
129 | + on_properties_changed_signal, |
130 | + gself, |
131 | + nullptr); |
132 | + |
133 | + self->m_bus.reset( |
134 | + bus, |
135 | + [watcher_id, subscription_id](GDBusConnection* o){ |
136 | + g_bus_unwatch_name(watcher_id); |
137 | + g_dbus_connection_signal_unsubscribe(o, subscription_id); |
138 | + g_clear_object(&o); |
139 | + } |
140 | + ); |
141 | + } |
142 | + } |
143 | + |
144 | + static void on_greeter_appeared( |
145 | + GDBusConnection* bus, |
146 | + const char* /*name*/, |
147 | + const char* name_owner, |
148 | + gpointer gself) |
149 | + { |
150 | + auto self = static_cast<Impl*>(gself); |
151 | + |
152 | + self->m_owner = name_owner; |
153 | + |
154 | + g_dbus_connection_call( |
155 | + bus, |
156 | + DBusNames::UnityGreeter::NAME, |
157 | + DBusNames::UnityGreeter::PATH, |
158 | + DBusNames::Properties::INTERFACE, |
159 | + "Get", |
160 | + g_variant_new("(ss)", DBusNames::UnityGreeter::INTERFACE, "IsActive"), |
161 | + G_VARIANT_TYPE("(v)"), |
162 | + G_DBUS_CALL_FLAGS_NONE, |
163 | + -1, |
164 | + self->m_cancellable.get(), |
165 | + on_get_is_active_ready, |
166 | + gself); |
167 | + } |
168 | + |
169 | + static void on_greeter_vanished( |
170 | + GDBusConnection* /*bus*/, |
171 | + const char* /*name*/, |
172 | + gpointer gself) |
173 | + { |
174 | + auto self = static_cast<Impl*>(gself); |
175 | + |
176 | + self->m_owner.clear(); |
177 | + self->set_state(State::UNAVAILABLE); |
178 | + } |
179 | + |
180 | + static void on_get_is_active_ready( |
181 | + GObject* source, |
182 | + GAsyncResult* res, |
183 | + gpointer gself) |
184 | { |
185 | GError* error {}; |
186 | auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error); |
187 | if (error != nullptr) { |
188 | if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) |
189 | - g_warning("UsbSnap: Error getting session bus: %s", error->message); |
190 | + g_warning("Greeter: Error getting IsActive property: %s", error->message); |
191 | g_clear_error(&error); |
192 | } else { |
193 | GVariant* is_active {}; |
194 | g_variant_get_child(v, 0, "v", &is_active); |
195 | - static_cast<Impl*>(gself)->m_is_active.set(g_variant_get_boolean(is_active)); |
196 | + static_cast<Impl*>(gself)->set_state(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE); |
197 | g_clear_pointer(&is_active, g_variant_unref); |
198 | } |
199 | g_clear_pointer(&v, g_variant_unref); |
200 | } |
201 | |
202 | - static void on_properties_changed_signal(GDBusConnection* /*connection*/, |
203 | - const gchar* /*sender_name*/, |
204 | - const gchar* object_path, |
205 | - const gchar* interface_name, |
206 | - const gchar* signal_name, |
207 | - GVariant* parameters, |
208 | - gpointer gself) |
209 | + static void on_properties_changed_signal( |
210 | + GDBusConnection* /*bus*/, |
211 | + const gchar* sender_name, |
212 | + const gchar* object_path, |
213 | + const gchar* interface_name, |
214 | + const gchar* signal_name, |
215 | + GVariant* parameters, |
216 | + gpointer gself) |
217 | { |
218 | + auto self = static_cast<Impl*>(gself); |
219 | + |
220 | + g_return_if_fail(!g_strcmp0(sender_name, self->m_owner.c_str())); |
221 | g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH)); |
222 | g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Properties::INTERFACE)); |
223 | g_return_if_fail(!g_strcmp0(signal_name, DBusNames::Properties::PropertiesChanged::NAME)); |
224 | g_return_if_fail(g_variant_is_of_type(parameters, G_VARIANT_TYPE(DBusNames::Properties::PropertiesChanged::ARGS_VARIANT_TYPE))); |
225 | |
226 | - auto v = g_variant_get_child_value (parameters, 1); |
227 | + auto v = g_variant_get_child_value(parameters, 1); |
228 | gboolean is_active {}; |
229 | if (g_variant_lookup(v, "IsActive", "b", &is_active)) |
230 | - { |
231 | - g_debug("%s is_active changed to %d", G_STRLOC, int(is_active)); |
232 | - static_cast<Impl*>(gself)->m_is_active.set(is_active); |
233 | - } |
234 | + self->set_state(is_active ? State::ACTIVE : State::INACTIVE); |
235 | g_clear_pointer(&v, g_variant_unref); |
236 | } |
237 | |
238 | - core::Property<bool> m_is_active; |
239 | - GCancellable* m_cancellable {}; |
240 | - GDBusConnection* m_bus {}; |
241 | - unsigned int m_subscription_id {}; |
242 | + core::Property<State> m_state {State::UNAVAILABLE}; |
243 | + std::shared_ptr<GCancellable> m_cancellable; |
244 | + std::shared_ptr<GDBusConnection> m_bus; |
245 | + std::string m_owner; |
246 | }; |
247 | |
248 | /*** |
249 | @@ -154,8 +203,8 @@ |
250 | |
251 | UnityGreeter::~UnityGreeter() =default; |
252 | |
253 | -core::Property<bool>& |
254 | -UnityGreeter::is_active() |
255 | +core::Property<Greeter::State>& |
256 | +UnityGreeter::state() |
257 | { |
258 | - return impl->is_active(); |
259 | + return impl->state(); |
260 | } |
261 | |
262 | === modified file 'src/greeter.h' |
263 | --- src/greeter.h 2016-03-21 23:15:05 +0000 |
264 | +++ src/greeter.h 2016-05-04 00:04:00 +0000 |
265 | @@ -29,7 +29,13 @@ |
266 | public: |
267 | Greeter(); |
268 | virtual ~Greeter(); |
269 | - virtual core::Property<bool>& is_active() =0; |
270 | + |
271 | + enum class State { UNAVAILABLE, INACTIVE, ACTIVE }; |
272 | +static inline const char* state_str(const State& state) { |
273 | + static constexpr char const * state_str[] = { "Unavailable", "Inactive", "Active" }; |
274 | + return state_str[int(state)]; |
275 | +} |
276 | + virtual core::Property<State>& state() =0; |
277 | }; |
278 | |
279 | |
280 | @@ -38,7 +44,7 @@ |
281 | public: |
282 | UnityGreeter(); |
283 | virtual ~UnityGreeter(); |
284 | - core::Property<bool>& is_active() override; |
285 | + core::Property<State>& state() override; |
286 | |
287 | protected: |
288 | class Impl; |
289 | |
290 | === modified file 'src/usb-manager.cpp' |
291 | --- src/usb-manager.cpp 2016-03-23 19:44:23 +0000 |
292 | +++ src/usb-manager.cpp 2016-05-04 00:04:00 +0000 |
293 | @@ -46,72 +46,84 @@ |
294 | m_greeter{greeter} |
295 | { |
296 | m_usb_monitor->on_usb_disconnected().connect([this](const std::string& /*usb_name*/) { |
297 | - restart(); |
298 | - }); |
299 | - |
300 | - m_greeter->is_active().changed().connect([this](bool /*is_active*/) { |
301 | - maybe_snap(); |
302 | - }); |
303 | - |
304 | - restart(); |
305 | + m_req.reset(); |
306 | + }); |
307 | + |
308 | + m_greeter->state().changed().connect([this](const Greeter::State& state) { |
309 | + if (state == Greeter::State::INACTIVE) |
310 | + maybe_snap(); |
311 | + else |
312 | + stop_snap(); |
313 | + }); |
314 | + |
315 | + // create a new adbd client |
316 | + m_adbd_client.reset(new GAdbdClient{m_socket_path}); |
317 | + m_adbd_client->on_pk_request().connect( |
318 | + [this](const AdbdClient::PKRequest& req) { |
319 | + g_debug("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str()); |
320 | + |
321 | + m_response = AdbdClient::PKResponse::DENY; // set the fallback response |
322 | + m_req.reset( |
323 | + new AdbdClient::PKRequest(req), |
324 | + [this](AdbdClient::PKRequest* r) { |
325 | + stop_snap(); |
326 | + r->respond(m_response); |
327 | + delete r; |
328 | + } |
329 | + ); |
330 | + maybe_snap(); |
331 | + } |
332 | + ); |
333 | } |
334 | |
335 | ~Impl() |
336 | { |
337 | - if (m_restart_idle_tag) |
338 | - g_source_remove(m_restart_idle_tag); |
339 | - |
340 | - clear(); |
341 | + if (m_request_complete_idle_tag) |
342 | + g_source_remove(m_request_complete_idle_tag); |
343 | } |
344 | |
345 | private: |
346 | |
347 | - void clear() |
348 | + void stop_snap() |
349 | { |
350 | - // clear out old state |
351 | m_snap_connections.clear(); |
352 | m_snap.reset(); |
353 | - m_req = decltype(m_req)(); |
354 | - m_adbd_client.reset(); |
355 | - } |
356 | - |
357 | - void restart() |
358 | - { |
359 | - clear(); |
360 | - |
361 | - // set a new client |
362 | - m_adbd_client.reset(new GAdbdClient{m_socket_path}); |
363 | - m_adbd_client->on_pk_request().connect( |
364 | - [this](const AdbdClient::PKRequest& req) { |
365 | - g_debug("%s got pk request: %s", G_STRLOC, req.fingerprint.c_str()); |
366 | - m_req = req; |
367 | - maybe_snap(); |
368 | - } |
369 | - ); |
370 | } |
371 | |
372 | void maybe_snap() |
373 | { |
374 | - // don't prompt in the greeter! |
375 | - if (!m_req.public_key.empty() && !m_greeter->is_active().get()) |
376 | - snap(); |
377 | + // only prompt if there's something to prompt about |
378 | + if (!m_req) |
379 | + return; |
380 | + |
381 | + // only prompt in an unlocked session |
382 | + if (m_greeter->state().get() != Greeter::State::INACTIVE) |
383 | + return; |
384 | + |
385 | + snap(); |
386 | } |
387 | |
388 | void snap() |
389 | { |
390 | - m_snap = std::make_shared<UsbSnap>(m_req.fingerprint); |
391 | + m_snap = std::make_shared<UsbSnap>(m_req->fingerprint); |
392 | m_snap_connections.insert((*m_snap).on_user_response().connect( |
393 | [this](AdbdClient::PKResponse response, bool remember_choice){ |
394 | - g_debug("%s user responded! response %d, remember %d", G_STRLOC, int(response), int(remember_choice)); |
395 | - m_req.respond(response); |
396 | + |
397 | if (remember_choice && (response == AdbdClient::PKResponse::ALLOW)) |
398 | - write_public_key(m_req.public_key); |
399 | - m_restart_idle_tag = g_idle_add([](gpointer gself){ |
400 | - auto self = static_cast<Impl*>(gself); |
401 | - self->m_restart_idle_tag = 0; |
402 | - self->restart(); |
403 | - return G_SOURCE_REMOVE; |
404 | - }, this); |
405 | + write_public_key(m_req->public_key); |
406 | + |
407 | + m_response = response; |
408 | + |
409 | + // defer finishing the request into an idle func because |
410 | + // ScopedConnections can't be destroyed inside their callbacks |
411 | + if (m_request_complete_idle_tag == 0) { |
412 | + m_request_complete_idle_tag = g_idle_add([](gpointer gself){ |
413 | + auto self = static_cast<Impl*>(gself); |
414 | + self->m_request_complete_idle_tag = 0; |
415 | + self->m_req.reset(); |
416 | + return G_SOURCE_REMOVE; |
417 | + }, this); |
418 | + } |
419 | } |
420 | )); |
421 | } |
422 | @@ -152,12 +164,13 @@ |
423 | const std::shared_ptr<UsbMonitor> m_usb_monitor; |
424 | const std::shared_ptr<Greeter> m_greeter; |
425 | |
426 | - unsigned int m_restart_idle_tag {}; |
427 | + unsigned int m_request_complete_idle_tag {}; |
428 | |
429 | std::shared_ptr<GAdbdClient> m_adbd_client; |
430 | - AdbdClient::PKRequest m_req; |
431 | std::shared_ptr<UsbSnap> m_snap; |
432 | std::set<core::ScopedConnection> m_snap_connections; |
433 | + AdbdClient::PKResponse m_response {AdbdClient::PKResponse::DENY}; |
434 | + std::shared_ptr<AdbdClient::PKRequest> m_req; |
435 | }; |
436 | |
437 | /*** |
438 | |
439 | === modified file 'src/usb-snap.cpp' |
440 | --- src/usb-snap.cpp 2016-03-23 17:16:06 +0000 |
441 | +++ src/usb-snap.cpp 2016-05-04 00:04:00 +0000 |
442 | @@ -208,6 +208,7 @@ |
443 | const bool remember_this_choice = response == AdbdClient::PKResponse::ALLOW; |
444 | |
445 | m_on_user_response(response, remember_this_choice); |
446 | + m_notification_id = 0; |
447 | } |
448 | |
449 | void on_notification_closed(uint32_t close_reason) |
450 | |
451 | === modified file 'tests/integration/usb-manager-test.cpp' |
452 | --- tests/integration/usb-manager-test.cpp 2016-03-23 17:16:06 +0000 |
453 | +++ tests/integration/usb-manager-test.cpp 2016-05-04 00:04:00 +0000 |
454 | @@ -143,7 +143,7 @@ |
455 | ); |
456 | |
457 | // confirm that the AdbdServer got the right response |
458 | - wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 2000); |
459 | + wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 5000); |
460 | ASSERT_EQ(1, adbd_server->m_responses.size()); |
461 | EXPECT_EQ("OK", adbd_server->m_responses.front()); |
462 | |
463 | @@ -163,13 +163,16 @@ |
464 | const std::shared_ptr<std::string> public_keys_path {new std::string{*m_tmpdir+"/adb_keys"}, file_deleter}; |
465 | |
466 | // start a mock AdbdServer ready to submit a request |
467 | + const size_t N_TESTS {3}; |
468 | const std::string public_key {"public_key"}; |
469 | - auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, std::vector<std::string>{"PK"+public_key}); |
470 | + const std::vector<std::string> requests(N_TESTS, "PK"+public_key); |
471 | + const std::vector<std::string> expected_responses(N_TESTS, "NO"); |
472 | + auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, requests); |
473 | |
474 | // set up a UsbManager to process the request |
475 | auto usb_manager = std::make_shared<UsbManager>(*socket_path, *public_keys_path, m_usb_monitor, m_greeter); |
476 | |
477 | - for (int i=0; i<3; i++) |
478 | + for (std::remove_const<decltype(N_TESTS)>::type i=0; i<N_TESTS; ++i) |
479 | { |
480 | // add a signal spy to listen to the notification daemon |
481 | QSignalSpy notificationsSpy( |
482 | @@ -194,6 +197,9 @@ |
483 | EXPECT_EQ("CloseNotification", notificationsSpy.at(0).at(0)); |
484 | notificationsSpy.clear(); |
485 | } |
486 | + |
487 | + EXPECT_TRUE(wait_for([adbd_server, N_TESTS](){return adbd_server->m_responses.size() == N_TESTS;}, 5000)); |
488 | + EXPECT_EQ(expected_responses, adbd_server->m_responses); |
489 | } |
490 | |
491 | TEST_F(UsbManagerFixture, Greeter) |
492 | @@ -206,7 +212,7 @@ |
493 | auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, std::vector<std::string>{"PK"+public_key}); |
494 | |
495 | // set up a UsbManager to process the request |
496 | - m_greeter->m_is_active.set(true); |
497 | + m_greeter->m_state.set(Greeter::State::ACTIVE); |
498 | auto usb_manager = std::make_shared<UsbManager>(*socket_path, *public_keys_path, m_usb_monitor, m_greeter); |
499 | |
500 | // add a signal spy to listen to the notification daemon |
501 | @@ -219,7 +225,7 @@ |
502 | EXPECT_FALSE(notificationsSpy.wait(2000)); |
503 | |
504 | // disable the greeter, the notification should appear |
505 | - m_greeter->m_is_active.set(false); |
506 | + m_greeter->m_state.set(Greeter::State::INACTIVE); |
507 | wait_for_signals(notificationsSpy, 1); |
508 | EXPECT_EQ("Notify", notificationsSpy.at(0).at(0)); |
509 | notificationsSpy.clear(); |
510 | |
511 | === modified file 'tests/unit/CMakeLists.txt' |
512 | --- tests/unit/CMakeLists.txt 2016-03-16 00:25:33 +0000 |
513 | +++ tests/unit/CMakeLists.txt 2016-05-04 00:04:00 +0000 |
514 | @@ -14,6 +14,10 @@ |
515 | ${GMOCK_LIBRARIES} |
516 | ) |
517 | |
518 | +add_definitions( |
519 | + -DGREETER_TEMPLATE="${CMAKE_SOURCE_DIR}/tests/utils/mock-unity-greeter.py" |
520 | +) |
521 | + |
522 | function(add_test_by_name name) |
523 | set(TEST_NAME ${name}) |
524 | add_executable (${TEST_NAME} ${TEST_NAME}.cpp) |
525 | @@ -31,4 +35,5 @@ |
526 | set_property(TEST ${TEST_NAME} APPEND PROPERTY ENVIRONMENT ${CTEST_ENVIRONMENT}) |
527 | target_link_libraries(${TEST_NAME} ${SERVICE_LINK_LIBRARIES} ${QT_LINK_LIBRARIES} ${TEST_LINK_LIBRARIES} ${THREAD_LINK_LIBRARIES}) |
528 | endfunction() |
529 | +add_qt_test_by_name(greeter-test) |
530 | add_qt_test_by_name(usb-snap-test) |
531 | |
532 | === added file 'tests/unit/greeter-test.cpp' |
533 | --- tests/unit/greeter-test.cpp 1970-01-01 00:00:00 +0000 |
534 | +++ tests/unit/greeter-test.cpp 2016-05-04 00:04:00 +0000 |
535 | @@ -0,0 +1,159 @@ |
536 | +/* |
537 | + * Copyright 2016 Canonical Ltd. |
538 | + * |
539 | + * This program is free software: you can redistribute it and/or modify it |
540 | + * under the terms of the GNU General Public License version 3, as published |
541 | + * by the Free Software Foundation. |
542 | + * |
543 | + * This program is distributed in the hope that it will be useful, but |
544 | + * WITHOUT ANY WARRANTY; without even the implied warranties of |
545 | + * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR |
546 | + * PURPOSE. See the GNU General Public License for more details. |
547 | + * |
548 | + * You should have received a copy of the GNU General Public License along |
549 | + * with this program. If not, see <http://www.gnu.org/licenses/>. |
550 | + * |
551 | + * Authors: |
552 | + * Charles Kerr <charles.kerr@canonical.com> |
553 | + */ |
554 | + |
555 | +#include <tests/utils/qt-fixture.h> |
556 | +#include <tests/utils/gtest-print-helpers.h> |
557 | + |
558 | +#include <src/dbus-names.h> |
559 | +#include <src/greeter.h> |
560 | + |
561 | +#include <libqtdbustest/DBusTestRunner.h> |
562 | +#include <libqtdbustest/QProcessDBusService.h> |
563 | +#include <libqtdbusmock/DBusMock.h> |
564 | + |
565 | +class GreeterFixture: public QtFixture |
566 | +{ |
567 | +private: |
568 | + |
569 | + using super = QtFixture; |
570 | + |
571 | +public: |
572 | + |
573 | + GreeterFixture() =default; |
574 | + ~GreeterFixture() =default; |
575 | + |
576 | +protected: |
577 | + |
578 | + std::shared_ptr<QtDBusTest::DBusTestRunner> m_dbus_runner; |
579 | + std::shared_ptr<QtDBusMock::DBusMock> m_dbus_mock; |
580 | + GDBusConnection* m_bus {}; |
581 | + |
582 | + void SetUp() override |
583 | + { |
584 | + super::SetUp(); |
585 | + |
586 | + // use a fresh bus for each test run |
587 | + m_dbus_runner.reset(new QtDBusTest::DBusTestRunner()); |
588 | + m_dbus_mock.reset(new QtDBusMock::DBusMock(*m_dbus_runner.get())); |
589 | + |
590 | + GError* error {}; |
591 | + m_bus = g_bus_get_sync (G_BUS_TYPE_SESSION, nullptr, &error); |
592 | + g_assert_no_error(error); |
593 | + g_dbus_connection_set_exit_on_close(m_bus, FALSE); |
594 | + } |
595 | + |
596 | + void TearDown() override |
597 | + { |
598 | + g_clear_object(&m_bus); |
599 | + |
600 | + super::TearDown(); |
601 | + } |
602 | + |
603 | + void start_greeter_service(bool is_active) |
604 | + { |
605 | + // set a watcher to look for our mock greeter to appear |
606 | + bool owned {}; |
607 | + QDBusServiceWatcher watcher( |
608 | + DBusNames::UnityGreeter::NAME, |
609 | + m_dbus_runner->sessionConnection() |
610 | + ); |
611 | + QObject::connect( |
612 | + &watcher, |
613 | + &QDBusServiceWatcher::serviceRegistered, |
614 | + [&owned](const QString&){owned = true;} |
615 | + ); |
616 | + |
617 | + // start the mock greeter |
618 | + QVariantMap parameters; |
619 | + parameters["IsActive"] = QVariant(is_active); |
620 | + m_dbus_mock->registerTemplate( |
621 | + DBusNames::UnityGreeter::NAME, |
622 | + GREETER_TEMPLATE, |
623 | + parameters, |
624 | + QDBusConnection::SessionBus |
625 | + ); |
626 | + m_dbus_runner->startServices(); |
627 | + |
628 | + // wait for the watcher |
629 | + ASSERT_TRUE(wait_for([&owned]{return owned;})); |
630 | + } |
631 | +}; |
632 | + |
633 | +#define ASSERT_PROPERTY_EQ_EVENTUALLY(expected_in, property_in) \ |
634 | + do { \ |
635 | + const auto& e = expected_in; \ |
636 | + const auto& p = property_in; \ |
637 | + ASSERT_TRUE(wait_for([e, &p](){return e == p.get();})) \ |
638 | + << "expected " << e << " but got " << p.get(); \ |
639 | + } while(0) |
640 | + |
641 | +/** |
642 | + * Test startup timing by looking at four different cases: |
643 | + * [unity greeter shows up on bus (before, after) we start listening] |
644 | + * x [unity greeter is (active, inactive)] |
645 | + */ |
646 | + |
647 | +TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher) |
648 | +{ |
649 | + constexpr bool is_active {true}; |
650 | + constexpr Greeter::State expected {Greeter::State::ACTIVE}; |
651 | + |
652 | + start_greeter_service(is_active); |
653 | + |
654 | + UnityGreeter greeter; |
655 | + |
656 | + ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state()); |
657 | +} |
658 | + |
659 | +TEST_F(GreeterFixture, WatcherStartsBeforeActiveService) |
660 | +{ |
661 | + constexpr bool is_active {true}; |
662 | + constexpr Greeter::State expected {Greeter::State::ACTIVE}; |
663 | + |
664 | + UnityGreeter greeter; |
665 | + |
666 | + start_greeter_service(is_active); |
667 | + |
668 | + ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state()); |
669 | +} |
670 | + |
671 | +TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher) |
672 | +{ |
673 | + constexpr bool is_active {false}; |
674 | + constexpr Greeter::State expected {Greeter::State::INACTIVE}; |
675 | + |
676 | + start_greeter_service(is_active); |
677 | + |
678 | + UnityGreeter greeter; |
679 | + |
680 | + ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state()); |
681 | +} |
682 | + |
683 | +TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService) |
684 | +{ |
685 | + constexpr bool is_active {false}; |
686 | + constexpr Greeter::State expected {Greeter::State::INACTIVE}; |
687 | + |
688 | + UnityGreeter greeter; |
689 | + |
690 | + start_greeter_service(is_active); |
691 | + |
692 | + ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state()); |
693 | +} |
694 | + |
695 | |
696 | === modified file 'tests/unit/usb-snap-test.cpp' |
697 | --- tests/unit/usb-snap-test.cpp 2016-03-23 17:16:06 +0000 |
698 | +++ tests/unit/usb-snap-test.cpp 2016-05-04 00:04:00 +0000 |
699 | @@ -130,14 +130,9 @@ |
700 | EXPECT_TRUE(user_response_set); |
701 | ASSERT_EQ(test.expected_response, user_response); |
702 | |
703 | - // confirm that the snap dtor cleans up the notification |
704 | + // confirm that the snap dtor doesn't try to close |
705 | + // the notification that's already been closed by user choice |
706 | snap.reset(); |
707 | - wait_for_signals(notificationsSpy, 1); |
708 | - { |
709 | - QVariantList const& call(notificationsSpy.at(0)); |
710 | - EXPECT_EQ("CloseNotification", call.at(0)); |
711 | - QVariantList const& args(call.at(1).toList()); |
712 | - EXPECT_EQ(id, args.at(0)); |
713 | - } |
714 | + EXPECT_FALSE(notificationsSpy.wait(1000)); |
715 | } |
716 | } |
717 | |
718 | === modified file 'tests/utils/adbd-server.h' |
719 | --- tests/utils/adbd-server.h 2016-03-24 16:01:16 +0000 |
720 | +++ tests/utils/adbd-server.h 2016-05-04 00:04:00 +0000 |
721 | @@ -38,7 +38,7 @@ |
722 | GAdbdServer(const std::string& socket_path, |
723 | const std::vector<std::string>& requests): |
724 | m_requests{requests}, |
725 | - m_socket_path{socket_path}, |
726 | + m_server_socket{create_server_socket(socket_path)}, |
727 | m_cancellable{g_cancellable_new()}, |
728 | m_worker_thread{&GAdbdServer::worker_func, this} |
729 | { |
730 | @@ -50,6 +50,7 @@ |
731 | g_cancellable_cancel(m_cancellable); |
732 | m_worker_thread.join(); |
733 | g_clear_object(&m_cancellable); |
734 | + g_clear_object(&m_server_socket); |
735 | } |
736 | |
737 | const std::vector<std::string> m_requests; |
738 | @@ -59,18 +60,14 @@ |
739 | |
740 | void worker_func() // runs in worker thread |
741 | { |
742 | - auto server_socket = create_server_socket(m_socket_path); |
743 | auto requests = m_requests; |
744 | |
745 | - GError* error {}; |
746 | - g_socket_listen (server_socket, &error); |
747 | - g_assert_no_error (error); |
748 | - |
749 | while (!g_cancellable_is_cancelled(m_cancellable) && !requests.empty()) |
750 | { |
751 | // wait for a client connection |
752 | g_message("GAdbdServer::Impl::worker_func() calling g_socket_accept()"); |
753 | - auto client_socket = g_socket_accept(server_socket, m_cancellable, &error); |
754 | + GError* error {}; |
755 | + auto client_socket = g_socket_accept(m_server_socket, m_cancellable, &error); |
756 | if (error != nullptr) { |
757 | if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) |
758 | g_message("GAdbdServer: Error accepting socket connection: %s", error->message); |
759 | @@ -121,8 +118,6 @@ |
760 | // cleanup |
761 | g_clear_object(&client_socket); |
762 | } |
763 | - |
764 | - g_clear_object(&server_socket); |
765 | } |
766 | |
767 | // bind to a local domain socket |
768 | @@ -139,11 +134,14 @@ |
769 | g_assert_no_error (error); |
770 | g_clear_object (&address); |
771 | |
772 | + g_socket_listen (socket, &error); |
773 | + g_assert_no_error (error); |
774 | + |
775 | return socket; |
776 | } |
777 | |
778 | - const std::string m_socket_path; |
779 | - GCancellable* m_cancellable = nullptr; |
780 | + GSocket* m_server_socket {}; |
781 | + GCancellable* m_cancellable {}; |
782 | std::thread m_worker_thread; |
783 | }; |
784 | |
785 | |
786 | === added file 'tests/utils/gtest-print-helpers.h' |
787 | --- tests/utils/gtest-print-helpers.h 1970-01-01 00:00:00 +0000 |
788 | +++ tests/utils/gtest-print-helpers.h 2016-05-04 00:04:00 +0000 |
789 | @@ -0,0 +1,18 @@ |
790 | + |
791 | +#pragma once |
792 | + |
793 | +#include <src/greeter.h> |
794 | + |
795 | +inline void PrintTo(const Greeter::State& state, std::ostream* os) { |
796 | + switch(state) { |
797 | + case Greeter::State::ACTIVE: *os << "Active"; break; |
798 | + case Greeter::State::INACTIVE: *os << "Inactive"; break; |
799 | + case Greeter::State::UNAVAILABLE: *os << "Unavailable"; break; |
800 | + } |
801 | +} |
802 | + |
803 | +inline std::ostream& operator<<(std::ostream& os, const Greeter::State& state) { |
804 | + PrintTo(state, &os); |
805 | + return os; |
806 | +} |
807 | + |
808 | |
809 | === modified file 'tests/utils/mock-greeter.h' |
810 | --- tests/utils/mock-greeter.h 2016-03-22 20:39:36 +0000 |
811 | +++ tests/utils/mock-greeter.h 2016-05-04 00:04:00 +0000 |
812 | @@ -26,7 +26,7 @@ |
813 | public: |
814 | MockGreeter() =default; |
815 | virtual ~MockGreeter() =default; |
816 | - core::Property<bool>& is_active() override {return m_is_active;} |
817 | - core::Property<bool> m_is_active {false}; |
818 | + core::Property<Greeter::State>& state() override {return m_state;} |
819 | + core::Property<Greeter::State> m_state {State::INACTIVE}; |
820 | }; |
821 | |
822 | |
823 | === added file 'tests/utils/mock-unity-greeter.py' |
824 | --- tests/utils/mock-unity-greeter.py 1970-01-01 00:00:00 +0000 |
825 | +++ tests/utils/mock-unity-greeter.py 2016-05-04 00:04:00 +0000 |
826 | @@ -0,0 +1,41 @@ |
827 | +'''unity greeter mock template |
828 | + |
829 | +Very basic template that just mocks the greeter is-active flag |
830 | +''' |
831 | + |
832 | +# This program is free software; you can redistribute it and/or modify it under |
833 | +# the terms of the GNU Lesser General Public License as published by the Free |
834 | +# Software Foundation; either version 3 of the License, or (at your option) any |
835 | +# later version. See http://www.gnu.org/copyleft/lgpl.html for the full text |
836 | +# of the license. |
837 | + |
838 | +__author__ = 'Charles Kerr' |
839 | +__email__ = 'charles.kerr@canonical.com' |
840 | +__copyright__ = '(c) 2016 Canonical Ltd.' |
841 | +__license__ = 'LGPL 3+' |
842 | + |
843 | +import dbus |
844 | +import os |
845 | + |
846 | +from dbusmock import MOCK_IFACE, mockobject |
847 | + |
848 | +BUS_NAME = 'com.canonical.UnityGreeter' |
849 | +MAIN_OBJ = '/' |
850 | +MAIN_IFACE = 'com.canonical.UnityGreeter' |
851 | +SYSTEM_BUS = False |
852 | + |
853 | + |
854 | +def load(mock, parameters): |
855 | + mock.AddMethods( |
856 | + MAIN_IFACE, [ |
857 | + ('HideGreeter', '', '', 'self.Set("com.canonical.UnityGreeter", "IsActive", False)'), |
858 | + ('ShowGreeter', '', '', 'self.Set("com.canonical.UnityGreeter", "IsActive", True)') |
859 | + ] |
860 | + ) |
861 | + mock.AddProperties( |
862 | + MAIN_IFACE, |
863 | + dbus.Dictionary({ |
864 | + 'IsActive': parameters.get('IsActive', False), |
865 | + }, signature='sv') |
866 | + ) |
867 | + |
FAILED: Continuous integration, rev:26 jenkins. qa.ubuntu. com/job/ indicator- display- ci/26/ jenkins. qa.ubuntu. com/job/ indicator- display- wily-amd64- ci/9/console jenkins. qa.ubuntu. com/job/ indicator- display- wily-armhf- ci/9/console
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/indicator- display- ci/26/rebuild
http://