Merge lp:~mzanetti/autopilot-qt/qt5support into lp:autopilot-qt

[Michael Zanetti]

This adds Qt5 support for autopilot-qt. UI tests are working for qmlscene powered QtQuick 2.0 apps.

This adds Qt5 support for autopilot-qt. UI tests are working for qmlscene powered QtQuick 2.0 apps.

I would suggest to have two separate packages for Qt4 and Qt5 to avoid having dependencies to both with one package.

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:28
http://s-jenkins:8080/job/autopilot-qt-ci/17/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/autopilot-qt-ci/./build=pbuilder,distribution=quantal,flavor=amd64/17/console

review: Approve (continuous-integration)

[Martin Mrazik (mrazik)]

So the idea here is to have 1 codebase and two packaging branches. Depending on which packaging branch is used the result is either autopilot-qt (for qt4) or autopilot-qt5 (for qt5). The qt5 packaging branch is here:
lp:~mzanetti/autopilot-qt/packaging-qt5

Sergio: could you have a look on the packaging bits?

[Thomi Richards (thomir-deactivatedaccount)]

Hi,

A few comments:

First, I'm not sure that this is correct:

150 + target.path = /opt/qt5/lib

Ubuntu packages typically don't install to /opt - I guess you're doing this because the qt5 packages install there? If that's the case then we should convince whoever packages qt5 to fix their packages sooner rather than later, so we can be conformant to Ubuntu packaging policy.

Second, regarding the multiple packaging branches, why not have one packaging branch that builds two binary packages? I'd suggest that having two packaging branches is going to cause trouble in the future. In fact, I recommend that we keep the packaging information in the source tree (I've come to the conclusion that separate packaging branches are more trouble than they're worth). The main advantage of this is that it's still only one step to build *all* the binary packages from the source branch - it also means we only need to maintain one change-log.

Finally, I'm not a huge fan of this:

#if QT_VERSION >= 0x050000

... but I can see that it's a necessary evil. I wonder if it wouldn't be better to do something like:

#if QT5_SUPPORT

and define that ourselves? Not sure on this one - thought I'd mention it anyway.

I'm not putting "Needs Fixing" on this MP, since I'm now officially washing my hands of responsibility for this project! Welcome aboard Michael, in your first week you're lumped with maintaining a project - good luck!

:P

[lp:~mzanetti/autopilot-qt/qt5support]

29. By Michael Zanetti on 2012-10-09

add missing properties of type LongLong and Double.

Really needed for QML UI testing as some of the most important properties are of type double (x, y, opacity, scale etc.)

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:29
http://s-jenkins:8080/job/autopilot-qt-ci/18/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/autopilot-qt-ci/./build=pbuilder,distribution=quantal,flavor=amd64/18/console

review: Approve (continuous-integration)

[Sergio Schvezov (sergiusens)]

> Hi,
>
> A few comments:
>
> First, I'm not sure that this is correct:
>
> 150 + target.path = /opt/qt5/lib
>
> Ubuntu packages typically don't install to /opt - I guess you're doing this
> because the qt5 packages install there? If that's the case then we should
> convince whoever packages qt5 to fix their packages sooner rather than later,
> so we can be conformant to Ubuntu packaging policy.

This is ppa:canonical-qt5-edgers/qt5-beta1 and sadly it's set to /opt/ and these are unofficial packages until the real ones land for R.

