[gnome-desktop-testing] Parse test files before execution, clean up internals

[Colin Walters <walters src gnome org>]

commit 281b859564440cb5cf6212ae436c0d9731d33a65
Author: Colin Walters <walters verbum org>
Date:   Wed May 29 18:41:33 2013 -0400

    Parse test files before execution, clean up internals
    
    This paves the way for a future commit which will respect e.g.
    Type=session-exclusive to ensure we don't execute tests in parallel.
    Also we should ideally handle the Description key.

 src/gnome-desktop-testing-runner.c |  167 ++++++++++++++++++++++++------------
 1 files changed, 113 insertions(+), 54 deletions(-)
---
diff --git a/src/gnome-desktop-testing-runner.c b/src/gnome-desktop-testing-runner.c
index 74055b6..ecacd42 100755
--- a/src/gnome-desktop-testing-runner.c
+++ b/src/gnome-desktop-testing-runner.c
@@ -40,16 +40,27 @@ typedef struct {
 
   int parallel;
   int test_index;
-  
-  int ntests;
-  int n_skipped_tests;
-  int n_failed_tests;
 } TestRunnerApp;
 
+typedef enum {
+  TEST_STATE_UNLOADED,
+  TEST_STATE_LOADED,
+  TEST_STATE_EXECUTING,
+  TEST_STATE_COMPLETE_SUCCESS,
+  TEST_STATE_COMPLETE_SKIPPED,
+  TEST_STATE_COMPLETE_FAILED,
+} TestState;
+
 typedef struct {
   volatile gint refcount;
   GFile *prefix_root;
   GFile *path;
+  GFile *tmpdir;
+
+  char *name;
+  char **argv;
+
+  TestState state;
 } Test;
 
 static void
@@ -57,17 +68,63 @@ test_unref (Test *test)
 {
   if (!g_atomic_int_dec_and_test (&test->refcount))
     return;
+  g_strfreev (test->argv);
+  g_clear_object (&test->tmpdir);
   g_object_unref (test->prefix_root);
   g_object_unref (test->path);
 }
 
-static Test *
-test_new (GFile *prefix_root, GFile *path)
+static Test*
+test_ref (Test *test)
+{
+  g_atomic_int_inc (&test->refcount);
+  return test;
+}
+
+static gboolean
+load_test (GFile         *prefix_root,
+           GFile         *path,
+           Test         **out_test,
+           GCancellable  *cancellable,
+           GError       **error)
 {
+  gboolean ret = FALSE;
+  GKeyFile *keyfile = NULL;
   Test *test = g_new0 (Test, 1);
+  int test_argc;
+  gs_free char *exec_key = NULL;
+  const char *test_path;
+
+  g_assert (test->state == TEST_STATE_UNLOADED);
+
   test->prefix_root = g_object_ref (prefix_root);
   test->path = g_object_ref (path);
-  return test;
+
+  test->name = g_file_get_relative_path (test->prefix_root, test->path);
+
+  test_path = gs_file_get_path_cached (test->path);
+
+  keyfile = g_key_file_new ();
+  if (!g_key_file_load_from_file (keyfile, test_path, 0, error))
+    goto out;
+
+  exec_key = g_key_file_get_string (keyfile, "Test", "Exec", error);
+  if (exec_key == NULL)
+    goto out;
+
+  if (!g_shell_parse_argv (exec_key, &test_argc, &test->argv, error))
+    goto out;
+  
+  test->state = TEST_STATE_LOADED;
+
+  ret = TRUE;
+ out:
+  g_clear_pointer (&keyfile, g_key_file_free);
+  if (!ret)
+    test_unref (test);
+  else
+    *out_test = test;
+  return ret;
 }
 
 static TestRunnerApp *app;
