Autopilot Qt Support

Merge lp:~autopilot/autopilot-qt/update-for-1.3 into lp:autopilot-qt

  1. update-for-1.3
  2. Merge into trunk
Proposed by Thomi Richards
Status: Merged
Approved by: Thomi Richards
Approved revision: 60
Merged at revision: 53
Proposed branch: lp:~autopilot/autopilot-qt/update-for-1.3
Merge into: lp:autopilot-qt
Diff against target: 233 lines (+43/-23)
7 files modified
debian/changelog (+6/-0)
debian/control (+2/-2)
driver/qtnode.cpp (+13/-7)
driver/qtnode.h (+3/-1)
driver/rootnode.cpp (+8/-3)
driver/rootnode.h (+1/-0)
tests/unittests/tst_introspection.cpp (+10/-10)
To merge this branch: bzr merge lp:~autopilot/autopilot-qt/update-for-1.3
Reviewer Review Type Date Requested Status
Thomi Richards (community) Approve
PS Jenkins bot continuous-integration Approve
Michael Zanetti (community) Needs Fixing
Review via email: mp+159545@code.launchpad.net

Commit message

Update dbus wire protocol to that needed for 1.3

Description of the change

This branch changes autopilot-qt to build against the latest libxpathselect, and updates it's dbus wire protocol to the latest version.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:56
http://jenkins.qa.ubuntu.com/job/autopilot-qt-ci/48/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-ci/17/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-ci/17/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Given that this is a Qt project I think we should try to adapt Qt quidelines.

Please use QString instead of std::string and one of Qt's shared pointer mechanisms. In fact, I'm not a big fan of shared pointers at all as in most cases they lead to bad design choices. So if not really necessary, try avoiding those.

review: Needs Fixing
Revision history for this message
Michael Zanetti (mzanetti) wrote :

I'd like to get the tests from the other MP merged before this for 2 reasons:
- knowing what this breaks before merging it
- the tests should still be released to raring while this will not.

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

> Given that this is a Qt project I think we should try to adapt Qt quidelines.
>
> Please use QString instead of std::string and one of Qt's shared pointer
> mechanisms.

Michael,

The GetName and GetPath methods are defined in xpathselect. We cannot use QStrings here since xpathselect does not (and should not) rely on Qt. Similarly, we use std::shared_ptr because that's what the interface defines.

> In fact, I'm not a big fan of shared pointers at all as in most
> cases they lead to bad design choices. So if not really necessary, try
> avoiding those.

I'm not sure what you mean - this is a classic case where shared_ptrs are necessary. Qt has it's fancy copy-on-write semantics which make passing objects around by value very cheap, but other toolkits don't have that. Since we're dealing with an interface that has to be toolkit-agnostic, we can't start using Qt's magic.

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

> I'd like to get the tests from the other MP merged before this for 2 reasons:
> - knowing what this breaks before merging it
> - the tests should still be released to raring while this will not.

I'm happy if you want to merge the tests with trunk, but please remember that trunk will never get released into raring. If you want your tests to go into raring you need to merge them in the stable branch.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:57
http://jenkins.qa.ubuntu.com/job/autopilot-qt-ci/49/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-ci/18/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-ci/18/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

7 + * Bump version number

Doesn't seem to describe what this changeset actually does

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:57
http://jenkins.qa.ubuntu.com/job/autopilot-qt-ci/50/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-ci/19/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-ci/19/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

hmm.. doesn't build in jenkins because libpathselect >= 1.3 cannot be found. Is that already in some PPA I could add to the job?

Revision history for this message
Michael Zanetti (mzanetti) wrote :

> The GetName and GetPath methods are defined in xpathselect. We cannot use
> QStrings here since xpathselect does not (and should not) rely on Qt.
> Similarly, we use std::shared_ptr because that's what the interface defines.

Right... missed that.

Revision history for this message
Michael Zanetti (mzanetti) wrote :

Tests are merged to trunk and they fail because of the GatPath changes.

This would need to be merged with them and updated to pass CI (once Jenkins can find libxpathselect >= 1.3.)

review: Needs Fixing
Revision history for this message
Michael Terry (mterry) wrote :

> Doesn't seem to describe what this changeset actually does

