AutoTest: Fix parsing of multiple test cases in single qml file

Quick tests allow definition of more than one TestCase inside a
qml file and even nesting is possible, so support this correctly.

Fixes: QTCREATORBUG-22761
Change-Id: I65fcc7cd6063d976d798c3e900d3299a12e2d73f
Reviewed-by: David Schulz <david.schulz@qt.io>

7 files changed, 108 insertions, 60 deletions

diff --git a/src/plugins/autotest/autotestunittests.cpp b/src/plugins/autotest/autotestunittests.cpp
index 614a61a6c27..b571ed13546 100644
--- a/src/plugins/autotest/autotestunittests.cpp
+++ b/src/plugins/autotest/autotestunittests.cpp
@@ -131,13 +131,13 @@ void AutoTestUnitTests::testCodeParser_data()
             << 1 << 0 << 0 << 0;
     QTest::newRow("mixedAutoTestAndQuickTests")
             << QString(m_tmpDir->path() + "/mixed_atp/mixed_atp.pro")
-            << 4 << 7 << 3 << 10;
+            << 4 << 10 << 4 << 10;
     QTest::newRow("plainAutoTestQbs")
             << QString(m_tmpDir->path() + "/plain/plain.qbs")
             << 1 << 0 << 0 << 0;
     QTest::newRow("mixedAutoTestAndQuickTestsQbs")
             << QString(m_tmpDir->path() + "/mixed_atp/mixed_atp.qbs")
-            << 4 << 7 << 3 << 10;
+            << 4 << 10 << 4 << 10;
 }
 
 void AutoTestUnitTests::testCodeParserSwitchStartup()
@@ -183,8 +183,8 @@ void AutoTestUnitTests::testCodeParserSwitchStartup_data()
             m_tmpDir->path() + "/mixed_atp/mixed_atp.qbs"});
 
     QList<int> expectedAutoTests = QList<int>()         << 1 << 4 << 1 << 4;
-    QList<int> expectedNamedQuickTests = QList<int>()   << 0 << 7 << 0 << 7;
-    QList<int> expectedUnnamedQuickTests = QList<int>() << 0 << 3 << 0 << 3;
+    QList<int> expectedNamedQuickTests = QList<int>()   << 0 << 10 << 0 << 10;
+    QList<int> expectedUnnamedQuickTests = QList<int>() << 0 << 4 << 0 << 4;
     QList<int> expectedDataTagsCount = QList<int>()     << 0 << 10 << 0 << 10;
 
     QTest::newRow("loadMultipleProjects")
