The Sync Menu

Merge lp:~charlesk/indicator-sync/lp-1040137 into lp:indicator-sync/12.10

  1. lp-1040137
  2. Merge into trunk
Proposed by Charles Kerr on 2012-08-29
Status: Merged
Approved by: Charles Kerr on 2012-09-04
Approved revision: 51
Merged at revision: 13
Proposed branch: lp:~charlesk/indicator-sync/lp-1040137
Merge into: lp:indicator-sync/12.10
Diff against target: 392 lines (+211/-46)
3 files modified
src/service/app-menu-item.c (+4/-4)
src/service/sync-service.c (+25/-24)
test/test-client.cpp (+182/-18)
To merge this branch: bzr merge lp:~charlesk/indicator-sync/lp-1040137
Reviewer Review Type Date Requested Status
Lars Karlitski (community) Approve on 2012-09-01
jenkins (community) continuous-integration 2012-08-29 Approve on 2012-08-31
Review via email: mp+121932@code.launchpad.net

Description of the Change

Re-proposing to trigger Jenkins

To post a comment you must log in.
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
Lars Karlitski (larsu) wrote :

I'm starting to feel bad about my testing with all those tests your adding to i-sync ;)

I think it would make sense to also test removing a sync client in TestClientCount.

TestState is impressive.

