diff --git a/src/NUnitCommon/nunit.common/PackageHelper.cs b/src/NUnitCommon/nunit.common/PackageHelper.cs index 4399a4e0e..d435a6b94 100644 --- a/src/NUnitCommon/nunit.common/PackageHelper.cs +++ b/src/NUnitCommon/nunit.common/PackageHelper.cs @@ -165,12 +165,11 @@ private static void ReadSettings(this PackageSettings packageSettings, XmlReader { var name = xmlReader.Name; var value = xmlReader.Value; - var prop = typeof(SettingDefinitions).GetProperty(name, BindingFlags.Public | BindingFlags.Static); - if (prop is not null) // Standard SettingDefinition + var settingDefinition = SettingDefinitions.Lookup(name); + if (settingDefinition is not null) // Known setting name { - var settingDefinition = (SettingDefinition)prop.GetValue(null)!; - + // A few settings require special handling when deserializing switch (name) { case nameof(SettingDefinitions.InternalTraceWriter): @@ -190,9 +189,14 @@ private static void ReadSettings(this PackageSettings packageSettings, XmlReader default: switch (settingDefinition.ValueType) { - case Type t when t.IsPrimitive || t.IsAssignableFrom(typeof(string)): - var data = Convert.ChangeType(value, t); - packageSettings.Add(settingDefinition.WithValue(data)); + case Type t when t.IsAssignableFrom(typeof(string)): + packageSettings.Add(name, value); + break; + case Type t when t.IsAssignableFrom(typeof(bool)): + packageSettings.Add(name, bool.Parse(value)); + break; + case Type t when t.IsAssignableFrom(typeof(int)): + packageSettings.Add(name, int.Parse(value)); break; default: // Setting doesn't match the expected type, ignore or throw an exception? diff --git a/src/NUnitEngine/nunit.engine.api/PackageSetting.cs b/src/NUnitEngine/nunit.engine.api/PackageSetting.cs index 81254abff..1821d65fa 100644 --- a/src/NUnitEngine/nunit.engine.api/PackageSetting.cs +++ b/src/NUnitEngine/nunit.engine.api/PackageSetting.cs @@ -1,5 +1,7 @@ // Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt +using System.Xml; + namespace NUnit.Engine { /// diff --git a/src/NUnitEngine/nunit.engine.api/PackageSettings.cs b/src/NUnitEngine/nunit.engine.api/PackageSettings.cs index fb4fe0ca7..0d7e50935 100644 --- a/src/NUnitEngine/nunit.engine.api/PackageSettings.cs +++ b/src/NUnitEngine/nunit.engine.api/PackageSettings.cs @@ -1,7 +1,10 @@ // Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt +using NUnit.Common; +using System; using System.Collections; using System.Collections.Generic; +using System.Xml.Linq; namespace NUnit.Engine { @@ -65,12 +68,20 @@ public T GetValueOrDefault(SettingDefinition definition) public void Add(PackageSetting setting) => _settings.Add(setting.Name, setting); /// - /// Creates and adds a custom string setting to the list, specifying the name and value. + /// Creates and adds a setting to the list, specified by the name and a string value. + /// The string value is converted to a typed PackageSetting if the name specifies a known SettingDefinition. /// /// The name of the setting. /// The corresponding value to set. public void Add(string name, string value) { + var definition = SettingDefinitions.Lookup(name); + + // If this is a known setting but not a string throw an exception + if (definition is not null && !definition.ValueType.IsAssignableFrom(typeof(string))) + throw (new ArgumentException($"The {name} setting requires a value of type {definition.ValueType.Name}")); + + // Otherwise add it Add(new PackageSetting(name, value)); } @@ -81,6 +92,12 @@ public void Add(string name, string value) /// The corresponding value to set. public void Add(string name, bool value) { + var definition = SettingDefinitions.Lookup(name); + + // If this is a known setting but not a boolean throw an exception + if (definition is not null && !definition.ValueType.IsAssignableFrom(typeof(bool))) + throw (new ArgumentException($"The {name} setting requires a value of type {definition.ValueType.Name}")); + Add(new PackageSetting(name, value)); } @@ -91,6 +108,12 @@ public void Add(string name, bool value) /// The corresponding value to set. public void Add(string name, int value) { + var definition = SettingDefinitions.Lookup(name); + + // If this is a known setting but not an int throw an exception + if (definition is not null && !definition.ValueType.IsAssignableFrom(typeof(int))) + throw (new ArgumentException($"The {name} setting requires a value of type {definition.ValueType.Name}")); + Add(new PackageSetting(name, value)); } @@ -109,5 +132,23 @@ public void Set(string name, T value) { Set(new PackageSetting(name, value)); } + + /// + /// Remove a setting + /// + /// The setting to remove + public void Remove(SettingDefinition setting) + { + _settings.Remove(setting.Name); + } + + /// + /// Remove a setting + /// + /// The name of the setting to remove + public void Remove(string settingName) + { + _settings.Remove(settingName); + } } } diff --git a/src/NUnitEngine/nunit.engine.api/SettingDefinition.cs b/src/NUnitEngine/nunit.engine.api/SettingDefinition.cs index 04cac7c17..e8419ac1d 100644 --- a/src/NUnitEngine/nunit.engine.api/SettingDefinition.cs +++ b/src/NUnitEngine/nunit.engine.api/SettingDefinition.cs @@ -37,7 +37,10 @@ public SettingDefinition(string name, Type valueType) public PackageSetting WithValue(T value) where T : notnull { - return new PackageSetting(Name, value); + if (ValueType.IsAssignableFrom(typeof(T))) + return new PackageSetting(Name, (T)Convert.ChangeType(value, ValueType)); + + throw (new ArgumentException($"The {Name} setting requires a value of type {ValueType.Name}")); } } diff --git a/src/NUnitEngine/nunit.engine.api/SettingDefinitions.cs b/src/NUnitEngine/nunit.engine.api/SettingDefinitions.cs index 7a157de4b..9edf96136 100644 --- a/src/NUnitEngine/nunit.engine.api/SettingDefinitions.cs +++ b/src/NUnitEngine/nunit.engine.api/SettingDefinitions.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Reflection; namespace NUnit.Common { @@ -15,6 +16,35 @@ namespace NUnit.Common /// public static class SettingDefinitions { + private static readonly Dictionary _knownSettings; + + static SettingDefinitions() + { + // Use Reflection to build a dictionary of all known setting definitions + var properties = typeof(SettingDefinitions).GetProperties(BindingFlags.Public | BindingFlags.Static); + + _knownSettings = new Dictionary(); + + foreach (var property in properties) + { + var propValue = property.GetValue(null, null) as SettingDefinition; + if (propValue is not null) + _knownSettings.Add(property.Name, propValue); + } + } + + /// + /// Lookup a known SettingDefinition by name + /// + /// The name of the setting + /// A SettingDefinition if there is one, otherwise null. + public static SettingDefinition? Lookup(string name) + { + if (_knownSettings.TryGetValue(name, out var definition)) + return definition; + return null; + } + #region Settings Used by the Engine /// diff --git a/src/NUnitEngine/nunit.engine.tests/Api/PackageSettingsTests.cs b/src/NUnitEngine/nunit.engine.tests/Api/PackageSettingsTests.cs new file mode 100644 index 000000000..0e16f15a0 --- /dev/null +++ b/src/NUnitEngine/nunit.engine.tests/Api/PackageSettingsTests.cs @@ -0,0 +1,154 @@ +// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using NUnit.Common; +using NUnit.Framework; + +namespace NUnit.Engine.Api +{ + public class PackageSettingsTests + { + private PackageSettings _settings; + + [SetUp] + public void SetUp() + { + _settings = new PackageSettings(); + } + + [Test] + public static void LookupSettingDefinitionSucceeds() + { + var definition = SettingDefinitions.MaxAgents; + var lookup = SettingDefinitions.Lookup("MaxAgents"); + + Assert.That(lookup, Is.SameAs(definition)); + } + + [Test] + public static void LookupSettingDefinitionFails() + { + var lookup = SettingDefinitions.Lookup("NonExistent"); + + Assert.That(lookup, Is.Null); + } + + [Test] + public void AddKnownSettingUsingSettingDefinition() + { + _settings.Add(SettingDefinitions.MaxAgents.WithValue(42)); + Assert.That(_settings.GetSetting("MaxAgents"), Is.EqualTo(42)); + } + + [Test] + public void AddKnownSettingUsingSettingDefinitionThrowsWithWrongType() + { + Assert.That(() => _settings.Add(SettingDefinitions.MaxAgents.WithValue("42")), Throws.ArgumentException); + } + + //[TestCase("ActiveConfig", "Release")] + //public void AddKnownSettingUsingName(string name, T value) + // where T : notnull + //{ + // var setting = SettingDefinitions.Lookup(name); + // Assert.That(setting, Is.Not.Null); + // var type = setting.ValueType; + // Assert.That(type, Is.EqualTo(typeof(T))); + + // _settings.Add(name, value); + //} + + [Test] + public void AddKnownStringSettingUsingName() + { + _settings.Add("ActiveConfig", "Release"); + Assert.That(_settings.GetSetting("ActiveConfig"), Is.EqualTo("Release")); + } + + [Test] + public void AddKnownStringSettingUsingNameThrowsWithWrongType() + { + Assert.That(() => _settings.Add("ActiveConfig", 5), Throws.ArgumentException); + } + + [Test] + public void AddKnownBoolSettingUsingName() + { + _settings.Add("RunAsX86", true); + Assert.That(_settings.GetSetting("RunAsX86"), Is.True); + } + + [Test] + public void AddKnownBoolSettingUsingNameThrowsWithWrongType() + { + Assert.That(() => _settings.Add("RunAsX86", 5), Throws.ArgumentException); + } + + [Test] + public void AddKnownIntSettingUsingName() + { + _settings.Add("MaxAgents", 42); + Assert.That(_settings.GetSetting("MaxAgents"), Is.EqualTo(42)); + } + + [Test] + public void AddKnownintSettingUsingNameThrowsWithWrongType() + { + Assert.That(() => _settings.Add("MaxAgents", "Bad"), Throws.ArgumentException); + } + + [Test] + public void AddUnknownStringSettingUsingName() + { + _settings.Add("MySetting", "Something"); + Assert.That(_settings.GetSetting("MySetting"), Is.EqualTo("Something")); + } + + [Test] + public void AddUnknownBoolSettingUsingName() + { + _settings.Add("MySetting", true); + Assert.That(_settings.GetSetting("MySetting"), Is.True); + } + + [Test] + public void AddUnknownIntSettingUsingName() + { + _settings.Add("MySetting", 999); + Assert.That(_settings.GetSetting("MySetting"), Is.EqualTo(999)); + } + + [Test] + public void CanRemoveSettingUsingSettingDefinition() + { + var setting = SettingDefinitions.MaxAgents.WithValue(20); + _settings.Add(setting); + Assert.That(_settings.HasSetting("MaxAgents")); + _settings.Remove(SettingDefinitions.MaxAgents); + Assert.That(_settings.HasSetting("MaxAgents"), Is.False); + } + + [Test] + public void CanRemoveSettingUsingName() + { + var setting = SettingDefinitions.MaxAgents.WithValue(20); + _settings.Add(setting); + Assert.That(_settings.HasSetting("MaxAgents")); + _settings.Remove("MaxAgents"); + Assert.That(_settings.HasSetting("MaxAgents"), Is.False); + } + + // Adding special types is NYI + //[Test] + //public void AddKnownSpecialSettingUsingName() + //{ + // var dict = new Dictionary(); + // _settings.Add("TestParametersDictionary", dict); + // Assert.That(_settings.GetSetting("TestParametersDictionary"), Is.SameAs(dict)); + //} + } +}