The other change will be added to the changelog by jenkins due to the linked bug.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:57
http://jenkins.qa.ubuntu.com/job/autopilot-qt-ci/51/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-ci/20/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-ci/20/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:58
http://jenkins.qa.ubuntu.com/job/autopilot-qt-ci/52/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-ci/21/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-ci/21/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:59
http://jenkins.qa.ubuntu.com/job/autopilot-qt-ci/53/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-ci/22/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-ci/22/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:60
http://jenkins.qa.ubuntu.com/job/autopilot-qt-ci/54/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-ci/24
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-ci/23/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-qt-autolanding/26/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-autolanding/10
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-autolanding/10/console

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:60
http://jenkins.qa.ubuntu.com/job/autopilot-qt-ci/55/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-amd64-ci/25
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-qt-raring-armhf-ci/24

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

review: Approve (continuous-integration)
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

CI job passes, tests pass. Approving now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2013-03-22 00:01:49 +0000
3+++ debian/changelog 2013-04-21 21:24:26 +0000
4@@ -1,3 +1,9 @@
5+autopilot-qt (1.3) UNRELEASED; urgency=low
6+
7+ * Bump version number
8+
9+ -- Christopher Lee <chris.lee@canonical.com> Thu, 18 Apr 2013 15:53:10 +1200
10+
11 autopilot-qt (0.8daily13.03.22-0ubuntu1) raring; urgency=low
12
13 [ Sebastien Bacher ]
14
15=== modified file 'debian/control'
16--- debian/control 2013-04-18 14:16:36 +0000
17+++ debian/control 2013-04-21 21:24:26 +0000
18@@ -10,7 +10,7 @@
19 libqt4-dev,
20 qtbase5-dev,
21 qtdeclarative5-dev,
22- libxpathselect-dev,
23+ libxpathselect-dev (>= 1.3),
24 mesa-common-dev,
25 libgl1-mesa-dev,
26 libgles2-mesa-dev,
27@@ -28,7 +28,7 @@
28 Section: libs
29 Architecture: amd64 armel armhf i386
30 Depends: ${misc:Depends},
31- libxpathselect1.2,
32+ libxpathselect1.3,
33 Replaces: autopilot-qt, autopilot-qt5
34 Conflicts: autopilot-qt, autopilot-qt5
35 Provides: autopilot-qt, autopilot-qt5
36
37=== modified file 'driver/qtnode.cpp'
38--- driver/qtnode.cpp 2013-01-17 19:48:27 +0000
39+++ driver/qtnode.cpp 2013-04-21 21:24:26 +0000
40@@ -18,9 +18,10 @@
41
42 const QByteArray AP_ID_NAME("_autopilot_id");
43
44-QtNode::QtNode(QObject *obj)
45+QtNode::QtNode(QObject *obj, std::string const& parent_path)
46 : object_(obj)
47 {
48+ full_path_ = parent_path + "/" + GetName();
49 }
50
51 QObject* QtNode::getWrappedObject() const
52@@ -31,7 +32,7 @@
53 QVariant QtNode::IntrospectNode() const
54 {
55 // return must be (name, state_map)
56- QString object_name = QString::fromStdString(GetName());
57+ QString object_name = QString::fromStdString(GetPath());
58 QVariantMap object_properties = GetNodeProperties(object_);
59 object_properties["id"] = GetObjectId();
60 QList<QVariant> object_tuple = { QVariant(object_name), QVariant(object_properties) };
61@@ -62,6 +63,11 @@
62 return name.toStdString();
63 }
64
65+std::string QtNode::GetPath() const
66+{
67+ return full_path_;
68+}
69+
70 bool QtNode::MatchProperty(const std::string& name, const std::string& value) const
71 {
72 if (name == "id")
73@@ -95,21 +101,21 @@
74
75 QQuickView *view = qobject_cast<QQuickView*>(object_);
76 if (view) {
77- children.push_back(std::make_shared<QtNode>(view->rootObject()));
78+ children.push_back(std::make_shared<QtNode>(view->rootObject(), GetPath()));
79 }
80
81 QQuickItem* item = qobject_cast<QQuickItem*>(object_);
82 if (item) {
83 foreach (QQuickItem *childItem, item->childItems()) {
84 if (childItem->parentItem() == item) {
85- children.push_back(std::make_shared<QtNode>(childItem));
86+ children.push_back(std::make_shared<QtNode>(childItem, GetPath()));
87 }
88 }
89 } else {
90 foreach (QObject *child, object_->children())
91 {
92 if (child->parent() == object_)
93- children.push_back(std::make_shared<QtNode>(child));
94+ children.push_back(std::make_shared<QtNode>(child, GetPath()));
95 }
96 }
97
98@@ -117,7 +123,7 @@
99 foreach (QObject *child, object_->children())
100 {
101 if (child->parent() == object_)
102- children.push_back(std::make_shared<QtNode>(child));
103+ children.push_back(std::make_shared<QtNode>(child, GetPath()));
104 }
105
106 // If our wrapped object is a QGraphicsScene, we need to explicitly grab any child graphics
107@@ -131,7 +137,7 @@
108 {
109 QGraphicsObject *obj = item->toGraphicsObject();
110 if (obj && ! obj->parent())
111- children.push_back(std::make_shared<QtNode>(obj));
112+ children.push_back(std::make_shared<QtNode>(obj, GetPath()));
113 }
114 }
115 #endif
116
117=== modified file 'driver/qtnode.h'
118--- driver/qtnode.h 2012-08-23 22:01:03 +0000
119+++ driver/qtnode.h 2013-04-21 21:24:26 +0000
120@@ -14,7 +14,7 @@
121 public:
122 typedef std::shared_ptr<QtNode> Ptr;
123
124- QtNode(QObject* object);
125+ QtNode(QObject* object, std::string const& parent_path);
126
127 QObject* getWrappedObject() const;
128
129@@ -22,10 +22,12 @@
130 virtual qint64 GetObjectId() const;
131
132 virtual std::string GetName() const;
133+ virtual std::string GetPath() const;
134 virtual bool MatchProperty(const std::string& name, const std::string& value) const;
135 virtual xpathselect::NodeList Children() const;
136 private:
137 QObject *object_;
138+ std::string full_path_;
139 };
140
141 #endif // QTNODE_H
142
143=== modified file 'driver/rootnode.cpp'
144--- driver/rootnode.cpp 2013-03-15 12:50:02 +0000
145+++ driver/rootnode.cpp 2013-04-21 21:24:26 +0000
146@@ -5,7 +5,7 @@
147 #include <QStringList>
148
149 RootNode::RootNode(QCoreApplication* application)
150- : QtNode(application)
151+ : QtNode(application, std::string())
152 , application_(application)
153 {
154 }
155@@ -13,7 +13,7 @@
156 QVariant RootNode::IntrospectNode() const
157 {
158 // return must be (name, state_map)
159- QString object_name = QString::fromStdString(GetName());
160+ QString object_name = QString::fromStdString(GetPath());
161 QStringList child_names;
162 foreach(QObject* child, children_)
163 {
164@@ -43,6 +43,11 @@
165 return appName.isEmpty() ? "Root" : appName.toStdString();
166 }
167
168+std::string RootNode::GetPath() const
169+{
170+ return "/" + GetName();
171+}
172+
173 bool RootNode::MatchProperty(const std::string& name, const std::string& value) const
174 {
175 if (name == "id")
176@@ -55,6 +60,6 @@
177 {
178 xpathselect::NodeList children;
179 foreach(QObject* child, children_)
180- children.push_back(std::make_shared<QtNode>(child));
181+ children.push_back(std::make_shared<QtNode>(child, GetPath()));
182 return children;
183 }
184
185=== modified file 'driver/rootnode.h'
186--- driver/rootnode.h 2012-08-11 00:46:21 +0000
187+++ driver/rootnode.h 2013-04-21 21:24:26 +0000
188@@ -19,6 +19,7 @@
189 void AddChild(QObject* child);
190
191 virtual std::string GetName() const;
192+ virtual std::string GetPath() const;
193 virtual bool MatchProperty(const std::string& name, const std::string& value) const;
194 virtual xpathselect::NodeList Children() const;
195 private:
196
197=== modified file 'tests/unittests/tst_introspection.cpp'
198--- tests/unittests/tst_introspection.cpp 2013-03-18 20:10:35 +0000
199+++ tests/unittests/tst_introspection.cpp 2013-04-21 21:24:26 +0000
200@@ -110,24 +110,24 @@
201 QTest::addColumn<QVariant>("firstResultPropertyValue");
202
203 #ifdef QT5_SUPPORT
204- QTest::newRow("/") << "/" << 1 << "tst_introspection" << "Children" << QVariant(QStringList() << "QMainWindow" << "QWidgetWindow");
205- QTest::newRow("//QWidget[id=6]") << "//QWidget[id=6]" << 1 << "QWidget" << "objectName" << QVariant("centralTestWidget");
206- QTest::newRow("//QPushButton[id=9]") << "//QPushButton[id=9]" << 1 << "QPushButton" << "objectName" << QVariant("myButton2");
207+ QTest::newRow("/") << "/" << 1 << "/tst_introspection" << "Children" << QVariant(QStringList() << "QMainWindow" << "QWidgetWindow");
208+ QTest::newRow("//QWidget[id=6]") << "//QWidget[id=6]" << 1 << "/tst_introspection/QMainWindow/QWidget" << "objectName" << QVariant("centralTestWidget");
209+ QTest::newRow("//QPushButton[id=9]") << "//QPushButton[id=9]" << 1 << "/tst_introspection/QMainWindow/QWidget/QPushButton" << "objectName" << QVariant("myButton2");
210 #else
211- QTest::newRow("/") << "/" << 1 << "tst_introspection" << "Children" << QVariant(QStringList() << "QMainWindow");
212- QTest::newRow("//QWidget[id=5]") << "//QWidget[id=5]" << 1 << "QWidget" << "objectName" << QVariant("centralTestWidget");
213+ QTest::newRow("/") << "/" << 1 << "/tst_introspection" << "Children" << QVariant(QStringList() << "QMainWindow");
214+ QTest::newRow("//QWidget[id=5]") << "//QWidget[id=5]" << 1 << "/tst_introspection/QMainWindow/QWidget" << "objectName" << QVariant("centralTestWidget");
215
216 // Depending on the environment, Qt4 could add a second QWidget at position 6. That moves other items down by one.
217 if (Introspect("//QWidget[id=6]").count() > 0) {
218- QTest::newRow("//QPushButton[id=9]") << "//QPushButton[id=9]" << 1 << "QPushButton" << "objectName" << QVariant("myButton2");
219+ QTest::newRow("//QPushButton[id=9]") << "//QPushButton[id=9]" << 1 << "/tst_introspection/QMainWindow/QWidget/QPushButton" << "objectName" << QVariant("myButton2");
220 } else {
221- QTest::newRow("//QPushButton[id=8]") << "//QPushButton[id=8]" << 1 << "QPushButton" << "objectName" << QVariant("myButton2");
222+ QTest::newRow("//QPushButton[id=8]") << "//QPushButton[id=8]" << 1 << "/tst_introspection/QMainWindow/QWidget/QPushButton" << "objectName" << QVariant("myButton2");
223 }
224 #endif
225
226- QTest::newRow("//GridLayout") << "//QGridLayout" << 1 << "QGridLayout" << "objectName" << QVariant("myTestLayout");
227- QTest::newRow("//QPushButton") << "//QPushButton" << 2 << "QPushButton" << "objectName" << QVariant("myButton1");
228- QTest::newRow("//QWidget/*") << "//QWidget/*" << 5 << "QGridLayout" << "objectName" << QVariant("myTestLayout");
229+ QTest::newRow("//GridLayout") << "//QGridLayout" << 1 << "/tst_introspection/QMainWindow/QWidget/QGridLayout" << "objectName" << QVariant("myTestLayout");
230+ QTest::newRow("//QPushButton") << "//QPushButton" << 2 << "/tst_introspection/QMainWindow/QWidget/QPushButton" << "objectName" << QVariant("myButton1");
231+ QTest::newRow("//QWidget/*") << "//QWidget/*" << 5 << "/tst_introspection/QMainWindow/QWidget/QGridLayout" << "objectName" << QVariant("myTestLayout");
232 QTest::newRow("broken query") << "broken query" << 0 << QString() << QString() << QVariant();
233 }
234

Subscribers

People subscribed via source and target branches

to all changes: