From 43969bf41aa01c7664135b8aee926dfd610df864 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 22 May 2020 11:04:34 +0100 Subject: [PATCH 1/2] tests: Port file test to use g_assert_*() rather than g_assert() g_assert() can be compiled out with G_DISABLE_ASSERT, which renders the test rather useless. Signed-off-by: Philip Withnall --- gio/tests/file.c | 74 ++++++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/gio/tests/file.c b/gio/tests/file.c index 8d3aafa63..06ff24908 100644 --- a/gio/tests/file.c +++ b/gio/tests/file.c @@ -18,11 +18,11 @@ test_basic_for_file (GFile *file, g_free (s); s = g_file_get_uri (file); - g_assert (g_str_has_prefix (s, "file://")); - g_assert (g_str_has_suffix (s, suffix)); + g_assert_true (g_str_has_prefix (s, "file://")); + g_assert_true (g_str_has_suffix (s, suffix)); g_free (s); - g_assert (g_file_has_uri_scheme (file, "file")); + g_assert_true (g_file_has_uri_scheme (file, "file")); s = g_file_get_uri_scheme (file); g_assert_cmpstr (s, ==, "file"); g_free (s); @@ -64,13 +64,13 @@ test_parent (void) file2 = g_file_new_for_path ("./some/directory"); root = g_file_new_for_path ("/"); - g_assert (g_file_has_parent (file, file2)); + g_assert_true (g_file_has_parent (file, file2)); parent = g_file_get_parent (file); - g_assert (g_file_equal (parent, file2)); + g_assert_true (g_file_equal (parent, file2)); g_object_unref (parent); - g_assert (g_file_get_parent (root) == NULL); + g_assert_null (g_file_get_parent (root)); g_object_unref (file); g_object_unref (file2); @@ -86,10 +86,10 @@ test_child (void) file = g_file_new_for_path ("./some/directory"); child = g_file_get_child (file, "child"); - g_assert (g_file_has_parent (child, file)); + g_assert_true (g_file_has_parent (child, file)); child2 = g_file_get_child_for_display_name (file, "child2", NULL); - g_assert (g_file_has_parent (child2, file)); + g_assert_true (g_file_has_parent (child2, file)); g_object_unref (child); g_object_unref (child2); @@ -137,7 +137,7 @@ test_parse_name (void) file = g_file_parse_name ("~foo"); name = g_file_get_parse_name (file); - g_assert (name != NULL); + g_assert_nonnull (name); g_object_unref (file); g_free (name); } @@ -209,12 +209,12 @@ iclosed_cb (GObject *source, error = NULL; ret = g_input_stream_close_finish (data->istream, res, &error); g_assert_no_error (error); - g_assert (ret); + g_assert_true (ret); - g_assert (g_input_stream_is_closed (data->istream)); + g_assert_true (g_input_stream_is_closed (data->istream)); ret = g_file_delete (data->file, NULL, &error); - g_assert (ret); + g_assert_true (ret); g_assert_no_error (error); /* work around file monitor bug: @@ -251,7 +251,7 @@ read_cb (GObject *source, else { g_assert_cmpstr (data->buffer, ==, data->data); - g_assert (!g_input_stream_is_closed (data->istream)); + g_assert_false (g_input_stream_is_closed (data->istream)); g_input_stream_close_async (data->istream, 0, NULL, iclosed_cb, data); } } @@ -346,8 +346,8 @@ oclosed_cb (GObject *source, error = NULL; ret = g_output_stream_close_finish (data->ostream, res, &error); g_assert_no_error (error); - g_assert (ret); - g_assert (g_output_stream_is_closed (data->ostream)); + g_assert_true (ret); + g_assert_true (g_output_stream_is_closed (data->ostream)); g_file_read_async (data->file, 0, NULL, opened_cb, data); } @@ -378,7 +378,7 @@ written_cb (GObject *source, } else { - g_assert (!g_output_stream_is_closed (data->ostream)); + g_assert_false (g_output_stream_is_closed (data->ostream)); g_output_stream_close_async (data->ostream, 0, NULL, oclosed_cb, data); } } @@ -409,7 +409,7 @@ created_cb (GObject *source, error = NULL; base = g_file_create_finish (G_FILE (source), res, &error); g_assert_no_error (error); - g_assert (g_file_query_exists (data->file, NULL)); + g_assert_true (g_file_query_exists (data->file, NULL)); if (data->buffersize == 0) data->ostream = G_OUTPUT_STREAM (g_object_ref (base)); @@ -461,13 +461,13 @@ test_create_delete (gconstpointer d) data->file = g_file_new_tmp ("g_file_create_delete_XXXXXX", &iostream, NULL); - g_assert (data->file != NULL); + g_assert_nonnull (data->file); g_object_unref (iostream); data->monitor_path = g_file_get_path (data->file); remove (data->monitor_path); - g_assert (!g_file_query_exists (data->file, NULL)); + g_assert_false (g_file_query_exists (data->file, NULL)); error = NULL; data->monitor = g_file_monitor_file (data->file, 0, NULL, &error); @@ -507,9 +507,9 @@ test_create_delete (gconstpointer d) g_assert_cmpint (data->monitor_deleted, ==, 1); g_assert_cmpint (data->monitor_changed, >, 0); - g_assert (!g_file_monitor_is_cancelled (data->monitor)); + g_assert_false (g_file_monitor_is_cancelled (data->monitor)); g_file_monitor_cancel (data->monitor); - g_assert (g_file_monitor_is_cancelled (data->monitor)); + g_assert_true (g_file_monitor_is_cancelled (data->monitor)); g_main_loop_unref (data->loop); g_object_unref (data->ostream); @@ -582,7 +582,7 @@ loaded_cb (GObject *source, error = NULL; ret = g_file_load_contents_finish (data->file, res, &contents, &length, NULL, &error); - g_assert (ret); + g_assert_true (ret); g_assert_no_error (error); g_assert_cmpint (length, ==, strlen (data->data)); g_assert_cmpstr (contents, ==, data->data); @@ -609,8 +609,8 @@ loaded_cb (GObject *source, error = NULL; ret = g_file_delete (data->file, NULL, &error); g_assert_no_error (error); - g_assert (ret); - g_assert (!g_file_query_exists (data->file, NULL)); + g_assert_true (ret); + g_assert_false (g_file_query_exists (data->file, NULL)); g_main_loop_quit (data->loop); } @@ -644,13 +644,13 @@ test_replace_load (void) data->file = g_file_new_tmp ("g_file_replace_load_XXXXXX", &iostream, NULL); - g_assert (data->file != NULL); + g_assert_nonnull (data->file); g_object_unref (iostream); path = g_file_peek_path (data->file); remove (path); - g_assert (!g_file_query_exists (data->file, NULL)); + g_assert_false (g_file_query_exists (data->file, NULL)); data->loop = g_main_loop_new (NULL, FALSE); @@ -717,11 +717,11 @@ test_replace_cancel (void) info = g_file_enumerator_next_file (fenum, NULL, &error); g_assert_no_error (error); - g_assert (info != NULL); + g_assert_nonnull (info); g_object_unref (info); info = g_file_enumerator_next_file (fenum, NULL, &error); g_assert_no_error (error); - g_assert (info != NULL); + g_assert_nonnull (info); g_object_unref (info); g_file_enumerator_close (fenum, NULL, &error); @@ -736,7 +736,7 @@ test_replace_cancel (void) while (TRUE) { gboolean ret = g_file_enumerator_iterate (fenum, &info, NULL, NULL, &error); - g_assert (ret); + g_assert_true (ret); g_assert_no_error (error); if (!info) break; @@ -758,13 +758,13 @@ test_replace_cancel (void) GFile *child; gboolean ret = g_file_enumerator_iterate (fenum, NULL, &child, NULL, &error); - g_assert (ret); + g_assert_true (ret); g_assert_no_error (error); if (!child) break; - g_assert (G_IS_FILE (child)); + g_assert_true (G_IS_FILE (child)); count++; } g_assert_cmpint (count, ==, 2); @@ -832,7 +832,7 @@ test_async_delete (void) g_assert_no_error (local_error); g_object_unref (iostream); - g_assert (g_file_query_exists (file, NULL)); + g_assert_true (g_file_query_exists (file, NULL)); loop = g_main_loop_new (NULL, TRUE); @@ -840,7 +840,7 @@ test_async_delete (void) g_main_loop_run (loop); - g_assert (!g_file_query_exists (file, NULL)); + g_assert_false (g_file_query_exists (file, NULL)); g_main_loop_unref (loop); g_object_unref (file); @@ -1034,7 +1034,7 @@ test_measure (void) &num_dirs, &num_files, &error); - g_assert (ok); + g_assert_true (ok); g_assert_no_error (error); if (size > 0) @@ -1087,7 +1087,7 @@ measure_done (GObject *source, gboolean ok; ok = g_file_measure_disk_usage_finish (G_FILE (source), res, &num_bytes, &num_dirs, &num_files, &error); - g_assert (ok); + g_assert_true (ok); g_assert_no_error (error); if (data->expected_bytes > 0) @@ -1160,7 +1160,7 @@ test_load_bytes (void) file = g_file_new_for_path (filename); bytes = g_file_load_bytes (file, NULL, NULL, &error); g_assert_no_error (error); - g_assert (bytes != NULL); + g_assert_nonnull (bytes); g_assert_cmpint (len, ==, g_bytes_get_size (bytes)); g_assert_cmpstr ("test_load_bytes", ==, (gchar *)g_bytes_get_data (bytes, NULL)); @@ -1188,7 +1188,7 @@ test_load_bytes_cb (GObject *object, data->bytes = g_file_load_bytes_finish (file, result, NULL, &error); g_assert_no_error (error); - g_assert (data->bytes != NULL); + g_assert_nonnull (data->bytes); g_main_loop_quit (data->main_loop); } From 26c433d26cd5c3e7723c2e0aa714f81be150add4 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 22 May 2020 11:05:27 +0100 Subject: [PATCH 2/2] tests: Speed up the file test and make it more reliable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sometimes this test was timing out due to the file monitor notifications taking longer than the arbitrary 2s delay before ending the test and checking its results at the end of `iclosed_cb()`. Avoid that timing-dependence by ending the test when the expected file monitor notifications are seen, or after a 10s timeout (if so, the test is failed). This makes the test run 4× faster in the normal case, as it’s no longer waiting for a timeout to elapse if the file monitor notifications come in sooner. Signed-off-by: Philip Withnall --- gio/tests/file.c | 51 ++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/gio/tests/file.c b/gio/tests/file.c index 06ff24908..c3877af4b 100644 --- a/gio/tests/file.c +++ b/gio/tests/file.c @@ -144,11 +144,11 @@ test_parse_name (void) typedef struct { + GMainContext *context; GFile *file; GFileMonitor *monitor; GOutputStream *ostream; GInputStream *istream; - GMainLoop *loop; gint buffersize; gint monitor_created; gint monitor_deleted; @@ -158,6 +158,8 @@ typedef struct const gchar *data; gchar *buffer; guint timeout; + gboolean file_deleted; + gboolean timed_out; } CreateDeleteData; static void @@ -183,18 +185,8 @@ monitor_changed (GFileMonitor *monitor, data->monitor_deleted++; if (event_type == G_FILE_MONITOR_EVENT_CHANGED) data->monitor_changed++; -} - -static gboolean -quit_idle (gpointer user_data) -{ - CreateDeleteData *data = user_data; - - g_source_remove (data->timeout); - g_main_loop_quit (data->loop); - - return FALSE; + g_main_context_wakeup (data->context); } static void @@ -217,11 +209,8 @@ iclosed_cb (GObject *source, g_assert_true (ret); g_assert_no_error (error); - /* work around file monitor bug: - * inotify events are only processed every 1000 ms, regardless - * of the rate limit set on the file monitor - */ - g_timeout_add (2000, quit_idle, data); + data->file_deleted = TRUE; + g_main_context_wakeup (data->context); } static void @@ -435,11 +424,14 @@ created_cb (GObject *source, } static gboolean -stop_timeout (gpointer data) +stop_timeout (gpointer user_data) { - g_assert_not_reached (); + CreateDeleteData *data = user_data; - return FALSE; + data->timed_out = TRUE; + g_main_context_wakeup (data->context); + + return G_SOURCE_REMOVE; } /* @@ -495,14 +487,23 @@ test_create_delete (gconstpointer d) g_signal_connect (data->monitor, "changed", G_CALLBACK (monitor_changed), data); - data->loop = g_main_loop_new (NULL, FALSE); - - data->timeout = g_timeout_add (10000, stop_timeout, NULL); + /* Use the global default main context */ + data->context = NULL; + data->timeout = g_timeout_add_seconds (10, stop_timeout, data); g_file_create_async (data->file, 0, 0, NULL, created_cb, data); - g_main_loop_run (data->loop); + while (!data->timed_out && + (data->monitor_created == 0 || + data->monitor_deleted == 0 || + data->monitor_changed == 0 || + !data->file_deleted)) + g_main_context_iteration (data->context, TRUE); + g_source_remove (data->timeout); + + g_assert_false (data->timed_out); + g_assert_true (data->file_deleted); g_assert_cmpint (data->monitor_created, ==, 1); g_assert_cmpint (data->monitor_deleted, ==, 1); g_assert_cmpint (data->monitor_changed, >, 0); @@ -511,7 +512,7 @@ test_create_delete (gconstpointer d) g_file_monitor_cancel (data->monitor); g_assert_true (g_file_monitor_is_cancelled (data->monitor)); - g_main_loop_unref (data->loop); + g_clear_pointer (&data->context, g_main_context_unref); g_object_unref (data->ostream); g_object_unref (data->istream);