Code cleanup and refactoring

.ci/unit-tests/LifeCycleAssessment_Engine_Tests/CombinationModulesTests.cs

@@ -33,7 +33,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Reflection;
 using System.Text;
 using System.Threading.Tasks;
 
@@ -45,16 +44,102 @@ public class CombinationModulesTests : NUnitTest
         /****   Public Methods                          ****/
         /***************************************************/
 
+        [TestCase(Module.A1, new[] { Module.A1toA3 })]
+        [TestCase(Module.A3, new[] { Module.A1toA3 })]
+        [TestCase(Module.A5_1, new[] { Module.A5 })]
+        [TestCase(Module.A5_3, new[] { Module.A5 })]
+        [TestCase(Module.B1_1, new[] { Module.B1, Module.B1toB3, Module.B1toB5, Module.B1toB7 })]
+        [TestCase(Module.B1, new[] { Module.B1toB3, Module.B1toB5, Module.B1toB7 })]
+        [TestCase(Module.B2, new[] { Module.B1toB3, Module.B1toB5, Module.B1toB7 })]
+        [TestCase(Module.B4_2, new[] { Module.B4, Module.B4toB5, Module.B1toB5, Module.B1toB7 })]
+        [TestCase(Module.B5, new[] { Module.B4toB5, Module.B1toB5, Module.B1toB7 })]
+        [TestCase(Module.B1toB3, new[] { Module.B1toB5, Module.B1toB7 })]
+        [TestCase(Module.B1toB5, new[] { Module.B1toB7 })]
+        [TestCase(Module.B6, new[] { Module.B1toB7 })]
+        [TestCase(Module.B7_3, new[] { Module.B7, Module.B1toB7 })]
+        [TestCase(Module.B8_2, new[] { Module.B8 })]
+        [TestCase(Module.C1, new[] { Module.C1toC4 })]
+        [TestCase(Module.C2, new[] { Module.C1toC4 })]
+        [TestCase(Module.C3, new[] { Module.C3toC4, Module.C1toC4 })]
+        [TestCase(Module.C4, new[] { Module.C3toC4, Module.C1toC4 })]
+        [TestCase(Module.C3toC4, new[] { Module.C1toC4 })]
+        [TestCase(Module.D_1, new[] { Module.D })]
+        [Description("Tests that PartOfCombinationModules returns every combination a module is part of, including combinations that contain it only indirectly, e.g. B1_1 is part of B1, B1toB3, B1toB5 and B1toB7. Expected values are derived by hand from the module hierarchy.")]
+        public void PartOfCombinationModulesIncludesAllParents(Module part, Module[] expectedCombinations)
+        {
+            var partOfCombinations = Query.PartOfCombinationModules();
+
+            partOfCombinations.Should().ContainKey(part);
+            partOfCombinations[part].Should().BeEquivalentTo(expectedCombinations);
+        }
+
+        /***************************************************/
+
+        [TestCase(Module.A4)]
+        [TestCase(Module.A1toA3)]
+        [TestCase(Module.A5)]
+        [TestCase(Module.B1toB7)]
+        [TestCase(Module.B8)]
+        [TestCase(Module.C1toC4)]
+        [TestCase(Module.D)]
+        [Description("Tests that modules that are not part of any combination, either stand-alone modules or the top level combinations, are not included in the PartOfCombinationModules result.")]
+        public void PartOfCombinationModulesExcludesModulesWithoutParents(Module module)
+        {
+            Query.PartOfCombinationModules().Should().NotContainKey(module);
+        }
+
+        /***************************************************/
+
         [Test]
-        [Description("Tests the PartOfCombinationModules query method by displaying all parts of combination modules. This is a visual inspection test with no assertions to verify the method returns the expected module combinations.")]
-        public void TestCombinationModuleParts()
+        [Description("Tests that PartOfCombinationModules is consistent with CombinationModules: for every combination, each of its parts must list that combination as a parent, and the parents of the combination itself must also be parents of the part.")]
+        public void PartOfCombinationModulesConsistentWithCombinationModules()
         {
-            var partOFCombinationMOdules = Query.PartOfCombinationModules();
-            foreach (var item in partOFCombinationMOdules)
+            var combinations = Query.CombinationModules();
+            var partOfCombinations = Query.PartOfCombinationModules();
+
+            Assert.Multiple(() =>
+            {
+                foreach (var combination in combinations)
+                {
+                    foreach (var part in combination.Value.Select(x => x.Item1))
+                    {
+                        Assert.That(partOfCombinations.ContainsKey(part), Is.True, $"{part} should be part of {combination.Key}");
+                        if (!partOfCombinations.ContainsKey(part))
+                            continue;
+
+                        Assert.That(partOfCombinations[part], Does.Contain(combination.Key), $"{part} should list {combination.Key} as a parent");
+
+                        if (partOfCombinations.TryGetValue(combination.Key, out var grandParents))
+                            Assert.That(partOfCombinations[part], Is.SupersetOf(grandParents), $"{part} should also list the parents of {combination.Key}");
+                    }
+                }
+            });
+        }
+
+        /***************************************************/
+
+        [Test]
+        [Description("Tests that the module hierarchy is free of cycles: no module is a part of itself, directly or through other combinations, and no combination lists duplicate parts.")]
+        public void CombinationModulesHaveNoCyclesOrDuplicates()
+        {
+            var combinations = Query.CombinationModules();
+            var partOfCombinations = Query.PartOfCombinationModules();
+
+            Assert.Multiple(() =>
             {
-                Console.WriteLine($"{item.Key}: {string.Join(", ", item.Value)}");
-            }
+                foreach (var combination in combinations)
+                {
+                    List<Module> parts = combination.Value.Select(x => x.Item1).ToList();
+                    Assert.That(parts, Is.Unique, $"{combination.Key} has duplicate parts");
+                    Assert.That(parts, Does.Not.Contain(combination.Key), $"{combination.Key} is listed as a part of itself");
+                }
 
+                foreach (var item in partOfCombinations)
+                {
+                    Assert.That(item.Value, Does.Not.Contain(item.Key), $"{item.Key} is indirectly a part of itself");
+                    Assert.That(item.Value, Is.Unique, $"{item.Key} has duplicate parents");
+                }
+            });
         }
 
         /***************************************************/

