Merge lp:~pitti/autopilot-gtk/enum-flags-properties into lp:autopilot-gtk

[Martin Pitt]

Handle enum and flags properties. (LP: #1193342)

This branch enables enum and flag properties, such as GtkButton.relief or
GtkWidget.events. The first commit refactors the MatchProperty() method, to avoid having to introduce more duplicated code, and fix the GValue leak. After that, introducing support for new property types is a matter of only adding them to the convert_value() helper function, and then introspecting and matching will both support it.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:49
http://jenkins.qa.ubuntu.com/job/autopilot-gtk-ci/57/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-amd64-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-gtk-saucy-armhf-ci/19

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-gtk-ci/57/rebuild

review: Approve (continuous-integration)

[Thomi Richards (thomir-deactivatedaccount)]

LGTM

review: Approve

=== modified file 'lib/GtkNode.cpp'
--- lib/GtkNode.cpp	2013-06-26 10:16:59 +0000
+++ lib/GtkNode.cpp	2013-06-27 04:52:28 +0000
@@ -37,6 +37,36 @@
   g_clear_object(&object_);
 }
 
+// we cannot represent GEnums, GFlags, etc. through D-BUS and autopilot's API,
+// so convert them to strings, ints, and other primitive types
+static void convert_value (GParamSpec *pspec, GValue *value)
+{
+  if (G_VALUE_HOLDS_ENUM(value)) {
+    GEnumValue *ev = g_enum_get_value(G_PARAM_SPEC_ENUM(pspec)->enum_class,
+                                      g_value_get_enum(value));
+    if (ev != NULL) {
+      //g_debug("attribute %s of type %s holds enum %s", g_param_spec_get_name(pspec),
+      //        g_type_name(pspec->value_type), ev->value_name);
+      g_value_unset(value);
+      *value = G_VALUE_INIT;
+      g_value_init(value, G_TYPE_STRING);
+      g_value_set_string(value, ev->value_name);
+    }
+  }
+
+  // representing flags as strings is too unwieldy; let's just represent them
+  // as integer
+  if (G_VALUE_HOLDS_FLAGS(value)) {
+      guint flags = g_value_get_flags(value);
+      g_warning("attribute %s of type %s holds flags %x", g_param_spec_get_name(pspec),
+              g_type_name(pspec->value_type), flags);
+      g_value_unset(value);
+      *value = G_VALUE_INIT;
+      g_value_init(value, G_TYPE_UINT);
+      g_value_set_uint(value, flags);
+  }
+}
+
 GVariant* GtkNode::Introspect() const
 {
   GVariantBuilder builder;
@@ -59,6 +89,7 @@
         GValue value = G_VALUE_INIT;
         g_value_init(&value, param_spec->value_type);
         g_object_get_property(object_, g_param_spec_get_name(param_spec), &value);
+        convert_value(param_spec, &value);
         builder_wrapper.add(param_spec->name, &value);
         g_value_unset(&value); //Free the memory accquired by the value object. Absence of this was causig the applications to crash.
       }
