From ae0e26dbfe753fcfabbee6e2b79dee873fadd9c4 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Wed, 8 Jan 2020 13:54:43 +0000
Subject: [PATCH] tests: Check error indicator before checking boolean result

If a GLib function that raises an error fails unexpectedly during
testing, it's a lot more informative to see an assertion failure that
includes details of the error, rather than just the boolean result.

For a conventional GLib function we should not look at the "out"
parameters until we have checked that the function was successful.

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 tests/graphics.c        |   4 +
 tests/system-info-cli.c | 169 ++++++++++++++++++++++------------------
 2 files changed, 99 insertions(+), 74 deletions(-)

diff --git a/tests/graphics.c b/tests/graphics.c
index d0d489704..e34c21261 100644
--- a/tests/graphics.c
+++ b/tests/graphics.c
@@ -890,6 +890,8 @@ assert_egl_icd (SrtEglIcd *icd)
 
   if (error_property == NULL)
     {
+      srt_egl_icd_check_error (icd, &error);
+      g_assert_no_error (error);
       g_assert_true (srt_egl_icd_check_error (icd, NULL));
       g_assert_true (srt_egl_icd_check_error (icd, &error));
       g_assert_no_error (error);
@@ -1258,6 +1260,8 @@ assert_vulkan_icd (SrtVulkanIcd *icd)
 
   if (error_property == NULL)
     {
+      srt_vulkan_icd_check_error (icd, &error);
+      g_assert_no_error (error);
       g_assert_true (srt_vulkan_icd_check_error (icd, NULL));
       g_assert_true (srt_vulkan_icd_check_error (icd, &error));
       g_assert_no_error (error);
diff --git a/tests/system-info-cli.c b/tests/system-info-cli.c
index 8ab9bb5f0..b13925758 100644
--- a/tests/system-info-cli.c
+++ b/tests/system-info-cli.c
@@ -89,6 +89,7 @@ static void
 libraries_presence (Fixture *f,
                     gconstpointer context)
 {
+  gboolean result;
   int exit_status = -1;
   JsonParser *parser = NULL;
   JsonNode *node = NULL;
@@ -103,25 +104,27 @@ libraries_presence (Fixture *f,
       "steam-runtime-system-info", "--expectations", expectations_in, NULL
     };
 
-  g_assert_true (g_spawn_sync (NULL,    /* working directory */
-                               (gchar **) argv,
-                               NULL,    /* envp */
-                               G_SPAWN_SEARCH_PATH,
-                               NULL,    /* child setup */
-                               NULL,    /* user data */
-                               &output, /* stdout */
-                               NULL,    /* stderr */
-                               &exit_status,
-                               &error));
+  result = g_spawn_sync (NULL,    /* working directory */
+                         (gchar **) argv,
+                         NULL,    /* envp */
+                         G_SPAWN_SEARCH_PATH,
+                         NULL,    /* child setup */
+                         NULL,    /* user data */
+                         &output, /* stdout */
+                         NULL,    /* stderr */
+                         &exit_status,
+                         &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   g_assert_cmpint (exit_status, ==, 0);
-  g_assert_null (error);
   g_assert_nonnull (output);
 
   /* We can't use `json_from_string()` directly because we are targeting an
    * older json-glib version */
   parser = json_parser_new ();
-  json_parser_load_from_data (parser, output, -1, &error);
+  result = json_parser_load_from_data (parser, output, -1, &error);
   g_assert_no_error (error);
+  g_assert_true (result);
   node = json_parser_get_root (parser);
   json = json_node_get_object (node);
 
@@ -210,6 +213,7 @@ static void
 libraries_missing (Fixture *f,
                    gconstpointer context)
 {
+  gboolean result;
   int exit_status = -1;
   JsonParser *parser = NULL;
   JsonNode *node = NULL;
@@ -224,24 +228,27 @@ libraries_missing (Fixture *f,
       "steam-runtime-system-info", "--expectations", expectations_in, NULL
     };
 
-  g_assert_true (g_spawn_sync (NULL,    /* working directory */
-                               (gchar **) argv,
-                               NULL,    /* envp */
-                               G_SPAWN_SEARCH_PATH,
-                               NULL,    /* child setup */
-                               NULL,    /* user data */
-                               &output, /* stdout */
-                               NULL,    /* stderr */
-                               &exit_status,
-                               &error));
+  result = g_spawn_sync (NULL,    /* working directory */
+                         (gchar **) argv,
+                         NULL,    /* envp */
+                         G_SPAWN_SEARCH_PATH,
+                         NULL,    /* child setup */
+                         NULL,    /* user data */
+                         &output, /* stdout */
+                         NULL,    /* stderr */
+                         &exit_status,
+                         &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   g_assert_cmpint (exit_status, ==, 0);
-  g_assert_null (error);
   g_assert_nonnull (output);
 
   /* We can't use `json_from_string()` directly because we are targeting an
    * older json-glib version */
   parser = json_parser_new ();
-  g_assert_true (json_parser_load_from_data (parser, output, -1, NULL));
+  result = json_parser_load_from_data (parser, output, -1, &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   node = json_parser_get_root (parser);
   json = json_node_get_object (node);
 
@@ -314,6 +321,7 @@ static void
 libraries_presence_verbose (Fixture *f,
                             gconstpointer context)
 {
+  gboolean result;
   int exit_status = -1;
   JsonParser *parser = NULL;
   JsonNode *node = NULL;
@@ -331,24 +339,27 @@ libraries_presence_verbose (Fixture *f,
       "steam-runtime-system-info", "--expectations", expectations_in, "--verbose", NULL
     };
 
-  g_assert_true (g_spawn_sync (NULL,    /* working directory */
-                               (gchar **) argv,
-                               NULL,    /* envp */
-                               G_SPAWN_SEARCH_PATH,
-                               NULL,    /* child setup */
-                               NULL,    /* user data */
-                               &output, /* stdout */
-                               NULL,    /* stderr */
-                               &exit_status,
-                               &error));
+  result = g_spawn_sync (NULL,    /* working directory */
+                         (gchar **) argv,
+                         NULL,    /* envp */
+                         G_SPAWN_SEARCH_PATH,
+                         NULL,    /* child setup */
+                         NULL,    /* user data */
+                         &output, /* stdout */
+                         NULL,    /* stderr */
+                         &exit_status,
+                         &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   g_assert_cmpint (exit_status, ==, 0);
-  g_assert_null (error);
   g_assert_nonnull (output);
 
   /* We can't use `json_from_string()` directly because we are targeting an
    * older json-glib version */
   parser = json_parser_new ();
-  g_assert_true (json_parser_load_from_data (parser, output, -1, NULL));
+  result = json_parser_load_from_data (parser, output, -1, &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   node = json_parser_get_root (parser);
   json = json_node_get_object (node);
 
@@ -388,6 +399,7 @@ static void
 no_arguments (Fixture *f,
               gconstpointer context)
 {
+  gboolean result;
   int exit_status = -1;
   JsonParser *parser = NULL;
   JsonNode *node = NULL;
@@ -398,24 +410,27 @@ no_arguments (Fixture *f,
   SrtSystemInfo *info = srt_system_info_new (NULL);
   const gchar *argv[] = { "steam-runtime-system-info", NULL };
 
-  g_assert_true (g_spawn_sync (NULL,    /* working directory */
-                               (gchar **) argv,
-                               NULL,    /* envp */
-                               G_SPAWN_SEARCH_PATH,
-                               NULL,    /* child setup */
-                               NULL,    /* user data */
-                               &output, /* stdout */
-                               NULL,    /* stderr */
-                               &exit_status,
-                               &error));
+  result = g_spawn_sync (NULL,    /* working directory */
+                         (gchar **) argv,
+                         NULL,    /* envp */
+                         G_SPAWN_SEARCH_PATH,
+                         NULL,    /* child setup */
+                         NULL,    /* user data */
+                         &output, /* stdout */
+                         NULL,    /* stderr */
+                         &exit_status,
+                         &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   g_assert_cmpint (exit_status, ==, 0);
-  g_assert_null (error);
   g_assert_nonnull (output);
 
   /* We can't use `json_from_string()` directly because we are targeting an
    * older json-glib version */
   parser = json_parser_new ();
-  g_assert_true (json_parser_load_from_data (parser, output, -1, NULL));
+  result = json_parser_load_from_data (parser, output, -1, &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   node = json_parser_get_root (parser);
   json = json_node_get_object (node);
 
@@ -446,6 +461,7 @@ static void
 steam_presence (Fixture *f,
                 gconstpointer context)
 {
+  gboolean result;
   int exit_status = -1;
   JsonParser *parser = NULL;
   JsonNode *node = NULL;
@@ -463,24 +479,27 @@ steam_presence (Fixture *f,
   fake_home = fake_home_new ();
   fake_home_create_structure (fake_home);
 
-  g_assert_true (g_spawn_sync (NULL,    /* working directory */
-                               (gchar **) argv,
-                               fake_home->env, /* envp */
-                               G_SPAWN_SEARCH_PATH,
-                               NULL,    /* child setup */
-                               NULL,    /* user data */
-                               &output, /* stdout */
-                               NULL,    /* stderr */
-                               &exit_status,
-                               &error));
+  result = g_spawn_sync (NULL,    /* working directory */
+                         (gchar **) argv,
+                         fake_home->env, /* envp */
+                         G_SPAWN_SEARCH_PATH,
+                         NULL,    /* child setup */
+                         NULL,    /* user data */
+                         &output, /* stdout */
+                         NULL,    /* stderr */
+                         &exit_status,
+                         &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   g_assert_cmpint (exit_status, ==, 0);
-  g_assert_null (error);
   g_assert_nonnull (output);
 
   /* We can't use `json_from_string()` directly because we are targeting an
    * older json-glib version */
   parser = json_parser_new ();
-  g_assert_true (json_parser_load_from_data (parser, output, -1, NULL));
+  result = json_parser_load_from_data (parser, output, -1, &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   node = json_parser_get_root (parser);
   json = json_node_get_object (node);
 
@@ -534,6 +553,7 @@ static void
 steam_issues (Fixture *f,
                 gconstpointer context)
 {
+  gboolean result;
   int exit_status = -1;
   JsonParser *parser = NULL;
   JsonNode *node = NULL;
@@ -548,32 +568,33 @@ steam_issues (Fixture *f,
   const gchar *argv[] = { "steam-runtime-system-info", NULL };
   FakeHome *fake_home;
 
-  g_assert_true (TRUE);
-
   fake_home = fake_home_new ();
   fake_home->create_pinning_libs = FALSE;
   fake_home->create_steam_symlink = FALSE;
   fake_home->create_steamrt_files = FALSE;
   fake_home_create_structure (fake_home);
 
-  g_assert_true (g_spawn_sync (NULL,    /* working directory */
-                               (gchar **) argv,
-                               fake_home->env, /* envp */
-                               G_SPAWN_SEARCH_PATH,
-                               NULL,    /* child setup */
-                               NULL,    /* user data */
-                               &output, /* stdout */
-                               NULL,    /* stderr */
-                               &exit_status,
-                               &error));
+  result = g_spawn_sync (NULL,    /* working directory */
+                         (gchar **) argv,
+                         fake_home->env, /* envp */
+                         G_SPAWN_SEARCH_PATH,
+                         NULL,    /* child setup */
+                         NULL,    /* user data */
+                         &output, /* stdout */
+                         NULL,    /* stderr */
+                         &exit_status,
+                         &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   g_assert_cmpint (exit_status, ==, 0);
-  g_assert_null (error);
   g_assert_nonnull (output);
 
   /* We can't use `json_from_string()` directly because we are targeting an
    * older json-glib version */
   parser = json_parser_new ();
-  g_assert_true (json_parser_load_from_data (parser, output, -1, NULL));
+  result = json_parser_load_from_data (parser, output, -1, &error);
+  g_assert_no_error (error);
+  g_assert_true (result);
   node = json_parser_get_root (parser);
   json = json_node_get_object (node);
 
-- 
GitLab