Merge lp:~thumper/unity/fix-881106 into lp:unity

[Tim Penhey]

Don't try to create a std::string with a null pointer.

dee_model_get_string can return NULL, which std::string doesn't like.

I'd like to make a test for this, but need to create a fake dee model.

Can someone recommend a very simple way to mock a dee model that has a null string value?

[Michal Hruby (mhr3)]

DeeModel *model = dee_sequence_model_new ();
dee_model_set_schema (model, "s", NULL);
dee_model_append (model, NULL);

Should do...

[Mikkel Kamstrup Erlandsen (kamstrup)]

On 24 November 2011 09:23, Michal Hruby <email address hidden> wrote:
> DeeModel *model = dee_sequence_model_new ();
> dee_model_set_schema (model, "s", NULL);
> dee_model_append (model, NULL);

I think you'll get wild complaints from dee about this, so your tests
should not fail if there's output on stderr. If it's just a matter of
getting dee_model_get_string() to return NULL you can also just do the
first two lines above and then dee_model_get_string(model, NULL, 27);

[Tim Penhey (thumper)]

Michal,

I have the following:

  DeeModel* model = dee_sequence_model_new();
  dee_model_set_schema(model, "s", NULL);
  // Add on a null string.
  DeeModelIter* iter = dee_model_append(model, NULL);

  // Check that the method we call is null.
  const gchar* value = dee_model_get_string(model, iter, 0);
  ASSERT_THAT(value, IsNull());

This fails, as dee_model_get_string actually returns a valid gchar* pointing to "".

This isn't what I want :(

The actual interesting part of the test is this:
   RowAdaptorBase row(model, iter);
   ASSERT_THAT(row.GetStringAt(0), Eq(""));

But I want o assert that the actual method call it is using returns a NULL, but it doesn't.

I can use 1 instead of 0, but his gives spewage on std::err as Mikkel says.

I've tried many different ways to get a null char* into the model, but I seem to be failing.
Is it legal?

[Mikkel Kamstrup Erlandsen (kamstrup)]

Hehe, that may be the thing :-) It is not legal to store an NULL in a
DeeModel, sorry if I misunderstood.

[Michal Hruby (mhr3)]

Right, we need an actual wrong usage, so something more along the lines of:

dee_model_set_schema (model, "i", NULL);
iter = dee_model_append (model, 0);

dee_model_get_string (model, iter, 0); // will definitely return NULL

[Marco Trevisan (Treviño) (3v1n0)]

Everything in the code is fine, tested and works properly.

review: Approve

=== modified file 'UnityCore/ModelRowAdaptor.cpp'
--- UnityCore/ModelRowAdaptor.cpp	2011-08-25 16:40:28 +0000
+++ UnityCore/ModelRowAdaptor.cpp	2011-11-28 01:15:28 +0000
@@ -41,7 +41,7 @@
   model_ = other.model_;
   iter_ = other.iter_;
   tag_ = other.tag_;
-  
+
   return *this;
 }
 
@@ -49,7 +49,11 @@
 {
   if (!model_ || !iter_)
     return "";
-  return dee_model_get_string(model_, iter_, position);
+  const gchar* value = dee_model_get_string(model_, iter_, position);
+  if (value)
+    return value;
+  else
+    return ""; // std::strings don't like null.
 }
 
 bool RowAdaptorBase::GetBoolAt(int position)

=== modified file 'tests/test_model.cpp'
--- tests/test_model.cpp	2011-08-02 08:38:34 +0000
+++ tests/test_model.cpp	2011-11-28 01:15:28 +0000
@@ -1,4 +1,4 @@
-#include <gtest/gtest.h>
+#include <gmock/gmock.h>
 #include <glib-object.h>
 
 #include <UnityCore/GLibWrapper.h>
@@ -8,6 +8,7 @@
 
 using namespace std;
 using namespace unity::dash;
+using namespace testing;
 
 namespace
 {
@@ -19,20 +20,18 @@
 {
 public:
   TestAdaptor(DeeModel* model, DeeModelIter* iter, DeeModelTag* tag)
+    : RowAdaptorBase(model, iter, tag)
   {
-    model_ = model;
-    iter_ = iter;
-    tag_ = tag;
   }
 
   unsigned int index()
   {
-    return dee_model_get_uint32(model_, iter_, 0);
+    return GetUIntAt(0);
   }
 
   string name()
   {
-    return dee_model_get_string(model_, iter_, 1);
+    return GetStringAt(1);
   }
 };
 
@@ -101,4 +100,72 @@
   }
 }
 
+
+
+void discard_g_log_calls(const gchar* log_domain,
+                         GLogLevelFlags log_level,
+                         const gchar* message,
+                         gpointer user_data)
+{
+  // do nothing for the messages.
+}
+
+class TestRowAdapterBase : public Test
+{
+public:
+  void SetUp() {
+    logger_ = g_log_set_default_handler(discard_g_log_calls, NULL);
+  }
+  void TearDown() {
+    g_log_set_default_handler(logger_, NULL);
+  }
+private:
+  GLogFunc logger_;
+};
+
+TEST_F(TestRowAdapterBase, TestGetStringNull)
+{
+  DeeModel* model = dee_sequence_model_new();
+  dee_model_set_schema(model, "i", NULL);
+  // Add in zero to an int field
+  DeeModelIter* iter = dee_model_append(model, 0);
+
+  RowAdaptorBase row(model, iter);
+
+  // Check that the method we call is null.
+  const gchar* value = dee_model_get_string(model, iter, 0);
+  ASSERT_THAT(value, IsNull());
+  ASSERT_THAT(model, NotNull());
+  ASSERT_THAT(iter, NotNull());
+  ASSERT_THAT(row.GetStringAt(0), Eq(""));
+}
+
+TEST_F(TestRowAdapterBase, TestGetStringEmpty)
+{
+  DeeModel* model = dee_sequence_model_new();
+  dee_model_set_schema(model, "s", NULL);
+  // Add on a empty string.
+  DeeModelIter* iter = dee_model_append(model, "");
+
+  RowAdaptorBase row(model, iter);
+
+  ASSERT_THAT(model, NotNull());
+  ASSERT_THAT(iter, NotNull());
+  ASSERT_THAT(row.GetStringAt(0), Eq(""));
+}
+
+TEST_F(TestRowAdapterBase, TestGetStringValue)
+{
+  DeeModel* model = dee_sequence_model_new();
+  dee_model_set_schema(model, "s", NULL);
+  // Add on a real string.
+  DeeModelIter* iter = dee_model_append(model, "Hello");
+
+  RowAdaptorBase row(model, iter);
+
+  ASSERT_THAT(model, NotNull());
+  ASSERT_THAT(iter, NotNull());
+  ASSERT_THAT(row.GetStringAt(0), Eq("Hello"));
+}
+
 }