[folks] TestCase: avoid circular refs between GTest and this

[Simon McVittie <smcv src gnome org>]

commit 8268c6efbe20a9274dc1b4ad2f61b18406dfb86c
Author: Simon McVittie <simon mcvittie collabora co uk>
Date:   Mon Mar 18 19:00:43 2013 +0000

    TestCase: avoid circular refs between GTest and this
    
    Instead of running tests via the Adaptor inner class, run them via
    a C helper that takes a weak ref to the TestCase. This relies on the
    caller keeping a ref to the TestCase until after Test.run() - but I
    recently converted the tests to do exactly that, so that's OK.
    
    Adding a debug message which prints this.ref_count to final_tear_down()
    reveals that some of our regression tests still leak the TestCase, but
    many don't now.
    
    Bug: https://bugzilla.gnome.org/show_bug.cgi?id=695381
    Signed-off-by: Simon McVittie <simon mcvittie collabora co uk>
    Reviewed-by: Philip Withnall <philip tecnocode co uk>

 tests/lib/Makefile.am        |   10 ++++-
 tests/lib/test-case-helper.c |   91 ++++++++++++++++++++++++++++++++++++++++++
 tests/lib/test-case.vala     |   50 ++++++-----------------
 3 files changed, 112 insertions(+), 39 deletions(-)
---
diff --git a/tests/lib/Makefile.am b/tests/lib/Makefile.am
index 5a6908b..ea56cf0 100644
--- a/tests/lib/Makefile.am
+++ b/tests/lib/Makefile.am
@@ -30,7 +30,11 @@ DIST_SUBDIRS = \
 
 noinst_LTLIBRARIES = libfolks-test.la
 
-libfolks_test_la_SOURCES = test-case.vala test-utils.vala
+libfolks_test_la_SOURCES = \
+       test-case.vala \
+       test-case-helper.c \
+       test-utils.vala \
+       $(NULL)
 
 libfolks_test_la_CFLAGS = \
        $(AM_CFLAGS) \
@@ -67,6 +71,10 @@ libfolks_test_la_VALAFLAGS = \
        -g \
        $(NULL)
 
+BUILT_SOURCES = \
+       libfolks_test_la_vala.stamp \
+       $(NULL)
+
 MAINTAINERCLEANFILES = \
        $(libfolks_test_la_SOURCES:.vala=.c) \
        libfolks_test_la_vala.stamp \
