diff --git a/FineCodeCoverageTests/CoverageProject_Settings_Tests.cs b/FineCodeCoverageTests/CoverageProject_Settings_Tests.cs index bf32a7b5..5335b8ac 100644 --- a/FineCodeCoverageTests/CoverageProject_Settings_Tests.cs +++ b/FineCodeCoverageTests/CoverageProject_Settings_Tests.cs @@ -5,6 +5,7 @@ using FineCodeCoverageTests.Test_helpers; using Moq; using NUnit.Framework; +using StructureMap.AutoMocking; using System; using System.Collections.Generic; using System.IO; @@ -142,13 +143,21 @@ public async Task Should_Return_Using_VsBuild_When_No_Labelled_PropertyGroup(boo public class SettingsMerger_Tests { + private AutoMoqer mocker; + private SettingsMerger settingsMerger; + + [SetUp] + public void SetUp() + { + mocker = new AutoMoqer(); + settingsMerger = mocker.Create(); + } [Test] public void Should_Use_Global_Settings_If_No_Project_Level_Or_FCC_Settings_Files() { var mockAppOptions = new Mock(MockBehavior.Strict); var appOptions = mockAppOptions.Object; - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge(appOptions, new List(), null); Assert.AreSame(appOptions, mergedSettings); @@ -161,7 +170,6 @@ public void Should_Overwrite_GlobalOptions_Bool_Properties_From_Settings_File() mockAppOptions.SetupSet(o => o.IncludeReferencedProjects = true); var appOptions = mockAppOptions.Object; - var settingsMerger = new SettingsMerger(null); var settingsFileElement = CreateIncludeReferencedProjectsElement(true); var mergedSettings = settingsMerger.Merge(appOptions, new List { settingsFileElement}, null); @@ -176,7 +184,6 @@ public void Should_Overwrite_GlobalOptions_Bool_Properties_From_Settings_File_In mockAppOptions.SetupAllProperties(); var appOptions = mockAppOptions.Object; - var settingsMerger = new SettingsMerger(null); var settingsFileElementTop = CreateIncludeReferencedProjectsElement(!last); var settingsFileElementLast = CreateIncludeReferencedProjectsElement(last); var mergedSettings = settingsMerger.Merge( @@ -196,7 +203,6 @@ public void Should_Overwrite_GlobalOptions_Bool_Properties_From_Project(bool las mockAppOptions.SetupAllProperties(); var appOptions = mockAppOptions.Object; - var settingsMerger = new SettingsMerger(null); var settingsFileElement = CreateIncludeReferencedProjectsElement(!last); var projectElement = CreateIncludeReferencedProjectsElement(last); var mergedSettings = settingsMerger.Merge( @@ -221,7 +227,6 @@ public void Should_Overwrite_Int_Properties() "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List {}, @@ -244,7 +249,6 @@ public void Should_Overwrite_Enum_Properties() "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List { }, @@ -267,7 +271,6 @@ public void Should_Overwrite_String_Properties() "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List { }, @@ -293,7 +296,6 @@ public void Should_Overwrite_String_Array_By_Default() "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List { }, @@ -319,7 +321,6 @@ public void Should_Overwrite_String_Array_DefaultMerge_False() "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List { }, @@ -345,7 +346,6 @@ public void Should_Overwrite_String_Array_DefaultMerge_True_Property_Merge_false "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List { }, @@ -371,7 +371,6 @@ public void Should_Overwrite_String_Array_DefaultMerge_Not_Bool() "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List { }, @@ -397,7 +396,6 @@ public void Should_Merge_String_Array_If_DefaultMerge() "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List { }, @@ -423,7 +421,6 @@ public void Should_Merge_If_Property_Element_Merge() "); - var settingsMerger = new SettingsMerger(null); var mergedSettings = settingsMerger.Merge( appOptions, new List { }, @@ -433,6 +430,54 @@ public void Should_Merge_If_Property_Element_Merge() Assert.AreEqual(new string[] { "global", "1", "2" }, appOptions.Exclude); } + [Test] + public void Should_Log_Failed_To_Get_Setting_From_Project_Settings_Exception_And_Not_Throw() + { + var mockAppOptions = new Mock(); + mockAppOptions.SetupAllProperties(); + var appOptions = mockAppOptions.Object; + var element = XElement.Parse($@" + + + DefaultX + + +"); + + var mergedSettings = settingsMerger.Merge( + appOptions, + new List { }, + element); + + var mockLogger = mocker.GetMock(); + mockLogger.Verify(logger => logger.Log("Failed to get 'OpenCoverRegister' setting from project settings", It.IsAny())); + Assert.AreEqual(mergedSettings.OpenCoverRegister, OpenCoverRegister.Default); + } + + [Test] + public void Should_Log_Failed_To_Get_Setting_From_Settings_File_Exception_And_Not_Throw() + { + var mockAppOptions = new Mock(); + mockAppOptions.SetupAllProperties(); + var appOptions = mockAppOptions.Object; + var element = XElement.Parse($@" + + + DefaultX + + +"); + + var mergedSettings = settingsMerger.Merge( + appOptions, + new List { element}, + null); + + var mockLogger = mocker.GetMock(); + mockLogger.Verify(logger => logger.Log("Failed to get 'OpenCoverRegister' setting from settings file", It.IsAny())); + Assert.AreEqual(mergedSettings.OpenCoverRegister, OpenCoverRegister.Default); + } + [Test] public void Should_Not_Throw_If_Merge_Current_Null_String_Array_Type() { @@ -460,7 +505,7 @@ public void Should_Not_Throw_If_Merge_Current_Null_String_Array_Type() } [TestCaseSource(nameof(XmlConversionCases))] - public void Should_Convert_Xml_Value_Correctly(string propertyElement,string propertyName,object expectedConversion, bool expectedException) + public void Should_Convert_Xml_Value_Correctly(string propertyElement,string propertyName,object expectedConversion) { var settingsMerger = new SettingsMerger(new Mock().Object); var settingsElement = XElement.Parse($"{propertyElement}"); @@ -474,10 +519,10 @@ public void Should_Convert_Xml_Value_Correctly(string propertyElement,string pro [Test] public void Should_Throw_For_Unsupported_Conversion() { - var settingsMerger = new SettingsMerger(new Mock().Object); var settingsElement = XElement.Parse($""); var unsupported = typeof(PropertyInfo).GetProperty(nameof(PropertyInfo.PropertyType)); - var expectedMessage = $"Cannot handle 'PropertyType' yet"; + + var expectedMessage = $"Unexpected settings type Type for setting PropertyType in settings merger GetValueFromXml"; Assert.Throws(() => settingsMerger.GetValueFromXml(settingsElement, unsupported), expectedMessage); } @@ -503,31 +548,31 @@ string CreateElement(string elementName, string value) var cases = new object[] { // boolean - new object[]{ CreateElement(hideFullyCovered, "true"),hideFullyCovered,true, false }, - new object[]{ CreateElement(hideFullyCovered, "false"), hideFullyCovered, false, false }, - new object[]{ CreateElement(hideFullyCovered, "bad"), hideFullyCovered, null, false }, - new object[]{ CreateElement(hideFullyCovered, ""), hideFullyCovered, null, false }, - new object[]{ CreateElement(hideFullyCovered, boolArray), hideFullyCovered, true, false }, + new object[]{ CreateElement(hideFullyCovered, "true"),hideFullyCovered,true }, + new object[]{ CreateElement(hideFullyCovered, "false"), hideFullyCovered, false }, + new object[]{ CreateElement(hideFullyCovered, "bad"), hideFullyCovered, null }, + new object[]{ CreateElement(hideFullyCovered, ""), hideFullyCovered, null }, + new object[]{ CreateElement(hideFullyCovered, boolArray), hideFullyCovered, true }, // int - new object[]{ CreateElement(thresholdForCrapScore, "1"), thresholdForCrapScore, 1, false }, - new object[]{ CreateElement(thresholdForCrapScore, "bad"), thresholdForCrapScore, null, false }, - new object[]{ CreateElement(thresholdForCrapScore, ""), thresholdForCrapScore, null, false }, + new object[]{ CreateElement(thresholdForCrapScore, "1"), thresholdForCrapScore, 1 }, + new object[]{ CreateElement(thresholdForCrapScore, "bad"), thresholdForCrapScore, null }, + new object[]{ CreateElement(thresholdForCrapScore, ""), thresholdForCrapScore, null }, // string - new object[]{ CreateElement(coverletConsoleCustomPath, "1"), coverletConsoleCustomPath, "1", false }, + new object[]{ CreateElement(coverletConsoleCustomPath, "1"), coverletConsoleCustomPath, "1" }, // breaking change ( previous ignored ) - new object[]{ CreateElement(coverletConsoleCustomPath, ""), coverletConsoleCustomPath, "", false }, + new object[]{ CreateElement(coverletConsoleCustomPath, ""), coverletConsoleCustomPath, "" }, // string[] - new object[]{ CreateElement(exclude, stringArray), exclude, new string[] { "1","2"}, false }, - new object[]{ CreateElement(exclude, ""), exclude, new string[] {}, false }, + new object[]{ CreateElement(exclude, stringArray), exclude, new string[] { "1","2"} }, + new object[]{ CreateElement(exclude, ""), exclude, new string[] {} }, // null for no property element - new object[]{ CreateElement(exclude, "true"), hideFullyCovered, null, false}, + new object[]{ CreateElement(exclude, "true"), hideFullyCovered, null}, //exception for no type conversion - new object[]{ CreateElement(enumConversion, "No"), enumConversion, RunMsCodeCoverage.No, false } + new object[]{ CreateElement(enumConversion, "No"), enumConversion, RunMsCodeCoverage.No } }; diff --git a/SharedProject/Core/Model/SettingsMerger.cs b/SharedProject/Core/Model/SettingsMerger.cs index 0fd6c570..0ad9f552 100644 --- a/SharedProject/Core/Model/SettingsMerger.cs +++ b/SharedProject/Core/Model/SettingsMerger.cs @@ -75,6 +75,7 @@ private class SettingsElementDefaultMerge { public XElement SettingsElement { get; set; } public bool DefaultMerge { get; set; } + public bool FromProjectSettings { get; internal set; } } private readonly PropertyInfo[] settingsPropertyInfos; @@ -90,13 +91,27 @@ ILogger logger } - public IAppOptions Merge(IAppOptions globalOptions, List settingsFileElements, XElement projectSettingsElement) + public IAppOptions Merge( + IAppOptions globalOptions, + List settingsFileElements, + XElement projectSettingsElement) { var settingsElementsWithDefaultMergeStrategy = - settingsFileElements.Select(e => new SettingsElementDefaultMerge { SettingsElement = e, DefaultMerge = settingsFileDefaultMerge }).ToList(); + settingsFileElements.Select(e => new SettingsElementDefaultMerge { + SettingsElement = e, + DefaultMerge = settingsFileDefaultMerge, + FromProjectSettings = false + }).ToList(); + if (projectSettingsElement != null) { - settingsElementsWithDefaultMergeStrategy.Add(new SettingsElementDefaultMerge { SettingsElement = projectSettingsElement, DefaultMerge = projectSettingsDefaultMerge }); + settingsElementsWithDefaultMergeStrategy.Add( + new SettingsElementDefaultMerge { + SettingsElement = projectSettingsElement, + DefaultMerge = projectSettingsDefaultMerge, + FromProjectSettings = true + } + ); } if (settingsElementsWithDefaultMergeStrategy.Count != 0) @@ -107,7 +122,11 @@ public IAppOptions Merge(IAppOptions globalOptions, List settingsFileE return globalOptions; } - private void Merge(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, List settingsElementsWithDefaultMergeStrategy) + private void Merge( + IAppOptions globalOptions, + PropertyInfo settingPropertyInfo, + List settingsElementsWithDefaultMergeStrategy + ) { var canMerge = settingsMergeLogic.CanMerge(settingPropertyInfo.PropertyType); if (canMerge) @@ -119,14 +138,15 @@ private void Merge(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, var propertyElement = GetPropertyElement(settingsElement, settingPropertyInfo.Name); if (propertyElement != null) { + var fromProjectSettings = settingsElementWithDefaultMerge.FromProjectSettings; var merge = GetMerge(defaultMerge, propertyElement); if (merge) { - Merge(globalOptions, settingPropertyInfo, settingsElement); + Merge(globalOptions, settingPropertyInfo, settingsElement,fromProjectSettings); } else { - Overwrite(globalOptions, settingPropertyInfo, settingsElement); + Overwrite(globalOptions, settingPropertyInfo, settingsElement,fromProjectSettings); } } @@ -134,14 +154,13 @@ private void Merge(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, } else { - var settingsElements = settingsElementsWithDefaultMergeStrategy.Select(x => x.SettingsElement); - Overwrite(globalOptions, settingPropertyInfo, settingsElements); + Overwrite(globalOptions, settingPropertyInfo, settingsElementsWithDefaultMergeStrategy); } } - private void Merge(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, XElement settingsElement) + private void Merge(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, XElement settingsElement,bool fromProjectSettings) { - var value = TryGetValueFromXml(settingsElement, settingPropertyInfo); + var value = TryGetValueFromXml(settingsElement, settingPropertyInfo,fromProjectSettings); if (value != null) { var currentValue = settingPropertyInfo.GetValue(globalOptions); @@ -179,17 +198,17 @@ private bool GetDefaultMerge(bool defaultDefaultMerge, XElement root) return defaultMergeAttribute.Value.ToLower() == "true"; } - private void Overwrite(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, IEnumerable settingsElements) + private void Overwrite(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, IEnumerable settingsElementsDefaultMerge) { - foreach (var settingsElement in settingsElements) + foreach (var settingsElementDefaultMerge in settingsElementsDefaultMerge) { - Overwrite(globalOptions, settingPropertyInfo, settingsElement); + Overwrite(globalOptions, settingPropertyInfo, settingsElementDefaultMerge.SettingsElement,settingsElementDefaultMerge.FromProjectSettings); } } - private void Overwrite(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, XElement settingsElement) + private void Overwrite(IAppOptions globalOptions, PropertyInfo settingPropertyInfo, XElement settingsElement,bool fromProjectSettings) { - var value = TryGetValueFromXml(settingsElement, settingPropertyInfo); + var value = TryGetValueFromXml(settingsElement, settingPropertyInfo,fromProjectSettings); if (value != null) { settingPropertyInfo.SetValue(globalOptions, value); @@ -201,7 +220,7 @@ private XElement GetPropertyElement(XElement settingsElement, string propertyNam return settingsElement.Descendants().FirstOrDefault(x => x.Name.LocalName.Equals(propertyName, StringComparison.OrdinalIgnoreCase)); } - private object TryGetValueFromXml(XElement settingsElement, PropertyInfo property) + private object TryGetValueFromXml(XElement settingsElement, PropertyInfo property,bool fromProjectSettings) { try { @@ -209,7 +228,8 @@ private object TryGetValueFromXml(XElement settingsElement, PropertyInfo propert } catch (Exception exception) { - logger.Log($"Failed to override '{property.Name}' setting", exception); + var from = fromProjectSettings ? "project settings" : "settings file"; + logger.Log($"Failed to get '{property.Name}' setting from {from}", exception); } return null; } @@ -372,7 +392,7 @@ internal object GetValueFromXml(XElement settingsElement, PropertyInfo property) else { - throw new Exception($"Cannot handle '{property.PropertyType.Name}' yet"); + throw new Exception($"Unexpected settings type '{property.PropertyType.Name}' for setting {property.Name} in settings merger GetValueFromXml"); } return null;