Merge lp:~aacid/dee-qt/support_arrays_and_tuples into lp:dee-qt/0.2

Proposed by Albert Astals Cid on 2012-11-09
Status: Merged
Approved by: Albert Astals Cid on 2012-11-12
Approved revision: 71
Merged at revision: 66
Proposed branch: lp:~aacid/dee-qt/support_arrays_and_tuples
Merge into: lp:dee-qt/0.2
Diff against target: 157 lines (+110/-3)
3 files modified
CMakeLists.txt (+8/-1)
conversiontest.cpp (+89/-0)
deelistmodel.cpp (+13/-2)
To merge this branch: bzr merge lp:~aacid/dee-qt/support_arrays_and_tuples
Reviewer Review Type Date Requested Status
Michal Hruby (community) Approve on 2012-11-12
Florian Boucault 2012-11-09 Pending
Review via email: mp+133677@code.launchpad.net

Commit message

Add support for G_VARIANT_CLASS_ARRAY and G_VARIANT_CLASS_TUPLE

Description of the change

Add support for G_VARIANT_CLASS_ARRAY and G_VARIANT_CLASS_TUPLE

To post a comment you must log in.
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-dee-qt/13/console reported an error when processing this lp:~aacid/dee-qt/support_arrays_and_tuples branch.
Not merging it.

Albert Astals Cid (aacid) wrote :

Reapproving since the only change from the last approval is the addition of the dbus-test-runner as instructed by Florian/Didier on IRC

Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-dee-qt/14/console reported an error when processing this lp:~aacid/dee-qt/support_arrays_and_tuples branch.
Not merging it.

Michal Hruby (mhr3) wrote :

134 + array << QVariantFromGVariant(g_variant_get_child_value(value, i));

Unless QVariantFromGVariant is stealing references, you're leaking a variant here.

review: Needs Fixing
69. By Albert Astals Cid on 2012-11-12

Fix memory leak as reported by Michal Hruby

70. By Albert Astals Cid on 2012-11-12

For the test

71. By Albert Astals Cid on 2012-11-12

Really fix the problem with the test

Michal Hruby (mhr3) :
review: Approve
Albert Astals Cid (aacid) wrote :

