testlib: Replace custom QTestCoreList with std::vector

The custom linked list implementation was implemented using
recursion, and as a result didn't handle long lists of test
cases, exhausting the stack on e.g. Windows where the default
stack is only 1MB. This was the case with e.g. the tst_QChar
test that produces 20K test cases.

Replacing with a std::vector should do nicely for our use-case.

No attempt has been made at further reducing the complexity
of QTestElement/QTestCoreElement/QTestElementAttribute.

Pick-to: 6.2
Change-Id: Ie295f7cf937ec6abdc4606b6120818551ad285c7
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/testlib/CMakeLists.txt

@@ -39,7 +39,6 @@ qt_internal_add_module(Test
         qtestblacklist.cpp qtestblacklist_p.h
         qtestcase.cpp qtestcase.h
         qtestcoreelement_p.h
-        qtestcorelist_p.h
         qtestdata.cpp qtestdata.h
         qtestelement.cpp qtestelement_p.h
         qtestelementattribute.cpp qtestelementattribute_p.h

src/testlib/qjunittestlogger.cpp

@@ -105,19 +105,19 @@ void QJUnitTestLogger::startLogging()
     property = new QTestElement(QTest::LET_Property);
     property->addAttribute(QTest::AI_Name, "QTestVersion");
     property->addAttribute(QTest::AI_PropertyValue, QTEST_VERSION_STR);
-    properties->addLogElement(property);
+    properties->addChild(property);
 
     property = new QTestElement(QTest::LET_Property);
     property->addAttribute(QTest::AI_Name, "QtVersion");
     property->addAttribute(QTest::AI_PropertyValue, qVersion());
-    properties->addLogElement(property);
+    properties->addChild(property);
 
     property = new QTestElement(QTest::LET_Property);
     property->addAttribute(QTest::AI_Name, "QtBuild");
     property->addAttribute(QTest::AI_PropertyValue, QLibraryInfo::build());
-    properties->addLogElement(property);
+    properties->addChild(property);
 
-    currentTestSuite->addLogElement(properties);
+    currentTestSuite->addChild(properties);
 
     elapsedTestcaseTime.start();
 }
@@ -141,14 +141,9 @@ void QJUnitTestLogger::stopLogging()
     currentTestSuite->addAttribute(QTest::AI_Time,
         toSecondsFormat(QTestLog::msecsTotalTime()).constData());
 
-    currentTestSuite->addLogElement(listOfTestcases);
+    for (auto *testCase : listOfTestcases)
-
+        currentTestSuite->addChild(testCase);
-    // For correct indenting, make sure every testcase knows its parent
+    listOfTestcases.clear();
-    QTestElement *testcase = listOfTestcases;
-    while (testcase) {
-        testcase->setParent(currentTestSuite);
-        testcase = testcase->nextElement();
-    }
 
     logFormatter->output(currentTestSuite);
 
@@ -168,7 +163,7 @@ void QJUnitTestLogger::enterTestCase(const char *name)
     currentTestCase = new QTestElement(QTest::LET_TestCase);
     currentTestCase->addAttribute(QTest::AI_Name, name);
     currentTestCase->addAttribute(QTest::AI_Classname, QTestResult::currentTestObjectName());
-    currentTestCase->addToList(&listOfTestcases);
+    listOfTestcases.push_back(currentTestCase);
 
     Q_ASSERT(!systemOutputElement && !systemErrorElement);
     systemOutputElement = new QTestElement(QTest::LET_SystemOutput);
@@ -212,13 +207,13 @@ void QJUnitTestLogger::leaveTestCase()
     currentTestCase->addAttribute(QTest::AI_Time,
         toSecondsFormat(elapsedTestCaseSeconds()).constData());
 
-    if (systemOutputElement->childElements())
+    if (!systemOutputElement->childElements().empty())
-        currentTestCase->addLogElement(systemOutputElement);
+        currentTestCase->addChild(systemOutputElement);
     else
         delete systemOutputElement;
 