.ci/unit-tests/LifeCycleAssessment_Engine_Tests/ConstructionEmssionsTests.cs

@@ -20,9 +20,10 @@
  * along with this code. If not, see <https://www.gnu.org/licenses/lgpl-3.0.html>.      
  */
 
+using BH.Engine.LifeCycleAssessment;
 using BH.oM.LifeCycleAssessment;
 using BH.oM.LifeCycleAssessment.MaterialFragments.Construction;
-using BH.Engine.LifeCycleAssessment;
+using BH.oM.Test.NUnit;
 using NUnit.Framework;
 using System;
 using System.Collections.Generic;
@@ -32,13 +33,14 @@
 
 namespace BH.Tests.Engine.LifeCycleAssessment
 {
-    public class ConstructionEmssionsTests
+    public class ConstructionEmssionsTests : NUnitTest
     {
 
         [TestCaseSource(nameof(AddConstructionWasteEmissionsTestCases))]
         [Description("Tests the AddConstructionWasteEmissions extension method by validating that construction waste emissions (A5_3 module) are correctly calculated based on waste rates and reuse scenarios. Tests both normal cases and edge cases where required modules are missing.")]
         public void TestAddConstructionWasteEmissions(Dictionary<Module, double> resultingValues, ConstructionWasteEmissions constructionEmissions, double assumedA5_3Value)
-        { 
+        {
+            BH.Engine.Base.Compute.ClearCurrentEvents(); //Make sure events raised by previous tests can not satisfy the assertions below
             resultingValues.AddConstructionWasteEmissions(constructionEmissions, MetricType.ClimateChangeTotal);
             if(assumedA5_3Value == 0)
             {
@@ -50,27 +52,76 @@ public void TestAddConstructionWasteEmissions(Dictionary<Module, double> resulti
             Assert.That(resultingValues[Module.A5_3], Is.EqualTo(assumedA5_3Value).Within(1e-9));
         }
 
-        private static IEnumerable<object[]> AddConstructionWasteEmissionsTestCases()
+        private static IEnumerable<TestCaseData> AddConstructionWasteEmissionsTestCases()
         {
             // Normal cases
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = false }, 140  };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = true }, 130 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1, 40 }, { Module.A2, 30 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = false }, 140 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1, 40 }, { Module.A2, 30 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = true }, 130 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1, 100 }, { Module.A2, 20 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 30 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.1 }, ResuedOnSite = false }, 23.3333333333333 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1, 40 }, { Module.A2, 30 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C4, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = true }, 130 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1, 40 }, { Module.A2, 30 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3toC4, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = true }, 130 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C3, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = true }, 130 };
+            yield return Case("A1toA3 with all required modules, not reused on site, C2 included",
+                new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = false }, 140);
+
+            yield return Case("A1toA3 with all required modules, reused on site, C2 excluded",
+                new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = true }, 130);
+
+            yield return Case("Separate A1, A2 and A3, not reused on site, C2 included",
+                new Dictionary<Module, double> { { Module.A1, 40 }, { Module.A2, 30 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = false }, 140);
+
+            yield return Case("Separate A1, A2 and A3, reused on site, C2 excluded",
+                new Dictionary<Module, double> { { Module.A1, 40 }, { Module.A2, 30 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = true }, 130);
+
+            yield return Case("Waste rate of 0.1 gives waste factor of 1/9 of the summed modules",
+                new Dictionary<Module, double> { { Module.A1, 100 }, { Module.A2, 20 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 30 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.1 }, ReusedOnSite = false }, 23.3333333333333);
+
+            yield return Case("C4 used as disposal module in place of C3",
+                new Dictionary<Module, double> { { Module.A1, 40 }, { Module.A2, 30 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C4, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = true }, 130);
+
+            yield return Case("C3toC4 used as disposal module in place of C3",
+                new Dictionary<Module, double> { { Module.A1, 40 }, { Module.A2, 30 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3toC4, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = true }, 130);
+
+            yield return Case("C2 not required when reused on site",
+                new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C3, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = true }, 130);
 
             // Missing modules cases
-            yield return new object[] { new Dictionary<Module, double> { { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = false }, 0 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.C2, 10 }, { Module.C3, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = false }, 0 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C3, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = false }, 0 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C2, 10 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ResuedOnSite = false }, 0 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A2, 20 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 30 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.1 }, ResuedOnSite = false }, 0 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1, 100 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 30 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.1 }, ResuedOnSite = false }, 0 };
-            yield return new object[] { new Dictionary<Module, double> { { Module.A1, 100 }, { Module.A2, 20 },  { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 30 } }, new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.1 }, ResuedOnSite = false }, 0 };
+            yield return Case("Missing A1toA3 gives error and no A5_3",
+                new Dictionary<Module, double> { { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = false }, 0);
 
+            yield return Case("Missing A4 gives error and no A5_3",
+                new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.C2, 10 }, { Module.C3, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = false }, 0);
+
+            yield return Case("Missing C2 when not reused on site gives error and no A5_3",
+                new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C3, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = false }, 0);
+
+            yield return Case("Missing disposal (C3, C4 or C3toC4) gives error and no A5_3",
+                new Dictionary<Module, double> { { Module.A1toA3, 100 }, { Module.A4, 20 }, { Module.C2, 10 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.5 }, ReusedOnSite = false }, 0);
+
+            yield return Case("Missing A1 when A2 and A3 are provided gives error and no A5_3",
+                new Dictionary<Module, double> { { Module.A2, 20 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 30 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.1 }, ReusedOnSite = false }, 0);
+
+            yield return Case("Missing A2 when A1 and A3 are provided gives error and no A5_3",
+                new Dictionary<Module, double> { { Module.A1, 100 }, { Module.A3, 30 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 30 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.1 }, ReusedOnSite = false }, 0);
+
+            yield return Case("Missing A3 when A1 and A2 are provided gives error and no A5_3",
+                new Dictionary<Module, double> { { Module.A1, 100 }, { Module.A2, 20 }, { Module.A4, 20 }, { Module.C2, 10 }, { Module.C3, 30 } },
+                new ConstructionWasteEmissions { WasteRate = new WasteRate { Rate = 0.1 }, ReusedOnSite = false }, 0);
+        }
+
+        /***************************************************/
+
+        private static TestCaseData Case(string name, Dictionary<Module, double> resultingValues, ConstructionWasteEmissions constructionEmissions, double assumedA5_3Value)
+        {
+            return new TestCaseData(resultingValues, constructionEmissions, assumedA5_3Value).SetArgDisplayNames(name);
         }
     }
 }

