Parametric google-test for mapping validation

CMakeLists.txt

@@ -2823,6 +2823,10 @@ if(BUILD_TESTING)
   endif()
 
   add_executable(mixxx-test ${src-mixxx-test})
+  target_compile_definitions(
+    mixxx-test
+    PUBLIC RESOURCE_FOLDER="${CMAKE_CURRENT_SOURCE_DIR}/res"
+  )
 
   if(QML)
     target_sources(
@@ -2848,6 +2852,30 @@ if(BUILD_TESTING)
   include(CTest)
   include(GoogleTest)
   enable_testing()
+
+  file(
+    GLOB_RECURSE CONTROLLER_BULK_MAPPING_LIST
+    "${CMAKE_CURRENT_SOURCE_DIR}/res/controllers/*.bulk.xml"
+  )
+  file(
+    GLOB_RECURSE CONTROLLER_HID_MAPPING_LIST
+    "${CMAKE_CURRENT_SOURCE_DIR}/res/controllers/*.hid.xml"
+  )
+  file(
+    GLOB_RECURSE CONTROLLER_MIDI_MAPPING_LIST
+    "${CMAKE_CURRENT_SOURCE_DIR}/res/controllers/*.midi.xml"
+  )
+  list(JOIN CONTROLLER_BULK_MAPPING_LIST "\",\"" CONTROLLER_BULK_MAPPINGS)
+  list(JOIN CONTROLLER_HID_MAPPING_LIST "\",\"" CONTROLLER_HID_MAPPINGS)
+  list(JOIN CONTROLLER_MIDI_MAPPING_LIST "\",\"" CONTROLLER_MIDI_MAPPINGS)
+
+  target_compile_definitions(
+    mixxx-test
+    PUBLIC CONTROLLER_BULK_MAPPINGS="${CONTROLLER_BULK_MAPPINGS}"
+    PUBLIC CONTROLLER_HID_MAPPINGS="${CONTROLLER_HID_MAPPINGS}"
+    PUBLIC CONTROLLER_MIDI_MAPPINGS="${CONTROLLER_MIDI_MAPPINGS}"
+  )
+
   gtest_discover_tests(
     mixxx-test
     EXTRA_ARGS --logLevel info

res/controllers/midi-components-0.0.js

@@ -130,14 +130,18 @@
         disconnect: function() {
             if (this.connections[0] !== undefined) {
                 this.connections.forEach(function(conn) {
-                    conn.disconnect();
+                    if (conn !== undefined) {
+                        conn.disconnect();
+                    }
                 });
             }
         },
         trigger: function() {
             if (this.connections[0] !== undefined) {
                 this.connections.forEach(function(conn) {
-                    conn.trigger();
+                    if (conn !== undefined) {
+                        conn.trigger();
+                    }
                 });
             }
         },

src/controllers/controller.h

@@ -210,7 +210,7 @@ class Controller : public QObject {
     // accesses lots of our stuff, but in the same thread
     friend class ControllerManager;
     // For testing
-    friend class LegacyControllerMappingValidationTest;
+    friend class MappingTestFixture;
     friend class MidiControllerTest;
 };
 

src/test/controller_mapping_validation_test.cpp

@@ -3,6 +3,8 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <QApplication>
+#include <QRegularExpression>
 #include <QUrl>
 
 #include "controllers/defs_controllers.h"
@@ -20,6 +22,10 @@
 #endif
 #include "moc_controller_mapping_validation_test.cpp"
 
+namespace {
+const QRegularExpression kNonWordPattern(QStringLiteral("[^\\w]+"));
+}
+
 FakeMidiControllerJSProxy::FakeMidiControllerJSProxy()
         : ControllerJSProxy(nullptr) {
 }
@@ -115,9 +121,12 @@ FakeController::~FakeController() {
 bool FakeController::isMappable() const {
     if (m_bMidiMapping) {
         return m_pMidiMapping->isMappable();
-    } else if (m_bHidMapping) {
+    }
+#ifdef __HID__
+    else if (m_bHidMapping) {
         return m_pHidMapping->isMappable();
     }
+#endif
     return false;
 }
 
@@ -129,9 +138,7 @@ void deleteTrack(Track* pTrack) {
 };
 #endif
 
-void LegacyControllerMappingValidationTest::SetUp() {
-    m_mappingPath = getTestDir().filePath(QStringLiteral("../../res/controllers/"));
-    m_pEnumerator.reset(new MappingInfoEnumerator(QList<QString>{m_mappingPath.absolutePath()}));
+void MappingTestFixture::SetUp() {
 #ifdef MIXXX_USE_QML
     // This setup mirrors coreservices -- it would be nice if we could use coreservices instead
     // but it does a lot of local disk / settings setup.
@@ -179,27 +186,30 @@ void LegacyControllerMappingValidationTest::SetUp() {
     m_pPlayerManager->bindToLibrary(m_pLibrary.get());
     mixxx::qml::QmlPlayerManagerProxy::registerPlayerManager(m_pPlayerManager);
     ControllerScriptEngineBase::registerTrackCollectionManager(m_pTrackCollectionManager);
+#endif
 }
 
-void LegacyControllerMappingValidationTest::TearDown() {
+void MappingTestFixture::TearDown() {
+#ifdef MIXXX_USE_QML
+    // Clean up in reverse order of initialization
     PlayerInfo::destroy();
     CoverArtCache::destroy();
     mixxx::qml::QmlPlayerManagerProxy::registerPlayerManager(nullptr);
     ControllerScriptEngineBase::registerTrackCollectionManager(nullptr);
 #endif
 }
 
-bool LegacyControllerMappingValidationTest::testLoadMapping(const MappingInfo& mapping) {
+bool MappingTestFixture::testLoadMapping(const QString& mappingPath) {
     std::shared_ptr<LegacyControllerMapping> pMapping =
             LegacyControllerMappingFileHandler::loadMapping(
-                    QFileInfo(mapping.getPath()), m_mappingPath);
+                    QFileInfo(mappingPath), QDir(RESOURCE_FOLDER "/controllers"));
     if (!pMapping) {
         return false;
     }
 
     FakeController controller;
     controller.setMapping(pMapping);
-    bool result = controller.applyMapping(getTestDir().filePath(QStringLiteral("../../res")));
+    bool result = controller.applyMapping(getTestDir().filePath(RESOURCE_FOLDER));
     controller.stopEngine();
     return result;
 }
@@ -239,39 +249,42 @@ bool lintMappingInfo(const MappingInfo& mapping) {
     return result;
 }
 
-TEST_F(LegacyControllerMappingValidationTest, MidiMappingsValid) {
-    foreach (const MappingInfo& mapping,
-            m_pEnumerator->getMappingsByExtension(MIDI_MAPPING_EXTENSION)) {
-        qDebug() << "Validating " << mapping.getPath();
-        std::string errorDescription = "Error while validating " + mapping.getPath().toStdString();
-        EXPECT_TRUE(mapping.isValid()) << errorDescription;
-        EXPECT_TRUE(lintMappingInfo(mapping)) << errorDescription;
-        EXPECT_TRUE(testLoadMapping(mapping)) << errorDescription;
-    }
+std::string PrintMappingName(const ::testing::TestParamInfo<std::string>& info) {
+    auto name = QFileInfo(QString::fromStdString(info.param));
+    return name.fileName().replace(kNonWordPattern, "_").toStdString();
 }
 
-#ifdef __HID__
-TEST_F(LegacyControllerMappingValidationTest, HidMappingsValid) {
-    foreach (const MappingInfo& mapping,
-            m_pEnumerator->getMappingsByExtension(HID_MAPPING_EXTENSION)) {
-        qDebug() << "Validating" << mapping.getPath();
-        std::string errorDescription = "Error while validating " + mapping.getPath().toStdString();
-        EXPECT_TRUE(mapping.isValid()) << errorDescription;
-        EXPECT_TRUE(lintMappingInfo(mapping)) << errorDescription;
-        EXPECT_TRUE(testLoadMapping(mapping)) << errorDescription;
-    }
+TEST_P(MappingTestFixture, ValidateMappingXML) {
+    QString mappingPath = QString::fromStdString(GetParam());
+    qDebug() << "ValidateMappingXML" << mappingPath;
+
+    MappingInfo mapping(mappingPath);
+    EXPECT_TRUE(mapping.isValid()) << "Error while validating XML file " << GetParam();
+    EXPECT_TRUE(lintMappingInfo(mapping)) << "Error while validating XML file " << GetParam();
 }
-#endif
 
-#ifdef __BULK__
-TEST_F(LegacyControllerMappingValidationTest, BulkMappingsValid) {
-    foreach (const MappingInfo& mapping,
-            m_pEnumerator->getMappingsByExtension(BULK_MAPPING_EXTENSION)) {
-        qDebug() << "Validating" << mapping.getPath();
-        std::string errorDescription = "Error while validating " + mapping.getPath().toStdString();
-        EXPECT_TRUE(mapping.isValid()) << errorDescription;
-        EXPECT_TRUE(lintMappingInfo(mapping)) << errorDescription;
-        EXPECT_TRUE(testLoadMapping(mapping)) << errorDescription;
-    }
+TEST_P(MappingTestFixture, LoadMapping) {
+    QString mappingPath = QString::fromStdString(GetParam());
+    qDebug() << "LoadMapping" << mappingPath;
+
+    EXPECT_TRUE(testLoadMapping(mappingPath)) << "Error while loading " << GetParam();
 }
+
+#ifdef __BULK__
+INSTANTIATE_TEST_SUITE_P(BulkMappings,
+        MappingTestFixture,
+        ::testing::Values(CONTROLLER_BULK_MAPPINGS),
+        PrintMappingName);
 #endif
+
+#ifdef __HID__
+INSTANTIATE_TEST_SUITE_P(HidMappings,
+        MappingTestFixture,
+        ::testing::Values(CONTROLLER_HID_MAPPINGS),
+        PrintMappingName);
+#endif
+
+INSTANTIATE_TEST_SUITE_P(MidiMappings,
+        MappingTestFixture,
+        ::testing::Values(CONTROLLER_MIDI_MAPPINGS),
+        PrintMappingName);

src/test/controller_mapping_validation_test.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <QObject>
+#include <QtGlobal>
 
 #include "control/controlindicatortimer.h"
 #include "controllers/controller.h"
@@ -214,16 +215,19 @@ class PlayerManager;
 
 // We can't inherit from LibraryTest because that creates a key_notation control object that is also
 // created by the Library object itself. The duplicated CO creation causes a debug assert.
-class LegacyControllerMappingValidationTest : public MixxxDbTest, SoundSourceProviderRegistration {
+class MappingTestFixture
+        : public MixxxDbTest,
+          SoundSourceProviderRegistration,
+          public ::testing::WithParamInterface<std::string> {
   public:
-    LegacyControllerMappingValidationTest()
+    MappingTestFixture()
             : MixxxDbTest(true) {
     }
 
   protected:
     void SetUp() override;
-#ifdef MIXXX_USE_QML
     void TearDown() override;
+#ifdef MIXXX_USE_QML
 
     TrackPointer getOrAddTrackByLocation(
             const QString& trackLocation) const {
@@ -241,8 +245,5 @@ class LegacyControllerMappingValidationTest : public MixxxDbTest, SoundSourcePro
     std::shared_ptr<Library> m_pLibrary;
 #endif
 
-    bool testLoadMapping(const MappingInfo& mapping);
-
-    QDir m_mappingPath;
-    QScopedPointer<MappingInfoEnumerator> m_pEnumerator;
+    bool testLoadMapping(const QString& mapping);
 };