CppEditor/CppTools: Don't continue in test function on failure

QVERIFY/QCOMPARE are meant to be called in the test function so that on
failure they just can "return" and thus skip subsequent code. Since we
use reusable test code in the test functions (the *TestCase classes), we
need to ensure that on failure no further test code is executed.

This mostly inlines the run function of the test classes into the
constructor.

Change-Id: I320ee032bdde0174ddfe3fdf3f9e18e19abf1d7f
Reviewed-by: Erik Verbruggen <erik.verbruggen@digia.com>

7 files changed, 115 insertions, 110 deletions

diff --git a/src/plugins/cpptools/cppcompletion_test.cpp b/src/plugins/cpptools/cppcompletion_test.cpp
index 173181c412..83387eaa70 100644
--- a/src/plugins/cpptools/cppcompletion_test.cpp
+++ b/src/plugins/cpptools/cppcompletion_test.cpp
@@ -63,6 +63,9 @@ public:
     CompletionTestCase(const QByteArray &sourceText, const QByteArray &textToInsert = QByteArray())
         : m_position(-1), m_editorWidget(0), m_textDocument(0), m_editor(0)
     {
+        QVERIFY(succeededSoFar());
+        m_succeededSoFar = false;
+
         m_source = sourceText;
         m_position = m_source.indexOf('@');
         QVERIFY(m_position != -1);
@@ -89,6 +92,8 @@ public:
 
         if (!textToInsert.isEmpty())
             insertText(textToInsert);
+
+        m_succeededSoFar = true;
     }
 
     QStringList getCompletions(bool *replaceAccessOperator = 0) const
@@ -162,6 +167,7 @@ void CppToolsPlugin::test_completion_basic_1()
             "    @\n"
             "}";
     CompletionTestCase test(source);
+    QVERIFY(test.succeededSoFar());
 
     QStringList basicCompletions = test.getCompletions();
     QVERIFY(!basicCompletions.contains(QLatin1String("foo")));
@@ -189,6 +195,7 @@ void CppToolsPlugin::test_completion_prefix_first_QTCREATORBUG_8737()
             "}\n"
             ;
     CompletionTestCase test(source, "a_c");
+    QVERIFY(test.succeededSoFar());
 
     QStringList completions = test.getCompletions();
 
@@ -214,9 +221,9 @@ void CppToolsPlugin::test_completion_prefix_first_QTCREATORBUG_9236()
             "};\n"
             ;
     CompletionTestCase test(source, "ret");
+    QVERIFY(test.succeededSoFar());
 
     QStringList completions = test.getCompletions();
-
     QVERIFY(completions.size() >= 2);
     QCOMPARE(completions.at(0), QLatin1String("return"));
     QCOMPARE(completions.at(1), QLatin1String("rETUCASE"));
@@ -233,9 +240,9 @@ void CppToolsPlugin::test_completion_template_function()
     QFETCH(QStringList, expectedCompletions);
 
     CompletionTestCase test(code);
+    QVERIFY(test.succeededSoFar());
 
     QStringList actualCompletions = test.getCompletions();
