feat: Node test fixtures

src/nodes/edge.cpp

@@ -3,6 +3,7 @@
 
 #include "nodes/edge.h"
 #include "nodes/graph.h"
+#include "nodes/inputs.h"
 #include "nodes/loopBack.h"
 #include "nodes/outputs.h"
 
@@ -31,18 +32,49 @@ class EdgeConstructor : public Edge
 // Create an edge from the supplied definition
 std::unique_ptr<Edge> Edge::create(Graph *parent, const EdgeDefinition &definition)
 {
+    // Get target node
+    auto targetNode = parent->findNode(definition.targetNode);
+    if (!targetNode)
+    {
+        Messenger::error("Target node '{}' does not exist in the graph.\n", definition.targetNode);
+        return {};
+    }
+
+    // Disallow circular edges (mostly a check for Graph -> Graph connections)
+    if (targetNode == parent)
+    {
+        Messenger::error("Target node is graph '{}' and cannot be the owner of the edge.", definition.targetNode);
+        return {};
+    }
+
     // Get source node and output
     auto sourceNode = parent->findNode(definition.sourceNode);
     if (!sourceNode)
     {
         Messenger::error("Source node '{}' does not exist in the graph.\n", definition.sourceNode);
         return {};
     }
+
     auto sourceOutput = sourceNode->findOutput(definition.sourceOutput);
     if (!sourceOutput)
     {
-        Messenger::error("Source node '{}' has no output parameter '{}'.\n", definition.sourceNode, definition.sourceOutput);
-        return {};
+        // If the source node is a Graph's own Inputs node, we will create an edge on the fly - else, throw an error
+        if (!dynamic_cast<InputsNode *>(sourceNode))
+        {
+            Messenger::error("Source node '{}' has no output parameter '{}'.\n", definition.sourceNode,
+                             definition.sourceOutput);
+            return {};
+        }
+
+        // The target node is the parent Graph's own Inputs node, so create a parameter link from the mapped input to the
+        // targetInput
+        auto link = targetNode->findInput(definition.targetInput)->createParameterLink(definition.sourceOutput);
+        if (!parent->addProxyInput(link.inputParameter, link.outputParameter))
+        {
+            Messenger::error("Failed to add mapped input '{}'.\n", definition.targetInput);
+            return {};
+        }
+        sourceOutput = parent->proxyInputs().findOutput(definition.sourceOutput);
     }
 
     // Confirm that the source is actually an output
@@ -53,35 +85,26 @@ std::unique_ptr<Edge> Edge::create(Graph *parent, const EdgeDefinition &definiti
         return {};
     }
 
-    // Get target node and input
-    auto targetNode = parent->findNode(definition.targetNode);
-    if (!targetNode)
-    {
-        Messenger::error("Target node '{}' does not exist in the graph.\n", definition.targetNode);
-        return {};
-    }
-
-    // Disallow circular edges (mostly a check for Graph -> Graph connections)
-    if (targetNode == parent)
-    {
-        Messenger::error("Target node is graph '{}' and cannot be the owner of the edge.", definition.targetNode);
-        return {};
-    }
-
     // We need to check carefully the target node, since we need to permit outside connections to the Graph object itself as
     // well as its Outputs node explicitly.
     std::shared_ptr<ParameterBase> targetInput{nullptr};
     if (dynamic_cast<Graph *>(targetNode))
     {
         // The target node is a Graph: create a parameter link from the sourceOutput and from it a mapped input
         auto graphNode = dynamic_cast<Graph *>(targetNode);
-        auto link = sourceOutput->createParameterLink(definition.targetInput);
-        if (!graphNode->addProxyInput(link.inputParameter, link.outputParameter))
+        auto existingTargetInput = graphNode->findInput(definition.targetInput);
+        if (!existingTargetInput.get())
         {
-            Messenger::error("Failed to add mapped input '{}'.\n", definition.targetInput);
-            return {};
+            auto link = sourceOutput->createParameterLink(definition.targetInput);
+            if (!graphNode->addProxyInput(link.inputParameter, link.outputParameter))
+            {
+                Messenger::error("Failed to add mapped input '{}'.\n", definition.targetInput);
+                return {};
+            }
+            targetInput = link.inputParameter;
         }
-        targetInput = link.inputParameter;
+        else
+            targetInput = existingTargetInput;
     }
     else if (dynamic_cast<OutputsNode *>(targetNode))
     {
@@ -288,3 +311,14 @@ void Edge::deserialise(const SerialisedValue &node)
     throw std::runtime_error("Cannot directly deserialise edges.  Please contact the Dissolve development team if you are "
                              "seeing this error - this is a bug and NOT your fault.\n");
 }
+
+// Express as a serialisable value
+void LoopEdge::serialise(std::string tag, SerialisedValue &target) const
+{
+    definition().serialise(tag, target);
+    target[tag]["targetNode"] = "LoopBacks";
+    target[tag]["analogue"] = analogue_;
+}
+
+// Read values from a serialisable value
+void LoopEdge::deserialise(const SerialisedValue &node) { Edge::deserialise(node); }

src/nodes/edge.h

@@ -115,4 +115,13 @@ class LoopEdge : public Edge
      *
      */
     ParameterBase *analogue_;
+
+    /*
+     * Serialisation
+     */
+    public:
+    // Express as a serialisable value
+    void serialise(std::string tag, SerialisedValue &target) const override;
+    // Read values from a serialisable value
+    void deserialise(const SerialisedValue &node) override;
 };

