Address modelview test "flakiness"v5.11.0-beta4

Expect there were two possible causes of flakiness.  First, re-using the
same nodes between tests can cause issues, as removing shared objects on
the bus is asynchronous and may not be complete before the next test starts.
So we now create new nodes for each test.

Second, the tests may depend on side effects from previous tests, so an
earlier failure could results in issues (and timeouts) in other tests.
The testDataChangedTree() tested seemed to have incorrect signalspy logic,
which has been cleaned up.

Change-Id: Iaf7ac943d0e80bedba100b3fdcfae0d588394cf3
Reviewed-by: Michael Brasser <michael.brasser@live.com>

1 files changed, 87 insertions, 44 deletions

diff --git a/tests/auto/modelview/tst_modelview.cpp b/tests/auto/modelview/tst_modelview.cpp
index 29b1728..2dfea2e 100644
--- a/tests/auto/modelview/tst_modelview.cpp
+++ b/tests/auto/modelview/tst_modelview.cpp
@@ -583,17 +583,27 @@ private:
 
 } // namespace
 
+#define _SETUP_TEST_ \
+    QRemoteObjectHost basicServer; \
+    QRemoteObjectNode client; \
+    QRemoteObjectRegistryHost registryServer; \
+    setup_models(basicServer, client, registryServer);
+
 class TestModelView: public QObject
 {
     Q_OBJECT
 
-    QRemoteObjectHost m_basicServer;
-    QRemoteObjectNode m_client;
-    QRemoteObjectRegistryHost m_registryServer;
     QStandardItemModel m_sourceModel;
     RolenamesListModel m_listModel;
 
+public:
+    void setup_models(QRemoteObjectHost &basicServer,
+                      QRemoteObjectNode &client,
+                      QRemoteObjectRegistryHost &registryServer);
+
 private slots:
+    // NB: The tests have side effects on the models used, and need to be run
+    // in order (they may depend on previous side effects).
     void initTestCase();
 
     void testEmptyModel();
@@ -629,18 +639,8 @@ void TestModelView::initTestCase()
 {
     QLoggingCategory::setFilterRules("qt.remoteobjects.warning=false");
 
-    //Setup registry
-    //Registry needs to be created first until we get the retry mechanism implemented
-    m_registryServer.setRegistryUrl(QUrl(QStringLiteral("tcp://127.0.0.1:65212")));
-
-    m_basicServer.setHostUrl(QUrl(QStringLiteral("tcp://127.0.0.1:65211")));
-    m_basicServer.setRegistryUrl(QUrl(QStringLiteral("tcp://127.0.0.1:65212")));
-
     static const int modelSize = 20;
 
-    // QStandardItem::flags are stored as data with Qt::UserRole - 1
-    QVector<int> roles = QVector<int>() << Qt::DisplayRole << Qt::BackgroundRole << (Qt::UserRole - 1);
-
     QHash<int,QByteArray> roleNames;
     roleNames[Qt::DisplayRole] = "text";
     roleNames[Qt::BackgroundRole] = "background";
@@ -664,7 +664,6 @@ void TestModelView::initTestCase()
         m_sourceModel.appendRow(row);
         list << QStringLiteral("FancyTextNumber %1").arg(i);
     }
-    m_basicServer.enableRemoting(&m_sourceModel, "test", roles);
 
     const int numElements = 1000;
     for (int i = 0; i < numElements; ++i) {
@@ -672,20 +671,37 @@ void TestModelView::initTestCase()
         QString pid = QString("%1").arg(i);
         m_listModel.addPair(name, pid);
     }
-    roles.clear();
-    roles << Qt::UserRole << Qt::UserRole+1;
-    m_basicServer.enableRemoting(&m_listModel, "testRoleNames", roles);
+}
+
+void TestModelView::setup_models(QRemoteObjectHost &basicServer, QRemoteObjectNode &client, QRemoteObjectRegistryHost &registryServer)
+{
+    static int port = 65211;
+    static const QString url = QStringLiteral("tcp://127.0.0.1:%1");
+
+    // QStandardItem::flags are stored as data with Qt::UserRole - 1
+    static const QVector<int> sourceModelRoles( {Qt::DisplayRole, Qt::BackgroundRole, (Qt::UserRole - 1)} );
 
-    m_client.setRegistryUrl(QUrl(QStringLiteral("tcp://127.0.0.1:65212")));
+    static const QVector<int> listModelRoles( {Qt::UserRole, Qt::UserRole+1} );
+
+    //Setup registry
+    //Registry needs to be created first until we get the retry mechanism implemented
+    basicServer.setHostUrl(QUrl(url.arg(port)));
+    registryServer.setRegistryUrl(QUrl(url.arg(port+1)));
+    basicServer.setRegistryUrl(QUrl(url.arg(port+1)));
+    basicServer.enableRemoting(&m_sourceModel, "test", sourceModelRoles);
+    basicServer.enableRemoting(&m_listModel, "testRoleNames", listModelRoles);
+    client.setRegistryUrl(QUrl(url.arg(port+1)));
+    port += 2;
 }
 
 void TestModelView::testEmptyModel()
 {
+    _SETUP_TEST_
     QVector<int> roles = QVector<int>() << Qt::DisplayRole << Qt::BackgroundRole;
     QStandardItemModel emptyModel;
-    m_basicServer.enableRemoting(&emptyModel, "emptyModel", roles);
+    basicServer.enableRemoting(&emptyModel, "emptyModel", roles);
 
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("emptyModel"));
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("emptyModel"));
     model->setRootCacheSize(1000);
 
     FetchData f(model.data());