-
     QString errorPattern(QLatin1String("Completion not found: %1"));
     foreach (const QString &completion, expectedCompletions) {
         QByteArray errorMessage = errorPattern.arg(completion).toUtf8();
@@ -290,6 +297,7 @@ void CppToolsPlugin::test_completion()
     QFETCH(QStringList, expectedCompletions);
 
     CompletionTestCase test(code, prefix);
+    QVERIFY(test.succeededSoFar());
 
     QStringList actualCompletions = test.getCompletions();
     actualCompletions.sort();
@@ -2092,6 +2100,7 @@ void CppToolsPlugin::test_completion_member_access_operator()
     QFETCH(bool, expectedReplaceAccessOperator);
 
     CompletionTestCase test(code, prefix);
+    QVERIFY(test.succeededSoFar());
 
     bool replaceAccessOperator = false;
     QStringList completions = test.getCompletions(&replaceAccessOperator);
diff --git a/src/plugins/cpptools/cpplocatorfilter_test.cpp b/src/plugins/cpptools/cpplocatorfilter_test.cpp
index 5a7e7f3a3a..9280fbcb72 100644
--- a/src/plugins/cpptools/cpplocatorfilter_test.cpp
+++ b/src/plugins/cpptools/cpplocatorfilter_test.cpp
@@ -67,12 +67,21 @@ class CppLocatorFilterTestCase
     , public CppTools::Tests::TestCase
 {
 public:
-    CppLocatorFilterTestCase(ILocatorFilter *filter, const QString &fileName)
+    CppLocatorFilterTestCase(ILocatorFilter *filter,
+                             const QString &fileName,
+                             const QString &searchText,
+                             const ResultDataList &expectedResults)
         : BasicLocatorFilterTest(filter)
         , m_fileName(fileName)
     {
+        QVERIFY(succeededSoFar());
         QVERIFY(!m_fileName.isEmpty());
         QVERIFY(garbageCollectGlobalSnapshot());
+
+        ResultDataList results = ResultData::fromFilterEntryList(matchesFor(searchText));
+//        ResultData::printFilterEntries(results);
+        QVERIFY(!results.isEmpty());
+        QCOMPARE(results, expectedResults);
     }
 
 private:
@@ -88,12 +97,19 @@ class CppCurrentDocumentFilterTestCase
     , public CppTools::Tests::TestCase
 {
 public:
-    CppCurrentDocumentFilterTestCase(const QString &fileName)
+    CppCurrentDocumentFilterTestCase(const QString &fileName,
+                                     const ResultDataList &expectedResults)
         : BasicLocatorFilterTest(PluginManager::getObject<CppCurrentDocumentFilter>())
         , m_editor(0)
         , m_fileName(fileName)
     {
+        QVERIFY(succeededSoFar());
         QVERIFY(!m_fileName.isEmpty());
+
+        ResultDataList results = ResultData::fromFilterEntryList(matchesFor());
+//        ResultData::printFilterEntries(results);
+        QVERIFY(!results.isEmpty());
+        QCOMPARE(results, expectedResults);
     }
 
 private:
@@ -130,11 +146,7 @@ void CppToolsPlugin::test_cpplocatorfilters_CppLocatorFilter()
     QFETCH(QString, searchText);
     QFETCH(ResultDataList, expectedResults);
 
-    CppLocatorFilterTestCase test(filter, testFile);
-    ResultDataList results = ResultData::fromFilterEntryList(test.matchesFor(searchText));
-//    ResultData::printFilterEntries(results);
-    QVERIFY(!results.isEmpty());
-    QCOMPARE(results, expectedResults);
+    CppLocatorFilterTestCase(filter, testFile, searchText, expectedResults);
 }
 
 void CppToolsPlugin::test_cpplocatorfilters_CppLocatorFilter_data()
@@ -277,9 +289,5 @@ void CppToolsPlugin::test_cpplocatorfilters_CppCurrentDocumentFilter()
         << ResultData(_("main()"), _(""))
         ;
 
-    CppCurrentDocumentFilterTestCase test(testFile);
-    ResultDataList results = ResultData::fromFilterEntryList(test.matchesFor());
-//    ResultData::printFilterEntries(results);
-    QVERIFY(!results.isEmpty());
-    QCOMPARE(expectedResults, results);
+    CppCurrentDocumentFilterTestCase(testFile, expectedResults);
 }
diff --git a/src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp b/src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp
index 45e87f2523..ffbffd7122 100644
--- a/src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp
+++ b/src/plugins/cpptools/cpppointerdeclarationformatter_test.cpp
@@ -70,8 +70,13 @@ QString stripCursor(const QString &source)
 class PointerDeclarationFormatterTestCase : public CppTools::Tests::TestCase
 {
 public:
-    PointerDeclarationFormatterTestCase(const QByteArray &source, Document::ParseMode parseMode)
+    PointerDeclarationFormatterTestCase(const QByteArray &source,
+                                        const QString &expectedSource,
+                                        Document::ParseMode parseMode,
+                                        PointerDeclarationFormatter::CursorHandling cursorHandling)
     {
+        QVERIFY(succeededSoFar());
+
         // Find cursor position and remove cursor marker '@'
         int cursorPosition = 0;
         QString sourceWithoutCursorMarker = QLatin1String(source);
@@ -83,58 +88,53 @@ public:
 
         // Write source to temprorary file
         const QString filePath = QDir::tempPath() + QLatin1String("/file.h");
-        m_document = Document::create(filePath);
-        QVERIFY(writeFile(m_document->fileName(), sourceWithoutCursorMarker.toLatin1()));
+        Document::Ptr document = Document::create(filePath);
+        QVERIFY(writeFile(document->fileName(), sourceWithoutCursorMarker.toLatin1()));
 
         // Preprocess source
-        Preprocessor preprocess(0, &m_env);
+        Environment env;
+        Preprocessor preprocess(0, &env);
         const QByteArray preprocessedSource = preprocess.run(filePath, sourceWithoutCursorMarker);
 
-        m_document->setUtf8Source(preprocessedSource);
-        m_document->parse(parseMode);
-        m_document->check();
-        m_translationUnit = m_document->translationUnit();
-        m_snapshot.insert(m_document);
-        m_editor = new TextEditor::PlainTextEditorWidget(0);
+        document->setUtf8Source(preprocessedSource);
+        document->parse(parseMode);
+        document->check();
+        AST *ast = document->translationUnit()->ast();
+        QVERIFY(ast);
+
+        // Open file
+        QScopedPointer<TextEditor::BaseTextEditorWidget> editorWidget(
+            new TextEditor::PlainTextEditorWidget(0));
         QString error;
-        m_editor->open(&error, m_document->fileName(), m_document->fileName());
+        editorWidget->open(&error, document->fileName(), document->fileName());
+        QVERIFY(error.isEmpty());
 
         // Set cursor position
-        QTextCursor cursor = m_editor->textCursor();
+        QTextCursor cursor = editorWidget->textCursor();
         cursor.movePosition(QTextCursor::NextCharacter, QTextCursor::MoveAnchor, cursorPosition);
-        m_editor->setTextCursor(cursor);
+        editorWidget->setTextCursor(cursor);
 
-        m_textDocument = m_editor->document();
-        m_cppRefactoringFile = CppRefactoringChanges::file(m_editor, m_document);
-    }
+        QTextDocument *textDocument = editorWidget->document();
+        CppRefactoringFilePtr cppRefactoringFile
+            = CppRefactoringChanges::file(editorWidget.data(), document);
 
-    void applyFormatting(AST *ast, PointerDeclarationFormatter::CursorHandling cursorHandling)
-    {
+        // Prepare for formatting
         Overview overview;
         overview.showReturnTypes = true;
         overview.showArgumentNames = true;
         overview.starBindFlags = Overview::StarBindFlags(0);
 
         // Run the formatter
-        PointerDeclarationFormatter formatter(m_cppRefactoringFile, overview, cursorHandling);
+        PointerDeclarationFormatter formatter(cppRefactoringFile, overview, cursorHandling);
         ChangeSet change = formatter.format(ast); // ChangeSet may be empty.
 
         // Apply change
-        QTextCursor cursor(m_textDocument);
-        change.apply(&cursor);
-    }
+        QTextCursor changeCursor(textDocument);
+        change.apply(&changeCursor);
 
-public:
-    QTextDocument *m_textDocument;
-    TranslationUnit *m_translationUnit;
-
-private:
-    QByteArray m_source;
-    Snapshot m_snapshot;
-    CppRefactoringFilePtr m_cppRefactoringFile;
-    TextEditor::BaseTextEditorWidget *m_editor;
-    Document::Ptr m_document;
-    Environment m_env;
+        // Compare
+        QCOMPARE(textDocument->toPlainText(), expectedSource);
+    }
 };
 
 } // anonymous namespace
@@ -144,13 +144,10 @@ void CppToolsPlugin::test_format_pointerdeclaration_in_simpledeclarations()
     QFETCH(QString, source);
     QFETCH(QString, reformattedSource);
 
-    PointerDeclarationFormatterTestCase test(source.toLatin1(), Document::ParseDeclaration);
-    AST *ast = test.m_translationUnit->ast();
-    QVERIFY(ast);
-
-    test.applyFormatting(ast, PointerDeclarationFormatter::RespectCursor);
-
-    QCOMPARE(test.m_textDocument->toPlainText(), reformattedSource);
+    PointerDeclarationFormatterTestCase(source.toLatin1(),
+                                        reformattedSource,
+                                        Document::ParseDeclaration,
+                                        PointerDeclarationFormatter::RespectCursor);
 }
 
 void CppToolsPlugin::test_format_pointerdeclaration_in_simpledeclarations_data()
@@ -371,13 +368,10 @@ void CppToolsPlugin::test_format_pointerdeclaration_in_controlflowstatements()
     QFETCH(QString, source);
     QFETCH(QString, reformattedSource);
 
-    PointerDeclarationFormatterTestCase test(source.toLatin1(), Document::ParseStatement);
-    AST *ast = test.m_translationUnit->ast();
-    QVERIFY(ast);
-
-    test.applyFormatting(ast, PointerDeclarationFormatter::RespectCursor);
-
-    QCOMPARE(test.m_textDocument->toPlainText(), reformattedSource);
+    PointerDeclarationFormatterTestCase(source.toLatin1(),
+                                        reformattedSource,
+                                        Document::ParseStatement,
+                                        PointerDeclarationFormatter::RespectCursor);
 }
 
 void CppToolsPlugin::test_format_pointerdeclaration_in_controlflowstatements_data()
@@ -449,13 +443,10 @@ void CppToolsPlugin::test_format_pointerdeclaration_multiple_declarators()
     QFETCH(QString, source);
     QFETCH(QString, reformattedSource);
 
-    PointerDeclarationFormatterTestCase test(source.toLatin1(), Document::ParseDeclaration);
-    AST *ast = test.m_translationUnit->ast();
-    QVERIFY(ast);
-
-    test.applyFormatting(ast, PointerDeclarationFormatter::RespectCursor);
-
-    QCOMPARE(test.m_textDocument->toPlainText(), reformattedSource);
+    PointerDeclarationFormatterTestCase(source.toLatin1(),
+                                        reformattedSource,
+                                        Document::ParseDeclaration,
+                                        PointerDeclarationFormatter::RespectCursor);
 }
 
 void CppToolsPlugin::test_format_pointerdeclaration_multiple_declarators_data()
@@ -507,13 +498,10 @@ void CppToolsPlugin::test_format_pointerdeclaration_multiple_matches()
     QFETCH(QString, source);
     QFETCH(QString, reformattedSource);
 
-    PointerDeclarationFormatterTestCase test(source.toLatin1(), Document::ParseTranlationUnit);
-    AST *ast = test.m_translationUnit->ast();
-    QVERIFY(ast);
-
-    test.applyFormatting(ast, PointerDeclarationFormatter::IgnoreCursor);
-
-    QCOMPARE(test.m_textDocument->toPlainText(), reformattedSource);
+    PointerDeclarationFormatterTestCase(source.toLatin1(),
+                                        reformattedSource,
+                                        Document::ParseTranlationUnit,
+                                        PointerDeclarationFormatter::IgnoreCursor);
 }
 
 void CppToolsPlugin::test_format_pointerdeclaration_multiple_matches_data()
@@ -593,13 +581,10 @@ void CppToolsPlugin::test_format_pointerdeclaration_macros()
     QFETCH(QString, source);
     QFETCH(QString, reformattedSource);
 
-    PointerDeclarationFormatterTestCase test(source.toLatin1(), Document::ParseTranlationUnit);
-    AST *ast = test.m_translationUnit->ast();
-    QVERIFY(ast);
-
-    test.applyFormatting(ast, PointerDeclarationFormatter::RespectCursor);
-
-    QCOMPARE(test.m_textDocument->toPlainText(), reformattedSource);
+    PointerDeclarationFormatterTestCase(source.toLatin1(),
+                                        reformattedSource,
+                                        Document::ParseTranlationUnit,
+                                        PointerDeclarationFormatter::RespectCursor);
 }
 
 void CppToolsPlugin::test_format_pointerdeclaration_macros_data()
diff --git a/src/plugins/cpptools/cpptoolstestcase.cpp b/src/plugins/cpptools/cpptoolstestcase.cpp
index be9e61fb03..80de9dfc1d 100644
--- a/src/plugins/cpptools/cpptoolstestcase.cpp
+++ b/src/plugins/cpptools/cpptoolstestcase.cpp
@@ -70,10 +70,12 @@ bool TestDocument::writeToDisk() const
 
 TestCase::TestCase(bool runGarbageCollector)
     : m_modelManager(CppModelManagerInterface::instance())
+    , m_succeededSoFar(false)
     , m_runGarbageCollector(runGarbageCollector)
 {
     if (m_runGarbageCollector)
         QVERIFY(garbageCollectGlobalSnapshot());
+    m_succeededSoFar = true;
 }
 
 TestCase::~TestCase()
@@ -85,6 +87,11 @@ TestCase::~TestCase()
         QVERIFY(garbageCollectGlobalSnapshot());
 }
 
+bool TestCase::succeededSoFar() const
+{
+    return m_succeededSoFar;
+}
+
 CPlusPlus::Snapshot TestCase::globalSnapshot()
 {
     return CppModelManagerInterface::instance()->snapshot();
diff --git a/src/plugins/cpptools/cpptoolstestcase.h b/src/plugins/cpptools/cpptoolstestcase.h
index 1c9b5b4f59..cdca7c3db7 100644
--- a/src/plugins/cpptools/cpptoolstestcase.h
+++ b/src/plugins/cpptools/cpptoolstestcase.h
@@ -70,6 +70,7 @@ public:
     TestCase(bool runGarbageCollector = true);
     ~TestCase();
 
+    bool succeededSoFar() const;
     void closeEditorAtEndOfTestCase(Core::IEditor *editor);
 
     static bool parseFiles(const QString &filePath);
@@ -86,6 +87,7 @@ public:
 
 protected:
     CppModelManagerInterface *m_modelManager;
+    bool m_succeededSoFar;
 
 private:
     QList<Core::IEditor *> m_editorsToClose;
diff --git a/src/plugins/cpptools/symbolsearcher_test.cpp b/src/plugins/cpptools/symbolsearcher_test.cpp
index 22d3e4ad11..6f41179dba 100644
--- a/src/plugins/cpptools/symbolsearcher_test.cpp
+++ b/src/plugins/cpptools/symbolsearcher_test.cpp
@@ -91,37 +91,39 @@ class SymbolSearcherTestCase : public CppTools::Tests::TestCase
 {
 public:
     /// Takes no ownership of indexingSupportToUse
-    SymbolSearcherTestCase(const QString &testFile, CppIndexingSupport *indexingSupportToUse)
-        : m_indexingSupportToUse(indexingSupportToUse)
-        , m_testFile(testFile)
+    SymbolSearcherTestCase(const QString &testFile,
+                           CppIndexingSupport *indexingSupportToUse,
+                           const SymbolSearcher::Parameters &searchParameters,
+                           const ResultDataList &expectedResults)
+        : m_indexingSupportToRestore(0)
+        , m_indexingSupportToUse(indexingSupportToUse)
     {
+        QVERIFY(succeededSoFar());
+
         QVERIFY(m_indexingSupportToUse);
-        QVERIFY(parseFiles(m_testFile));
+        QVERIFY(parseFiles(testFile));
         m_indexingSupportToRestore = m_modelManager->indexingSupport();
         m_modelManager->setIndexingSupport(m_indexingSupportToUse);
-    }
 
-    ResultDataList run(const SymbolSearcher::Parameters &searchParameters) const
-    {
         CppIndexingSupport *indexingSupport = m_modelManager->indexingSupport();
         SymbolSearcher *symbolSearcher = indexingSupport->createSymbolSearcher(searchParameters,
-            QSet<QString>() << m_testFile);
+            QSet<QString>() << testFile);
         QFuture<Find::SearchResultItem> search
             = QtConcurrent::run(&SymbolSearcher::runSearch, symbolSearcher);
         search.waitForFinished();
         ResultDataList results = ResultData::fromSearchResultList(search.results());
-        return results;
+        QCOMPARE(results, expectedResults);
     }
 
     ~SymbolSearcherTestCase()
     {
-        m_modelManager->setIndexingSupport(m_indexingSupportToRestore);
+        if (m_indexingSupportToRestore)
+            m_modelManager->setIndexingSupport(m_indexingSupportToRestore);
     }
 
 private:
     CppIndexingSupport *m_indexingSupportToRestore;
     CppIndexingSupport *m_indexingSupportToUse;
-    const QString m_testFile;
 };
 
 } // anonymous namespace
