From 131243b3c8d71febe5294f0035844cea8a9e3049 Mon Sep 17 00:00:00 2001
From: Simon Hausmann <simon.hausmann@qt.io>
Date: Wed, 6 Sep 2017 10:30:03 +0200
Subject: Stabilize the threadSignal() test

We've seen the case in the CI where we delete the worker thread object
before it has had a chance (resulting in a crash). This patch attempts
to stabilize this by waiting for the thread to terminate properly.

In addition QSignalSpy's connection to the done(QString) signal is
forced to be direct, which means the spy's internal list is accessed
from the gui thread (via QCOMPARE) at the same time as the thread may be
emitting the signal and calling the signalspy's slot (metacall), which
helgrind complains about (rightly so).

I don't see any purpose in connecting to the signal, so let's remove
that code. The test continues to cover the threading code in
QQmlData::signalEmitted, once as the thread is triggered via C++ and
once via QML (doIt invocation).

Change-Id: I5e8a4ae65e2d0890a26491d25c73de1ba33a6668
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
---
 tests/auto/qml/qqmlecmascript/testtypes.cpp        |  9 ++++++++-
 tests/auto/qml/qqmlecmascript/testtypes.h          |  5 +++++
 .../auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 22 +++++++++-------------
 3 files changed, 22 insertions(+), 14 deletions(-)

(limited to 'tests/auto/qml/qqmlecmascript')

diff --git a/tests/auto/qml/qqmlecmascript/testtypes.cpp b/tests/auto/qml/qqmlecmascript/testtypes.cpp
index 63c2918325..13bb0435cd 100644
--- a/tests/auto/qml/qqmlecmascript/testtypes.cpp
+++ b/tests/auto/qml/qqmlecmascript/testtypes.cpp
@@ -245,9 +245,16 @@ public:
     MyWorkerObject *o;
 };
 
+MyWorkerObject::~MyWorkerObject()
+{
+    if (m_thread)
+        m_thread->wait();
+}
+
 void MyWorkerObject::doIt()
 {
-    new MyWorkerObjectThread(this);
+    Q_ASSERT(!m_thread);
+    m_thread = new MyWorkerObjectThread(this);
 }
 
 class MyDateClass : public QObject
diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h
index eedeb66647..e15a05a00c 100644
--- a/tests/auto/qml/qqmlecmascript/testtypes.h
+++ b/tests/auto/qml/qqmlecmascript/testtypes.h
@@ -1532,12 +1532,17 @@ private:
 class MyWorkerObject : public QObject
 {
     Q_OBJECT
+public:
+    ~MyWorkerObject();
 
 public Q_SLOTS:
     void doIt();
 
 Q_SIGNALS:
     void done(const QString &result);
+
+private:
+    QThread *m_thread = 0;
 };
 
 class MyUnregisteredEnumTypeObject : public QObject
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 7b9a43dc38..14c2aa18bf 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -7440,21 +7440,17 @@ void tst_qqmlecmascript::signalEmitted()
 void tst_qqmlecmascript::threadSignal()
 {
     {
-    QQmlComponent c(&engine, testFileUrl("threadSignal.qml"));
-    QObject *object = c.create();
-    QVERIFY(object != 0);
-    QTRY_VERIFY(object->property("passed").toBool());
-    delete object;
+        QQmlComponent c(&engine, testFileUrl("threadSignal.qml"));
+        QScopedPointer<QObject> object(c.create());
+        QVERIFY(!object.isNull());
+        QTRY_VERIFY(object->property("passed").toBool());
     }
     {
-    QQmlComponent c(&engine, testFileUrl("threadSignal.2.qml"));
-    QObject *object = c.create();
-    QVERIFY(object != 0);
-    QSignalSpy doneSpy(object, SIGNAL(done(QString)));
-    QMetaObject::invokeMethod(object, "doIt");
-    QTRY_VERIFY(object->property("passed").toBool());
-    QCOMPARE(doneSpy.count(), 1);
-    delete object;
+        QQmlComponent c(&engine, testFileUrl("threadSignal.2.qml"));
+        QScopedPointer<QObject> object(c.create());
+        QVERIFY(!object.isNull());
+        QMetaObject::invokeMethod(object.data(), "doIt");
+        QTRY_VERIFY(object->property("passed").toBool());
     }
 }
 
-- 
cgit v1.2.3