@@ -697,7 +713,8 @@ void TestModelView::testEmptyModel()
 
 void TestModelView::testInitialData()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -708,7 +725,8 @@ void TestModelView::testInitialData()
 
 void TestModelView::testInitialDataTree()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -719,7 +737,8 @@ void TestModelView::testInitialDataTree()
 
 void TestModelView::testHeaderData()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -741,7 +760,8 @@ void TestModelView::testHeaderData()
 
 void TestModelView::testDataChangedTree()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -749,6 +769,7 @@ void TestModelView::testDataChangedTree()
 
     compareTreeData(&m_sourceModel, model.data());
     QSignalSpy dataChangedSpy(model.data(), SIGNAL(dataChanged(QModelIndex,QModelIndex,QVector<int>)));
+    QSet<int> expected;
     for (int i = 10; i < 20; ++i) {
         const QModelIndex parent = m_sourceModel.index(i,0);
         const int rowCount = m_sourceModel.rowCount(parent);
@@ -762,6 +783,7 @@ void TestModelView::testDataChangedTree()
             }
         }
         m_sourceModel.setData(m_sourceModel.index(i, 1), QColor(Qt::magenta), Qt::BackgroundRole);
+        expected << i;
     }
 
     bool signalsReceived = false;
@@ -770,7 +792,12 @@ void TestModelView::testDataChangedTree()
     while (runs < maxRuns) {
         if (dataChangedSpy.wait() &&!dataChangedSpy.isEmpty()) {
             signalsReceived = true;
-            if (dataChangedSpy.takeFirst().at(1).value<QModelIndex>().row() == 19)
+            for (auto args : dataChangedSpy) {
+                int row = args.at(1).value<QModelIndex>().row();
+                if (row && expected.contains(row))
+                    expected.remove(row);
+            }
+            if (expected.size() == 0)
                 break;
         }
         ++runs;
@@ -781,7 +808,8 @@ void TestModelView::testDataChangedTree()
 
 void TestModelView::testFlags()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -806,7 +834,8 @@ void TestModelView::testFlags()
 
 void TestModelView::testDataChanged()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -828,7 +857,8 @@ void TestModelView::testDataChanged()
 
 void TestModelView::testDataInsertion()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -891,7 +921,8 @@ void TestModelView::testDataInsertion()
 
 void TestModelView::testDataInsertionTree()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -978,7 +1009,8 @@ void TestModelView::testDataInsertionTree()
 
 void TestModelView::testDataRemoval()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
     qputenv("QTRO_NODES_CACHE_SIZE", "1000");
     model->setRootCacheSize(1000);
     FetchData f(model.data());
@@ -1018,7 +1050,8 @@ void TestModelView::testDataRemoval()
 
 void TestModelView::testRoleNames()
 {
-    QScopedPointer<QAbstractItemModelReplica> repModel( m_client.acquireModel(QStringLiteral("testRoleNames")));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> repModel( client.acquireModel(QStringLiteral("testRoleNames")));
     // Set a bigger cache enough to keep all the data otherwise the last test will fail
     repModel->setRootCacheSize(1500);
     FetchData f(repModel.data());
@@ -1034,17 +1067,19 @@ void TestModelView::testRoleNames()
 
 void TestModelView::testDataRemovalTree()
 {
+    _SETUP_TEST_
     m_sourceModel.removeRows(2, 4);
     //Maybe some checks here?
 }
 
 void TestModelView::testServerInsertDataTree()
 {
+    _SETUP_TEST_
     QVector<int> roles = QVector<int>() << Qt::DisplayRole << Qt::BackgroundRole;
     QStandardItemModel testTreeModel;
-    m_basicServer.enableRemoting(&testTreeModel, "testTreeModel", roles);
+    basicServer.enableRemoting(&testTreeModel, "testTreeModel", roles);
 
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("testTreeModel"));
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("testTreeModel"));
 
     QTRY_COMPARE(testTreeModel.rowCount(), model->rowCount());
 
@@ -1076,7 +1111,8 @@ void TestModelView::testServerInsertDataTree()
 
 void TestModelView::testModelTest()
 {
-    QScopedPointer<QAbstractItemModelReplica> repModel( m_client.acquireModel(QStringLiteral("test")));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> repModel( client.acquireModel(QStringLiteral("test")));
     ModelTest test(repModel.data());
 
     FetchData f(repModel.data());
@@ -1087,7 +1123,8 @@ void TestModelView::testModelTest()
 
 void TestModelView::testSortFilterModel()
 {
-    QScopedPointer<QAbstractItemModelReplica> repModel( m_client.acquireModel(QStringLiteral("test")));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> repModel( client.acquireModel(QStringLiteral("test")));
 
     FetchData f(repModel.data());
     f.addAll();
@@ -1105,7 +1142,8 @@ void TestModelView::testSortFilterModel()
 
 void TestModelView::testSetData()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -1135,7 +1173,8 @@ void TestModelView::testSetData()
 
 void TestModelView::testSetDataTree()
 {
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("test"));
+    _SETUP_TEST_
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("test"));
 
     FetchData f(model.data());
     f.addAll();
@@ -1181,14 +1220,15 @@ void TestModelView::testSetDataTree()
 
 void TestModelView::testSelectionFromReplica()
 {
+    _SETUP_TEST_
     QVector<int> roles = QVector<int>() << Qt::DisplayRole << Qt::BackgroundRole;
     QStandardItemModel simpleModel;
     for (int i = 0; i < 4; ++i)
         simpleModel.appendRow(new QStandardItem(QString("item %0").arg(i)));
     QItemSelectionModel selectionModel(&simpleModel);
-    m_basicServer.enableRemoting(&simpleModel, "simpleModelFromReplica", roles, &selectionModel);
+    basicServer.enableRemoting(&simpleModel, "simpleModelFromReplica", roles, &selectionModel);
 
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("simpleModelFromReplica"));
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("simpleModelFromReplica"));
     QItemSelectionModel *replicaSelectionModel = model->selectionModel();
 
     FetchData f(model.data());
@@ -1201,14 +1241,15 @@ void TestModelView::testSelectionFromReplica()
 
 void TestModelView::testSelectionFromSource()
 {
+    _SETUP_TEST_
     QVector<int> roles = QVector<int>() << Qt::DisplayRole << Qt::BackgroundRole;
     QStandardItemModel simpleModel;
     for (int i = 0; i < 4; ++i)
         simpleModel.appendRow(new QStandardItem(QString("item %0").arg(i)));
     QItemSelectionModel selectionModel(&simpleModel);
-    m_basicServer.enableRemoting(&simpleModel, "simpleModelFromSource", roles, &selectionModel);
+    basicServer.enableRemoting(&simpleModel, "simpleModelFromSource", roles, &selectionModel);
 
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("simpleModelFromSource"));
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("simpleModelFromSource"));
     QItemSelectionModel *replicaSelectionModel = model->selectionModel();
 
     FetchData f(model.data());