r49 is the best.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/service/app-menu-item.c'
2--- src/service/app-menu-item.c 2012-08-25 20:27:47 +0000
3+++ src/service/app-menu-item.c 2012-08-31 00:21:18 +0000
4@@ -316,11 +316,11 @@
5 */
6
7 static gchar*
8-get_iconstr (const gchar * desktop_filename, GAppInfo * appinfo)
9+get_iconstr (const gchar * desktop_filename, GAppInfo * app_info)
10 {
11 gchar * iconstr = NULL;
12
13- if (iconstr == NULL)
14+ if ((iconstr == NULL) && (desktop_filename != NULL))
15 {
16 /* see if the .desktop file has an entry specifying its sync menu icon */
17 GKeyFile * keyfile = g_key_file_new ();
18@@ -331,9 +331,9 @@
19 g_key_file_free (keyfile);
20 }
21
22- if (iconstr == NULL)
23+ if ((iconstr == NULL) && (app_info != NULL))
24 {
25- GIcon * icon = g_app_info_get_icon (appinfo);
26+ GIcon * icon = g_app_info_get_icon (app_info);
27 iconstr = g_icon_to_string (icon);
28 }
29
30
31=== modified file 'src/service/sync-service.c'
32--- src/service/sync-service.c 2012-08-25 21:43:05 +0000
33+++ src/service/sync-service.c 2012-08-31 00:21:18 +0000
34@@ -266,7 +266,8 @@
35 dbusmenu_menuitem_child_append (root, mi);
36
37 /* add the client's custom menuitems */
38- service_menu_append_client_menu (root, entry->menu_client);
39+ if (entry->menu_client != NULL)
40+ service_menu_append_client_menu (root, entry->menu_client);
41
42 /* add a separator before the next client */
43 if (l->next != NULL)
44@@ -430,13 +431,21 @@
45
46 const gchar * name = g_dbus_proxy_get_name (G_DBUS_PROXY(entry->sync_menu_app));
47 const gchar * path = dbus_sync_menu_app_get_menu_path (entry->sync_menu_app);
48- entry->menu_client = dbusmenu_client_new (name, path);
49-
50- entry->menu_client_root_handler_id = g_signal_connect (
51- entry->menu_client,
52- DBUSMENU_CLIENT_SIGNAL_ROOT_CHANGED,
53- G_CALLBACK(on_client_menu_root_changed),
54- service);
55+
56+ if (name && *name && path && *path)
57+ {
58+ DbusmenuClient * client;
59+ gulong id;
60+
61+ client = dbusmenu_client_new (name, path);
62+ id = g_signal_connect (client,
63+ DBUSMENU_CLIENT_SIGNAL_ROOT_CHANGED,
64+ G_CALLBACK(on_client_menu_root_changed),
65+ service);
66+
67+ entry->menu_client = client;
68+ entry->menu_client_root_handler_id = id;
69+ }
70 }
71
72 static void
73@@ -580,9 +589,17 @@
74 {
75 g_error ("unable to get bus: %s", err->message);
76 g_clear_error (&err);
77+ g_main_loop_quit (sync_service.mainloop);
78 }
79 else
80 {
81+ service->indicator_service = indicator_service_new_version (SYNC_SERVICE_DBUS_NAME,
82+ 1);
83+ g_signal_connect_swapped (sync_service.indicator_service,
84+ INDICATOR_SERVICE_SIGNAL_SHUTDOWN,
85+ G_CALLBACK(g_main_loop_quit),
86+ sync_service.mainloop);
87+
88 g_dbus_interface_skeleton_export (
89 G_DBUS_INTERFACE_SKELETON(service->skeleton),
90 connection,
91@@ -614,14 +631,6 @@
92 }
93 }
94
95-static void
96-service_shutdown (IndicatorService * service G_GNUC_UNUSED, gpointer user_data)
97-{
98- SyncService * sync_service = user_data;
99- g_debug ("sync-service shutting down by request");
100- g_main_loop_quit (sync_service->mainloop);
101-}
102-
103 int
104 main (int argc, char ** argv)
105 {
106@@ -633,14 +642,6 @@
107
108 g_type_init ();
109
110- sync_service.indicator_service =
111- indicator_service_new_version (SYNC_SERVICE_DBUS_NAME, 1);
112-
113- g_signal_connect (sync_service.indicator_service,
114- INDICATOR_SERVICE_SIGNAL_SHUTDOWN,
115- G_CALLBACK(service_shutdown),
116- &sync_service);
117-
118 sync_service.skeleton = dbus_sync_service_skeleton_new ();
119
120 g_bus_get (G_BUS_TYPE_SESSION, NULL, on_got_bus, &sync_service);
121
122=== modified file 'test/test-client.cpp'
123--- test/test-client.cpp 2012-08-27 15:47:25 +0000
124+++ test/test-client.cpp 2012-08-31 00:21:18 +0000
125@@ -9,6 +9,9 @@
126 #include "sync-service-dbus.h"
127 #include "sync-menu/sync-app.h"
128
129+#define INDICATOR_SERVICE_OBJECT_PATH "/org/ayatana/indicator/service"
130+#define INDICATOR_SERVICE_INTERFACE_NAME "org.ayatana.indicator.service"
131+
132 class ClientTest : public ::testing::Test
133 {
134 protected:
135@@ -29,6 +32,8 @@
136 if (!ran_once_init)
137 {
138 g_type_init();
139+ g_setenv ("INDICATOR_SERVICE_SHUTDOWN_TIMEOUT", "5000", TRUE);
140+ g_unsetenv ("INDICATOR_ALLOW_NO_WATCHERS");
141 g_unsetenv ("INDICATOR_SERVICE_REPLACE_MODE");
142 ran_once_init = true;
143 }
144@@ -81,6 +86,7 @@
145 g_error ("unable to create service proxy: %s", err->message);
146 g_debug (G_STRLOC" the service proxy %p refcount is %d", service_proxy, G_OBJECT(service_proxy)->ref_count);
147 g_debug (G_STRLOC" the dbus connection %p refcount is %d", session_bus, G_OBJECT(session_bus)->ref_count);
148+ g_debug (G_STRLOC" proxy's name owner is \"%s\"", g_dbus_proxy_get_name_owner(G_DBUS_PROXY(service_proxy)));
149
150 ASSERT_TRUE (err == NULL);
151 ASSERT_EQ (dbus_sync_service_get_client_count (service_proxy), 0);
152@@ -96,27 +102,33 @@
153 return is_owned;
154 }
155
156- void ServiceProxyShutdown ()
157+ void CallIndicatorServiceMethod (const gchar * method)
158 {
159- GError * err;
160 GVariant * ret;
161
162 ASSERT_TRUE (session_bus != NULL);
163 ASSERT_TRUE (service_proxy != NULL);
164+ ASSERT_TRUE (ServiceProxyIsOwned ());
165
166- err = NULL;
167 ret = g_dbus_connection_call_sync (
168 session_bus,
169 SYNC_SERVICE_DBUS_NAME,
170- "/org/ayatana/indicator/service",
171- "org.ayatana.indicator.service",
172- "Shutdown", NULL,
173+ INDICATOR_SERVICE_OBJECT_PATH,
174+ INDICATOR_SERVICE_INTERFACE_NAME,
175+ method, NULL,
176 NULL,
177 G_DBUS_CALL_FLAGS_NONE,
178- -1, NULL, &err);
179+ -1, NULL, NULL);
180+
181 g_clear_pointer (&ret, g_variant_unref);
182-
183- ASSERT_TRUE (err == NULL);
184+ }
185+
186+ void ServiceProxyShutdown ()
187+ {
188+ CallIndicatorServiceMethod ("Shutdown");
189+ WaitForSignal (service_proxy, "notify::g-name-owner");
190+
191+ ASSERT_FALSE (ServiceProxyIsOwned ());
192 }
193
194 // undo SetUpServiceProxy
195@@ -124,6 +136,9 @@
196 {
197 ASSERT_TRUE (service_proxy != NULL);
198
199+ if (ServiceProxyIsOwned ())
200+ ServiceProxyShutdown ();
201+
202 g_clear_object (&service_proxy);
203 }
204
205@@ -142,8 +157,7 @@
206 ****
207 ***/
208
209-
210-TEST_F(ClientTest, TestCanStartService)
211+TEST_F (ClientTest, TestCanStartService)
212 {
213 ASSERT_TRUE (service_proxy == NULL);
214 SetUpServiceProxy (true);
215@@ -151,8 +165,7 @@
216 TearDownServiceProxy ();
217 }
218
219-
220-TEST_F(ClientTest, AppCanStartService)
221+TEST_F (ClientTest, AppCanStartService)
222 {
223 SyncMenuApp * app;
224 SetUpServiceProxy (false);
225@@ -164,11 +177,162 @@
226 WaitForSignal (service_proxy, "notify::client-count");
227 ASSERT_EQ (1, dbus_sync_service_get_client_count (service_proxy));
228
229- ServiceProxyShutdown ();
230- WaitForSignal (service_proxy, "notify::g-name-owner");
231- ASSERT_FALSE (ServiceProxyIsOwned ());
232- ASSERT_EQ (0, dbus_sync_service_get_client_count (service_proxy));
233-
234 TearDownServiceProxy ();
235 g_clear_object (&app);
236 }
237+
238+/***
239+**** Add a handful of SyncMenuApps & confirm that the service counts them
240+***/
241+
242+TEST_F (ClientTest, TestClientCount)
243+{
244+ SyncMenuApp * app[3];
245+
246+ ASSERT_TRUE (service_proxy == NULL);
247+ SetUpServiceProxy (true);
248+ ASSERT_TRUE (ServiceProxyIsOwned ());
249+
250+ app[0] = sync_menu_app_new ("a.desktop");
251+ WaitForSignal (service_proxy, "notify::client-count");
252+ ASSERT_EQ (1, dbus_sync_service_get_client_count (service_proxy));
253+
254+ app[1] = sync_menu_app_new ("b.desktop");
255+ WaitForSignal (service_proxy, "notify::client-count");
256+ ASSERT_EQ (2, dbus_sync_service_get_client_count (service_proxy));
257+
258+ app[2] = sync_menu_app_new ("c.desktop");
259+ WaitForSignal (service_proxy, "notify::client-count");
260+ ASSERT_EQ (3, dbus_sync_service_get_client_count (service_proxy));
261+
262+ g_clear_object (&app[2]);
263+ g_clear_object (&app[1]);
264+ g_clear_object (&app[0]);
265+ TearDownServiceProxy ();
266+}
267+
268+/***
269+**** Confirm that the service's 'paused' property is true
270+**** iff any of the clients are paused
271+***/
272+
273+TEST_F (ClientTest, TestPaused)
274+{
275+ /* start up the service */
276+ ASSERT_TRUE (service_proxy == NULL);
277+ SetUpServiceProxy (true);
278+ ASSERT_TRUE (ServiceProxyIsOwned ());
279+
280+ /* add three SyncMenuApps */
281+ SyncMenuApp * app[3];
282+ app[0] = sync_menu_app_new ("a.desktop");
283+ app[1] = sync_menu_app_new ("b.desktop");
284+ app[2] = sync_menu_app_new ("c.desktop");
285+ while (dbus_sync_service_get_client_count (service_proxy) != 3)
286+ WaitForSignal (service_proxy, "notify::client-count");
287+
288+ /* confirm that setting any one of the SyncMenuApps
289+ to paused will set the service's "Paused" property */
290+ ASSERT_FALSE (dbus_sync_service_get_paused (service_proxy));
291+ for (int i=0; i<3; i++)
292+ {
293+ sync_menu_app_set_paused (app[i], true);
294+ WaitForSignal (service_proxy, "notify::paused");
295+ ASSERT_TRUE (dbus_sync_service_get_paused (service_proxy));
296+
297+ sync_menu_app_set_paused (app[i], false);
298+ WaitForSignal (service_proxy, "notify::paused");
299+ ASSERT_FALSE (dbus_sync_service_get_paused (service_proxy));
300+ }
301+
302+ /* cleanup */
303+ g_clear_object (&app[2]);
304+ g_clear_object (&app[1]);
305+ g_clear_object (&app[0]);
306+ TearDownServiceProxy ();
307+}
308+
309+/***
310+**** Test that SyncService correctly reports the right SyncMenuState
311+**** based on the SyncMenuApps that are connected to it
312+***/
313+
314+TEST_F (ClientTest, TestState)
315+{
316+ /* start up the service */
317+ ASSERT_TRUE (service_proxy == NULL);
318+ SetUpServiceProxy (true);
319+ ASSERT_TRUE (ServiceProxyIsOwned ());
320+
321+ /* add three SyncMenuApps */
322+ SyncMenuApp * app[3];
323+ app[0] = sync_menu_app_new ("a.desktop");
324+ app[1] = sync_menu_app_new ("b.desktop");
325+ app[2] = sync_menu_app_new ("c.desktop");
326+ while (dbus_sync_service_get_client_count (service_proxy) != 3)
327+ WaitForSignal (service_proxy, "notify::client-count");
328+
329+ /* the default state is "idle" */
330+ ASSERT_TRUE (sync_menu_app_get_state (app[0]) == SYNC_MENU_STATE_IDLE);
331+ ASSERT_TRUE (sync_menu_app_get_state (app[1]) == SYNC_MENU_STATE_IDLE);
332+ ASSERT_TRUE (sync_menu_app_get_state (app[2]) == SYNC_MENU_STATE_IDLE);
333+ ASSERT_TRUE (dbus_sync_service_get_state (service_proxy) == SYNC_MENU_STATE_IDLE);
334+
335+ /* test all the state combinations for three SyncMenuApps */
336+ const SyncMenuState test_states[27][4] =
337+ {
338+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_IDLE },
339+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
340+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING },
341+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR },
342+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
343+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR },
344+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING },
345+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
346+ { SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING },
347+
348+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING },
349+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
350+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING },
351+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR },
352+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
353+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR },
354+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING },
355+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
356+ { SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING },
357+
358+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR },
359+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
360+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR },
361+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR },
362+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
363+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR },
364+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_IDLE, SYNC_MENU_STATE_ERROR },
365+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_ERROR },
366+ { SYNC_MENU_STATE_ERROR, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_SYNCING, SYNC_MENU_STATE_ERROR }
367+ };
368+
369+ for (int i=0; i<27; i++)
370+ {
371+ /* set the three apps' states */
372+ for (int j=0; j<3; j++)
373+ {
374+ SyncMenuApp * a = app[j];
375+ const SyncMenuState state = test_states[i][j];
376+ sync_menu_app_set_state (a, state);
377+ ASSERT_EQ (state, sync_menu_app_get_state(a));
378+ }
379+
380+ /* test to see the service's state reflects */
381+ const SyncMenuState expected = test_states[i][3];
382+ if (dbus_sync_service_get_state (service_proxy) != expected)
383+ WaitForSignal (service_proxy, "notify::state");
384+ ASSERT_EQ (expected, dbus_sync_service_get_state (service_proxy));
385+ }
386+
387+ /* cleanup */
388+ g_clear_object (&app[2]);
389+ g_clear_object (&app[1]);
390+ g_clear_object (&app[0]);
391+ TearDownServiceProxy ();
392+}

Subscribers

People subscribed via source and target branches

to all changes: