Merge pull request #3105 from uklotzde/2.3_test_dependencies

Testing: Reduce dependencies / Fix non-functional test

7 files changed, 159 insertions, 96 deletions

diff --git a/src/test/controllerengine_test.cpp b/src/test/controllerengine_test.cpp
index 642b4b9896..2fd534dd78 100644
--- a/src/test/controllerengine_test.cpp
+++ b/src/test/controllerengine_test.cpp
@@ -1,19 +1,33 @@
+#include "controllers/controllerengine.h"
+
+#include <QScopedPointer>
+#include <QTemporaryFile>
 #include <QThread>
 #include <QtDebug>
+#include <memory>
 
 #include "control/controlobject.h"
 #include "control/controlpotmeter.h"
 #include "controllers/controllerdebug.h"
-#include "controllers/controllerengine.h"
 #include "controllers/softtakeover.h"
 #include "preferences/usersettings.h"
 #include "test/mixxxtest.h"
 #include "util/color/colorpalette.h"
-#include "util/memory.h"
 #include "util/time.h"
 
+typedef std::unique_ptr<QTemporaryFile> ScopedTemporaryFile;
+
 class ControllerEngineTest : public MixxxTest {
   protected:
+    static ScopedTemporaryFile makeTemporaryFile(const QString& contents) {
+        QByteArray contentsBa = contents.toLocal8Bit();
+        ScopedTemporaryFile pFile = std::make_unique<QTemporaryFile>();
+        pFile->open();
+        pFile->write(contentsBa);
+        pFile->close();
+        return pFile;
+    }
+
     void SetUp() override {
         mixxx::Time::setTestMode(true);
         mixxx::Time::setTestElapsedTime(mixxx::Duration::fromMillis(10));
diff --git a/src/test/learningutilstest.cpp b/src/test/learningutilstest.cpp
index c6e2057503..8741085c38 100644
--- a/src/test/learningutilstest.cpp
+++ b/src/test/learningutilstest.cpp
@@ -1,7 +1,8 @@
 #include <stdint.h>
 
-#include "test/mixxxtest.h"
+#include "control/controlobject.h"
 #include "controllers/learningutils.h"
+#include "test/mixxxtest.h"
 #include "util/memory.h"
 
 std::ostream& operator<<(std::ostream& stream, const MidiInputMapping& mapping) {
diff --git a/src/test/librarytest.h b/src/test/librarytest.h
index c7e99c645c..d114320c1a 100644
--- a/src/test/librarytest.h
+++ b/src/test/librarytest.h
@@ -2,13 +2,13 @@
 
 #include <memory>
 
-#include "test/mixxxtest.h"
-
+#include "control/controlobject.h"
 #include "database/mixxxdb.h"
 #include "library/trackcollection.h"
 #include "library/trackcollectionmanager.h"
-#include "util/db/dbconnectionpooler.h"
+#include "test/mixxxtest.h"
 #include "util/db/dbconnectionpooled.h"
+#include "util/db/dbconnectionpooler.h"
 
 class LibraryTest : public MixxxTest {
   protected:
diff --git a/src/test/mixxxtest.cpp b/src/test/mixxxtest.cpp
index 07dbf28da6..b881b8dccb 100644
--- a/src/test/mixxxtest.cpp
+++ b/src/test/mixxxtest.cpp
@@ -1,5 +1,8 @@
 #include "test/mixxxtest.h"
 
+#include <QTemporaryFile>
+
+#include "control/control.h"
 #include "library/coverartutils.h"
 #include "sources/soundsourceproxy.h"
 #include "util/cmdlineargs.h"
@@ -65,3 +68,82 @@ MixxxTest::~MixxxTest() {
         pControl->deleteCreatorCO();
     }
 }
+
+void MixxxTest::saveAndReloadConfig() {
+    m_pConfig->save();
+    m_pConfig = UserSettingsPointer(
+            new UserSettings(getTestDataDir().filePath("test.cfg")));
+    ControlDoublePrivate::setUserConfig(m_pConfig);
+}
+
+namespace mixxxtest {
+
+FileRemover::~FileRemover() {
+    VERIFY_OR_DEBUG_ASSERT(
+            m_fileName.isEmpty() ||
+            QFile::remove(m_fileName) ||
+            !QFile::exists(m_fileName)) {
+        // unexpected
+    }
+}
+
+QString generateTemporaryFileName(const QString& fileNameTemplate) {
+    auto tmpFile = QTemporaryFile(fileNameTemplate);
+    // The file must be opened to create it and to obtain
+    // its file name!
+    VERIFY_OR_DEBUG_ASSERT(tmpFile.open()) {
+        return QString();
+    }
+    const auto tmpFileName = tmpFile.fileName();
+    DEBUG_ASSERT(!tmpFileName.isEmpty());
+    // The empty temporary file will be removed upon returning
+    // from this function
+    return tmpFileName;
+}
+
+QString createEmptyTemporaryFile(const QString& fileNameTemplate) {
+    auto emptyFile = QTemporaryFile(fileNameTemplate);
+    VERIFY_OR_DEBUG_ASSERT(emptyFile.open()) {
+        return QString();
+    }
+
+    // Retrieving the file's name after opening it is required to actually
+    // create a named file on Linux.
+    const auto fileName = emptyFile.fileName();
+    DEBUG_ASSERT(!fileName.isEmpty());
+    VERIFY_OR_DEBUG_ASSERT(emptyFile.exists()) {
+        return QString();
+    }
+    VERIFY_OR_DEBUG_ASSERT(emptyFile.size() == 0) {
+        return QString();
+    }
+
+    emptyFile.setAutoRemove(false);
+    return fileName;
+}
+
+bool copyFile(const QString& srcFileName, const QString& dstFileName) {
+    auto srcFile = QFile(srcFileName);
+    DEBUG_ASSERT(srcFile.exists());
+    VERIFY_OR_DEBUG_ASSERT(srcFile.copy(dstFileName)) {
+        qWarning()
+                << srcFile.errorString()
+                << "- Failed to copy file"
+                << srcFile.fileName()
+                << "->"
+                << dstFileName;
+        return false;
+    }
+    auto dstFileRemover = FileRemover(dstFileName);
+    auto dstFile = QFile(dstFileName);
+    VERIFY_OR_DEBUG_ASSERT(dstFile.exists()) {
+        return false;
+    }
+    VERIFY_OR_DEBUG_ASSERT(srcFile.size() == dstFile.size()) {
+        return false;
+    }
+    dstFileRemover.keepFile();
+    return true;
+}
+
+} // namespace mixxxtest
diff --git a/src/test/mixxxtest.h b/src/test/mixxxtest.h
index 92aa601218..92c1df7e95 100644
--- a/src/test/mixxxtest.h
+++ b/src/test/mixxxtest.h
@@ -1,24 +1,17 @@
-#ifndef MIXXXTEST_H
-#define MIXXXTEST_H
+#pragma once
 
 #include <gtest/gtest.h>
 
 #include <QDir>
-#include <QTemporaryFile>
-#include <QTemporaryDir>
 #include <QScopedPointer>
+#include <QTemporaryDir>
 
 #include "mixxxapplication.h"
-
 #include "preferences/usersettings.h"
-#include "control/controlobject.h"
-#include "control/controlproxy.h"
 
 #define EXPECT_QSTRING_EQ(expected, test) EXPECT_STREQ(qPrintable(expected), qPrintable(test))
 #define ASSERT_QSTRING_EQ(expected, test) ASSERT_STREQ(qPrintable(expected), qPrintable(test))
 
-typedef QScopedPointer<QTemporaryFile> ScopedTemporaryFile;
-
 class MixxxTest : public testing::Test {
   public:
     MixxxTest();
@@ -29,7 +22,7 @@ class MixxxTest : public testing::Test {
     // and destroying the QApplication multiple times in the same process.
     // http://stackoverflow.com/questions/14243858/qapplication-segfaults-in-googletest
     class ApplicationScope {
-    public:
+      public:
         ApplicationScope(int& argc, char** argv);
         ~ApplicationScope();
     };
@@ -45,21 +38,7 @@ class MixxxTest : public testing::Test {
     }
 
     // Simulate restarting Mixxx by saving and reloading the UserSettings.
-    void saveAndReloadConfig() {
-        m_pConfig->save();
-        m_pConfig = UserSettingsPointer(
-            new UserSettings(getTestDataDir().filePath("test.cfg")));
-        ControlDoublePrivate::setUserConfig(m_pConfig);
-    }
-
-    QTemporaryFile* makeTemporaryFile(const QString& contents) const {
-        QByteArray contentsBa = contents.toLocal8Bit();
-        QTemporaryFile* file = new QTemporaryFile();
-        file->open();
-        file->write(contentsBa);
-        file->close();
-        return file;
-    }
+    void saveAndReloadConfig();
 
     QDir getTestDataDir() const {
         return m_testDataDir.path();
@@ -74,4 +53,33 @@ class MixxxTest : public testing::Test {
     UserSettingsPointer m_pConfig;
 };
 
-#endif /* MIXXXTEST_H */
+namespace mixxxtest {
+
+/// Returns the full, non-empty file path on success.
+///
+/// For the format of fileNameTemplate refer to QTemporaryFile.
+QString generateTemporaryFileName(const QString& fileNameTemplate);
+
+/// Returns the full, non-empty file path on success.
+///
+/// For the format of fileNameTemplate refer to QTemporaryFile.
+QString createEmptyTemporaryFile(const QString& fileNameTemplate);
+
+bool copyFile(const QString& srcFileName, const QString& dstFileName);
+
+class FileRemover final {
+  public:
+    explicit FileRemover(const QString& fileName)
+            : m_fileName(fileName) {
+    }
+    ~FileRemover();
+
+    void keepFile() {
+        m_fileName = QString();
+    }
+
+  private:
+    QString m_fileName;
+};
+
+} // namespace mixxxtest
diff --git a/src/test/soundproxy_test.cpp b/src/test/soundproxy_test.cpp
index 32e3825e25..e6eb9b79e2 100644
--- a/src/test/soundproxy_test.cpp
+++ b/src/test/soundproxy_test.cpp
@@ -1,10 +1,9 @@
 #include <QTemporaryFile>
 #include <QtDebug>
 
-#include "test/mixxxtest.h"
-
-#include "sources/soundsourceproxy.h"
 #include "sources/audiosourcestereoproxy.h"
+#include "sources/soundsourceproxy.h"
+#include "test/mixxxtest.h"
 #include "track/trackmetadata.h"
 #include "util/samplebuffer.h"
 
@@ -35,7 +34,7 @@ const CSAMPLE kMaxDecodingError = 0.01f;
 
 } // anonymous namespace
 
-class SoundSourceProxyTest: public MixxxTest {
+class SoundSourceProxyTest : public MixxxTest {
   protected:
     static QStringList getFileNameSuffixes() {
         QStringList availableFileNameSuffixes;
@@ -178,14 +177,14 @@ TEST_F(SoundSourceProxyTest, open) {
 
 TEST_F(SoundSourceProxyTest, openEmptyFile) {
     for (const auto& fileNameSuffix: getFileNameSuffixes()) {
-        QTemporaryFile tempFile("emptyXXXXXX" + fileNameSuffix);
-        qDebug() << "Created testing to open empty file:"
-                << tempFile.fileName();
-        tempFile.open();
-        tempFile.close();
-
-        ASSERT_TRUE(SoundSourceProxy::isFileNameSupported(tempFile.fileName()));
-        auto pTrack = Track::newTemporary(tempFile.fileName());
+        const auto tmpFileName =
+                mixxxtest::createEmptyTemporaryFile("emptyXXXXXX" + fileNameSuffix);
+        const mixxxtest::FileRemover tmpFileRemover(tmpFileName);
+
+        ASSERT_TRUE(QFile::exists(tmpFileName));
+        ASSERT_TRUE(!tmpFileName.isEmpty());
+        ASSERT_TRUE(SoundSourceProxy::isFileNameSupported(tmpFileName));
+        auto pTrack = Track::newTemporary(tmpFileName);
         SoundSourceProxy proxy(pTrack);
 
         auto pAudioSource = proxy.openAudioSource();
diff --git a/src/test/taglibtest.cpp b/src/test/taglibtest.cpp
index ba3464cc4b..8a980aaa2d 100644
--- a/src/test/taglibtest.cpp
+++ b/src/test/taglibtest.cpp
@@ -1,68 +1,27 @@
-#include <gtest/gtest.h>
-
 #include <QDir>
-#include <QFile>
-#include <QTemporaryFile>
 #include <QtDebug>
 
 #include "sources/metadatasourcetaglib.h"
+#include "test/mixxxtest.h"
 
 namespace {
 
-const QDir kTestDir(QDir::current().absoluteFilePath("src/test/id3-test-data"));
+const QDir kTestDir = QDir::current().absoluteFilePath("src/test/id3-test-data");
 
-class TagLibTest : public testing::Test {
-  protected:
-    static QString generateTemporaryFileName(const QString& fileNameTemplate) {
-        QTemporaryFile tmpFile(fileNameTemplate);
-        // The file must be opened to create it and to obtain
-        // its file name!
-        tmpFile.open();
-        const QString tmpFileName = tmpFile.fileName();
-        DEBUG_ASSERT(!tmpFileName.isEmpty());
-        return tmpFileName;
-    }
-
-    static bool copyFile(const QString& srcFileName, const QString& dstFileName) {
-        QFile srcFile(srcFileName);
-        DEBUG_ASSERT(srcFile.exists());
-        if (!srcFile.copy(dstFileName)) {
-            qWarning()
-                    << srcFile.errorString()
-                    << "- Failed to copy file"
-                    << srcFileName
-                    << "->"
-                    << dstFileName;
-            return false;
-        }
-        QFile dstFile(dstFileName);
-        DEBUG_ASSERT(dstFile.exists());
-        DEBUG_ASSERT(srcFile.size() == dstFile.size());
-        return true;
-    }
-};
+} // anonymous namespace
 
-class FileRemover final {
-public:
-    explicit FileRemover(const QString& fileName)
-        : m_fileName(fileName) {
-    }
-    ~FileRemover() {
-        QFile::remove(m_fileName);
-    }
-private:
-    QString m_fileName;
+class TagLibTest : public testing::Test {
 };
 
 TEST_F(TagLibTest, WriteID3v2Tag) {
     // Generate a file name for the temporary file
-    const QString tmpFileName = generateTemporaryFileName("no_id3v1_mp3");
+    const QString tmpFileName = mixxxtest::generateTemporaryFileName("no_id3v1_mp3");
 
     // Create the temporary file by copying an existing file
-    copyFile(kTestDir.absoluteFilePath("empty.mp3"), tmpFileName);
+    mixxxtest::copyFile(kTestDir.absoluteFilePath("empty.mp3"), tmpFileName);
 
     // Ensure that the temporary file is removed after the test
-    FileRemover tmpFileRemover(tmpFileName);
+    mixxxtest::FileRemover tmpFileRemover(tmpFileName);
 
     // Verify that the file has no tags
     {
@@ -80,7 +39,8 @@ TEST_F(TagLibTest, WriteID3v2Tag) {
     trackMetadata.refTrackInfo().setTitle("title");
     const auto exported =
             mixxx::MetadataSourceTagLib(
-                    tmpFileName, mixxx::taglib::FileType::MP3).exportTrackMetadata(trackMetadata);
+                    tmpFileName, mixxx::taglib::FileType::MP3)
+                    .exportTrackMetadata(trackMetadata);
     ASSERT_EQ(mixxx::MetadataSource::ExportResult::Succeeded, exported.first);
     ASSERT_FALSE(exported.second.isNull());
 
@@ -99,7 +59,8 @@ TEST_F(TagLibTest, WriteID3v2Tag) {
     trackMetadata.refTrackInfo().setTitle("title2");
     const auto exported2 =
             mixxx::MetadataSourceTagLib(
-                    tmpFileName, mixxx::taglib::FileType::MP3).exportTrackMetadata(trackMetadata);
+                    tmpFileName, mixxx::taglib::FileType::MP3)
+                    .exportTrackMetadata(trackMetadata);
     ASSERT_EQ(mixxx::MetadataSource::ExportResult::Succeeded, exported.first);
     ASSERT_FALSE(exported.second.isNull());
 
@@ -112,5 +73,3 @@ TEST_F(TagLibTest, WriteID3v2Tag) {
         EXPECT_FALSE(mixxx::taglib::hasAPETag(mpegFile));
     }
 }
-
-}  // anonymous namespace