@@ -181,55 +212,50 @@
 
 bool GtkNode::MatchProperty(const std::string& name,
                             const std::string& value) const {
-
-  // g_debug("attempting to match a node's property");
-
-  if (name == "id") {
+  if (name == "id")
     return value == std::to_string(GetObjectId());
 
-  } else {
-    GObjectClass* klass = G_OBJECT_GET_CLASS(object_);
-    GParamSpec* pspec = g_object_class_find_property(klass, name.c_str());
-    if (pspec == NULL)
+  GObjectClass* klass = G_OBJECT_GET_CLASS(object_);
+  GParamSpec* pspec = g_object_class_find_property(klass, name.c_str());
+  if (pspec == NULL)
+    return false;
+
+  // read the property into a GValue
+  g_debug("Matching property %s of type (%s).", g_param_spec_get_name(pspec),
+          g_type_name(G_PARAM_SPEC_VALUE_TYPE(pspec)));
+
+  GValue dest_value = G_VALUE_INIT;
+  g_value_init(&dest_value, G_PARAM_SPEC_VALUE_TYPE(pspec));
+  g_object_get_property(object_, name.c_str(), &dest_value);
+  convert_value(pspec, &dest_value);
+  std::string dest_string;
+
+  // convert it to a string; always doing string comparison avoids having to do
+  // type comparison, conversion and error handling on value
+  switch (G_VALUE_TYPE(&dest_value)) {
+    case G_TYPE_INT:
+      dest_string = std::to_string(g_value_get_int(&dest_value));
+      break;
+    case G_TYPE_UINT:
+      dest_string = std::to_string(g_value_get_uint(&dest_value));
+      break;
+    case G_TYPE_DOUBLE:
+      dest_string = std::to_string(g_value_get_double(&dest_value));
+      break;
+    case G_TYPE_STRING: {
+      const gchar *str = g_value_get_string(&dest_value);
+      dest_string = (str != NULL) ? str : "";
+      break;
+      }
+    default:
+      g_debug("Unhandled type %s for matching property %s",
+              g_type_name(G_VALUE_TYPE(&dest_value)), g_param_spec_get_name(pspec));
+      g_value_unset(&dest_value);
       return false;
-
-    g_debug("Matching a property of type (%s).", g_type_name(G_PARAM_SPEC_VALUE_TYPE(pspec)));
-
-    if (pspec && G_PARAM_SPEC_VALUE_TYPE(pspec) == G_TYPE_INT) {
-      GValue dest_value = G_VALUE_INIT;
-      g_value_init(&dest_value, G_TYPE_INT);
-      g_object_get_property(object_, name.c_str(), &dest_value);
-
-      const gint cint = g_value_get_int(&dest_value);
-      g_value_unset(&dest_value);
-      std::stringstream out;
-      out << cint;
-      std::string dest_string(out.str());
-      return dest_string == value;
-
-    } else if (pspec && G_PARAM_SPEC_VALUE_TYPE(pspec) == G_TYPE_DOUBLE) {
-      GValue dest_value = G_VALUE_INIT;
-      g_value_init(&dest_value, G_TYPE_DOUBLE);
-      g_object_get_property(object_, name.c_str(), &dest_value);
-
-      const gdouble cdbl = g_value_get_double(&dest_value);
-      g_value_unset(&dest_value);
-      std::stringstream out;
-      out << cdbl;
-      std::string dest_string(out.str());
-      return dest_string == value;
-
-    } else if (pspec && G_PARAM_SPEC_VALUE_TYPE(pspec) == G_TYPE_STRING) {
-      gchar *strval = NULL;
-      g_object_get(object_, name.c_str(), &strval, NULL);
-      if (strval == NULL)
-        return false;
-
-      std::string dest_string(strval);
-      return dest_string == value;
-    }
-    return false;
   }
+
+  g_value_unset(&dest_value);
+  return dest_string == value;
 }
 
 xpathselect::NodeList GtkNode::Children() const {

=== modified file 'tests/autopilot/tests/test_properties.py'
--- tests/autopilot/tests/test_properties.py	2013-06-26 07:21:14 +0000
+++ tests/autopilot/tests/test_properties.py	2013-06-27 04:52:28 +0000
@@ -90,10 +90,20 @@
         self.assertEqual(entry_name.text_length, 0)
         self.assertEqual(entry_color.text_length, 0)
 
-    #https://launchpad.net/bugs/1193342
-    @unittest.expectedFailure
-    def test_enum_properties(self):
-        '''enum properties'''
+    def test_enum_flags_properties(self):
+        '''enum and flags properties'''
 
+        # enum
         btn_greet = self.app.select_single('GtkButton', label='Greet')
-        self.assertTrue(hasattr(btn_greet, 'relief'))
+        self.assertEqual(btn_greet.relief, 'GTK_RELIEF_NORMAL')
+        self.assertEqual(btn_greet.resize_mode, 'GTK_RESIZE_PARENT')
+
+        res = self.app.select_many(relief='GTK_RELIEF_NORMAL', visible=True)
+        self.assertGreaterEqual(len(res), 3)
+        self.assertIn('Button', str(type(res[0])))
+
+        # flags
+        self.assertGreaterEqual(btn_greet.events, 0)
+
+        res = self.app.select_many('GtkButton', events=btn_greet.events)
+        self.assertGreater(len(res), 0)