Handle duplicate nodes key checks in MiniYaml in a better place.

Moving the key duplication check allows a redundant check on top-level nodes to be avoided. Add tests to ensure key checks are functioning as expected.

OpenRA.Game/MiniYaml.cs

@@ -416,6 +416,9 @@ namespace OpenRA
 
 		static MiniYaml MergePartial(MiniYaml existingNodes, MiniYaml overrideNodes)
 		{
+			existingNodes?.Nodes.ToDictionaryWithConflictLog(x => x.Key, "MiniYaml.Merge", null, x => $"{x.Key} (at {x.Location})");
+			overrideNodes?.Nodes.ToDictionaryWithConflictLog(x => x.Key, "MiniYaml.Merge", null, x => $"{x.Key} (at {x.Location})");
+
 			if (existingNodes == null)
 				return overrideNodes;
 
@@ -435,9 +438,6 @@ namespace OpenRA
 
 			var ret = new List<MiniYamlNode>(existingNodes.Count + overrideNodes.Count);
 
-			var existingDict = existingNodes.ToDictionaryWithConflictLog(x => x.Key, "MiniYaml.Merge", null, x => $"{x.Key} (at {x.Location})");
-			var overrideDict = overrideNodes.ToDictionaryWithConflictLog(x => x.Key, "MiniYaml.Merge", null, x => $"{x.Key} (at {x.Location})");
-
 			foreach (var node in existingNodes.Concat(overrideNodes))
 			{
 				// Append Removal nodes to the result.

OpenRA.Test/OpenRA.Game/MiniYamlTest.cs

@@ -18,7 +18,10 @@ namespace OpenRA.Test
 	[TestFixture]
 	public class MiniYamlTest
 	{
-		readonly string yamlTabStyle = @"
+		[TestCase(TestName = "Mixed tabs & spaces indents")]
+		public void TestIndents()
+		{
+			var yamlTabStyle = @"
 Root1:
 	Child1:
 		Attribute1: Test
@@ -31,7 +34,7 @@ Root2:
 		Attribute1: Test
 ";
 
-		readonly string yamlMixedStyle = @"
+			var yamlMixedStyle = @"
 Root1:
     Child1:
         Attribute1: Test
@@ -43,10 +46,6 @@ Root2:
     Child1:
 		Attribute1: Test
 ";
-
-		[TestCase(TestName = "Mixed tabs & spaces indents")]
-		public void TestIndents()
-		{
 			var tabs = MiniYaml.FromString(yamlTabStyle, "yamlTabStyle").WriteToString();
 			Console.WriteLine(tabs);
 			var mixed = MiniYaml.FromString(yamlMixedStyle, "yamlMixedStyle").WriteToString();
@@ -300,6 +299,142 @@ Test:
 			Assert.That(mergeNode.Nodes[0].Value.Value, Is.EqualTo("override"), "Merge node Child value should be 'override', but is not");
 		}
 
+		[TestCase(TestName = "Duplicated nodes across multiple sources are correctly merged")]
+		public void TestSelfMergingMultiSource()
+		{
+			var firstYaml = @"
+Test:
+	Merge: original
+		Child: original
+	Original:
+";
+			var secondYaml = @"
+Test:
+	Merge: original
+		Child: original
+	Original:
+Test:
+	Merge: override
+		Child: override
+	Override:
+";
+
+			var result = MiniYaml.Merge(new[] { firstYaml, secondYaml }.Select(s => MiniYaml.FromString(s, "")));
+			Assert.That(result.Count(n => n.Key == "Test"), Is.EqualTo(1), "Result should have exactly one Test node.");
+
+			var testNodes = result.First(n => n.Key == "Test").Value.Nodes;
+			Assert.That(testNodes.Select(n => n.Key), Is.EqualTo(new[] { "Merge", "Original", "Override" }), "Merged Test node has incorrect child nodes.");
+
+			var mergeNode = testNodes.First(n => n.Key == "Merge").Value;
+			Assert.That(mergeNode.Value, Is.EqualTo("override"), "Merge node has incorrect value.");
+			Assert.That(mergeNode.Nodes[0].Value.Value, Is.EqualTo("override"), "Merge node Child value should be 'override', but is not");
+		}
+
+		[TestCase(TestName = "Duplicated child nodes do not throw if parent does not require merging")]
+		public void TestMergeConflictsNoMerge()
+		{
+			var baseYaml = @"
+Test:
+	Merge:
+		Child:
+		Child:
+";
+
+			var result = MiniYaml.Merge(new[] { baseYaml }.Select(s => MiniYaml.FromString(s, "")));
+			var testNodes = result.First(n => n.Key == "Test").Value.Nodes;
+			var mergeNode = testNodes.First(n => n.Key == "Merge").Value;
+			Assert.That(mergeNode.Nodes.Count, Is.EqualTo(2));
+		}
+
+		[TestCase(TestName = "Duplicated child nodes throw merge error if first parent requires merging")]
+		public void TestMergeConflictsFirstParent()
+		{
+			var baseYaml = @"
+Test:
+	Merge:
+		Child1:
+		Child1:
+	Merge:
+";
+
+			void Merge() => MiniYaml.Merge(new[] { baseYaml }.Select(s => MiniYaml.FromString(s, "test-filename")));
+			Assert.That(Merge, Throws.Exception.TypeOf<ArgumentException>().And.Message.EqualTo(
+				"MiniYaml.Merge, duplicate values found for the following keys: Child1: [Child1 (at test-filename:4),Child1 (at test-filename:5)]"));
+		}
+
+		[TestCase(TestName = "Duplicated child nodes throw merge error if second parent requires merging")]
+		public void TestMergeConflictsSecondParent()
+		{
+			var baseYaml = @"
+Test:
+	Merge:
+	Merge:
+		Child2:
+		Child2:
+";
+			void Merge() => MiniYaml.Merge(new[] { baseYaml }.Select(s => MiniYaml.FromString(s, "test-filename")));
+			Assert.That(Merge, Throws.Exception.TypeOf<ArgumentException>().And.Message.EqualTo(
+				"MiniYaml.Merge, duplicate values found for the following keys: Child2: [Child2 (at test-filename:5),Child2 (at test-filename:6)]"));
+		}
+
+		[TestCase(TestName = "Duplicated child nodes across multiple sources do not throw")]
+		public void TestMergeConflictsMultiSourceMerge()
+		{
+			var firstYaml = @"
+Test:
+	Merge:
+		Child:
+";
+			var secondYaml = @"
+Test:
+	Merge:
+		Child:
+";
+
+			var result = MiniYaml.Merge(new[] { firstYaml, secondYaml }.Select(s => MiniYaml.FromString(s, "")));
+			var testNodes = result.First(n => n.Key == "Test").Value.Nodes;
+			var mergeNode = testNodes.First(n => n.Key == "Merge").Value;
+			Assert.That(mergeNode.Nodes.Count, Is.EqualTo(1));
+		}
+
+		[TestCase(TestName = "Duplicated child nodes across multiple sources throw merge error if first parent requires merging")]
+		public void TestMergeConflictsMultiSourceFirstParent()
+		{
+			var firstYaml = @"
+Test:
+	Merge:
+		Child1:
+		Child1:
+";
+			var secondYaml = @"
+Test:
+	Merge:
+";
+
+			void Merge() => MiniYaml.Merge(new[] { firstYaml, secondYaml }.Select(s => MiniYaml.FromString(s, "test-filename")));
+			Assert.That(Merge, Throws.Exception.TypeOf<ArgumentException>().And.Message.EqualTo(
+				"MiniYaml.Merge, duplicate values found for the following keys: Child1: [Child1 (at test-filename:4),Child1 (at test-filename:5)]"));
+		}
+
+		[TestCase(TestName = "Duplicated child nodes across multiple sources throw merge error if second parent requires merging")]
+		public void TestMergeConflictsMultiSourceSecondParent()
+		{
+			var firstYaml = @"
+Test:
+	Merge:
+";
+			var secondYaml = @"
+Test:
+	Merge:
+		Child2:
+		Child2:
+";
+
+			void Merge() => MiniYaml.Merge(new[] { firstYaml, secondYaml }.Select(s => MiniYaml.FromString(s, "test-filename")));
+			Assert.That(Merge, Throws.Exception.TypeOf<ArgumentException>().And.Message.EqualTo(
+				"MiniYaml.Merge, duplicate values found for the following keys: Child2: [Child2 (at test-filename:4),Child2 (at test-filename:5)]"));
+		}
+
 		[TestCase(TestName = "Comments are correctly separated from values")]
 		public void TestEscapedHashInValues()
 		{