Fixes and tests.

CMakeLists.txt

@@ -26,11 +26,10 @@ add_subdirectory(src/proxy)
 
 # force plugin_python to build mobase
 add_dependencies(plugin_python mobase)
+set_property(DIRECTORY ${PROJECT_SOURCE_DIR} PROPERTY VS_STARTUP_PROJECT plugin_python)
 
 set(PLUGIN_PYTHON_TESTS ${PLUGIN_PYTHON_TESTS} CACHE BOOL "build tests for plugin_python")
-
 if (PLUGIN_PYTHON_TESTS)
+	enable_testing()
 	add_subdirectory(tests)
 endif()
-
-set_property(DIRECTORY ${PROJECT_SOURCE_DIR} PROPERTY VS_STARTUP_PROJECT plugin_python)

src/proxy/plugin_python_en.ts

@@ -16,58 +16,58 @@ Do you want to try initializing python again (at the risk of another crash)?
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="169"/>
+        <location filename="proxypython.cpp" line="173"/>
         <source>Python Proxy</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="179"/>
+        <location filename="proxypython.cpp" line="183"/>
         <source>Proxy Plugin to allow plugins written in python to be loaded</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="251"/>
+        <location filename="proxypython.cpp" line="255"/>
         <source>ModOrganizer path contains a semicolon</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="253"/>
+        <location filename="proxypython.cpp" line="257"/>
         <source>Python DLL not found</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="255"/>
+        <location filename="proxypython.cpp" line="259"/>
         <source>Invalid Python DLL</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="257"/>
+        <location filename="proxypython.cpp" line="261"/>
         <source>Initializing Python failed</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="259"/>
-        <location filename="proxypython.cpp" line="289"/>
+        <location filename="proxypython.cpp" line="263"/>
+        <location filename="proxypython.cpp" line="293"/>
         <source>invalid problem key %1</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="267"/>
+        <location filename="proxypython.cpp" line="271"/>
         <source>The path to Mod Organizer (%1) contains a semicolon. &lt;br&gt;While this is legal on NTFS drives, many softwares do not handle it correctly.&lt;br&gt;Unfortunately MO depends on libraries that seem to fall into that group.&lt;br&gt;As a result the python plugin cannot be loaded, and the only solution we canoffer is to remove the semicolon or move MO to a path without a semicolon.</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="278"/>
+        <location filename="proxypython.cpp" line="282"/>
         <source>The Python plugin DLL was not found, maybe your antivirus deleted it. Re-installing MO2 might fix the problem.</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="281"/>
+        <location filename="proxypython.cpp" line="285"/>
         <source>The Python plugin DLL is invalid, maybe your antivirus is blocking it. Re-installing MO2 and adding exclusions for it to your AV might fix the problem.</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="proxypython.cpp" line="286"/>
+        <location filename="proxypython.cpp" line="290"/>
         <source>The initialization of the Python plugin DLL failed, unfortunately without any details.</source>
         <translation type="unfinished"></translation>
     </message>

src/proxy/proxypython.cpp