diff --git a/src/plugins/autotest/quick/quicktestparser.cpp b/src/plugins/autotest/quick/quicktestparser.cpp
index 25423255102..14e04c8a2fb 100644
--- a/src/plugins/autotest/quick/quicktestparser.cpp
+++ b/src/plugins/autotest/quick/quicktestparser.cpp
@@ -190,20 +190,26 @@ static bool checkQmlDocumentForQuickTestCode(QFutureInterface<TestParseResultPtr
     if (!qmlVisitor.isValid())
         return false;
 
-    const QString testCaseName = qmlVisitor.testCaseName();
-    const TestCodeLocationAndType tcLocationAndType = qmlVisitor.testCaseLocation();
-    const QMap<QString, TestCodeLocationAndType> &testFunctions = qmlVisitor.testFunctions();
-
-    QuickTestParseResult *parseResult = new QuickTestParseResult(id);
-    parseResult->proFile = proFile;
-    parseResult->itemType = TestTreeItem::TestCase;
-    QMap<QString, TestCodeLocationAndType>::ConstIterator it = testFunctions.begin();
-    const QMap<QString, TestCodeLocationAndType>::ConstIterator end = testFunctions.end();
-    for ( ; it != end; ++it) {
-        const TestCodeLocationAndType &loc = it.value();
+    const QVector<QuickTestCaseSpec> &testFunctions = qmlVisitor.testFunctions();
+
+    for (const QuickTestCaseSpec &it : testFunctions) {
+        const QString testCaseName = it.m_caseName;
+        const QString functionName = it.m_functionName;
+        const TestCodeLocationAndType &loc = it.m_functionLocationAndType;
+
+        QuickTestParseResult *parseResult = new QuickTestParseResult(id);
+        parseResult->proFile = proFile;
+        parseResult->itemType = TestTreeItem::TestCase;
+        if (!testCaseName.isEmpty()) {
+            parseResult->fileName = it.m_name;
+            parseResult->name = testCaseName;
+            parseResult->line = it.m_line;
+            parseResult->column = it.m_column;
+        }
+
         QuickTestParseResult *funcResult = new QuickTestParseResult(id);
-        funcResult->name = it.key();
-        funcResult->displayName = it.key();
+        funcResult->name = functionName;
+        funcResult->displayName = functionName;
         funcResult->itemType = loc.m_type;
         funcResult->fileName = loc.m_name;
         funcResult->line = loc.m_line;
@@ -211,14 +217,9 @@ static bool checkQmlDocumentForQuickTestCode(QFutureInterface<TestParseResultPtr
         funcResult->proFile = proFile;
 
         parseResult->children.append(funcResult);
+
+        futureInterface.reportResult(TestParseResultPtr(parseResult));
     }
-    if (!testCaseName.isEmpty()) {
-        parseResult->fileName = tcLocationAndType.m_name;
-        parseResult->name = testCaseName;
-        parseResult->line = tcLocationAndType.m_line;
-        parseResult->column = tcLocationAndType.m_column;
-    }
-    futureInterface.reportResult(TestParseResultPtr(parseResult));
     return true;
 }
 
diff --git a/src/plugins/autotest/quick/quicktesttreeitem.cpp b/src/plugins/autotest/quick/quicktesttreeitem.cpp
index 52d3796463e..c18de0ee9e2 100644
--- a/src/plugins/autotest/quick/quicktesttreeitem.cpp
+++ b/src/plugins/autotest/quick/quicktesttreeitem.cpp
@@ -323,11 +323,11 @@ TestTreeItem *QuickTestTreeItem::find(const TestParseResult *result)
             TestTreeItem *group = findFirstLevelChild([path](TestTreeItem *group) {
                     return group->filePath() == path;
             });
-            return group ? group->findChildByFile(result->fileName) : nullptr;
+            return group ? group->findChildByNameAndFile(result->name, result->fileName) : nullptr;
         }
-        return findChildByFile(result->fileName);
+        return findChildByNameAndFile(result->name, result->fileName);
     case GroupNode:
-        return findChildByFile(result->fileName);
+        return findChildByNameAndFile(result->name, result->fileName);
     case TestCase:
         return name().isEmpty() ? findChildByNameAndFile(result->name, result->fileName)
                                 : findChildByName(result->name);
@@ -345,9 +345,9 @@ TestTreeItem *QuickTestTreeItem::findChild(const TestTreeItem *other)
     case Root:
         if (otherType == TestCase && other->name().isEmpty())
             return unnamedQuickTests();
-        return findChildByFileAndType(other->filePath(), otherType);
+        return findChildByFileNameAndType(other->filePath(), other->name(), otherType);
     case GroupNode:
-        return findChildByFileAndType(other->filePath(), otherType);
+        return findChildByFileNameAndType(other->filePath(), other->name(), otherType);
     case TestCase:
         if (otherType != TestFunction && otherType != TestDataFunction && otherType != TestSpecialFunction)
             return nullptr;
@@ -444,6 +444,16 @@ void QuickTestTreeItem::markForRemovalRecursively(const QString &filePath)
     }
 }
 