@@ -125,7 +182,10 @@ gather_all_tests_recurse (GFile         *prefix_root,
 
       if (type == G_FILE_TYPE_REGULAR && g_str_has_suffix (name, ".test"))
         {
-          g_ptr_array_add (tests, test_new (prefix_root, child));
+          Test *test;
+          if (!load_test (prefix_root, child, &test, cancellable, error))
+            goto out;
+          g_ptr_array_add (tests, test);
         }
       else if (type == G_FILE_TYPE_DIRECTORY)
         {
@@ -158,14 +218,13 @@ on_test_exited (GObject       *obj,
   GSSubprocess *proc = GS_SUBPROCESS (obj);
   GTask *task = G_TASK (user_data);
   GCancellable *cancellable = g_task_get_cancellable (task);
-  GFile *test_path;
+  Test *test;
   GFile *test_tmpdir_f;
-  const char *testname;
   gboolean failed = FALSE;
 
-  testname = g_object_get_data ((GObject*)task, "gdtr-test-name");
-  test_path = g_object_get_data ((GObject*)task, "gdtr-test");
-  test_tmpdir_f = g_object_get_data ((GObject*)task, "gdtr-test-tmpdir");
+  test = g_object_get_data ((GObject*)task, "gdtr-test");
+
+  g_assert (test->state == TEST_STATE_EXECUTING);
 
   if (!gs_subprocess_wait_finish (proc, result, &estatus, error))
     goto out;
@@ -173,15 +232,15 @@ on_test_exited (GObject       *obj,
     {
       if (g_error_matches (tmp_error, G_SPAWN_EXIT_ERROR, 77))
         {
+          test->state = TEST_STATE_COMPLETE_SKIPPED;
           gs_log_structured_print_id_v (ONE_TEST_SKIPPED_MSGID,
-                                        "Test %s skipped (exit code 77)", testname);
-          app->n_skipped_tests++;
+                                        "Test %s skipped (exit code 77)", test->name);
         }
       else
         {
+          test->state = TEST_STATE_COMPLETE_FAILED;
           gs_log_structured_print_id_v (ONE_TEST_FAILED_MSGID,
-                                        "Test %s failed: %s", testname, tmp_error->message); 
-          app->n_failed_tests++;
+                                        "Test %s failed: %s", test->name, tmp_error->message); 
           failed = TRUE;
         }
       /* Individual test failures don't count as failure of the whole process */
@@ -189,14 +248,14 @@ on_test_exited (GObject       *obj,
     }
   else
     {
-      gs_log_structured_print_id_v (ONE_TEST_SUCCESS_MSGID, "PASS: %s", testname);
-      app->ntests += 1;
+      gs_log_structured_print_id_v (ONE_TEST_SUCCESS_MSGID, "PASS: %s", test->name);
+      test->state = TEST_STATE_COMPLETE_SUCCESS;
     }
   
   /* Keep around temporaries from failed tests */
   if (!(failed && opt_report_directory))
     {
-      if (!gs_shutil_rm_rf (test_tmpdir_f, cancellable, error))
+      if (!gs_shutil_rm_rf (test->tmpdir, cancellable, error))
         goto out;
     }
 
@@ -218,7 +277,6 @@ run_test_async (Test                *test,
 
   GError *local_error = NULL;
   GError **error = &local_error;
-  GKeyFile *keyfile = NULL;
   gs_free char *testname = NULL;
   gs_free char *exec_key = NULL;
   gs_free char *test_tmpdir = NULL;
@@ -236,6 +294,8 @@ run_test_async (Test                *test,
   int estatus;
   GTask *task;
 
+  g_assert (test->state == TEST_STATE_LOADED);
+
   if (g_once_init_enter (&initialized))
     {
       slash_regex = g_regex_new ("/", 0, 0, NULL);
@@ -245,23 +305,9 @@ run_test_async (Test                *test,
   
   task = g_task_new (test->path, cancellable, callback, user_data); 
 
-  testname = g_file_get_relative_path (test->prefix_root, test->path);
-  test_path = gs_file_get_path_cached (test->path);
-
-  keyfile = g_key_file_new ();
-  if (!g_key_file_load_from_file (keyfile, test_path, 0, error))
-    goto out;
-
-  exec_key = g_key_file_get_string (keyfile, "Test", "Exec", error);
-  if (exec_key == NULL)
-    goto out;
-
-  if (!g_shell_parse_argv (exec_key, &test_argc, &test_argv, error))
-    goto out;
-  
-  g_print ("Running test: %s\n", test_path);
+  g_print ("Running test: %s\n", test->name);
 
-  test_squashed_name = g_regex_replace_literal (slash_regex, testname, -1,
+  test_squashed_name = g_regex_replace_literal (slash_regex, test->name, -1,
                                                 0, "_", 0, NULL);
   if (!opt_report_directory)
     {
@@ -269,37 +315,33 @@ run_test_async (Test                *test,
       test_tmpdir = g_dir_make_tmp (test_tmpname, error);
       if (!test_tmpdir)
         goto out;
-      test_tmpdir_f = g_file_new_for_path (test_tmpdir);
+      test->tmpdir = g_file_new_for_path (test_tmpdir);
     }
   else
     {
       test_tmpdir = g_build_filename (opt_report_directory, test_squashed_name, NULL);
-      test_tmpdir_f = g_file_new_for_path (test_tmpdir);
+      test->tmpdir = g_file_new_for_path (test_tmpdir);
       if (!gs_shutil_rm_rf (test_tmpdir_f, cancellable, error))
         goto out;
       if (!gs_file_ensure_directory (test_tmpdir_f, TRUE, cancellable, error))
         goto out;
     }
 
-  proc_context = gs_subprocess_context_new (test_argv);
+  proc_context = gs_subprocess_context_new (test->argv);
   gs_subprocess_context_set_cwd (proc_context, test_tmpdir);
 
   proc = gs_subprocess_new (proc_context, cancellable, error);
   if (!proc)
     goto out;
 
-  g_object_set_data_full ((GObject*)task, "gdtr-test-name",
-                          g_strdup (testname), (GDestroyNotify)g_free);
+  test->state = TEST_STATE_EXECUTING;
+
   g_object_set_data_full ((GObject*)task, "gdtr-test",
-                          g_object_ref (test->path), (GDestroyNotify)g_object_unref);
-  g_object_set_data_full ((GObject*)task, "gdtr-test-tmpdir",
-                          g_object_ref (test_tmpdir_f), (GDestroyNotify)g_object_unref);
+                          test_ref (test), (GDestroyNotify)test_unref);
   
   gs_subprocess_wait (proc, cancellable, on_test_exited, task);
 
  out:
-  g_clear_pointer (&keyfile, g_key_file_free);
-  g_clear_pointer (&test_argv, g_strfreev);
   if (local_error)
     {
       g_task_report_error (test->path, callback, user_data, run_test_async, local_error);
@@ -381,6 +423,7 @@ main (int argc, char **argv)
   TestRunnerApp appstruct;
   GPtrArray *test_paths;
   const char *const *datadirs_iter;
+  int n_passed, n_skipped, n_failed;
 
   memset (&appstruct, 0, sizeof (appstruct));
   app = &appstruct;
@@ -426,11 +469,10 @@ main (int argc, char **argv)
         {
           gboolean matches = FALSE;
           Test *test = app->tests->pdata[j];
-          gs_free char *test_relname = g_file_get_relative_path (test->prefix_root, test->path);
           for (i = 1; i < argc; i++)
             {
               const char *prefix = argv[i];
-              if (g_str_has_prefix (test_relname, prefix))
+              if (g_str_has_prefix (test->name, prefix))
                 {
                   matches = TRUE;
                   break;
@@ -450,9 +492,7 @@ main (int argc, char **argv)
       for (i = 0; i < app->tests->len; i++)
         {
           Test *test = app->tests->pdata[i];
-          const char *path = gs_file_get_path_cached (test->path);
-          gs_free char *test_relname = g_file_get_relative_path (test->prefix_root, test->path);
-          g_print ("%s (%s)\n", test_relname, gs_file_get_path_cached (test->prefix_root));
+          g_print ("%s (%s)\n", test->name, gs_file_get_path_cached (test->prefix_root));
         }
     }
   else
@@ -468,13 +508,32 @@ main (int argc, char **argv)
 
   ret = TRUE;
  out:
-  g_clear_pointer (&app->tests, g_ptr_array_unref);
   if (!opt_list)
     {
+      n_passed = n_skipped = n_failed = 0;
+      for (i = 0; i < app->tests->len; i++)
+        {
+          Test *test = app->tests->pdata[i];
+          switch (test->state)
+            {
+            case TEST_STATE_COMPLETE_SUCCESS:
+              n_passed++;
+              break;
+            case TEST_STATE_COMPLETE_SKIPPED:
+              n_skipped++;
+              break;
+            case TEST_STATE_COMPLETE_FAILED:
+              n_failed++;
+              break;
+            default:
+              g_assert_not_reached ();
+            }
+        }
       gs_log_structured_print_id_v (TESTS_COMPLETE_MSGID,
                                     "SUMMARY: total: %u passed: %d skipped: %d failed: %d",
-                                    total_tests, app->ntests, app->n_skipped_tests, app->n_failed_tests);
+                                    total_tests, n_passed, n_skipped, n_failed);
     }
+  g_clear_pointer (&app->tests, g_ptr_array_unref);
   if (!ret)
     {
       g_assert (local_error);