@@ -148,10 +150,10 @@ void CppToolsPlugin::test_builtinsymbolsearcher()
     QFETCH(ResultDataList, expectedResults);
 
     QScopedPointer<CppIndexingSupport> builtinIndexingSupport(new BuiltinIndexingSupport);
-
-    SymbolSearcherTestCase test(testFile, builtinIndexingSupport.data());
-    const ResultDataList results = test.run(searchParameters);
-    QCOMPARE(results, expectedResults);
+    SymbolSearcherTestCase(testFile,
+                           builtinIndexingSupport.data(),
+                           searchParameters,
+                           expectedResults);
 }
 
 void CppToolsPlugin::test_builtinsymbolsearcher_data()
diff --git a/src/plugins/cpptools/typehierarchybuilder_test.cpp b/src/plugins/cpptools/typehierarchybuilder_test.cpp
index fc776f824b..1f57596c41 100644
--- a/src/plugins/cpptools/typehierarchybuilder_test.cpp
+++ b/src/plugins/cpptools/typehierarchybuilder_test.cpp
@@ -101,15 +101,12 @@ class TypeHierarchyBuilderTestCase : public CppTools::Tests::TestCase
 public:
     TypeHierarchyBuilderTestCase(const QList<Tests::TestDocument> &documents,
                                  const QString &expectedHierarchy)
-        : m_documents(documents),
-          m_expectedHierarchy(expectedHierarchy)
-    {}
-
-    void run()
     {
+        QVERIFY(succeededSoFar());
+
         // Write files
         QStringList filePaths;
-        foreach (const Tests::TestDocument &document, m_documents) {
+        foreach (const Tests::TestDocument &document, documents) {
             QVERIFY(document.writeToDisk());
             filePaths << document.filePath();
         }
@@ -130,12 +127,8 @@ public:
         const QString actualHierarchy = toString(hierarchy);
 //        Uncomment for updating/generating reference data:
 //        qDebug() << actualHierarchy;
-        QCOMPARE(actualHierarchy, m_expectedHierarchy);
+        QCOMPARE(actualHierarchy, expectedHierarchy);
     }
-
-private:
-    QList<Tests::TestDocument> m_documents;
-    QString m_expectedHierarchy;
 };
 
 } // anonymous namespace
@@ -192,6 +185,5 @@ void CppToolsPlugin::test_typehierarchy()
     QFETCH(QList<Tests::TestDocument>, documents);
     QFETCH(QString, expectedHierarchy);
 
-    TypeHierarchyBuilderTestCase testCase(documents, expectedHierarchy);
-    testCase.run();
+    TypeHierarchyBuilderTestCase(documents, expectedHierarchy);
 }