> Second, regarding the multiple packaging branches, why not have one packaging
> branch that builds two binary packages? I'd suggest that having two packaging
> branches is going to cause trouble in the future. In fact, I recommend that we
> keep the packaging information in the source tree (I've come to the conclusion
> that separate packaging branches are more trouble than they're worth). The
> main advantage of this is that it's still only one step to build *all* the
> binary packages from the source branch - it also means we only need to
> maintain one change-log.

+1

It's nice to see that more people are getting convinced by this :-)
I can help out.

> Finally, I'm not a huge fan of this:
>
> #if QT_VERSION >= 0x050000
>
> ... but I can see that it's a necessary evil. I wonder if it wouldn't be
> better to do something like:
>
> #if QT5_SUPPORT
>
> and define that ourselves? Not sure on this one - thought I'd mention it
> anyway.

That looks cleaner to me. But no strong opinion and not sure about the 'Qt' way.

[Sergio Schvezov (sergiusens)]

There's no MR for the packaging stuff; that would ideally be solved if thomi's comment is addressed.

The debian/substvars file is usually generated and modified dynamically by debian/rules targets, in which case it must be removed by the clean target.

Each debhelper command will record when it's successfully run in
debian/package.debhelper.log. (Which dh_clean deletes.) So dh can tell
which commands have already been run, for which packages, and skip
running those commands again.

That mean remove the substvars and debhelper.log from revision control

--

This is more of a question, is autopilot-qt1.dirs needed? I thought that was more for empty directories
dirs

This file specifies any directories which we need but which are not created by the normal installation procedure (make install DESTDIR=... invoked by dh_auto_install). This generally means there is a problem with the Makefile.

Files listed in an install file don't need their directories created first. See Section 5.11, “install”.

It is best to try to run the installation first and only use this if you run into trouble. There is no preceding slash on the directory names listed in the dirs file.

--

debian/changelog

Just make it one version since it is basically the first and remove the '-' (dash)

autopilot-qt5 (0.1) quantal; urgency=low

  * added missing build dependency to mesa

 -- Michael Zanetti <email address hidden> Mon, 08 Oct 2012 16:58:28 +0200

autopilot-qt5 (0.1-0) quantal; urgency=low

  * Initial release, based on autopilot-qt

 -- Michael Zanetti <email address hidden> Mon, 08 Oct 2012 14:23:32 +0200

--

debian/copyright

Copyright: 2012 <email address hidden>
and the other entry

Should be
Copyright: 2012 Canonical Ltd.

--

delete debian/docs

--

debian/control

I don't think I am the maintainer of this :-)
make sure you are depending on the latest version of debhelper (I think it's 9.0.0)

review: Needs Fixing (packaging)

[lp:~mzanetti/autopilot-qt/qt5support]

30. By Michael Zanetti on 2012-10-09

replace Qt version check macros with self defined, cleaner one

31. By Michael Zanetti on 2012-10-09

added packaging files for Qt4 and Qt5 based libraries

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:31
http://s-jenkins:8080/job/autopilot-qt-ci/19/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/autopilot-qt-ci/./build=pbuilder,distribution=quantal,flavor=amd64/19/console

review: Needs Fixing (continuous-integration)

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:31
http://s-jenkins:8080/job/autopilot-qt-ci/20/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/autopilot-qt-ci/./build=pbuilder,distribution=quantal,flavor=amd64/20/console

review: Needs Fixing (continuous-integration)

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:31
http://s-jenkins:8080/job/autopilot-qt-ci/21/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/autopilot-qt-ci/./build=pbuilder,distribution=quantal,flavor=amd64/21/console

review: Approve (continuous-integration)

[Thomi Richards (thomir-deactivatedaccount)]

review: Approve

[Sergio Schvezov (sergiusens)]

> --
>
> delete debian/docs

I think you missed this comment.

110 +autopilot-qt binary: non-dev-pkg-with-shlib-symlink

Add a new line at EOF.

What follows is just a question, is the symlink needed and can it be removed from the qmake stuff?

22 + -- Michael Zanetti <email address hidden> Mon, 08 Oct 2012 14:23:32 +0200

This is not important, but is that the email you wanted? I don't think there is a problem with that though, just a question.

review: Needs Fixing (packaging)

[lp:~mzanetti/autopilot-qt/qt5support]

32. By Michael Zanetti on 2012-10-10

removed unneeded docs file from packaging

33. By Michael Zanetti on 2012-10-10

fix lintian file EOF

34. By Michael Zanetti on 2012-10-10

fixed email address

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:34
http://s-jenkins:8080/job/autopilot-qt-ci/22/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/autopilot-qt-ci/./build=pbuilder,distribution=quantal,flavor=amd64/22/console

review: Approve (continuous-integration)

[Michael Zanetti (mzanetti)]

Ok... Should be fine now. About the symlink: It probably could go as well as its not really needed, but given we don't have a -dev package for this, it could be useful at some point.

[Sergio Schvezov (sergiusens)]

Just for the books

review: Approve

=== added directory 'debian'
=== added file 'debian/autopilot-qt.install'
--- debian/autopilot-qt.install	1970-01-01 00:00:00 +0000
+++ debian/autopilot-qt.install	2012-10-10 07:46:20 +0000
@@ -0,0 +1,1 @@
+usr/lib/lib*.so.*

=== added file 'debian/autopilot-qt5.install'
--- debian/autopilot-qt5.install	1970-01-01 00:00:00 +0000
+++ debian/autopilot-qt5.install	2012-10-10 07:46:20 +0000
@@ -0,0 +1,1 @@
+opt/qt5/lib/lib*.so.*

=== added file 'debian/changelog'
--- debian/changelog	1970-01-01 00:00:00 +0000
+++ debian/changelog	2012-10-10 07:46:20 +0000
@@ -0,0 +1,5 @@
+autopilot-qt (0.1) quantal; urgency=low
+
+  * Initial release
+
+ -- Michael Zanetti <[e-mail]>  Mon, 08 Oct 2012 14:23:32 +0200

=== added file 'debian/compat'
--- debian/compat	1970-01-01 00:00:00 +0000
+++ debian/compat	2012-10-10 07:46:20 +0000
@@ -0,0 +1,1 @@
+8

=== added file 'debian/control'
--- debian/control	1970-01-01 00:00:00 +0000
+++ debian/control	2012-10-10 07:46:20 +0000
@@ -0,0 +1,23 @@
+Source: autopilot-qt
+Priority: extra
+Maintainer: Michael Zanetti <[e-mail]>
+Build-Depends: debhelper (>= 8.0.0), libindicate-qt-dev, pkg-config, qt4-qmake, libqt4-dev, qtbase, qtdeclarative, libxpathselect-dev, mesa-common-dev, libgl1-mesa-dev
+Standards-Version: 3.9.3
+Section: libs
+Homepage: https://launchpad.net/autopilot-qt
+
+Package: autopilot-qt
+Section: libs
+Architecture: any
+Depends: ${shlibs:Depends}, ${misc:Depends}
+Description: This project makes Qt4 applications introspectable by autopilot.
+ This allows autopilot to test any existing Qt4 application, 
+ without having to rebuild the application under test.
+
+Package: autopilot-qt5
+Section: libs
+Architecture: any
+Depends: ${shlibs:Depends}, ${misc:Depends}
+Description: This project makes Qt5 applications introspectable by autopilot.
+ This allows autopilot to test any existing Qt5 application, 
+ without having to rebuild the application under test.

=== added file 'debian/copyright'
--- debian/copyright	1970-01-01 00:00:00 +0000
+++ debian/copyright	2012-10-10 07:46:20 +0000
@@ -0,0 +1,42 @@
+Format: http://dep.debian.net/deps/dep5
+Upstream-Name: autopilot-qt
+Source: https://code.launchpad.net/~private-ps-quality-team/autopilot-qt/trunk
+
+Files: *
+Copyright: 2012 Canonical Ltd.
+License: 
+ This program is free software: you can redistribute it and/or modify it
+ under the terms of the the GNU General Public License version 3, as
+ published by the Free Software Foundation.
+ .
+ This program is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranties of
+ MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ PURPOSE.  See the applicable version of the GNU Lesser General Public
+ License for more details.
+ .
+ You should have received a copy of the GNU General Public License
+ along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ .
+ On Debian systems, the complete text of the GNU General
+ Public License v3 can be found in `/usr/share/common-licenses/GPL-3'.
+
+Files: debian/*
+Copyright: 2012 Canonical Ltd.
+License: GPL-2+
+ This package is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2 of the License, or
+ (at your option) any later version.
+ .
+ This package 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 General Public License for more details.
+ .
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>
+ .
+ On Debian systems, the complete text of the GNU General
+ Public License version 2 can be found in "/usr/share/common-licenses/GPL-2".
+

=== added file 'debian/lintian-overrides'
--- debian/lintian-overrides	1970-01-01 00:00:00 +0000
+++ debian/lintian-overrides	2012-10-10 07:46:20 +0000
@@ -0,0 +1,1 @@
+autopilot-qt binary: non-dev-pkg-with-shlib-symlink

=== added file 'debian/rules'
--- debian/rules	1970-01-01 00:00:00 +0000
+++ debian/rules	2012-10-10 07:46:20 +0000
@@ -0,0 +1,34 @@
+#!/usr/bin/make -f
+# -*- makefile -*-
+# Sample debian/rules that uses debhelper.
+# This file was originally written by Joey Hess and Craig Small.
+# As a special exception, when this file is copied by dh-make into a
+# dh-make output file, you may use that output file without restriction.
+# This special exception was added by Craig Small in version 0.37 of dh-make.
+
+# Uncomment this to turn on verbose mode.
+export DH_VERBOSE=1
+
+%:
+	dh $@ 
+
+override_dh_auto_configure:
+	mkdir qt4
+	mkdir qt5
+	cd qt4 && qmake ../autopilot-qt.pro
+	cd qt5 && /opt/qt5/bin/qmake ../autopilot-qt.pro
+
+override_dh_auto_build:
+	cd qt4 && make
+	cd qt5 && make
+
+override_dh_clean:
+	dh_clean
+	rm -rf qt4
+	rm -rf qt5
+
+override_dh_install:
+	mkdir -p $(CURDIR)/debian/autopilot-qt/usr/lib/
+	mkdir -p $(CURDIR)/debian/autopilot-qt5/opt/qt5/lib/
+	cp -pP qt4/libqttestability* $(CURDIR)/debian/autopilot-qt/usr/lib/
+	cp -pP qt5/libqttestability* $(CURDIR)/debian/autopilot-qt5/opt/qt5/lib/

=== modified file 'lib/dbus_object.cpp'
--- lib/dbus_object.cpp	2012-09-06 05:29:12 +0000
+++ lib/dbus_object.cpp	2012-10-10 07:46:20 +0000
@@ -13,7 +13,13 @@
 #include <QList>
 #include <QVariantMap>
 #include <QDebug>
-#include <QApplication>
+
+#ifdef QT5_SUPPORT
+  #include <QtWidgets/QApplication>
+#else
+  #include <QApplication>
+#endif
+
 #include <QDBusConnection>
 #include <QThread>
 
@@ -125,7 +131,11 @@
             QMetaMethod method = meta->method(i);
             if (method.methodType() == QMetaMethod::Signal)
             {
+#ifdef QT5_SUPPORT
+                QString signature = QString::fromLatin1(method.methodSignature());
+#else
                 QString signature = QString::fromLatin1(method.signature());
+#endif
                 signal_list.append(QVariant(signature));
             }
         }
@@ -157,7 +167,11 @@
             if (method.methodType() == QMetaMethod::Slot ||
                 method.methodType() == QMetaMethod::Method)
             {
+#ifdef QT5_SUPPORT
+                QString signature = QString::fromLatin1(method.methodSignature());
+#else
                 QString signature = QString::fromLatin1(method.signature());
+#endif
                 method_list.append(QVariant(signature));
             }
         }
@@ -199,8 +213,14 @@
 
     qDebug() << "Method parameter names:" << method.parameterNames();
     qDebug() << "Method parameter types:" << method.parameterTypes();
+
+#ifdef QT5_SUPPORT
+    qDebug() << "Method signature:" << method.methodSignature()
+             << "return type:" << method.typeName();
+#else
     qDebug() << "Method signature:" << method.signature()
              << "return type:" << method.typeName();
+#endif
 
     QVector<QGenericArgument> generic_args(10);
     QList<QByteArray> parameterTypes = method.parameterTypes();

=== modified file 'lib/introspection.cpp'
--- lib/introspection.cpp	2012-09-07 02:03:32 +0000
+++ lib/introspection.cpp	2012-10-10 07:46:20 +0000
@@ -10,17 +10,29 @@
 #include <xpathselect/node.h>
 #include <xpathselect/xpathselect.h>
 
-#include <QApplication>
 #include <QDebug>
+
+#ifdef QT5_SUPPORT
+  #include <QtGui/QGuiApplication>
+  #include <QtWidgets/QGraphicsItem>
+  #include <QtWidgets/QGraphicsScene>
+  #include <QtWidgets/QGraphicsView>
+  #include <QtWidgets/QWidget>
+  #include <QtQuick/QQuickItem>
+  #include <QtQuick/QQuickWindow>
+#else
+  #include <QGraphicsItem>
+  #include <QGraphicsScene>
+  #include <QGraphicsView>
+  #include <QApplication>
+  #include <QWidget>
+#endif
+
 #include <QMap>
 #include <QMetaProperty>
 #include <QObject>
 #include <QStringList>
-#include <QGraphicsItem>
-#include <QGraphicsScene>
-#include <QGraphicsView>
 #include <QVariant>
-#include <QWidget>
 #include <QRect>
 
 #include "introspection.h"
@@ -49,13 +61,19 @@
 
 QList<QtNode::Ptr> GetNodesThatMatchQuery(QString const& query_string)
 {
+#ifdef QT5_SUPPORT
+    std::shared_ptr<RootNode> root = std::make_shared<RootNode>(QGuiApplication::instance());
+    foreach (QWindow *widget, QGuiApplication::topLevelWindows())
+    {
+        root->AddChild((QObject*) widget);
+    }
+#else
     std::shared_ptr<RootNode> root = std::make_shared<RootNode>(QApplication::instance());
-
     foreach (QWidget *widget, QApplication::topLevelWidgets())
     {
         root->AddChild((QObject*) widget);
     }
-
+#endif
     QList<QtNode::Ptr> node_list;
 
     xpathselect::NodeList list = xpathselect::SelectNodes(root, query_string.toStdString());
@@ -148,7 +166,17 @@
                     scene_rect.size());
         properties["globalRect"] = PackProperty(global_rect);
     }
-
+#ifdef QT5_SUPPORT
+    // ... and support for QQuickItems (aka. Qt5 Declarative items)
+    else if (QQuickItem *i = qobject_cast<QQuickItem*>(obj))
+    {
+        QQuickWindow *view = i->window();
+        QRectF bounding_rect = i->boundingRect();
+        bounding_rect = i->mapRectToScene(bounding_rect);
+        QRect global_rect = QRect(view->mapToGlobal(bounding_rect.toRect().topLeft()), bounding_rect.size().toSize());
+        properties["globalRect"] = PackProperty(global_rect);
+    }
+#endif
 }
 
 QVariant PackProperty(QVariant const& prop)
@@ -159,8 +187,10 @@
     case QVariant::Bool:
     case QVariant::String:
     case QVariant::UInt:
+    case QVariant::LongLong:
     case QVariant::ULongLong:
     case QVariant::StringList:
+    case QVariant::Double:
     {
         return prop;
     }

=== modified file 'lib/lib.pro'
--- lib/lib.pro	2012-08-23 22:01:03 +0000
+++ lib/lib.pro	2012-10-10 07:46:20 +0000
@@ -25,5 +25,13 @@
     dbus_adaptor_qt.h
 
 target.file = libtestability*
-target.path = /usr/lib
+
+#version check qt
+contains(QT_VERSION, ^5\\..\\..*) {
+    DEFINES += QT5_SUPPORT
+    target.path = /opt/qt5/lib
+} else {
+    target.path = /usr/lib
+}
+
 INSTALLS += target

=== modified file 'lib/qtnode.cpp'
--- lib/qtnode.cpp	2012-08-23 22:01:03 +0000
+++ lib/qtnode.cpp	2012-10-10 07:46:20 +0000
@@ -2,8 +2,19 @@
 
 #include "introspection.h"
 
-#include <QGraphicsScene>
-#include <QGraphicsObject>
+#include <QDebug>
+
+#ifdef QT5_SUPPORT
+  #include <QtWidgets/QGraphicsScene>
+  #include <QtWidgets/QGraphicsObject>
+  #include <QtQml/QQmlEngine>
+  #include <QtQml/QQmlContext>
+  #include <QtQuick/QQuickView>
+  #include <QtQuick/QQuickItem>
+#else
+  #include <QGraphicsScene>
+  #include <QGraphicsObject>
+#endif
 
 const QByteArray AP_ID_NAME("_autopilot_id");
 
@@ -75,11 +86,40 @@
 xpathselect::NodeList QtNode::Children() const
 {
     xpathselect::NodeList children;
+
+#ifdef QT5_SUPPORT
+    // Qt5's hierarchy for QML has changed a bit:
+    // - On top there's a QQuickView which holds all the QQuick items
+    // - QQuickItems don't always follow the QObject type hierarchy (e.g. QQuickListView does not), therefore we use the QQuickItem's childItems()
+    // - In case it is not a QQuickItem, fall back to the standard QObject hierarchy
+
+    QQuickView *view = qobject_cast<QQuickView*>(object_);
+    if (view) {
+        children.push_back(std::make_shared<QtNode>(view->rootItem()));
+    }
+
+    QQuickItem* item = qobject_cast<QQuickItem*>(object_);
+    if (item) {
+        foreach (QQuickItem *childItem, item->childItems()) {
+            if (childItem->parentItem() == item) {
+                children.push_back(std::make_shared<QtNode>(childItem));
+            }
+        }
+    } else {
+        foreach (QObject *child, object_->children())
+        {
+            if (child->parent() == object_)
+                children.push_back(std::make_shared<QtNode>(child));
+        }
+    }
+
+#else
     foreach (QObject *child, object_->children())
     {
         if (child->parent() == object_)
             children.push_back(std::make_shared<QtNode>(child));
     }
+
     // If our wrapped object is a QGraphicsScene, we need to explicitly grab any child graphics
     // items that are derived from QObjects. Declarative UIs use this idiom, so this need to be
     // done to support QML applications.
@@ -94,5 +134,7 @@
                 children.push_back(std::make_shared<QtNode>(obj));
         }
     }
+#endif
+
     return children;
 }