From dfa2a4ae75af50d6dfa66cdfb14efb382140d22a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 5 Sep 2018 11:25:03 +0100 Subject: [PATCH 1/4] gtestutils: Print non-matching stderr/stdout output on trap failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When running a test as a subprocess and matching its output, it’s very annoying for GLib to tell you that the output didn’t match your pattern, *but not actually say what the output was*. Fix that. Signed-off-by: Philip Withnall --- glib/gtestutils.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/glib/gtestutils.c b/glib/gtestutils.c index 7b29c274e..119c55d92 100644 --- a/glib/gtestutils.c +++ b/glib/gtestutils.c @@ -3343,7 +3343,8 @@ g_test_trap_assertions (const char *domain, logged_child_output = logged_child_output || log_child_output (process_id); - msg = g_strdup_printf ("stdout of child process (%s) %s: %s", process_id, match_error, stdout_pattern); + msg = g_strdup_printf ("stdout of child process (%s) %s: %s\nstderr was:\n%s", + process_id, match_error, stdout_pattern, test_trap_last_stdout); g_assertion_message (domain, file, line, func, msg); g_free (msg); } @@ -3353,7 +3354,8 @@ g_test_trap_assertions (const char *domain, logged_child_output = logged_child_output || log_child_output (process_id); - msg = g_strdup_printf ("stderr of child process (%s) %s: %s", process_id, match_error, stderr_pattern); + msg = g_strdup_printf ("stderr of child process (%s) %s: %s\nstderr was:\n%s", + process_id, match_error, stderr_pattern, test_trap_last_stderr); g_assertion_message (domain, file, line, func, msg); g_free (msg); } From 35d1ef678aacdf9e7b616465e5ce20368f4f106e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 5 Sep 2018 11:25:58 +0100 Subject: [PATCH 2/4] gliststore: Simplify a GType check on construction When setting the GListStore:item-type property we need to check the GType is a GObject (or subclass). There was some explicit code for this, but when actually testing it and looking at the code coverage, it turns out that the GObject property type check coming from g_param_spec_gtype() does everything we want, and the custom g_critical() can never be hit. So turn it into an assertion. Signed-off-by: Philip Withnall --- gio/gliststore.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/gio/gliststore.c b/gio/gliststore.c index c91dcb334..8d9fbfe35 100644 --- a/gio/gliststore.c +++ b/gio/gliststore.c @@ -125,10 +125,8 @@ g_list_store_set_property (GObject *object, switch (property_id) { case PROP_ITEM_TYPE: /* construct-only */ + g_assert (g_type_is_a (g_value_get_gtype (value), G_TYPE_OBJECT)); store->item_type = g_value_get_gtype (value); - if (!g_type_is_a (store->item_type, G_TYPE_OBJECT)) - g_critical ("GListStore cannot store items of type '%s'. Items must be GObjects.", - g_type_name (store->item_type)); break; default: From 7e33c50dd3705e1b8f43d6f26f4eb7f687e9bd30 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 5 Sep 2018 11:27:06 +0100 Subject: [PATCH 3/4] tests: Add more GListStore tests to get it too 100% coverage Signed-off-by: Philip Withnall --- gio/tests/glistmodel.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/gio/tests/glistmodel.c b/gio/tests/glistmodel.c index dcf571f29..b12251b96 100644 --- a/gio/tests/glistmodel.c +++ b/gio/tests/glistmodel.c @@ -21,6 +21,38 @@ #include +/* Test that constructing/getting/setting properties on a #GListStore works. */ +static void +test_store_properties (void) +{ + GListStore *store = NULL; + GType item_type; + + store = g_list_store_new (G_TYPE_MENU_ITEM); + g_object_get (store, "item-type", &item_type, NULL); + g_assert_cmpint (item_type, ==, G_TYPE_MENU_ITEM); + + g_clear_object (&store); +} + +/* Test that #GListStore rejects non-GObject item types. */ +static void +test_store_non_gobjects (void) +{ + if (g_test_subprocess ()) + { + /* We have to use g_object_new() since g_list_store_new() checks the item + * type. We want to check the property setter code works properly. */ + g_object_new (G_TYPE_LIST_STORE, "item-type", G_TYPE_LONG, NULL); + return; + } + + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*WARNING*value * of type 'GType' is invalid or " + "out of range for property 'item-type'*"); +} + static void test_store_boundaries (void) { @@ -736,6 +768,8 @@ int main (int argc, char *argv[]) g_test_init (&argc, &argv, NULL); g_test_bug_base ("https://bugzilla.gnome.org/"); + g_test_add_func ("/glistmodel/store/properties", test_store_properties); + g_test_add_func ("/glistmodel/store/non-gobjects", test_store_non_gobjects); g_test_add_func ("/glistmodel/store/boundaries", test_store_boundaries); g_test_add_func ("/glistmodel/store/refcounts", test_store_refcounts); g_test_add_func ("/glistmodel/store/sorted", test_store_sorted); From 53a6689ebf3d7e8f823819e31200b033eb7b293d Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 5 Sep 2018 11:36:13 +0100 Subject: [PATCH 4/4] tests: Test g_list_model_get_object() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It wasn’t being tested. It should behave the same as g_list_model_get_item(), so write a wrapper for the two. This brings the code coverage of glistmodel.c up to 100%. Signed-off-by: Philip Withnall --- gio/tests/glistmodel.c | 75 +++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 30 deletions(-) diff --git a/gio/tests/glistmodel.c b/gio/tests/glistmodel.c index b12251b96..533e2e47d 100644 --- a/gio/tests/glistmodel.c +++ b/gio/tests/glistmodel.c @@ -21,6 +21,21 @@ #include +/* Wrapper around g_list_model_get_item() and g_list_model_get_object() which + * checks they return the same thing. */ +static gpointer +list_model_get (GListModel *model, + guint position) +{ + GObject *item = g_list_model_get_item (model, position); + GObject *object = g_list_model_get_object (model, position); + + g_assert_true (item == object); + + g_clear_object (&object); + return g_steal_pointer (&item); +} + /* Test that constructing/getting/setting properties on a #GListStore works. */ static void test_store_properties (void) @@ -124,7 +139,7 @@ test_store_refcounts (void) store = g_list_store_new (G_TYPE_MENU_ITEM); g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, 0); - g_assert_null (g_list_model_get_item (G_LIST_MODEL (store), 0)); + g_assert_null (list_model_get (G_LIST_MODEL (store), 0)); n_items = G_N_ELEMENTS (items); for (i = 0; i < n_items; i++) @@ -138,17 +153,17 @@ test_store_refcounts (void) } g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, n_items); - g_assert_null (g_list_model_get_item (G_LIST_MODEL (store), n_items)); + g_assert_null (list_model_get (G_LIST_MODEL (store), n_items)); - tmp = g_list_model_get_item (G_LIST_MODEL (store), 3); - g_assert (tmp == items[3]); + tmp = list_model_get (G_LIST_MODEL (store), 3); + g_assert_true (tmp == items[3]); g_object_unref (tmp); g_list_store_remove (store, 4); g_assert_null (items[4]); n_items--; g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, n_items); - g_assert_null (g_list_model_get_item (G_LIST_MODEL (store), n_items)); + g_assert_null (list_model_get (G_LIST_MODEL (store), n_items)); g_object_unref (store); for (i = 0; i < G_N_ELEMENTS (items); i++) @@ -221,8 +236,8 @@ test_store_sorted (void) GObject *a, *b; /* should see our two copies */ - a = g_list_model_get_item (G_LIST_MODEL (store), i * 2); - b = g_list_model_get_item (G_LIST_MODEL (store), i * 2 + 1); + a = list_model_get (G_LIST_MODEL (store), i * 2); + b = list_model_get (G_LIST_MODEL (store), i * 2 + 1); g_assert (compare_items (a, b, GUINT_TO_POINTER(0x1234)) == 0); g_assert (a != b); @@ -231,7 +246,7 @@ test_store_sorted (void) { GObject *c; - c = g_list_model_get_item (G_LIST_MODEL (store), i * 2 - 1); + c = list_model_get (G_LIST_MODEL (store), i * 2 - 1); g_assert (c != a); g_assert (c != b); @@ -273,13 +288,13 @@ test_store_splice_replace_middle (void) g_list_store_splice (store, 0, 0, array->pdata, 3); g_assert_cmpuint (g_list_model_get_n_items (model), ==, 3); - item = g_list_model_get_item (model, 0); + item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "1"); g_object_unref (item); - item = g_list_model_get_item (model, 1); + item = list_model_get (model, 1); g_assert_cmpstr (g_action_get_name (item), ==, "2"); g_object_unref (item); - item = g_list_model_get_item (model, 2); + item = list_model_get (model, 2); g_assert_cmpstr (g_action_get_name (item), ==, "3"); g_object_unref (item); @@ -287,16 +302,16 @@ test_store_splice_replace_middle (void) g_list_store_splice (store, 1, 1, array->pdata + 3, 2); g_assert_cmpuint (g_list_model_get_n_items (model), ==, 4); - item = g_list_model_get_item (model, 0); + item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "1"); g_object_unref (item); - item = g_list_model_get_item (model, 1); + item = list_model_get (model, 1); g_assert_cmpstr (g_action_get_name (item), ==, "4"); g_object_unref (item); - item = g_list_model_get_item (model, 2); + item = list_model_get (model, 2); g_assert_cmpstr (g_action_get_name (item), ==, "5"); g_object_unref (item); - item = g_list_model_get_item (model, 3); + item = list_model_get (model, 3); g_assert_cmpstr (g_action_get_name (item), ==, "3"); g_object_unref (item); @@ -328,10 +343,10 @@ test_store_splice_replace_all (void) g_list_store_splice (store, 0, 0, array->pdata, 2); g_assert_cmpuint (g_list_model_get_n_items (model), ==, 2); - item = g_list_model_get_item (model, 0); + item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "1"); g_object_unref (item); - item = g_list_model_get_item (model, 1); + item = list_model_get (model, 1); g_assert_cmpstr (g_action_get_name (item), ==, "2"); g_object_unref (item); @@ -339,10 +354,10 @@ test_store_splice_replace_all (void) g_list_store_splice (store, 0, 2, array->pdata + 2, 2); g_assert_cmpuint (g_list_model_get_n_items (model), ==, 2); - item = g_list_model_get_item (model, 0); + item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "3"); g_object_unref (item); - item = g_list_model_get_item (model, 1); + item = list_model_get (model, 1); g_assert_cmpstr (g_action_get_name (item), ==, "4"); g_object_unref (item); @@ -376,7 +391,7 @@ test_store_splice_noop (void) g_list_store_splice (store, 1, 0, NULL, 0); g_assert_cmpuint (g_list_model_get_n_items (model), ==, 1); - item = g_list_model_get_item (model, 0); + item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "1"); g_object_unref (item); @@ -396,7 +411,7 @@ model_array_equal (GListModel *model, GPtrArray *array) GObject *ptr; gboolean ptrs_equal; - ptr = g_list_model_get_item (model, i); + ptr = list_model_get (model, i); ptrs_equal = (g_ptr_array_index (array, i) == ptr); g_object_unref (ptr); if (!ptrs_equal) @@ -621,33 +636,33 @@ test_store_get_item_cache (void) g_list_store_append (store, item2); /* Clear the cache */ - g_assert_null (g_list_model_get_item (model, 42)); + g_assert_null (list_model_get (model, 42)); /* Access the same position twice */ - temp = g_list_model_get_item (model, 1); + temp = list_model_get (model, 1); g_assert (temp == item2); g_object_unref (temp); - temp = g_list_model_get_item (model, 1); + temp = list_model_get (model, 1); g_assert (temp == item2); g_object_unref (temp); - g_assert_null (g_list_model_get_item (model, 42)); + g_assert_null (list_model_get (model, 42)); /* Access forwards */ - temp = g_list_model_get_item (model, 0); + temp = list_model_get (model, 0); g_assert (temp == item1); g_object_unref (temp); - temp = g_list_model_get_item (model, 1); + temp = list_model_get (model, 1); g_assert (temp == item2); g_object_unref (temp); - g_assert_null (g_list_model_get_item (model, 42)); + g_assert_null (list_model_get (model, 42)); /* Access backwards */ - temp = g_list_model_get_item (model, 1); + temp = list_model_get (model, 1); g_assert (temp == item2); g_object_unref (temp); - temp = g_list_model_get_item (model, 0); + temp = list_model_get (model, 0); g_assert (temp == item1); g_object_unref (temp);