+TestTreeItem *QuickTestTreeItem::findChildByFileNameAndType(const QString &filePath,
+                                                            const QString &name,
+                                                            TestTreeItem::Type tType)
+
+{
+    return findFirstLevelChild([filePath, name, tType](const TestTreeItem *other) {
+        return other->type() == tType && other->name() == name && other->filePath() == filePath;
+    });
+}
+
 TestTreeItem *QuickTestTreeItem::unnamedQuickTests() const
 {
     if (type() != Root)
diff --git a/src/plugins/autotest/quick/quicktesttreeitem.h b/src/plugins/autotest/quick/quicktesttreeitem.h
index b9f06d6a079..a9e48fca940 100644
--- a/src/plugins/autotest/quick/quicktesttreeitem.h
+++ b/src/plugins/autotest/quick/quicktesttreeitem.h
@@ -57,6 +57,8 @@ public:
     QSet<QString> internalTargets() const override;
     void markForRemovalRecursively(const QString &filePath) override;
 private:
+    TestTreeItem *findChildByFileNameAndType(const QString &filePath, const QString &name,
+                                             Type tType);
     TestTreeItem *unnamedQuickTests() const;
 };
 
diff --git a/src/plugins/autotest/quick/quicktestvisitors.cpp b/src/plugins/autotest/quick/quicktestvisitors.cpp
index f9934e735d2..7243530700a 100644
--- a/src/plugins/autotest/quick/quicktestvisitors.cpp
+++ b/src/plugins/autotest/quick/quicktestvisitors.cpp
@@ -31,6 +31,7 @@
 #include <qmljs/qmljslink.h>
 #include <qmljs/qmljsutils.h>
 #include <utils/algorithm.h>