.ci/unit-tests/LifeCycleAssessment_Engine_Tests/DataSource.cs

@@ -159,7 +159,7 @@ private static WasteAndDisposalFactors DummyDisposalFactors(ref double v, double
         /***************************************************/
 
         [Description("Returns a dummy takeoff and material template to be used when testing evaluation.")]
-        public static IEnumerable<object[]> DummyTakeoffAndTemplates(double initialV, double increace, bool setA5ToWaste)
+        public static IEnumerable<TestCaseData> DummyTakeoffAndTemplates(double initialV, double increace, bool setA5ToWaste)
         {
 
             double v = initialV;
@@ -171,7 +171,7 @@ public static IEnumerable<object[]> DummyTakeoffAndTemplates(double initialV, do
                 MaterialTakeoffItems = templates.Select((x, i) => new TakeoffItem { Material = new Material { Name = x.Name, Density = 1 }, Volume = (i + 1) * 42.543, Mass = (i + 1) * 42.543 * x.Density }).ToList(),
             };
 
-            yield return new object[] { takeoff, templates, true };
+            yield return new TestCaseData(new object[] { takeoff, templates, true }).SetArgDisplayNames("Takeoff and templates with EPDs");
 
             List<Material> newTemplates = new List<Material>();
 
@@ -180,7 +180,7 @@ public static IEnumerable<object[]> DummyTakeoffAndTemplates(double initialV, do
                 newTemplates.Add(new Material { Name = templates[i].Name, Density = templates[i].Density, Properties = new List<IMaterialProperties> { DummyCombinedFactors(ref v, inc, setA5ToWaste, i, templates[i].Name + "EPD NAME", templates[i].Properties.OfType<EnvironmentalProductDeclaration>().FirstOrDefault().QuantityType) } });
             }
 
-            yield return new object[] { takeoff, newTemplates, false };
+            yield return new TestCaseData(new object[] { takeoff, newTemplates, false }).SetArgDisplayNames("Takeoff and templates with CombinedFactors");
         }
 
         /***************************************************/
@@ -268,7 +268,7 @@ private static ConstructionWasteEmissions DummyConstructionsFactor(double v, dou
         {
             return new ConstructionWasteEmissions
             {
-                ResuedOnSite = ((int)Math.Round(v)) % 2 == 0,
+                ReusedOnSite = ((int)Math.Round(v)) % 2 == 0,
                 WasteRate = new WasteRate { Rate = 0.5 },
             };
         }