src/nodes/graph.cpp

@@ -283,7 +283,7 @@ void Graph::deserialise(const SerialisedValue &node)
 }
 
 /*
- *Mermaid processing code
+ * Mermaid processing code
  */
 
 // Node types that represent data sources

src/nodes/iterableGraph.cpp

@@ -85,6 +85,12 @@ LoopEdge *IterableGraph::findLoopEdge(const EdgeDefinition &definition) const
     return {};
 }
 
+// Add edge between nodes
+bool IterableGraph::addLoopEdge(std::unique_ptr<Edge> edge, std::string_view source)
+{
+    return addOutputLoopEdge(source, loopEdges_.emplace_back(LoopEdge::makeLoopEdge(edge.release(), proxyInputs())).get());
+}
+
 // Add edge to node map
 Edge *IterableGraph::addOutputLoopEdge(std::string_view sourceOutput, Edge *edge)
 {
@@ -128,23 +134,26 @@ Edge *IterableGraph::removeOutputLoopEdge(std::string_view sourceOutput, Edge *e
 // Add edge between nodes
 bool IterableGraph::addEdge(const EdgeDefinition &definition)
 {
-    if (dynamic_cast<InputsNode *>(parentGraph()->findNode(definition.sourceNode)))
-        setLoopBacks();
-    else if (loopBacks_->findInput(definition.targetInput))
-    {
-        auto edge =
-            Edge::create(this, {definition.sourceNode, definition.sourceOutput, definition.targetNode, definition.targetInput});
-        if (!edge)
-            return false;
-
-        loopEdges_.emplace_back(LoopEdge::makeLoopEdge(edge.release(), proxyInputs()));
-
-        addOutputLoopEdge(definition.sourceOutput, loopEdges_.back().get());
-
-        return true;
-    }
-
-    return Graph::addEdge(definition);
+    // Refresh the graph loopbacks
+    setLoopBacks();
+
+    // Check if the connection is invertible.
+    // Invertibility is satisfied when the source node (internal to the graph) can output to an existing loopback,
+    // which discounts any edge for which no loopbacks correspond to the target input, as well as the graphs own InputsNode.
+    auto nonInvertible = dynamic_cast<InputsNode *>(parentGraph()->findNode(definition.sourceNode)) ||
+                         !loopBacks_->findInput(definition.targetInput);
+
+    // If not invertible, create and return a standard edge
+    if (nonInvertible)
+        return Graph::addEdge(definition);
+
+    // Create loop edge
+    auto edge =
+        Edge::create(this, {definition.sourceNode, definition.sourceOutput, definition.targetNode, definition.targetInput});
+    if (!edge)
+        return false;
+
+    return addLoopEdge(std::move(edge), definition.sourceOutput);
 }
 
 // Remove edge between nodes
@@ -183,3 +192,32 @@ NodeConstants::ProcessResult IterableGraph::process()
 
     return NodeConstants::ProcessResult::Success;
 }
+
+/*
+ * Serialisation
+ */
+
+// Express as a serialisable value
+void IterableGraph::serialise(std::string tag, SerialisedValue &target) const
+{
+    Graph::serialise(tag, target);
+    auto &result = target[tag];
+    fromVector(loopEdges_, "loopEdges", result);
+}
+
+// Read values from a serialisable value
+void IterableGraph::deserialise(const SerialisedValue &node)
+{
+    Graph::deserialise(node);
+    toVector(node, "loopEdges",
+             [this](const auto &value)
+             {
+                 auto definition = toml::get<EdgeDefinition>(value);
+                 auto edge = Edge::create(
+                     this, {definition.sourceNode, definition.sourceOutput, definition.targetNode, definition.targetInput});
+                 if (!edge)
+                     return false;
+
+                 return addLoopEdge(std::move(edge), definition.sourceOutput);
+             });
+}

src/nodes/iterableGraph.h

@@ -46,6 +46,8 @@ class IterableGraph : public Graph
     void releaseLoopBack(const std::string &name);
 
     private:
+    // Add edge between nodes
+    bool addLoopEdge(std::unique_ptr<Edge> edge, std::string_view source);
     // Add edge to node map
     Edge *addOutputLoopEdge(std::string_view sourceOutput, Edge *edge);
     // Remove edge from node map
@@ -80,4 +82,13 @@ class IterableGraph : public Graph
     protected:
     // Perform processing
     NodeConstants::ProcessResult process() override;
+
+    /*
+     * Serialisation
+     */
+    public:
+    // Express as a serialisable value
+    void serialise(std::string tag, SerialisedValue &target) const override;
+    // Read values from a serialisable value
+    void deserialise(const SerialisedValue &node) override;
 };