+#include <utils/qtcassert.h>
 
 namespace Autotest {
 namespace Internal {
@@ -96,18 +97,23 @@ bool TestQmlVisitor::visit(QmlJS::AST::UiObjectDefinition *ast)
 
     m_typeIsTestCase = true;
     m_insideTestCase = true;
-    m_currentTestCaseName.clear();
     const auto sourceLocation = ast->firstSourceLocation();
-    m_testCaseLocation.m_name = m_currentDoc->fileName();
-    m_testCaseLocation.m_line = sourceLocation.startLine;
-    m_testCaseLocation.m_column = sourceLocation.startColumn - 1;
-    m_testCaseLocation.m_type = TestTreeItem::TestCase;
+    QuickTestCaseSpec currentSpec;
+    currentSpec.m_name = m_currentDoc->fileName();
+    currentSpec.m_line = sourceLocation.startLine;
+    currentSpec.m_column = sourceLocation.startColumn - 1;
+    currentSpec.m_type = TestTreeItem::TestCase;
+    m_testCases.push(currentSpec);
     return true;
 }
 
 void TestQmlVisitor::endVisit(QmlJS::AST::UiObjectDefinition *)
 {
-    m_insideTestCase = m_objectStack.pop() == "TestCase";
+    if (!m_objectStack.isEmpty() && m_objectStack.pop() == "TestCase") {
+        if (!m_testCases.isEmpty())
+            m_testCases.pop();
+        m_insideTestCase = !m_objectStack.isEmpty() && m_objectStack.top() == "TestCase";
+    }
 }
 
 bool TestQmlVisitor::visit(QmlJS::AST::ExpressionStatement *ast)
@@ -148,15 +154,22 @@ bool TestQmlVisitor::visit(QmlJS::AST::FunctionDeclaration *ast)
         else
             locationAndType.m_type = TestTreeItem::TestFunction;
 
-        m_testFunctions.insert(name.toString(), locationAndType);
+        if (m_testCases.isEmpty()) // invalid qml code
+            return false;
+
+        QuickTestCaseSpec testCaseWithFunc = m_testCases.top();
+        testCaseWithFunc.m_functionName = name.toString();
+        testCaseWithFunc.m_functionLocationAndType = locationAndType;
+        m_testFunctions.append(testCaseWithFunc);
     }
     return false;
 }
 
 bool TestQmlVisitor::visit(QmlJS::AST::StringLiteral *ast)
 {
-    if (m_expectTestCaseName && m_currentTestCaseName.isEmpty()) {
-        m_currentTestCaseName = ast->value.toString();
+    if (m_expectTestCaseName) {
+        QTC_ASSERT(!m_testCases.isEmpty(), return false);
+        m_testCases.top().m_caseName = ast->value.toString();
         m_expectTestCaseName = false;
     }
     return false;
diff --git a/src/plugins/autotest/quick/quicktestvisitors.h b/src/plugins/autotest/quick/quicktestvisitors.h
index 025ce0198db..b4245a5abc5 100644
--- a/src/plugins/autotest/quick/quicktestvisitors.h
+++ b/src/plugins/autotest/quick/quicktestvisitors.h
@@ -37,6 +37,14 @@
 namespace Autotest {
 namespace Internal {
 
+class QuickTestCaseSpec : public TestCodeLocationAndType
+{
+public:
+    QString m_caseName;
+    QString m_functionName;
+    TestCodeLocationAndType m_functionLocationAndType;
+};
+
 class TestQmlVisitor : public QmlJS::AST::Visitor
 {
 public:
@@ -50,17 +58,14 @@ public:
     bool visit(QmlJS::AST::FunctionDeclaration *ast) override;
     bool visit(QmlJS::AST::StringLiteral *ast) override;
 
-    QString testCaseName() const { return m_currentTestCaseName; }
-    TestCodeLocationAndType testCaseLocation() const { return m_testCaseLocation; }
-    QMap<QString, TestCodeLocationAndType> testFunctions() const { return m_testFunctions; }
+    QVector<QuickTestCaseSpec> testFunctions() const { return m_testFunctions; }
     bool isValid() const { return m_typeIsTestCase; }
 
 private:
     QmlJS::Document::Ptr m_currentDoc;
     QmlJS::Snapshot m_snapshot;
-    QString m_currentTestCaseName;
-    TestCodeLocationAndType m_testCaseLocation;
-    QMap<QString, TestCodeLocationAndType> m_testFunctions;
+    QStack<QuickTestCaseSpec> m_testCases;
+    QVector<QuickTestCaseSpec> m_testFunctions;
     QStack<QString> m_objectStack;
     bool m_typeIsTestCase = false;
     bool m_insideTestCase = false;
diff --git a/src/plugins/autotest/unit_test/mixed_atp/tests/auto/quickauto/tst_test2.qml b/src/plugins/autotest/unit_test/mixed_atp/tests/auto/quickauto/tst_test2.qml
index d0115c5dd74..b6330ac4ea6 100644
--- a/src/plugins/autotest/unit_test/mixed_atp/tests/auto/quickauto/tst_test2.qml
+++ b/src/plugins/autotest/unit_test/mixed_atp/tests/auto/quickauto/tst_test2.qml
@@ -34,21 +34,38 @@ TestCase {
         verify(blubb == bla, "Comparing concat equality")
     }
 
-// nested TestCases actually fail
-//    TestCase {
-//        name: "boo"
-
-//        function test_boo() {
-//            verify(true);
-//        }
-
-//        TestCase {
-//            name: "far"
-
-//            function test_far() {
-//                verify(true);
-//            }
-//        }
-//    }
+    TestCase {
+        name: "boo"
+
+        function test_boo() {
+            verify(true);
+        }
+
+        TestCase {
+            name: "far"
+
+            function test_far() {
+                verify(true);
+            }
+        }
+
+        function test_boo2() { // should not get added to "far", but to "boo"
+            verify(false);
+        }
+    }
+
+    TestCase {
+        name: "secondBoo"
+
+        function test_bar() {
+            compare(1, 1);
+        }
+    }
+
+    TestCase { // unnamed
+        function test_func() {
+            verify(true);
+        }
+    }
 }