Ok, now that Michal is happy with the memory leak fix and i think i have finally fixed the problem with the test not running in jenkins lets try to merge again

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2012-02-20 16:32:05 +0000
3+++ CMakeLists.txt 2012-11-12 11:52:11 +0000
4@@ -41,6 +41,7 @@
5 ${QT_QTCORE_INCLUDE_DIR}
6 ${QT_QTDECLARATIVE_INCLUDE_DIR}
7 ${DEE_INCLUDE_DIRS}
8+ ${QT_QTTEST_INCLUDE_DIR}
9 )
10
11 ## QtDee
12@@ -79,7 +80,6 @@
13 target_link_libraries(test
14 QtDee
15 )
16-add_custom_target(check)
17
18 # Install
19 set(IMPORT_INSTALL_DIR lib/qt4/imports/dee)
20@@ -114,3 +114,10 @@
21 DESTINATION ${IMPORT_INSTALL_DIR}
22 )
23
24+## Unit-Test
25+enable_testing()
26+qt4_generate_moc(conversiontest.cpp conversiontest.moc)
27+add_executable(conversiontest conversiontest.cpp conversiontest.moc)
28+target_link_libraries(conversiontest ${QT_QTTEST_LIBRARIES} QtDee)
29+add_test(NAME conversiontest COMMAND "${CMAKE_CURRENT_BINARY_DIR}/conversiontest" "-xunitxml" "-o" "conversiontest-xunit.xml")
30+set_property(TEST conversiontest PROPERTY ENVIRONMENT "LD_LIBRARY_PATH=.")
31
32=== added file 'conversiontest.cpp'
33--- conversiontest.cpp 1970-01-01 00:00:00 +0000
34+++ conversiontest.cpp 2012-11-12 11:52:11 +0000
35@@ -0,0 +1,89 @@
36+/*
37+ * Copyright (C) 2012 Canonical, Ltd.
38+ *
39+ * This program is free software; you can redistribute it and/or modify
40+ * it under the terms of the GNU General Public License as published by
41+ * the Free Software Foundation; version 3.
42+ *
43+ * This program is distributed in the hope that it will be useful,
44+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
45+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
46+ * GNU General Public License for more details.
47+ *
48+ * You should have received a copy of the GNU General Public License
49+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
50+ */
51+
52+#include <QtTest>
53+#include <QObject>
54+
55+#include "deelistmodel.h"
56+
57+#include <dee.h>
58+
59+class ConversionTest : public QObject
60+{
61+ Q_OBJECT
62+
63+private Q_SLOTS:
64+ void initTestCase()
65+ {
66+ g_type_init();
67+ }
68+
69+ void GVariantToQVariantConversionTest()
70+ {
71+ DeeModel* model = dee_shared_model_new("com.dee-qt.test");
72+ dee_model_set_schema(model, "b", "y", "n", "q", "i", "u", "x", "t", "d", "s", "a(ii)", NULL);
73+
74+ GVariant **tuples = g_new(GVariant *, 2);
75+
76+ GVariant **t1 = g_new(GVariant *, 2);
77+ t1[0] = g_variant_new_int32(1);
78+ t1[1] = g_variant_new_int32(2);
79+ tuples[0] = g_variant_new_tuple(t1, 2);
80+
81+ GVariant **t2 = g_new(GVariant *, 2);
82+ t2[0] = g_variant_new_int32(3);
83+ t2[1] = g_variant_new_int32(4);
84+ tuples[1] = g_variant_new_tuple(t2, 2);
85+
86+ GVariant* array_of_tuples = g_variant_new_array(((const GVariantType *) "(ii)"), tuples, 2);
87+
88+ dee_model_append(model, TRUE, 7, INT16_MIN, UINT16_MAX, INT32_MIN, UINT32_MAX, INT64_MIN, UINT64_MAX, 3.1415, "giraffe", array_of_tuples);
89+
90+ DeeListModel model_qt;
91+ QCOMPARE(model_qt.count(), 0);
92+
93+ model_qt.setModel(model);
94+ QCOMPARE(model_qt.count(), 1);
95+
96+ const QModelIndex row0Index = model_qt.index(0, 0);
97+ QCOMPARE(model_qt.data(row0Index, 0), QVariant(true));
98+ QCOMPARE(model_qt.data(row0Index, 1), QVariant(7));
99+ QCOMPARE(model_qt.data(row0Index, 2), QVariant(INT16_MIN));
100+ QCOMPARE(model_qt.data(row0Index, 3), QVariant(UINT16_MAX));
101+ QCOMPARE(model_qt.data(row0Index, 4), QVariant(INT32_MIN));
102+ QCOMPARE(model_qt.data(row0Index, 5), QVariant(UINT32_MAX));
103+ QCOMPARE(model_qt.data(row0Index, 6), QVariant((qint64)INT64_MIN));
104+ QCOMPARE(model_qt.data(row0Index, 7), QVariant((quint64)UINT64_MAX));
105+ QCOMPARE(model_qt.data(row0Index, 8), QVariant(3.1415));
106+ QCOMPARE(model_qt.data(row0Index, 9), QVariant("giraffe"));
107+
108+ QList< QVariant > expected_array;
109+ QList< QVariant > tuple1, tuple2;
110+ tuple1 << 1 << 2;
111+ tuple2 << 3 << 4;
112+ expected_array << QVariant(tuple1) << QVariant(tuple2);
113+ QCOMPARE(model_qt.data(row0Index, 10), QVariant(expected_array));
114+
115+ g_free(tuples);
116+ g_free(t1);
117+ g_free(t2);
118+ g_object_unref(model);
119+ }
120+};
121+
122+QTEST_MAIN(ConversionTest)
123+
124+#include "conversiontest.moc"
125
126=== modified file 'deelistmodel.cpp'
127--- deelistmodel.cpp 2012-02-10 13:24:33 +0000
128+++ deelistmodel.cpp 2012-11-12 11:52:11 +0000
129@@ -49,6 +49,19 @@
130 return QVariant(g_variant_get_double(value));
131 case G_VARIANT_CLASS_STRING:
132 return QVariant(QString::fromUtf8(g_variant_get_string(value, NULL)));
133+ case G_VARIANT_CLASS_ARRAY:
134+ case G_VARIANT_CLASS_TUPLE:
135+ {
136+ const gsize nChildren = g_variant_n_children(value);
137+ QList<QVariant> array;
138+ for (gsize i = 0; i < nChildren; ++i)
139+ {
140+ GVariant* gvariant = g_variant_get_child_value(value, i);
141+ array << QVariantFromGVariant(gvariant);
142+ g_variant_unref(gvariant);
143+ }
144+ return array;
145+ }
146 default:
147 /* Fallback on an empty QVariant.
148 FIXME: Missing conversion of following GVariant types:
149@@ -57,8 +70,6 @@
150 - G_VARIANT_CLASS_SIGNATURE
151 - G_VARIANT_CLASS_VARIANT
152 - G_VARIANT_CLASS_MAYBE
153- - G_VARIANT_CLASS_ARRAY
154- - G_VARIANT_CLASS_TUPLE
155 - G_VARIANT_CLASS_DICT_ENTRY
156 */
157 return QVariant();

Subscribers

People subscribed via source and target branches