-    if (systemErrorElement->childElements())
+    if (!systemErrorElement->childElements().empty())
-        currentTestCase->addLogElement(systemErrorElement);
+        currentTestCase->addChild(systemErrorElement);
     else
         delete systemErrorElement;
 
@@ -252,8 +247,7 @@ void QJUnitTestLogger::addFailure(QTest::LogElementType elementType,
     if (elementType == QTest::LET_Failure) {
         // Make sure we're not adding failure when we already have error,
         // or adding additional failures when we already have a failure.
-        for (auto *childElement = currentTestCase->childElements();
+        for (auto *childElement : currentTestCase->childElements()) {
-                   childElement; childElement = childElement->nextElement()) {
             if (childElement->elementType() == QTest::LET_Error ||
                 childElement->elementType() == QTest::LET_Failure)
                 return;
@@ -272,10 +266,10 @@ void QJUnitTestLogger::addFailure(QTest::LogElementType elementType,
     if (!details.isEmpty()) {
         auto textNode = new QTestElement(QTest::LET_Text);
         textNode->addAttribute(QTest::AI_Value, details.toUtf8().constData());
-        failureElement->addLogElement(textNode);
+        failureElement->addChild(textNode);
     }
 
-    currentTestCase->addLogElement(failureElement);
+    currentTestCase->addChild(failureElement);
 
     switch (elementType) {
     case QTest::LET_Failure: ++failureCounter; break;
@@ -292,7 +286,7 @@ void QJUnitTestLogger::addMessage(MessageTypes type, const QString &message, con
     if (type == QAbstractTestLogger::Skip) {
         auto skippedElement = new QTestElement(QTest::LET_Skipped);
         skippedElement->addAttribute(QTest::AI_Message, message.toUtf8().constData());
-        currentTestCase->addLogElement(skippedElement);
+        currentTestCase->addChild(skippedElement);
         return;
     } else if (type == QAbstractTestLogger::QFatal) {
         addFailure(QTest::LET_Error, "qfatal", message);
@@ -316,7 +310,7 @@ void QJUnitTestLogger::addMessage(MessageTypes type, const QString &message, con
 
     auto textNode = new QTestElement(QTest::LET_Text);
     textNode->addAttribute(QTest::AI_Value, message.toUtf8().constData());
-    systemLogElement->addLogElement(textNode);
+    systemLogElement->addChild(textNode);
 }
 
 QT_END_NAMESPACE

src/testlib/qjunittestlogger_p.h

@@ -51,9 +51,13 @@
 // We mean it.
 //
 
+#include <QtTest/qttestglobal.h>
+
 #include <QtTest/private/qabstracttestlogger_p.h>
 #include <QtTest/private/qtestelementattribute_p.h>
 
+#include <vector>
+
 QT_BEGIN_NAMESPACE
 
 class QTestJUnitStreamer;
@@ -88,7 +92,7 @@ class QJUnitTestLogger : public QAbstractTestLogger
             const char *failureType, const QString &failureDescription);
 
         QTestElement *currentTestSuite = nullptr;
-        QTestElement *listOfTestcases = nullptr;
+        std::vector<QTestElement*> listOfTestcases;
         QTestElement *currentTestCase = nullptr;
         QTestElement *systemOutputElement = nullptr;
         QTestElement *systemErrorElement = nullptr;

src/testlib/qtestcoreelement_p.h

@@ -51,21 +51,23 @@
 // We mean it.
 //
 
-#include <QtTest/private/qtestcorelist_p.h>
+#include <QtTest/qttestglobal.h>
 #include <QtTest/private/qtestelementattribute_p.h>
 
+#include <vector>
+
 QT_BEGIN_NAMESPACE
 
 
 template <class ElementType>
-class QTestCoreElement: public QTestCoreList<ElementType>
+class QTestCoreElement
 {
     public:
         QTestCoreElement( int type = -1 );
         virtual ~QTestCoreElement();
 
         void addAttribute(const QTest::AttributeIndex index, const char *value);
-        QTestElementAttribute *attributes() const;
+        const std::vector<QTestElementAttribute*> &attributes() const;
         const char *attributeValue(QTest::AttributeIndex index) const;
         const char *attributeName(QTest::AttributeIndex index) const;
         const QTestElementAttribute *attribute(QTest::AttributeIndex index) const;
@@ -74,7 +76,7 @@ class QTestCoreElement: public QTestCoreList<ElementType>
         QTest::LogElementType elementType() const;
 
     private:
-        QTestElementAttribute *listOfAttributes = nullptr;
+        std::vector<QTestElementAttribute*> listOfAttributes;
         QTest::LogElementType type;
 };
 
@@ -87,7 +89,8 @@ QTestCoreElement<ElementType>::QTestCoreElement(int t)
 template<class ElementType>
 QTestCoreElement<ElementType>::~QTestCoreElement()
 {
-    delete listOfAttributes;
+    for (auto *attribute : listOfAttributes)
+        delete attribute;
 }
 
 template <class ElementType>
@@ -98,11 +101,11 @@ void QTestCoreElement<ElementType>::addAttribute(const QTest::AttributeIndex att
 
     QTestElementAttribute *testAttribute = new QTestElementAttribute;
     testAttribute->setPair(attributeIndex, value);
-    testAttribute->addToList(&listOfAttributes);
+    listOfAttributes.push_back(testAttribute);
 }
 
 template <class ElementType>
-QTestElementAttribute *QTestCoreElement<ElementType>::attributes() const
+const std::vector<QTestElementAttribute*> &QTestCoreElement<ElementType>::attributes() const
 {
     return listOfAttributes;
 }
@@ -159,12 +162,9 @@ QTest::LogElementType QTestCoreElement<ElementType>::elementType() const
 template <class ElementType>
 const QTestElementAttribute *QTestCoreElement<ElementType>::attribute(QTest::AttributeIndex index) const
 {
-    QTestElementAttribute *iterator = listOfAttributes;
+    for (auto *attribute : listOfAttributes) {
-    while (iterator) {
+        if (attribute->index() == index)
-        if (iterator->index() == index)
+            return attribute;
-            return iterator;
-
-        iterator = iterator->nextElement();
     }
 
     return nullptr;

src/testlib/qtestcorelist_p.h

@@ -1,123 +0,0 @@
-/****************************************************************************
-**
-** Copyright (C) 2016 The Qt Company Ltd.
-** Contact: https://www.qt.io/licensing/
-**
-** This file is part of the QtTest module of the Qt Toolkit.
-**
-** $QT_BEGIN_LICENSE:LGPL$
-** Commercial License Usage
-** Licensees holding valid commercial Qt licenses may use this file in
-** accordance with the commercial license agreement provided with the
-** Software or, alternatively, in accordance with the terms contained in
-** a written agreement between you and The Qt Company. For licensing terms
-** and conditions see https://www.qt.io/terms-conditions. For further
-** information use the contact form at https://www.qt.io/contact-us.
-**
-** GNU Lesser General Public License Usage
-** Alternatively, this file may be used under the terms of the GNU Lesser
-** General Public License version 3 as published by the Free Software
-** Foundation and appearing in the file LICENSE.LGPL3 included in the
-** packaging of this file. Please review the following information to
-** ensure the GNU Lesser General Public License version 3 requirements
-** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
-**
-** GNU General Public License Usage
-** Alternatively, this file may be used under the terms of the GNU
-** General Public License version 2.0 or (at your option) the GNU General
-** Public license version 3 or any later version approved by the KDE Free
-** Qt Foundation. The licenses are as published by the Free Software
-** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
-** included in the packaging of this file. Please review the following
-** information to ensure the GNU General Public License requirements will
-** be met: https://www.gnu.org/licenses/gpl-2.0.html and
-** https://www.gnu.org/licenses/gpl-3.0.html.
-**
-** $QT_END_LICENSE$
-**
-****************************************************************************/
-
-#ifndef QTESTCORELIST_P_H
-#define QTESTCORELIST_P_H
-
-//
-//  W A R N I N G
-//  -------------
-//
-// This file is not part of the Qt API.  It exists purely as an
-// implementation detail.  This header file may change from version to
-// version without notice, or even be removed.
-//
-// We mean it.
-//
-
-#include <QtCore/qglobal.h>
-
-QT_BEGIN_NAMESPACE
-
-
-template <class T>
-class QTestCoreList
-{
-    public:
-        QTestCoreList();
-        virtual ~QTestCoreList();
-
-        void addToList(T **list);
-        T *nextElement();
-        T *previousElement();
-    private:
-        T *next;
-        T *prev;
-};
-
-template <class T>
-QTestCoreList<T>::QTestCoreList()
-    : next(nullptr)
-    , prev(nullptr)
-{
-}
-
-template <class T>
-QTestCoreList<T>::~QTestCoreList()
-{
-    if (prev) {
-        prev->next = nullptr;
-    }
-    delete prev;
-
-    if (next) {
-        next->prev = nullptr;
-    }
-    delete next;
-}
-
-template <class T>
-void QTestCoreList<T>::addToList(T **list)
-{
-    if (next)
-        next->addToList(list);
-    else {
-        next = *list;
-        if (next)
-            next->prev = static_cast<T*>(this);
-    }
-
-    *list = static_cast<T*>(this);
-}
-
-template <class T>
-T *QTestCoreList<T>::nextElement()
-{
-    return next;
-}
-
-template <class T>
-T *QTestCoreList<T>::previousElement()
-{
-    return prev;
-}
-
-QT_END_NAMESPACE
-
-#endif

src/testlib/qtestelement.cpp

@@ -48,16 +48,17 @@ QTestElement::QTestElement(int type)
 
 QTestElement::~QTestElement()
 {
-    delete listOfChildren;
+    for (auto *child : listOfChildren)
+        delete child;
 }
 
-bool QTestElement::addLogElement(QTestElement *element)
+bool QTestElement::addChild(QTestElement *element)
 {
     if (!element)
         return false;
 
     if (element->elementType() != QTest::LET_Undefined) {
-        element->addToList(&listOfChildren);
+        listOfChildren.push_back(element);
         element->setParent(this);
         return true;
     }
@@ -65,7 +66,7 @@ bool QTestElement::addLogElement(QTestElement *element)
     return false;
 }
 
-QTestElement *QTestElement::childElements() const
+const std::vector<QTestElement*> &QTestElement::childElements() const
 {
     return listOfChildren;
 }

src/testlib/qtestelement_p.h

@@ -51,26 +51,27 @@
 // We mean it.
 //
 
+#include <QtTest/qttestglobal.h>
 #include <QtTest/private/qtestcoreelement_p.h>
 
 QT_BEGIN_NAMESPACE
 
 
-class QTestElement: public QTestCoreElement<QTestElement>
+class QTestElement : public QTestCoreElement<QTestElement>
 {
     public:
         QTestElement(int type = -1);
         ~QTestElement();
 
-        bool addLogElement(QTestElement *element);
+        bool addChild(QTestElement *element);
-        QTestElement *childElements() const;
+        const std::vector<QTestElement*> &childElements() const;
 
         const QTestElement *parentElement() const;
         void setParent(const QTestElement *p);
 
     private:
-        QTestElement *listOfChildren = nullptr;
+        std::vector<QTestElement*> listOfChildren;
-        const QTestElement * parent = nullptr;
+        const QTestElement *parent = nullptr;
 
 };
 

src/testlib/qtestelementattribute_p.h

@@ -51,7 +51,7 @@
 // We mean it.
 //
 
-#include <QtTest/private/qtestcorelist_p.h>
+#include <QtTest/qttestglobal.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -92,7 +92,7 @@ namespace QTest {
     };
 }
 
-class QTestElementAttribute: public QTestCoreList<QTestElementAttribute>
+class QTestElementAttribute
 {
     public:
         QTestElementAttribute();

src/testlib/qtestjunitstreamer.cpp

@@ -95,7 +95,7 @@ void QTestJUnitStreamer::formatEnd(const QTestElement *element, QTestCharBuffer
     if (!element || !formatted )
         return;
 
-    if (!element->childElements()) {
+    if (element->childElements().empty()) {
         formatted->data()[0] = '\0';
         return;
     }
@@ -135,7 +135,7 @@ void QTestJUnitStreamer::formatAfterAttributes(const QTestElement *element, QTes
         return;
     }
 
-    if (!element->childElements())
+    if (element->childElements().empty())
         QTest::qt_asprintf(formatted, "/>\n");
     else
         QTest::qt_asprintf(formatted, ">\n");
@@ -145,54 +145,39 @@ void QTestJUnitStreamer::output(QTestElement *element) const
 {
     QTEST_ASSERT(element);
 
-    outputString("<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n");
+    if (!element->parentElement())
-    outputElements(element);
+        outputString("<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n");
-}
 
-void QTestJUnitStreamer::outputElements(QTestElement *element, bool) const
-{
     QTestCharBuffer buf;
-    bool hasChildren;
 
-    // Elements are in reverse order of occurrence, so
+    formatStart(element, &buf);
-    // start from the end and work our way backwards.
+    outputString(buf.data());
-    while (element && element->nextElement())
-        element = element->nextElement();
 
-    while (element) {
+    outputElementAttributes(element, element->attributes());
-        hasChildren = element->childElements();
 
-        formatStart(element, &buf);
+    formatAfterAttributes(element, &buf);
-        outputString(buf.data());
+    outputString(buf.data());
 
-        outputElementAttributes(element, element->attributes());
+    if (!element->childElements().empty())
+        outputElements(element->childElements());
 
-        formatAfterAttributes(element, &buf);
+    formatEnd(element, &buf);
-        outputString(buf.data());
+    outputString(buf.data());
-
-        if (hasChildren)
-            outputElements(element->childElements(), true);
-
-        formatEnd(element, &buf);
-        outputString(buf.data());
-
-        element = element->previousElement();
-    }
 }
 
-void QTestJUnitStreamer::outputElementAttributes(const QTestElement* element, QTestElementAttribute *attribute) const
+void QTestJUnitStreamer::outputElements(const std::vector<QTestElement*> &elements) const
+{
+    for (auto *element : elements)
+        output(element);
+}
+
+void QTestJUnitStreamer::outputElementAttributes(const QTestElement* element, const std::vector<QTestElementAttribute*> &attributes) const
 {
     QTestCharBuffer buf;
 
-    // Attributes are in reverse order of occurrence, so
+    for (auto *attribute : attributes) {
-    // start from the end and work our way backwards.
-    while (attribute && attribute->nextElement())
-        attribute = attribute->nextElement();
-
-    while (attribute) {
         formatAttributes(element, attribute, &buf);
         outputString(buf.data());
-        attribute = attribute->previousElement();
     }
 }
 

src/testlib/qtestjunitstreamer_p.h

@@ -72,8 +72,8 @@ class QTestJUnitStreamer
         void formatAfterAttributes(const QTestElement *element, QTestCharBuffer *formatted) const;
         void formatAttributes(const QTestElement *element, const QTestElementAttribute *attribute, QTestCharBuffer *formatted) const;
         void output(QTestElement *element) const;
-        void outputElements(QTestElement *element, bool isChildElement = false) const;
+        void outputElements(const std::vector<QTestElement*> &) const;
-        void outputElementAttributes(const QTestElement *element, QTestElementAttribute *attribute) const;
+        void outputElementAttributes(const QTestElement *element, const std::vector<QTestElementAttribute*> &attributes) const;
 
         void outputString(const char *msg) const;