@@ -1221,8 +1262,9 @@ void TestModelView::testSelectionFromSource()
 
 void TestModelView::testCacheData()
 {
+    _SETUP_TEST_
     QVector<int> roles = QVector<int>() << Qt::UserRole << Qt::UserRole + 1;
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("testRoleNames", QtRemoteObjects::PrefetchData, roles));
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("testRoleNames", QtRemoteObjects::PrefetchData, roles));
     model->setRootCacheSize(1000);
 
     QEventLoop l;
@@ -1234,6 +1276,7 @@ void TestModelView::testCacheData()
 
 void TestModelView::testChildSelection()
 {
+    _SETUP_TEST_
     QVector<int> roles = {Qt::DisplayRole, Qt::BackgroundRole};
     QStandardItemModel simpleModel;
     QStandardItem *parentItem = simpleModel.invisibleRootItem();
@@ -1243,9 +1286,9 @@ void TestModelView::testChildSelection()
         parentItem = item;
     }
     QItemSelectionModel selectionModel(&simpleModel);
-    m_basicServer.enableRemoting(&simpleModel, "treeModelFromSource", roles, &selectionModel);
+    basicServer.enableRemoting(&simpleModel, "treeModelFromSource", roles, &selectionModel);
 
-    QScopedPointer<QAbstractItemModelReplica> model(m_client.acquireModel("treeModelFromSource", QtRemoteObjects::PrefetchData, roles));
+    QScopedPointer<QAbstractItemModelReplica> model(client.acquireModel("treeModelFromSource", QtRemoteObjects::PrefetchData, roles));
     QItemSelectionModel *replicaSelectionModel = model->selectionModel();
 
     QTRY_COMPARE(simpleModel.rowCount(), model->rowCount());