diff --git a/tests/lib/test-case-helper.c b/tests/lib/test-case-helper.c
new file mode 100644
index 0000000..2b0c6d5
--- /dev/null
+++ b/tests/lib/test-case-helper.c
@@ -0,0 +1,91 @@
+/* test-case-helper.c
+ *
+ * Copyright © 2013 Intel Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301  USA
+ *
+ * Author:
+ *     Simon McVittie <simon mcvittie collabora co uk>
+ */
+
+#include "folks-test.h"
+
+#include <glib.h>
+#include <glib-object.h>
+
+typedef struct {
+    gpointer self;
+    FolksTestCaseTestMethod test;
+} FolksTestCaseWeakMethod;
+
+static void
+folks_test_case_weak_method_setup (gpointer fixture G_GNUC_UNUSED,
+    gconstpointer test_data)
+{
+  const FolksTestCaseWeakMethod *wm = test_data;
+
+  g_return_if_fail (wm->self != NULL);
+  g_return_if_fail (FOLKS_IS_TEST_CASE (wm->self));
+
+  folks_test_case_set_up (wm->self);
+}
+
+static void
+folks_test_case_weak_method_test (gpointer fixture G_GNUC_UNUSED,
+    gconstpointer test_data)
+{
+  const FolksTestCaseWeakMethod *wm = test_data;
+
+  g_return_if_fail (wm->self != NULL);
+  g_return_if_fail (FOLKS_IS_TEST_CASE (wm->self));
+
+  wm->test (wm->self);
+}
+
+static void
+folks_test_case_weak_method_teardown (gpointer fixture G_GNUC_UNUSED,
+    gconstpointer test_data)
+{
+  const FolksTestCaseWeakMethod *wm = test_data;
+
+  g_return_if_fail (wm->self != NULL);
+  g_return_if_fail (FOLKS_IS_TEST_CASE (wm->self));
+
+  folks_test_case_tear_down (wm->self);
+}
+
+GTestCase *
+folks_test_case_add_test_helper (FolksTestCase *self,
+    const gchar *name,
+    FolksTestCaseTestMethod test,
+    void *test_target)
+{
+  FolksTestCaseWeakMethod *wm;
+
+  g_return_if_fail (self == (FolksTestCase *) test_target);
+
+  /* This will never be freed, so make sure not to hold references. */
+  wm = g_new0 (FolksTestCaseWeakMethod, 1);
+  wm->self = self;
+  wm->test = test;
+  g_object_add_weak_pointer (G_OBJECT (self), &wm->self);
+
+  return g_test_create_case (name,
+      0,
+      wm,
+      folks_test_case_weak_method_setup,
+      folks_test_case_weak_method_test,
+      folks_test_case_weak_method_teardown);
+}
diff --git a/tests/lib/test-case.vala b/tests/lib/test-case.vala
index dd7afc2..76eecee 100644
--- a/tests/lib/test-case.vala
+++ b/tests/lib/test-case.vala
@@ -35,12 +35,11 @@
 public abstract class Folks.TestCase : Object
 {
   private GLib.TestSuite _suite;
-  private Adaptor[] _adaptors = new Adaptor[0];
-
   public delegate void TestMethod ();
 
   public TestCase (string name)
     {
+      LogAdaptor.set_up ();
       this._suite = new GLib.TestSuite (name);
 
       /* By default, no backend is allowed. Subclasses must override. */
@@ -49,18 +48,17 @@ public abstract class Folks.TestCase : Object
 
   public void register ()
     {
-      TestSuite.get_root ().add_suite (this.get_suite ());
+      TestSuite.get_root ().add_suite (this._suite);
     }
 
   public void add_test (string name, TestMethod test)
     {
-      var adaptor = new Adaptor (name, test, this);
-      this._adaptors += adaptor;
-
-      this._suite.add (new GLib.TestCase (
-            adaptor.name, adaptor.set_up, adaptor.run, adaptor.tear_down));
+      this._suite.add (add_test_helper (name, test));
     }
 
+  /* implemented in test-case-helper.c */
+  private extern GLib.TestCase add_test_helper (string name, TestMethod test);
+
   /**
    * Set up for one test. If you have more than one test, this will
    * be called once per test.
@@ -103,30 +101,16 @@ public abstract class Folks.TestCase : Object
       this.final_tear_down ();
     }
 
-  private GLib.TestSuite get_suite ()
+  private class LogAdaptor
     {
-      return this._suite;
-    }
-
-  private class Adaptor
-    {
-      public string name { get; private set; }
-      private unowned TestMethod _test;
-      private TestCase _test_case;
-
-      public Adaptor (string name, TestMethod test, TestCase test_case)
+      public LogAdaptor ()
         {
-          this._name = name;
-          this._test = test;
-          this._test_case = test_case;
         }
 
-      public void set_up (void* fixture)
+      public static void set_up ()
         {
-          GLib.set_printerr_handler (Adaptor._printerr_func_stack_trace);
-          Log.set_default_handler (this._log_func_stack_trace);
-
-          this._test_case.set_up ();
+          GLib.set_printerr_handler (LogAdaptor._printerr_func_stack_trace);
+          Log.set_default_handler (LogAdaptor._log_func_stack_trace);
         }
 
       private static void _printerr_func_stack_trace (string? text)
@@ -140,7 +124,7 @@ public abstract class Folks.TestCase : Object
           GLib.on_error_stack_trace ("libtool --mode=execute gdb");
         }
 
-      private void _log_func_stack_trace (string? log_domain,
+      private static void _log_func_stack_trace (string? log_domain,
           LogLevelFlags log_levels,
           string message)
         {
@@ -156,15 +140,5 @@ public abstract class Folks.TestCase : Object
               GLib.on_error_stack_trace ("libtool --mode=execute gdb");
             }
         }
-
-      public void run (void* fixture)
-        {
-          this._test ();
-        }
-
-      public void tear_down (void* fixture)
-        {
-          this._test_case.tear_down ();
-        }
     }
 }