@@ -142,7 +142,11 @@ bool ProxyPython::init(IOrganizer* moInfo)
     m_Runner = std::unique_ptr<IPythonRunner>{createPythonRunner()};
 
     if (m_Runner) {
-        m_Runner->initialize(pluginFolder / "libs");
+        const auto libpath = pluginFolder / "libs";
+        const QStringList paths{
+            QFileInfo(libpath / "pythoncore.zip").absoluteFilePath(),
+            QFileInfo(libpath).absoluteFilePath(), IOrganizer::getPluginDataPath()};
+        m_Runner->initialize(paths);
     }
 
     if (m_MOInfo) {

src/pybind11-qt/pybind11_qt_basic.cpp

@@ -6,6 +6,9 @@
 
 #include "pybind11_qt/details/pybind11_qt_utils.h"
 
+// need to import containers to get QVariantList and QVariantMap
+#include "pybind11_qt/pybind11_qt_containers.h"
+
 namespace pybind11::detail {
 
     template <class CharT>
@@ -83,13 +86,22 @@ namespace pybind11::detail {
 
     bool type_caster<QVariant>::load(handle src, bool implicit)
     {
-        if (PyList_Check(src.ptr())) {
+        // test for string first otherwise PyList_Check also works
+        if (PyBytes_Check(src.ptr()) || PyUnicode_Check(src.ptr())) {
+            value = src.cast<QString>();
+            return true;
+        }
+        else if (PySequence_Check(src.ptr())) {
             // we could check if all the elements can be converted to QString
             // and store a QStringList in the QVariant but I am not sure that is
             // really useful.
             value = src.cast<QVariantList>();
             return true;
         }
+        else if (PyMapping_Check(src.ptr())) {
+            value = src.cast<QVariantMap>();
+            return true;
+        }
         else if (src == Py_None) {
             value = QVariant();
             return true;
@@ -98,10 +110,6 @@ namespace pybind11::detail {
             value = src.cast<QVariantMap>();
             return true;
         }
-        else if (PyBytes_Check(src.ptr()) || PyUnicode_Check(src.ptr())) {
-            value = src.cast<QString>();
-            return true;
-        }
         // PyBool will also return true for PyLong_Check but not the other way
         // around, so the order here is relevant.
         else if (PyBool_Check(src.ptr())) {

src/runner/CMakeLists.txt

@@ -6,10 +6,12 @@ mo2_configure_library(pythonrunner
 	WARNINGS OFF
 	AUTOMOC ON
 	TRANSLATIONS OFF
-	PRIVATE_DEPENDS uibase Qt::Core
+	PUBLIC_DEPENDS uibase Qt::Core
 )
 target_link_libraries(pythonrunner PRIVATE pybind11::embed pybind11::qt)
-target_include_directories(pythonrunner PRIVATE ${PYTHON_ROOT}/Include)
+target_include_directories(pythonrunner
+	PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}
+	PRIVATE ${PYTHON_ROOT}/Include)
 
 # this is kind of broken but it only works with this...
 target_compile_definitions(pythonrunner

src/runner/pythonrunner.cpp

@@ -34,7 +34,7 @@ public:
     QList<QObject*> load(const QString& identifier) override;
     void unload(const QString& identifier) override;
 
-    bool initialize(std::filesystem::path const& libpath) override;
+    bool initialize(QStringList const& paths) override;
     bool isInitialized() const override;
 
 private:
@@ -58,7 +58,7 @@ IPythonRunner* CreatePythonRunner()
     return new PythonRunner();
 }
 
-bool PythonRunner::initialize(std::filesystem::path const& libpath)
+bool PythonRunner::initialize(QStringList const& paths)
 {
     // we only initialize Python once for the whole lifetime of the program, even if MO2
     // is restarted and the proxy or PythonRunner objects are deleted and recreated,
@@ -81,10 +81,9 @@ bool PythonRunner::initialize(std::filesystem::path const& libpath)
 
         // initialize the core Path of Python, this must be done before initialization
         //
-        const QStringList paths{
-            QFileInfo(libpath / "pythoncore.zip").absoluteFilePath(),
-            QFileInfo(libpath).absoluteFilePath(), IOrganizer::getPluginDataPath()};
-        Py_SetPath(paths.join(';').toStdWString().c_str());
+        if (!paths.isEmpty()) {
+            Py_SetPath(paths.join(';').toStdWString().c_str());
+        }
 
         Py_OptimizeFlag = 2;
         Py_NoSiteFlag   = 1;

src/runner/pythonrunner.h

@@ -6,6 +6,7 @@
 
 #include <QObject>
 #include <QString>
+#include <QStringList>
 
 #include <dllimport.h>
 #include <imoinfo.h>
@@ -17,9 +18,11 @@ public:
 
     // initialize Python
     //
-    // libpath should be the folder containing the Python library (pythonxxx.zip, etc.)
+    // paths contains the list of built-in paths for the Python library (pythonxxx.zip,
+    // etc.), an empty list uses the default Python paths (e.g., the PYTHONPATH
+    // environment variable)
     //
-    virtual bool initialize(std::filesystem::path const& libpath) = 0;
+    virtual bool initialize(QStringList const& paths = {}) = 0;
 
     // check if the runner has been initialized, i.e., initialize() has been called and
     // succeeded

tests/python/CMakeLists.txt

@@ -24,7 +24,6 @@ mo2_python_pip_install(pytest
 	PACKAGES pytest PyQt6==6.3.0)
 add_dependencies(pytest mobase)
 
-
 file(GLOB test_files CONFIGURE_DEPENDS "test_*.cpp")
 foreach (test_file ${test_files})
 	get_filename_component(target ${test_file} NAME_WLE)

tests/python/test_qt.cpp

@@ -2,6 +2,8 @@
 
 #include <pybind11/pybind11.h>
 
+#include <tuple>
+
 using namespace pybind11::literals;
 
 PYBIND11_MODULE(qt, m)
@@ -33,6 +35,7 @@ PYBIND11_MODULE(qt, m)
     });
 
     // QMap
+
     m.def("qmap_to_length", [](QMap<QString, QString> const& map) {
         QMap<QString, int> res;
         for (auto it = map.begin(); it != map.end(); ++it) {
@@ -56,4 +59,61 @@ PYBIND11_MODULE(qt, m)
             return datetime.toString(format);
         },
         "datetime"_a, "format"_a = Qt::DateFormat::ISODate);
+
+    // QVariant
+
+    m.def("qvariant_from_none", [](QVariant const& variant) {
+        return std::make_tuple(variant.userType() == QVariant::Invalid,
+                               variant.isValid());
+    });
+    m.def("qvariant_from_int", [](QVariant const& variant) {
+        return std::make_tuple(variant.userType() == QVariant::Int, variant.toInt());
+    });
+    m.def("qvariant_from_bool", [](QVariant const& variant) {
+        return std::make_tuple(variant.userType() == QVariant::Bool, variant.toBool());
+    });
+    m.def("qvariant_from_str", [](QVariant const& variant) {
+        return std::make_tuple(variant.userType() == QVariant::String,
+                               variant.toString());
+    });
+    m.def("qvariant_from_list", [](QVariant const& variant) {
+        return std::make_tuple(variant.userType() == QVariant::List, variant.toList());
+    });
+    m.def("qvariant_from_map", [](QVariant const& variant) {
+        return std::make_tuple(variant.userType() == QVariant::Map, variant.toMap());
+    });
+
+    m.def("qvariant_none", []() {
+        return QVariant();
+    });
+    m.def("qvariant_int", []() {
+        return QVariant(42);
+    });
+    m.def("qvariant_bool", []() {
+        return QVariant(true);
+    });
+    m.def("qvariant_str", []() {
+        return QVariant("hello world");
+    });
+    m.def("qvariant_list", [] {
+        QVariantMap subMap;
+        subMap["bar"] = 42;
+        subMap["moo"] = QVariantList{44, true};
+        QVariantList list;
+        list.push_back(33);
+        list.push_back(QVariantList{4, "foo"});
+        list.push_back(false);
+        list.push_back("hello");
+        list.push_back(QVariant());
+        list.push_back(subMap);
+        list.push_back(45);
+        return QVariant(list);
+    });
+    m.def("qvariant_map", []() {
+        QVariantMap map;
+        map["bar"] = 42;
+        map["moo"] = true;
+        map["baz"] = "world hello";
+        return map;
+    });
 }

tests/python/test_qt.py

@@ -1,6 +1,5 @@
 import pytest
 from PyQt6.QtCore import QDateTime, Qt
-from PyQt6.QtWidgets import QWidget
 
 m = pytest.importorskip("mobase_tests.qt")
 
@@ -48,3 +47,46 @@ def test_qdatetime():
     assert m.datetime_to_string(date, Qt.DateFormat.TextDate) == date.toString(
         Qt.DateFormat.TextDate
     )
+
+
+def test_qvariant():
+
+    # Python -> C++
+
+    assert m.qvariant_from_none(None) == (True, False)
+
+    assert m.qvariant_from_int(-52) == (True, -52)
+    assert m.qvariant_from_int(0) == (True, 0)
+    assert m.qvariant_from_int(33) == (True, 33)
+
+    assert m.qvariant_from_bool(True) == (True, True)
+    assert m.qvariant_from_bool(False) == (True, False)
+
+    assert m.qvariant_from_str("a string") == (True, "a string")
+
+    assert m.qvariant_from_list([]) == (True, [])
+    assert m.qvariant_from_list([1, "hello", False]) == (True, [1, "hello", False])
+
+    assert m.qvariant_from_map({"a": 33, "b": False, "c": ["a", "b"]}) == (
+        True,
+        {"a": 33, "b": False, "c": ["a", "b"]},
+    )
+
+    # C++ -> Python (see .cpp file for the value)
+
+    assert m.qvariant_none() is None
+    assert m.qvariant_int() == 42
+    assert m.qvariant_bool() is True
+    assert m.qvariant_str() == "hello world"
+
+    assert m.qvariant_map() == {"baz": "world hello", "bar": 42, "moo": True}
+
+    assert m.qvariant_list() == [
+        33,
+        [4, "foo"],
+        False,
+        "hello",
+        None,
+        {"bar": 42, "moo": [44, True]},
+        45,
+    ]

tests/runner/CMakeLists.txt

@@ -1,8 +1,42 @@
-cmake_minimum_required(VERSION 3.16)
+cmake_minimum_required(VERSION 3.22)
 
-add_executable(pythonrunner-tests)
-mo2_configure_tests(pythonrunner-tests
-    WARNINGS OFF)
-mo2_add_dependencies(pythonrunner-tests PUBLIC uibase)
+# setting-up the tests for the runner is a bit complex because we need a tons of
+# things
+
+# first we configure the tests as with other tests
+add_executable(pythonrunner-tests EXCLUDE_FROM_ALL)
+mo2_configure_tests(pythonrunner-tests WARNINGS OFF)
+
+# add mocks
 target_include_directories(pythonrunner-tests
     PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../mocks)
+
+# link to pythonrunner - we set PYTHONRUNNER_LIBRARY to not export the symbol but
+# loads it directly, we are going to add the DLL path below
+target_compile_definitions(pythonrunner-tests PUBLIC PYTHONRUNNER_LIBRARY)
+target_link_libraries(pythonrunner-tests PUBLIC pythonrunner)
+
+set(PYLIB_DIR ${CMAKE_CURRENT_BINARY_DIR}/pylibs)
+mo2_python_pip_install(pythonrunner-tests
+	DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/pylibs
+	PACKAGES pytest PyQt6==6.3.0)
+
+add_dependencies(pythonrunner-tests mobase)
+
+# we set multiple properties, including:
+# - updated PATH for DLLs (Qt, etc.), Python and pythonrunner
+# - PYTHONPATH
+
+set(pythoncore "${PYTHON_ROOT}/PCbuild/amd64/pythoncore")
+file(GLOB pythoncorezip "${pythoncore}/python*.zip")
+set(PYTHONPATH "${PYLIB_DIR}\\;$<TARGET_FILE_DIR:mobase>\\;${pythoncore}\\;${pythoncorezip}")
+
+set(extra_paths "${MO2_INSTALL_PATH}/bin/dlls")
+string(APPEND extra_paths "\\;${PYTHON_ROOT}/PCbuild/amd64")
+string(APPEND extra_paths "\\;$<TARGET_FILE_DIR:pythonrunner>")
+set_tests_properties(${pythonrunner-tests_gtests}
+    PROPERTIES
+    WORKING_DIRECTORY "${MO2_INSTALL_PATH}/bin"
+    ENVIRONMENT "PLUGIN_DIR=${CMAKE_CURRENT_SOURCE_DIR}/plugins"
+    ENVIRONMENT_MODIFICATION
+    "PATH=path_list_prepend:${extra_paths};PYTHONPATH=set:${PYTHONPATH}")

tests/runner/plugins/dummy-iplugin.py

@@ -0,0 +1,31 @@
+# -*- encoding: utf-8 -*-
+
+import mobase
+
+
+class DummyPlugin(mobase.IPlugin):
+    def init(self, organizer: mobase.IOrganizer) -> bool:
+        return True
+
+    def author(self) -> str:
+        return "The Author"
+
+    def name(self) -> str:
+        return "The Name"
+
+    def description(self) -> str:
+        return "The Description"
+
+    def version(self) -> mobase.VersionInfo:
+        return mobase.VersionInfo("1.3.0")
+
+    def settings(self) -> list[mobase.PluginSetting]:
+        return [
+            mobase.PluginSetting(
+                "a setting", "the setting description", default_value=12
+            )
+        ]
+
+
+def createPlugin() -> mobase.IPlugin:
+    return DummyPlugin()

tests/runner/test_iplugin.cpp

@@ -0,0 +1,42 @@
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "MockOrganizer.h"
+#include "iplugin.h"
+#include "pythonrunner.h"
+
+#include <QCoreApplication>
+
+using namespace MOBase;
+
+TEST(IPlugin, Basic)
+{
+    const auto plugins_folder = QString(std::getenv("PLUGIN_DIR"));
+
+    std::unique_ptr<IPythonRunner> runner(CreatePythonRunner());
+    runner->initialize();
+
+    // load objects
+    const auto objects = runner->load(plugins_folder + "/dummy-iplugin.py");
+    EXPECT_EQ(objects.size(), 1);
+
+    // load the IPlugin
+    const IPlugin* plugin = qobject_cast<IPlugin*>(objects[0]);
+    EXPECT_NE(plugin, nullptr);
+
+    EXPECT_EQ(plugin->author(), "The Author");
+    EXPECT_EQ(plugin->name(), "The Name");
+    EXPECT_EQ(plugin->version(), VersionInfo(1, 3, 0));
+    EXPECT_EQ(plugin->description(), "The Description");
+
+    // settings
+    const auto settings = plugin->settings();
+    EXPECT_EQ(settings.size(), 1);
+    EXPECT_EQ(settings[0].key, "a setting");
+    EXPECT_EQ(settings[0].description, "the setting description");
+    EXPECT_EQ(settings[0].defaultValue.userType(), QVariant::Type::Int);
+    EXPECT_EQ(settings[0].defaultValue.toInt(), 12);
+
+    // no translation, no custom implementation -> name()
+    EXPECT_EQ(plugin->localizedName(), "The Name");
+}

tests/runner/test_lifetime.cpp

@@ -0,0 +1,47 @@
+#include "gtest/gtest.h"
+
+#include "MockOrganizer.h"
+#include "pythonrunner.h"
+
+#include <QCoreApplication>
+
+TEST(Lifetime, Plugins)
+{
+    const auto plugins_folder = QString(std::getenv("PLUGIN_DIR"));
+
+    std::unique_ptr<IPythonRunner> runner(CreatePythonRunner());
+    runner->initialize();
+
+    {
+        const auto objects = runner->load(plugins_folder + "/dummy-iplugin.py");
+
+        // we found one plugin
+        EXPECT_EQ(objects.size(), 1);
+
+        // check that deleting the object actually destroys it
+        bool destroyed = false;
+        QObject::connect(objects[0], &QObject::destroyed, [&destroyed]() {
+            destroyed = true;
+        });
+        delete objects[0];
+        EXPECT_EQ(destroyed, true);
+    }
+
+    // same things but with a parent
+    {
+        QObject* dummy_parent = new QObject();
+        const auto objects    = runner->load(plugins_folder + "/dummy-iplugin.py");
+
+        // we found one plugin
+        EXPECT_EQ(objects.size(), 1);
+        objects[0]->setParent(dummy_parent);
+
+        // check that deleting the object actually destroys it
+        bool destroyed = false;
+        QObject::connect(objects[0], &QObject::destroyed, [&destroyed]() {
+            destroyed = true;
+        });
+        delete dummy_parent;
+        EXPECT_EQ(destroyed, true);
+    }
+}

tests/runner/test_organizer.cpp

@@ -1,13 +0,0 @@
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
-
-#include "MockOrganizer.h"
-
-using ::testing::Eq;
-using ::testing::NaggyMock;
-using ::testing::Return;
-
-TEST(Organizer, Basic)
-{
-    MockOrganizer mock;
-}