Merge lp:~charlesk/indicator-datetime/always-get-initial-tzid-from-timedate1 into lp:indicator-datetime/15.10
- always-get-initial-tzid-from-timedate1
- Merge into trunk.15.10
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Renato Araujo Oliveira Filho | ||||
Approved revision: | 448 | ||||
Merged at revision: | 442 | ||||
Proposed branch: | lp:~charlesk/indicator-datetime/always-get-initial-tzid-from-timedate1 | ||||
Merge into: | lp:indicator-datetime/15.10 | ||||
Diff against target: |
796 lines (+362/-266) 7 files modified
include/datetime/dbus-shared.h (+42/-0) include/datetime/timezone-timedated.h (+4/-4) include/datetime/timezones-live.h (+2/-3) src/main.cpp (+12/-2) src/timezone-timedated.cpp (+139/-135) src/timezones-live.cpp (+7/-4) tests/test-timezone-timedated.cpp (+156/-118) |
||||
To merge this branch: | bzr merge lp:~charlesk/indicator-datetime/always-get-initial-tzid-from-timedate1 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Renato Araujo Oliveira Filho (community) | Approve | ||
Antti Kaijanmäki (community) | Approve | ||
PS Jenkins bot (community) | continuous-integration | Approve | |
Review via email: mp+291421@code.launchpad.net |
Commit message
Fix possible startup timing issues when watching for a timezone from timedate1
Description of the change
Possible fix to bug #1567940, fixes a couple of bugs in the code that got tzid changes from org.freedesktop
1. Watch for timedate1 to show up on the bus, and query it when it does, so that there's no possible timing issue between indicator-
2. Use G_BUS_NAME_
3. Get the initial value by querying timedate1's properties instead of reading from /etc/timezone
4. If 'Timezone' shows up in timedate1's invalidated properties signal, again query for the value instead of reading /etc/timezone
- 444. By Charles Kerr
-
in tests/test-
timezone- timedated, fix copy-paste error in comments - 445. By Charles Kerr
-
in timezone-timedated, check for error!=nullptr before passing it to g_error_matches()
PS Jenkins bot (ps-jenkins) wrote : | # |
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:445
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Renato Araujo Oliveira Filho (renatofilho) wrote : | # |
code looks good, some small inline code comments.
Renato Araujo Oliveira Filho (renatofilho) : | # |
Antti Kaijanmäki (kaijanmaki) wrote : | # |
code looks good, but the juggling with m_connection looks a bit brittle.
How about initializing m_connection in the constructor with g_bus_get_sync(), unreffing it in the destructor and using g_bus_watch_
- 446. By Charles Kerr
-
in TimedatedTimezone, take a GDBusConnection argument in the ctor to simplify state management
- 447. By Charles Kerr
-
in LiveTimezones, pass the primary timezone to it on construction. We used to create it implicitly but can't do that anymore now that TimedatedTimezone takes its own ctor argument.
- 448. By Charles Kerr
-
sync tests to new ctor arguments for TimedatedTimezone and LiveTimezones
Charles Kerr (charlesk) wrote : | # |
Thanks for the feedback. Since both of you commented on the clumsiness of m_connection and m_subscription's state management, I've cleaned it up in r446..448 by passing the GDBusConnection* in the ctor.
This way we can unconditionally watch & subscribe in the ctor, then unconditionally unsubscribe and unwatch in the dtor, no edge cases or having to check to see if the pointers exist yet. :)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:448
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
Renato Araujo Oliveira Filho (renatofilho) wrote : | # |
looks good.
Preview Diff
1 | === modified file 'include/datetime/dbus-shared.h' |
2 | --- include/datetime/dbus-shared.h 2014-09-17 16:51:51 +0000 |
3 | +++ include/datetime/dbus-shared.h 2016-04-12 17:11:17 +0000 |
4 | @@ -28,5 +28,47 @@ |
5 | #define BUS_POWERD_PATH "/com/canonical/powerd" |
6 | #define BUS_POWERD_INTERFACE "com.canonical.powerd" |
7 | |
8 | +namespace unity { |
9 | +namespace indicator { |
10 | +namespace datetime { |
11 | + |
12 | +namespace Bus |
13 | +{ |
14 | + namespace Timedate1 |
15 | + { |
16 | + static constexpr char const * BUSNAME {"org.freedesktop.timedate1"}; |
17 | + static constexpr char const * ADDR {"/org/freedesktop/timedate1"}; |
18 | + static constexpr char const * IFACE {"org.freedesktop.timedate1"}; |
19 | + |
20 | + namespace Properties |
21 | + { |
22 | + static constexpr char const * TIMEZONE {"Timezone"}; |
23 | + } |
24 | + |
25 | + namespace Methods |
26 | + { |
27 | + static constexpr char const * SET_TIMEZONE {"SetTimezone"}; |
28 | + } |
29 | + } |
30 | + |
31 | + namespace Properties |
32 | + { |
33 | + static constexpr char const * IFACE {"org.freedesktop.DBus.Properties"}; |
34 | + |
35 | + namespace Methods |
36 | + { |
37 | + static constexpr char const * GET {"Get"}; |
38 | + } |
39 | + |
40 | + namespace Signals |
41 | + { |
42 | + static constexpr char const * PROPERTIES_CHANGED {"PropertiesChanged"}; |
43 | + } |
44 | + } |
45 | +} |
46 | + |
47 | +} // namespace datetime |
48 | +} // namespace indicator |
49 | +} // namespace unity |
50 | |
51 | #endif /* INDICATOR_DATETIME_DBUS_SHARED_H */ |
52 | |
53 | === modified file 'include/datetime/timezone-timedated.h' |
54 | --- include/datetime/timezone-timedated.h 2015-09-03 09:54:20 +0000 |
55 | +++ include/datetime/timezone-timedated.h 2016-04-12 17:11:17 +0000 |
56 | @@ -20,10 +20,10 @@ |
57 | #ifndef INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H |
58 | #define INDICATOR_DATETIME_TIMEDATED_TIMEZONE_H |
59 | |
60 | -#define DEFAULT_FILENAME "/etc/timezone" |
61 | - |
62 | #include <datetime/timezone.h> // base class |
63 | |
64 | +#include <gio/gio.h> // GDBusConnection* |
65 | + |
66 | #include <string> // std::string |
67 | |
68 | namespace unity { |
69 | @@ -31,12 +31,12 @@ |
70 | namespace datetime { |
71 | |
72 | /** |
73 | - * \brief A #Timezone that gets its information from monitoring a file, such as /etc/timezone |
74 | + * \brief A #Timezone that gets its information from org.freedesktop.timedate1 |
75 | */ |
76 | class TimedatedTimezone: public Timezone |
77 | { |
78 | public: |
79 | - TimedatedTimezone(std::string filename = DEFAULT_FILENAME); |
80 | + TimedatedTimezone(GDBusConnection* connection); |
81 | ~TimedatedTimezone(); |
82 | |
83 | private: |
84 | |
85 | === modified file 'include/datetime/timezones-live.h' |
86 | --- include/datetime/timezones-live.h 2015-09-01 09:52:13 +0000 |
87 | +++ include/datetime/timezones-live.h 2016-04-12 17:11:17 +0000 |
88 | @@ -23,7 +23,6 @@ |
89 | #include <datetime/settings.h> |
90 | #include <datetime/timezones.h> |
91 | #include <datetime/timezone-geoclue.h> |
92 | -#include <datetime/timezone-timedated.h> |
93 | |
94 | #include <memory> // shared_ptr<> |
95 | |
96 | @@ -38,13 +37,13 @@ |
97 | class LiveTimezones: public Timezones |
98 | { |
99 | public: |
100 | - LiveTimezones(const std::shared_ptr<const Settings>& settings); |
101 | + LiveTimezones(const std::shared_ptr<const Settings>& settings, const std::shared_ptr<Timezone>& primary_timezone); |
102 | |
103 | private: |
104 | void update_geolocation(); |
105 | void update_timezones(); |
106 | |
107 | - TimedatedTimezone m_file; |
108 | + std::shared_ptr<Timezone> m_primary_timezone; |
109 | std::shared_ptr<const Settings> m_settings; |
110 | std::shared_ptr<GeoclueTimezone> m_geo; |
111 | }; |
112 | |
113 | === modified file 'src/main.cpp' |
114 | --- src/main.cpp 2015-10-13 16:06:35 +0000 |
115 | +++ src/main.cpp 2016-04-12 17:11:17 +0000 |
116 | @@ -68,7 +68,7 @@ |
117 | { |
118 | // create the live objects |
119 | auto live_settings = std::make_shared<LiveSettings>(); |
120 | - auto live_timezones = std::make_shared<LiveTimezones>(live_settings); |
121 | + auto live_timezones = std::make_shared<LiveTimezones>(live_settings, timezone_); |
122 | auto live_clock = std::make_shared<LiveClock>(timezone_); |
123 | |
124 | // create a full-month planner currently pointing to the current month |
125 | @@ -127,8 +127,17 @@ |
126 | bindtextdomain(GETTEXT_PACKAGE, GNOMELOCALEDIR); |
127 | textdomain(GETTEXT_PACKAGE); |
128 | |
129 | + // get the system bus |
130 | + GError* error {}; |
131 | + auto system_bus = g_bus_get_sync(G_BUS_TYPE_SYSTEM, nullptr, &error); |
132 | + if (error != nullptr) { |
133 | + g_critical("Unable to get system bus: %s", error->message); |
134 | + g_clear_error(&error); |
135 | + return 0; |
136 | + } |
137 | + |
138 | auto engine = create_engine(); |
139 | - auto timezone_ = std::make_shared<TimedatedTimezone>(); |
140 | + auto timezone_ = std::make_shared<TimedatedTimezone>(system_bus); |
141 | auto state = create_state(engine, timezone_); |
142 | auto actions = std::make_shared<LiveActions>(state); |
143 | MenuFactory factory(actions, state); |
144 | @@ -165,5 +174,6 @@ |
145 | g_main_loop_run(loop); |
146 | |
147 | g_main_loop_unref(loop); |
148 | + g_clear_object(&system_bus); |
149 | return 0; |
150 | } |
151 | |
152 | === modified file 'src/timezone-timedated.cpp' |
153 | --- src/timezone-timedated.cpp 2015-09-03 16:00:36 +0000 |
154 | +++ src/timezone-timedated.cpp 2016-04-12 17:11:17 +0000 |
155 | @@ -17,6 +17,7 @@ |
156 | * Charles Kerr <charles.kerr@canonical.com> |
157 | */ |
158 | |
159 | +#include <datetime/dbus-shared.h> |
160 | #include <datetime/timezone-timedated.h> |
161 | |
162 | #include <gio/gio.h> |
163 | @@ -36,166 +37,169 @@ |
164 | { |
165 | public: |
166 | |
167 | - Impl(TimedatedTimezone& owner, std::string filename): |
168 | - m_owner(owner), |
169 | - m_filename(filename) |
170 | + Impl(TimedatedTimezone& owner, GDBusConnection* connection): |
171 | + m_owner{owner}, |
172 | + m_connection{G_DBUS_CONNECTION(g_object_ref(G_OBJECT(connection)))}, |
173 | + m_cancellable{g_cancellable_new()} |
174 | { |
175 | - g_debug("Filename is '%s'", filename.c_str()); |
176 | - monitor_timezone_property(); |
177 | + // set the fallback value |
178 | + m_owner.timezone.set("Etc/Utc"); |
179 | + |
180 | + // watch for timedate1 on the bus |
181 | + m_watcher_id = g_bus_watch_name_on_connection( |
182 | + m_connection, |
183 | + Bus::Timedate1::BUSNAME, |
184 | + G_BUS_NAME_WATCHER_FLAGS_AUTO_START, |
185 | + on_timedate1_appeared, |
186 | + on_timedate1_vanished, |
187 | + this, |
188 | + nullptr); |
189 | + |
190 | + // listen for changed properties |
191 | + m_signal_subscription_id = g_dbus_connection_signal_subscribe( |
192 | + m_connection, |
193 | + Bus::Timedate1::IFACE, |
194 | + Bus::Properties::IFACE, |
195 | + Bus::Properties::Signals::PROPERTIES_CHANGED, |
196 | + Bus::Timedate1::ADDR, |
197 | + nullptr, |
198 | + G_DBUS_SIGNAL_FLAGS_NONE, |
199 | + on_properties_changed, |
200 | + this, |
201 | + nullptr); |
202 | } |
203 | |
204 | ~Impl() |
205 | { |
206 | - clear(); |
207 | + g_cancellable_cancel(m_cancellable); |
208 | + g_clear_object(&m_cancellable); |
209 | + |
210 | + g_bus_unwatch_name(m_watcher_id); |
211 | + |
212 | + g_dbus_connection_signal_unsubscribe(m_connection, m_signal_subscription_id); |
213 | + |
214 | + g_clear_object(&m_connection); |
215 | } |
216 | |
217 | private: |
218 | |
219 | - void clear() |
220 | - { |
221 | - if (m_connection && m_signal_subscription_id) |
222 | - { |
223 | - g_dbus_connection_signal_unsubscribe (m_connection, m_signal_subscription_id); |
224 | - m_signal_subscription_id = 0; |
225 | - } |
226 | - |
227 | - g_clear_object(&m_connection); |
228 | - } |
229 | - |
230 | - static void on_properties_changed (GDBusConnection *connection G_GNUC_UNUSED, |
231 | - const gchar *sender_name G_GNUC_UNUSED, |
232 | - const gchar *object_path G_GNUC_UNUSED, |
233 | - const gchar *interface_name G_GNUC_UNUSED, |
234 | - const gchar *signal_name G_GNUC_UNUSED, |
235 | - GVariant *parameters, |
236 | - gpointer gself) |
237 | + static void on_timedate1_appeared(GDBusConnection * /*connection*/, |
238 | + const gchar * name, |
239 | + const gchar * /*name_owner*/, |
240 | + gpointer gself) |
241 | + { |
242 | + g_debug("%s appeared on bus", name); |
243 | + |
244 | + static_cast<Impl*>(gself)->ask_for_timezone(); |
245 | + } |
246 | + |
247 | + static void on_timedate1_vanished(GDBusConnection * /*connection*/, |
248 | + const gchar * name, |
249 | + gpointer /*gself*/) |
250 | + { |
251 | + g_debug("%s not present on bus", name); |
252 | + } |
253 | + |
254 | + static void on_properties_changed(GDBusConnection * /*connection*/, |
255 | + const gchar * /*sender_name*/, |
256 | + const gchar * /*object_path*/, |
257 | + const gchar * /*interface_name*/, |
258 | + const gchar * /*signal_name*/, |
259 | + GVariant * parameters, |
260 | + gpointer gself) |
261 | { |
262 | auto self = static_cast<Impl*>(gself); |
263 | - const char *tz; |
264 | - GVariant *changed_properties; |
265 | - gchar **invalidated_properties; |
266 | - |
267 | - g_variant_get (parameters, "(s@a{sv}^as)", NULL, &changed_properties, &invalidated_properties); |
268 | - |
269 | - if (g_variant_lookup(changed_properties, "Timezone", "&s", &tz, NULL)) |
270 | - self->notify_timezone(tz); |
271 | - else if (g_strv_contains (invalidated_properties, "Timezone")) |
272 | - self->notify_timezone(self->get_timezone_from_file(self->m_filename)); |
273 | - |
274 | - g_variant_unref (changed_properties); |
275 | - g_strfreev (invalidated_properties); |
276 | - } |
277 | - |
278 | - void monitor_timezone_property() |
279 | - { |
280 | - GError *err = nullptr; |
281 | - |
282 | - /* |
283 | - * There is an unlikely race which happens if there is an activation |
284 | - * and timezone change before our match rule is added. |
285 | - */ |
286 | - notify_timezone(get_timezone_from_file(m_filename)); |
287 | - |
288 | - /* |
289 | - * Make sure the bus is around at least until we add the match rules, |
290 | - * otherwise things (tests) are sad. |
291 | - */ |
292 | - m_connection = g_bus_get_sync(G_BUS_TYPE_SYSTEM, |
293 | - nullptr, |
294 | - &err); |
295 | - |
296 | - if (err) |
297 | - { |
298 | - g_warning("Couldn't get bus connection: '%s'", err->message); |
299 | - g_error_free(err); |
300 | - return; |
301 | - } |
302 | - |
303 | - m_signal_subscription_id = g_dbus_connection_signal_subscribe(m_connection, |
304 | - "org.freedesktop.timedate1", |
305 | - "org.freedesktop.DBus.Properties", |
306 | - "PropertiesChanged", |
307 | - "/org/freedesktop/timedate1", |
308 | - NULL, G_DBUS_SIGNAL_FLAGS_NONE, |
309 | - on_properties_changed, |
310 | - this, nullptr); |
311 | - } |
312 | - |
313 | - void notify_timezone(std::string new_timezone) |
314 | - { |
315 | - g_debug("notify_timezone '%s'", new_timezone.c_str()); |
316 | - if (!new_timezone.empty()) |
317 | - m_owner.timezone.set(new_timezone); |
318 | - } |
319 | - |
320 | - std::string get_timezone_from_file(const std::string& filename) |
321 | - { |
322 | - GError * error; |
323 | - GIOChannel * io_channel; |
324 | - std::string ret; |
325 | - |
326 | - // read through filename line-by-line until we fine a nonempty non-comment line |
327 | - error = nullptr; |
328 | - io_channel = g_io_channel_new_file(filename.c_str(), "r", &error); |
329 | - if (error == nullptr) |
330 | - { |
331 | - auto line = g_string_new(nullptr); |
332 | - |
333 | - while(ret.empty()) |
334 | - { |
335 | - const auto io_status = g_io_channel_read_line_string(io_channel, line, nullptr, &error); |
336 | - if ((io_status == G_IO_STATUS_EOF) || (io_status == G_IO_STATUS_ERROR)) |
337 | - break; |
338 | - if (error != nullptr) |
339 | - break; |
340 | - |
341 | - g_strstrip(line->str); |
342 | - |
343 | - if (!line->len) // skip empty lines |
344 | - continue; |
345 | - |
346 | - if (*line->str=='#') // skip comments |
347 | - continue; |
348 | - |
349 | - ret = line->str; |
350 | - } |
351 | - |
352 | - g_string_free(line, true); |
353 | - } else |
354 | - /* Default to UTC */ |
355 | - ret = "Etc/Utc"; |
356 | - |
357 | - if (io_channel != nullptr) |
358 | - { |
359 | - g_io_channel_shutdown(io_channel, false, nullptr); |
360 | - g_io_channel_unref(io_channel); |
361 | - } |
362 | - |
363 | + |
364 | + GVariant* changed_properties {}; |
365 | + gchar** invalidated_properties {}; |
366 | + g_variant_get(parameters, "(s@a{sv}^as)", NULL, &changed_properties, &invalidated_properties); |
367 | + |
368 | + const char* tz {}; |
369 | + if (g_variant_lookup(changed_properties, Bus::Timedate1::Properties::TIMEZONE, "&s", &tz, NULL)) |
370 | + { |
371 | + if (tz != nullptr) |
372 | + self->set_timezone(tz); |
373 | + else |
374 | + g_warning("%s no timezone found", G_STRLOC); |
375 | + } |
376 | + else if (g_strv_contains(invalidated_properties, Bus::Timedate1::Properties::TIMEZONE)) |
377 | + { |
378 | + self->ask_for_timezone(); |
379 | + } |
380 | + |
381 | + g_variant_unref(changed_properties); |
382 | + g_strfreev(invalidated_properties); |
383 | + } |
384 | + |
385 | + void ask_for_timezone() |
386 | + { |
387 | + g_dbus_connection_call( |
388 | + m_connection, |
389 | + Bus::Timedate1::BUSNAME, |
390 | + Bus::Timedate1::ADDR, |
391 | + Bus::Properties::IFACE, |
392 | + Bus::Properties::Methods::GET, |
393 | + g_variant_new("(ss)", Bus::Timedate1::IFACE, Bus::Timedate1::Properties::TIMEZONE), |
394 | + G_VARIANT_TYPE("(v)"), |
395 | + G_DBUS_CALL_FLAGS_NONE, |
396 | + -1, |
397 | + m_cancellable, |
398 | + on_get_timezone_ready, |
399 | + this); |
400 | + } |
401 | + |
402 | + static void on_get_timezone_ready(GObject * connection, |
403 | + GAsyncResult * res, |
404 | + gpointer gself) |
405 | + { |
406 | + GError* error {}; |
407 | + GVariant* v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(connection), res, &error); |
408 | if (error != nullptr) |
409 | { |
410 | - g_warning("%s Unable to read timezone file '%s': %s", G_STRLOC, filename.c_str(), error->message); |
411 | - g_error_free(error); |
412 | - } |
413 | - |
414 | - return ret; |
415 | + if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) |
416 | + g_warning("%s Couldn't get timezone: %s", G_STRLOC, error->message); |
417 | + } |
418 | + else if (v != nullptr) |
419 | + { |
420 | + GVariant* tzv {}; |
421 | + g_variant_get(v, "(v)", &tzv); |
422 | + const char* tz = g_variant_get_string(tzv, nullptr); |
423 | + |
424 | + if (tz != nullptr) |
425 | + static_cast<Impl*>(gself)->set_timezone(tz); |
426 | + else |
427 | + g_warning("%s no timezone found", G_STRLOC); |
428 | + |
429 | + g_clear_pointer(&tzv, g_variant_unref); |
430 | + g_clear_pointer(&v, g_variant_unref); |
431 | + } |
432 | + } |
433 | + |
434 | + void set_timezone(const std::string& tz) |
435 | + { |
436 | + g_return_if_fail(!tz.empty()); |
437 | + |
438 | + g_debug("set timezone: '%s'", tz.c_str()); |
439 | + m_owner.timezone.set(tz); |
440 | } |
441 | |
442 | /*** |
443 | **** |
444 | ***/ |
445 | |
446 | - TimedatedTimezone & m_owner; |
447 | - GDBusConnection *m_connection = nullptr; |
448 | - unsigned long m_signal_subscription_id = 0; |
449 | - std::string m_filename; |
450 | + TimedatedTimezone& m_owner; |
451 | + GDBusConnection* m_connection {}; |
452 | + GCancellable* m_cancellable {}; |
453 | + unsigned long m_signal_subscription_id {}; |
454 | + unsigned int m_watcher_id {}; |
455 | }; |
456 | |
457 | /*** |
458 | **** |
459 | ***/ |
460 | |
461 | -TimedatedTimezone::TimedatedTimezone(std::string filename): |
462 | - impl(new Impl{*this, filename}) |
463 | +TimedatedTimezone::TimedatedTimezone(GDBusConnection* connection): |
464 | + impl{new Impl{*this, connection}} |
465 | { |
466 | } |
467 | |
468 | |
469 | === modified file 'src/timezones-live.cpp' |
470 | --- src/timezones-live.cpp 2015-08-31 21:08:36 +0000 |
471 | +++ src/timezones-live.cpp 2016-04-12 17:11:17 +0000 |
472 | @@ -25,11 +25,14 @@ |
473 | namespace indicator { |
474 | namespace datetime { |
475 | |
476 | -LiveTimezones::LiveTimezones(const std::shared_ptr<const Settings>& settings): |
477 | - m_file(), |
478 | +LiveTimezones::LiveTimezones( |
479 | + const std::shared_ptr<const Settings>& settings, |
480 | + const std::shared_ptr<Timezone>& primary_timezone |
481 | +): |
482 | + m_primary_timezone(primary_timezone), |
483 | m_settings(settings) |
484 | { |
485 | - m_file.timezone.changed().connect([this](const std::string&){update_timezones();}); |
486 | + m_primary_timezone->timezone.changed().connect([this](const std::string&){update_timezones();}); |
487 | |
488 | m_settings->show_detected_location.changed().connect([this](bool){update_geolocation();}); |
489 | update_geolocation(); |
490 | @@ -53,7 +56,7 @@ |
491 | |
492 | void LiveTimezones::update_timezones() |
493 | { |
494 | - const auto a = m_file.timezone.get(); |
495 | + const auto a = m_primary_timezone->timezone.get(); |
496 | const auto b = m_geo ? m_geo->timezone.get() : ""; |
497 | |
498 | timezone.set(a.empty() ? b : a); |
499 | |
500 | === modified file 'tests/test-timezone-timedated.cpp' |
501 | --- tests/test-timezone-timedated.cpp 2015-09-09 16:16:41 +0000 |
502 | +++ tests/test-timezone-timedated.cpp 2016-04-12 17:11:17 +0000 |
503 | @@ -1,9 +1,5 @@ |
504 | - |
505 | /* |
506 | - * Copyright 2013 Canonical Ltd. |
507 | - * |
508 | - * Authors: |
509 | - * Charles Kerr <charles.kerr@canonical.com> |
510 | + * Copyright © 2014-2016 Canonical Ltd. |
511 | * |
512 | * This program is free software: you can redistribute it and/or modify it |
513 | * under the terms of the GNU General Public License version 3, as published |
514 | @@ -16,127 +12,169 @@ |
515 | * |
516 | * You should have received a copy of the GNU General Public License along |
517 | * with this program. If not, see <http://www.gnu.org/licenses/>. |
518 | + * |
519 | + * Authors: |
520 | + * Charles Kerr <charles.kerr@canonical.com> |
521 | + * Ted Gould <ted.gould@canonical.com> |
522 | */ |
523 | |
524 | -#include "timedated-fixture.h" |
525 | +#include "glib-fixture.h" |
526 | |
527 | +#include <datetime/dbus-shared.h> |
528 | #include <datetime/timezone-timedated.h> |
529 | |
530 | -using unity::indicator::datetime::TimedatedTimezone; |
531 | +#include <gio/gio.h> |
532 | + |
533 | + |
534 | +using namespace unity::indicator::datetime; |
535 | + |
536 | + |
537 | +struct Timedate1Fixture: public GlibFixture |
538 | +{ |
539 | +private: |
540 | + |
541 | + typedef GlibFixture super; |
542 | + |
543 | +protected: |
544 | + |
545 | + GDBusConnection* m_bus {}; |
546 | + GTestDBus* m_test_bus {}; |
547 | + |
548 | + void SetUp() override |
549 | + { |
550 | + super::SetUp(); |
551 | + |
552 | + // use a fake bus |
553 | + m_test_bus = g_test_dbus_new(G_TEST_DBUS_NONE); |
554 | + g_test_dbus_up(m_test_bus); |
555 | + const char * address = g_test_dbus_get_bus_address(m_test_bus); |
556 | + g_setenv("DBUS_SYSTEM_BUS_ADDRESS", address, true); |
557 | + g_setenv("DBUS_SESSION_BUS_ADDRESS", address, true); |
558 | + g_debug("test_dbus's address is %s", address); |
559 | + |
560 | + // get the bus |
561 | + m_bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, nullptr); |
562 | + g_dbus_connection_set_exit_on_close(m_bus, FALSE); |
563 | + } |
564 | + |
565 | + void TearDown() override |
566 | + { |
567 | + // tear down the bus |
568 | + bool bus_finished = false; |
569 | + g_object_weak_ref( |
570 | + G_OBJECT(m_bus), |
571 | + [](gpointer gbus_finished, GObject*){*static_cast<bool*>(gbus_finished) = true;}, |
572 | + &bus_finished |
573 | + ); |
574 | + g_clear_object(&m_bus); |
575 | + EXPECT_TRUE(wait_for([&bus_finished](){return bus_finished;})); |
576 | + |
577 | + // tear down test bus |
578 | + g_clear_object(&m_test_bus); |
579 | + |
580 | + super::TearDown(); |
581 | + } |
582 | + |
583 | + void start_timedate1(const std::string& tzid) |
584 | + { |
585 | + // start dbusmock with the timedated template |
586 | + auto json_parameters = g_strdup_printf("{\"Timezone\": \"%s\"}", tzid.c_str()); |
587 | + const gchar* child_argv[] = { "python3", "-m", "dbusmock", "--template", "timedated", "--parameters", json_parameters, nullptr }; |
588 | + GError* error = nullptr; |
589 | + g_spawn_async(nullptr, (gchar**)child_argv, nullptr, G_SPAWN_SEARCH_PATH, nullptr, nullptr, nullptr, &error); |
590 | + g_assert_no_error(error); |
591 | + g_free(json_parameters); |
592 | + |
593 | + // wait for it to appear on the bus |
594 | + wait_for_name_owned(m_bus, Bus::Timedate1::BUSNAME); |
595 | + } |
596 | + |
597 | + bool wait_for_tzid(const std::string& tzid, Timezone& tz) |
598 | + { |
599 | + return wait_for([&tzid, &tz](){return tzid == tz.timezone.get();}); |
600 | + } |
601 | + |
602 | + void set_timedate1_timezone(const std::string& tzid) |
603 | + { |
604 | + GError* error {}; |
605 | + auto v = g_dbus_connection_call_sync( |
606 | + m_bus, |
607 | + Bus::Timedate1::BUSNAME, |
608 | + Bus::Timedate1::ADDR, |
609 | + Bus::Timedate1::IFACE, |
610 | + Bus::Timedate1::Methods::SET_TIMEZONE, |
611 | + g_variant_new("(sb)", tzid.c_str(), FALSE), |
612 | + nullptr, |
613 | + G_DBUS_CALL_FLAGS_NONE, |
614 | + -1, |
615 | + nullptr, |
616 | + &error); |
617 | + g_clear_pointer(&v, g_variant_unref); |
618 | + g_assert_no_error(error); |
619 | + } |
620 | +}; |
621 | + |
622 | +#define EXPECT_TZID(expected_tzid, tmp) \ |
623 | + EXPECT_TRUE(wait_for_tzid(expected_tzid, tmp)) \ |
624 | + << "expected " << expected_tzid \ |
625 | + << " got " << tmp.timezone.get(); |
626 | |
627 | /*** |
628 | **** |
629 | ***/ |
630 | |
631 | -#define TIMEZONE_FILE (SANDBOX"/timezone") |
632 | - |
633 | -class TimezoneFixture: public TimedateFixture |
634 | -{ |
635 | - private: |
636 | - |
637 | - typedef TimedateFixture super; |
638 | - |
639 | - protected: |
640 | - |
641 | - void SetUp() override |
642 | - { |
643 | - super::SetUp(); |
644 | - } |
645 | - |
646 | - void TearDown() override |
647 | - { |
648 | - super::TearDown(); |
649 | - } |
650 | - |
651 | - public: |
652 | - |
653 | - /* convenience func to set the timezone file */ |
654 | - void set_file(const std::string& text) |
655 | - { |
656 | - g_debug("set_file %s %s", TIMEZONE_FILE, text.c_str()); |
657 | - auto fp = fopen(TIMEZONE_FILE, "w+"); |
658 | - fprintf(fp, "%s\n", text.c_str()); |
659 | - fclose(fp); |
660 | - sync(); |
661 | - } |
662 | -}; |
663 | - |
664 | -/** |
665 | - * Test that timezone-timedated warns, but doesn't crash, if the timezone file doesn't exist |
666 | - */ |
667 | -TEST_F(TimezoneFixture, NoFile) |
668 | -{ |
669 | - remove(TIMEZONE_FILE); |
670 | - ASSERT_FALSE(g_file_test(TIMEZONE_FILE, G_FILE_TEST_EXISTS)); |
671 | - |
672 | - expectLogMessage(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, "*No such file or directory*"); |
673 | - TimedatedTimezone tz(TIMEZONE_FILE); |
674 | -} |
675 | - |
676 | -/** |
677 | - * Test that timezone-timedated gives a default of UTC if the file doesn't exist |
678 | - */ |
679 | -TEST_F(TimezoneFixture, DefaultValueNoFile) |
680 | -{ |
681 | - const std::string expected_timezone = "Etc/Utc"; |
682 | - remove(TIMEZONE_FILE); |
683 | - ASSERT_FALSE(g_file_test(TIMEZONE_FILE, G_FILE_TEST_EXISTS)); |
684 | - |
685 | - expectLogMessage(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, "*No such file or directory*"); |
686 | - TimedatedTimezone tz(TIMEZONE_FILE); |
687 | - ASSERT_EQ(expected_timezone, tz.timezone.get()); |
688 | -} |
689 | - |
690 | -/** |
691 | - * Test that timezone-timedated picks up the initial value |
692 | - */ |
693 | -TEST_F(TimezoneFixture, InitialValue) |
694 | -{ |
695 | - const std::string expected_timezone = "America/Chicago"; |
696 | - set_file(expected_timezone); |
697 | - TimedatedTimezone tz(TIMEZONE_FILE); |
698 | -} |
699 | - |
700 | -/** |
701 | - * Test that changing the tz after we are running works. |
702 | - */ |
703 | -TEST_F(TimezoneFixture, ChangedValue) |
704 | -{ |
705 | - const std::string initial_timezone = "America/Chicago"; |
706 | - const std::string changed_timezone = "America/New_York"; |
707 | - |
708 | - set_file(initial_timezone); |
709 | - |
710 | - TimedatedTimezone tz(TIMEZONE_FILE); |
711 | - ASSERT_EQ(initial_timezone, tz.timezone.get()); |
712 | - |
713 | - bool changed = false; |
714 | - tz.timezone.changed().connect( |
715 | - [&changed, this](const std::string& s){ |
716 | - g_message("timezone changed to %s", s.c_str()); |
717 | - changed = true; |
718 | - g_main_loop_quit(loop); |
719 | - }); |
720 | - |
721 | - g_idle_add([](gpointer gself){ |
722 | - static_cast<TimedateFixture*>(gself)->set_timezone("America/New_York"); |
723 | - return G_SOURCE_REMOVE; |
724 | - }, this); |
725 | - |
726 | - g_main_loop_run(loop); |
727 | - |
728 | - ASSERT_TRUE(changed); |
729 | - ASSERT_EQ(changed_timezone, tz.timezone.get()); |
730 | -} |
731 | - |
732 | -/** |
733 | - * Test that timezone-timedated picks up the initial value |
734 | - */ |
735 | -TEST_F(TimezoneFixture, IgnoreComments) |
736 | -{ |
737 | - const std::string comment = "# Created by cloud-init v. 0.7.5 on Thu, 24 Apr 2014 14:03:29 +0000"; |
738 | - const std::string expected_timezone = "Europe/Berlin"; |
739 | - set_file(comment + "\n" + expected_timezone); |
740 | - TimedatedTimezone tz(TIMEZONE_FILE); |
741 | - ASSERT_EQ(expected_timezone, tz.timezone.get()); |
742 | +TEST_F(Timedate1Fixture, HelloWorld) |
743 | +{ |
744 | +} |
745 | + |
746 | +/** |
747 | + * Test that the tzid is right if timedated isn't available |
748 | + */ |
749 | +TEST_F(Timedate1Fixture, DefaultTimezone) |
750 | +{ |
751 | + const std::string expected_tzid{"Etc/Utc"}; |
752 | + |
753 | + TimedatedTimezone tmp {m_bus}; |
754 | + EXPECT_TZID(expected_tzid, tmp); |
755 | +} |
756 | + |
757 | +/** |
758 | + * Test that the tzid is right if timedated shows BEFORE we start |
759 | + */ |
760 | +TEST_F(Timedate1Fixture, Timedate1First) |
761 | +{ |
762 | + const std::string expected_tzid{"America/Chicago"}; |
763 | + |
764 | + start_timedate1(expected_tzid); |
765 | + TimedatedTimezone tmp {m_bus}; |
766 | + EXPECT_TZID(expected_tzid, tmp); |
767 | +} |
768 | + |
769 | +/** |
770 | + * Test that the tzid is right if timedated shows AFTER we start |
771 | + */ |
772 | +TEST_F(Timedate1Fixture, Timedate1Last) |
773 | +{ |
774 | + const std::string expected_tzid("America/Los_Angeles"); |
775 | + |
776 | + TimedatedTimezone tmp {m_bus}; |
777 | + start_timedate1(expected_tzid); |
778 | + EXPECT_TZID(expected_tzid, tmp); |
779 | +} |
780 | + |
781 | +/** |
782 | + * Test that the tzid is right if timedated's property changes |
783 | + */ |
784 | +TEST_F(Timedate1Fixture, TimezoneChange) |
785 | +{ |
786 | + const std::vector<std::string> expected_tzids{"America/Los_Angeles", "America/Chicago", "Etc/Utc"}; |
787 | + |
788 | + TimedatedTimezone tmp {m_bus}; |
789 | + start_timedate1("America/New_York"); |
790 | + |
791 | + for(const auto& expected_tzid : expected_tzids) |
792 | + { |
793 | + set_timedate1_timezone(expected_tzid); |
794 | + EXPECT_TZID(expected_tzid, tmp); |
795 | + } |
796 | } |
PASSED: Continuous integration, rev:443 jenkins. qa.ubuntu. com/job/ indicator- datetime- ci/370/ jenkins. qa.ubuntu. com/job/ indicator- datetime- wily-amd64- ci/60 jenkins. qa.ubuntu. com/job/ indicator- datetime- wily-armhf- ci/60 jenkins. qa.ubuntu. com/job/ indicator- datetime- wily-armhf- ci/60/artifact/ work/output/ *zip*/output. zip
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/indicator- datetime- ci/370/ rebuild
http://