src/nodes/node.cpp

@@ -356,15 +356,29 @@ void Node::deserialise(const SerialisedValue &node)
           [this](const auto &k, const auto &v)
           {
               if (inputs_.contains(k))
-                  inputs_[k]->deserialise(v);
+                  try
+                  {
+                      inputs_[k]->deserialise(v);
+                  }
+                  catch (std::exception &ex)
+                  {
+                      Messenger::exception("Error reading input {} in node {} ({}).", k, name(), ex.what());
+                  }
               else
                   Messenger::exception("Node {} does not contain a parameter {}", name(), k);
           });
     toMap(node, "options",
           [this](const auto &k, const auto &v)
           {
               if (options_.contains(k))
-                  options_[k]->deserialise(v);
+                  try
+                  {
+                      options_[k]->deserialise(v);
+                  }
+                  catch (std::exception &ex)
+                  {
+                      Messenger::exception("Error reading option {} in node {} ({}).", k, name(), ex.what());
+                  }
               else
                   Messenger::exception("Node {} does not contain an option {}", name(), k);
           });

src/nodes/parameter.h

@@ -606,23 +606,23 @@ template <typename DataClass> class SerialisableParameter : public Parameter<Dat
         else if constexpr (HasEnumOptions<DataClass>)
         {
             DataClass proxy; // Fake T value to get the correct overload
-            Parameter<DataClass>::data_ = getEnumOptions(proxy).deserialise(node);
+            Parameter<DataClass>::data_ = getEnumOptions(proxy).enumeration(toml::find<std::string>(node, "data"));
         }
-        else if constexpr (std::is_convertible<DataClass, std::optional<double>>::value)
+        else if constexpr (std::is_same_v<DataClass, std::optional<double>>)
         {
             if (node.contains("data"))
                 Parameter<DataClass>::data_ = toml::find<double>(node, "data");
             else
                 Parameter<DataClass>::data_ = {};
         }
-        else if constexpr (std::is_convertible<DataClass, std::optional<Number>>::value)
+        else if constexpr (std::is_same_v<DataClass, std::optional<Number>>)
         {
             if (node.contains("data"))
                 Parameter<DataClass>::data_ = toml::find<Number>(node, "data");
             else
                 Parameter<DataClass>::data_ = {};
         }
-        else if constexpr (std::is_convertible<DataClass, std::optional<Data1D>>::value)
+        else if constexpr (std::is_same_v<DataClass, std::optional<Data1D>>)
         {
             if (node.contains("data"))
                 Parameter<DataClass>::data_ = toml::find<Data1D>(node, "data");

tests/CMakeLists.txt

@@ -62,7 +62,7 @@ function(dissolve_add_test)
 
 endfunction()
 
-add_library(testing testing.cpp testGraph.cpp testing.h testGraph.h)
+add_library(testing testing.cpp testGraph.cpp testing.h testGraph.h testGraphFixture.h)
 target_link_libraries(testing PRIVATE GTest::gtest_main)
 target_include_directories(
   testing PRIVATE ${PROJECT_SOURCE_DIR}/src ${PROJECT_BINARY_DIR}/src ${PROJECT_SOURCE_DIR} ${CONAN_INCLUDE_DIRS_GTEST}