QmlDebug tests: Return the right value from connect(...)

If the process terminates shortly after connecting, we would return the
wrong value because the services would be NotConnected again, due to the
timeout mechanism we used to detect this. Instead, rather directly react
to the stateChanged signals and return as soon as all services are in
the correct state (or if the debug connection is dropped before, or on
timeout, which are failures).

Change-Id: I3f0c1c8519fc450627a803c76ec9b0a703104022
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

3 files changed, 83 insertions, 35 deletions

diff --git a/tests/auto/qml/debugger/qqmlprofilerservice/tst_qqmlprofilerservice.cpp b/tests/auto/qml/debugger/qqmlprofilerservice/tst_qqmlprofilerservice.cpp
index dbb264bf77..624e488c91 100644
--- a/tests/auto/qml/debugger/qqmlprofilerservice/tst_qqmlprofilerservice.cpp
+++ b/tests/auto/qml/debugger/qqmlprofilerservice/tst_qqmlprofilerservice.cpp
@@ -631,7 +631,7 @@ void tst_QQmlProfilerService::scenegraphData()
 
 void tst_QQmlProfilerService::profileOnExit()
 {
-    connect(true, "exit.qml");
+    QCOMPARE(connect(true, "exit.qml"), ConnectSuccess);
     checkProcessTerminated();
 
     checkTraceReceived();
@@ -713,7 +713,7 @@ void tst_QQmlProfilerService::flushInterval()
 
 void tst_QQmlProfilerService::translationBinding()
 {
-    connect(true, "qstr.qml");
+    QCOMPARE(connect(true, "qstr.qml"), ConnectSuccess);
     checkProcessTerminated();
 
     checkTraceReceived();
@@ -729,7 +729,7 @@ void tst_QQmlProfilerService::translationBinding()
 
 void tst_QQmlProfilerService::memory()
 {
-    connect(true, "memory.qml");
+    QCOMPARE(connect(true, "memory.qml"), ConnectSuccess);
     checkProcessTerminated();
 
     checkTraceReceived();
diff --git a/tests/auto/qml/debugger/shared/debugutil.cpp b/tests/auto/qml/debugger/shared/debugutil.cpp
index 46f504f6d3..602e533388 100644
--- a/tests/auto/qml/debugger/shared/debugutil.cpp
+++ b/tests/auto/qml/debugger/shared/debugutil.cpp
@@ -120,19 +120,6 @@ void QQmlDebugTestClient::messageReceived(const QByteArray &ba)
     emit serverMessage(ba);
 }
 
-template<typename F>
-struct Finalizer {
-    F m_lambda;
-    Finalizer(F &&lambda) : m_lambda(std::forward<F>(lambda)) {}
-    ~Finalizer() { m_lambda(); }
-};
-
-template<typename F>
-static Finalizer<F> defer(F &&lambda)
-{
-    return Finalizer<F>(std::forward<F>(lambda));
-}
-
 QQmlDebugTest::ConnectResult QQmlDebugTest::connect(
         const QString &executable, const QString &services, const QString &extraArgs,
         bool block)
@@ -159,31 +146,27 @@ QQmlDebugTest::ConnectResult QQmlDebugTest::connect(
     if (m_clients.contains(nullptr))
         return ClientsFailed;
 
-    auto allEnabled = [this]() {
-        for (QQmlDebugClient *client : m_clients) {
-            if (client->state() != QQmlDebugClient::Enabled)
-                return false;
-        }
-        return true;
-    };
+    ClientStateHandler stateHandler(m_clients, createOtherClients(m_connection), services.isEmpty()
+                                    ? QQmlDebugClient::Enabled : QQmlDebugClient::Unavailable);
 
-    QList<QQmlDebugClient *> others = createOtherClients(m_connection);
-    auto deleter = defer([&others]() { qDeleteAll(others); });
-    Q_UNUSED(deleter);
 
     const int port = m_process->debugPort();
     m_connection->connectToHost(QLatin1String("127.0.0.1"), port);
-    for (int tries = 0; tries < 100 && !allEnabled(); ++tries)
-        QTest::qWait(50);
-    if (!allEnabled())
+
+    QEventLoop loop;
+    QTimer timer;
+    QObject::connect(&stateHandler, &ClientStateHandler::allOk, &loop, &QEventLoop::quit);
+    QObject::connect(m_connection, &QQmlDebugConnection::disconnected, &loop, &QEventLoop::quit);
+    QObject::connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit);
+
+    timer.start(5000);
+    loop.exec();
+
+    if (!stateHandler.allEnabled())
         return EnableFailed;
 
-    const QQmlDebugClient::State expectedState = services.isEmpty() ? QQmlDebugClient::Enabled
-                                                                    : QQmlDebugClient::Unavailable;
-    for (QQmlDebugClient *other : others) {
-        if (other->state() != expectedState)
-            return RestrictFailed;
-    }
+    if (!stateHandler.othersAsExpected())
+        return RestrictFailed;
 
     return ConnectSuccess;
 }
@@ -231,3 +214,41 @@ void QQmlDebugTest::cleanup()
         m_process = nullptr;
     }
 }
+
+ClientStateHandler::ClientStateHandler(const QList<QQmlDebugClient *> &clients,
+                                       const QList<QQmlDebugClient *> &others,
+                                       QQmlDebugClient::State expectedOthers) :
+    m_clients(clients), m_others(others), m_expectedOthers(expectedOthers)
+{
+    for (QQmlDebugClient *client : m_clients) {
+        QObject::connect(client, &QQmlDebugClient::stateChanged,
+                         this, &ClientStateHandler::checkStates);
+    }
+    for (QQmlDebugClient *client : m_others) {
+        QObject::connect(client, &QQmlDebugClient::stateChanged,
+                         this, &ClientStateHandler::checkStates);
+    }
+}
+
+ClientStateHandler::~ClientStateHandler()
+{
+    qDeleteAll(m_others);
+}
+
+void ClientStateHandler::checkStates()
+{
+    for (QQmlDebugClient *client : m_clients) {
+        if (client->state() != QQmlDebugClient::Enabled)
+            return;
+    }
+
+    m_allEnabled = true;
+
+    for (QQmlDebugClient *other : m_others) {
+        if (other->state() != m_expectedOthers)
+            return;
+    }
+
+    m_othersAsExpected = true;
+    emit allOk();
+}
diff --git a/tests/auto/qml/debugger/shared/debugutil_p.h b/tests/auto/qml/debugger/shared/debugutil_p.h
index 05665f8a28..cc7c9bd19d 100644
--- a/tests/auto/qml/debugger/shared/debugutil_p.h
+++ b/tests/auto/qml/debugger/shared/debugutil_p.h
@@ -114,4 +114,31 @@ public:
     }
 };
 
+class ClientStateHandler : public QObject
+{
+    Q_OBJECT
+public:
+    ClientStateHandler(const QList<QQmlDebugClient *> &clients,
+                       const QList<QQmlDebugClient *> &others,
+                       QQmlDebugClient::State expectedOthers);
+
+    ~ClientStateHandler();
+
+    bool allEnabled() const { return m_allEnabled; }
+    bool othersAsExpected() const { return m_othersAsExpected; }
+
+signals:
+    void allOk();
+
+private:
+    void checkStates();
+
+    const QList<QQmlDebugClient *> m_clients;
+    const QList<QQmlDebugClient *> m_others;
+    const QQmlDebugClient::State m_expectedOthers;
+
+    bool m_allEnabled = false;
+    bool m_othersAsExpected = false;
+};
+
 #endif // DEBUGUTIL_P_H