Skip to content

Commit f2ef56f

Browse files
CharliePooleCharliePoole
committed
PackageSettings improvements
1 parent 91d0a87 commit f2ef56f

File tree

6 files changed

+243
-9
lines changed

6 files changed

+243
-9
lines changed

‎src/NUnitCommon/nunit.common/PackageHelper.cs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,11 @@ private static void ReadSettings(this PackageSettings packageSettings, XmlReader
165165
{
166166
var name = xmlReader.Name;
167167
var value = xmlReader.Value;
168-
var prop = typeof(SettingDefinitions).GetProperty(name, BindingFlags.Public | BindingFlags.Static);
169168

170-
if (prop is not null) // Standard SettingDefinition
169+
var settingDefinition = SettingDefinitions.Lookup(name);
170+
if (settingDefinition is not null) // Known setting name
171171
{
172-
var settingDefinition = (SettingDefinition)prop.GetValue(null)!;
173-
172+
// A few settings require special handling when deserializing
174173
switch (name)
175174
{
176175
case nameof(SettingDefinitions.InternalTraceWriter):
@@ -190,9 +189,14 @@ private static void ReadSettings(this PackageSettings packageSettings, XmlReader
190189
default:
191190
switch (settingDefinition.ValueType)
192191
{
193-
case Type t when t.IsPrimitive || t.IsAssignableFrom(typeof(string)):
194-
var data = Convert.ChangeType(value, t);
195-
packageSettings.Add(settingDefinition.WithValue(data));
192+
case Type t when t.IsAssignableFrom(typeof(string)):
193+
packageSettings.Add(name, value);
194+
break;
195+
case Type t when t.IsAssignableFrom(typeof(bool)):
196+
packageSettings.Add(name, bool.Parse(value));
197+
break;
198+
case Type t when t.IsAssignableFrom(typeof(int)):
199+
packageSettings.Add(name, int.Parse(value));
196200
break;
197201
default:
198202
// Setting doesn't match the expected type, ignore or throw an exception?

‎src/NUnitEngine/nunit.engine.api/PackageSetting.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
22

3+
using System.Xml;
4+
35
namespace NUnit.Engine
46
{
57
/// <summary>

‎src/NUnitEngine/nunit.engine.api/PackageSettings.cs‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
22

3+
using NUnit.Common;
4+
using System;
35
using System.Collections;
46
using System.Collections.Generic;
7+
using System.Xml.Linq;
58

69
namespace NUnit.Engine
710
{
@@ -65,12 +68,20 @@ public T GetValueOrDefault<T>(SettingDefinition<T> definition)
6568
public void Add(PackageSetting setting) => _settings.Add(setting.Name, setting);
6669

6770
/// <summary>
68-
/// Creates and adds a custom string setting to the list, specifying the name and value.
71+
/// Creates and adds a setting to the list, specified by the name and a string value.
72+
/// The string value is converted to a typed PackageSetting if the name specifies a known SettingDefinition.
6973
/// </summary>
7074
/// <param name="name">The name of the setting.</param>
7175
/// <param name="value">The corresponding value to set.</param>
7276
public void Add(string name, string value)
7377
{
78+
var definition = SettingDefinitions.Lookup(name);
79+
80+
// If this is a known setting but not a string throw an exception
81+
if (definition is not null && !definition.ValueType.IsAssignableFrom(typeof(string)))
82+
throw (new ArgumentException($"The {name} setting requires a value of type {definition.ValueType.Name}"));
83+
84+
// Otherwise add it
7485
Add(new PackageSetting<string>(name, value));
7586
}
7687

@@ -81,6 +92,12 @@ public void Add(string name, string value)
8192
/// <param name="value">The corresponding value to set.</param>
8293
public void Add(string name, bool value)
8394
{
95+
var definition = SettingDefinitions.Lookup(name);
96+
97+
// If this is a known setting but not a boolean throw an exception
98+
if (definition is not null && !definition.ValueType.IsAssignableFrom(typeof(bool)))
99+
throw (new ArgumentException($"The {name} setting requires a value of type {definition.ValueType.Name}"));
100+
84101
Add(new PackageSetting<bool>(name, value));
85102
}
86103

@@ -91,6 +108,12 @@ public void Add(string name, bool value)
91108
/// <param name="value">The corresponding value to set.</param>
92109
public void Add(string name, int value)
93110
{
111+
var definition = SettingDefinitions.Lookup(name);
112+
113+
// If this is a known setting but not an int throw an exception
114+
if (definition is not null && !definition.ValueType.IsAssignableFrom(typeof(int)))
115+
throw (new ArgumentException($"The {name} setting requires a value of type {definition.ValueType.Name}"));
116+
94117
Add(new PackageSetting<int>(name, value));
95118
}
96119

@@ -109,5 +132,23 @@ public void Set<T>(string name, T value)
109132
{
110133
Set(new PackageSetting<T>(name, value));
111134
}
135+
136+
/// <summary>
137+
/// Remove a setting
138+
/// </summary>
139+
/// <param name="setting">The setting to remove</param>
140+
public void Remove(SettingDefinition setting)
141+
{
142+
_settings.Remove(setting.Name);
143+
}
144+
145+
/// <summary>
146+
/// Remove a setting
147+
/// </summary>
148+
/// <param name="settingName">The name of the setting to remove</param>
149+
public void Remove(string settingName)
150+
{
151+
_settings.Remove(settingName);
152+
}
112153
}
113154
}

‎src/NUnitEngine/nunit.engine.api/SettingDefinition.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ public SettingDefinition(string name, Type valueType)
3737
public PackageSetting WithValue<T>(T value)
3838
where T : notnull
3939
{
40-
return new PackageSetting<T>(Name, value);
40+
if (ValueType.IsAssignableFrom(typeof(T)))
41+
return new PackageSetting<T>(Name, (T)Convert.ChangeType(value, ValueType));
42+
43+
throw (new ArgumentException($"The {Name} setting requires a value of type {ValueType.Name}"));
4144
}
4245
}
4346

‎src/NUnitEngine/nunit.engine.api/SettingDefinitions.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.IO;
7+
using System.Reflection;
78

89
namespace NUnit.Common
910
{
@@ -15,6 +16,35 @@ namespace NUnit.Common
1516
/// </summary>
1617
public static class SettingDefinitions
1718
{
19+
private static readonly Dictionary<string, SettingDefinition> _knownSettings;
20+
21+
static SettingDefinitions()
22+
{
23+
// Use Reflection to build a dictionary of all known setting definitions
24+
var properties = typeof(SettingDefinitions).GetProperties(BindingFlags.Public | BindingFlags.Static);
25+
26+
_knownSettings = new Dictionary<string, SettingDefinition>();
27+
28+
foreach (var property in properties)
29+
{
30+
var propValue = property.GetValue(null, null) as SettingDefinition;
31+
if (propValue is not null)
32+
_knownSettings.Add(property.Name, propValue);
33+
}
34+
}
35+
36+
/// <summary>
37+
/// Lookup a known SettingDefinition by name
38+
/// </summary>
39+
/// <param name="name">The name of the setting</param>
40+
/// <returns>A SettingDefinition if there is one, otherwise null.</returns>
41+
public static SettingDefinition? Lookup(string name)
42+
{
43+
if (_knownSettings.TryGetValue(name, out var definition))
44+
return definition;
45+
return null;
46+
}
47+
1848
#region Settings Used by the Engine
1949

2050
/// <summary>

‎src/NUnitEngine/nunit.engine.tests/Api/PackageSettingsTests.cs‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (c) Charlie Poole, Rob Prouse and Contributors. MIT License - see LICENSE.txt
2+
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
using System.Text;
7+
using System.Threading.Tasks;
8+
using NUnit.Common;
9+
using NUnit.Framework;
10+
11+
namespace NUnit.Engine.Api
12+
{
13+
public class PackageSettingsTests
14+
{
15+
private PackageSettings _settings;
16+
17+
[SetUp]
18+
public void SetUp()
19+
{
20+
_settings = new PackageSettings();
21+
}
22+
23+
[Test]
24+
public static void LookupSettingDefinitionSucceeds()
25+
{
26+
var definition = SettingDefinitions.MaxAgents;
27+
var lookup = SettingDefinitions.Lookup("MaxAgents");
28+
29+
Assert.That(lookup, Is.SameAs(definition));
30+
}
31+
32+
[Test]
33+
public static void LookupSettingDefinitionFails()
34+
{
35+
var lookup = SettingDefinitions.Lookup("NonExistent");
36+
37+
Assert.That(lookup, Is.Null);
38+
}
39+
40+
[Test]
41+
public void AddKnownSettingUsingSettingDefinition()
42+
{
43+
_settings.Add(SettingDefinitions.MaxAgents.WithValue(42));
44+
Assert.That(_settings.GetSetting("MaxAgents"), Is.EqualTo(42));
45+
}
46+
47+
[Test]
48+
public void AddKnownSettingUsingSettingDefinitionThrowsWithWrongType()
49+
{
50+
Assert.That(() => _settings.Add(SettingDefinitions.MaxAgents.WithValue("42")), Throws.ArgumentException);
51+
}
52+
53+
//[TestCase("ActiveConfig", "Release")]
54+
//public void AddKnownSettingUsingName<T>(string name, T value)
55+
// where T : notnull
56+
//{
57+
// var setting = SettingDefinitions.Lookup(name);
58+
// Assert.That(setting, Is.Not.Null);
59+
// var type = setting.ValueType;
60+
// Assert.That(type, Is.EqualTo(typeof(T)));
61+
62+
// _settings.Add<T>(name, value);
63+
//}
64+
65+
[Test]
66+
public void AddKnownStringSettingUsingName()
67+
{
68+
_settings.Add("ActiveConfig", "Release");
69+
Assert.That(_settings.GetSetting("ActiveConfig"), Is.EqualTo("Release"));
70+
}
71+
72+
[Test]
73+
public void AddKnownStringSettingUsingNameThrowsWithWrongType()
74+
{
75+
Assert.That(() => _settings.Add("ActiveConfig", 5), Throws.ArgumentException);
76+
}
77+
78+
[Test]
79+
public void AddKnownBoolSettingUsingName()
80+
{
81+
_settings.Add("RunAsX86", true);
82+
Assert.That(_settings.GetSetting("RunAsX86"), Is.True);
83+
}
84+
85+
[Test]
86+
public void AddKnownBoolSettingUsingNameThrowsWithWrongType()
87+
{
88+
Assert.That(() => _settings.Add("RunAsX86", 5), Throws.ArgumentException);
89+
}
90+
91+
[Test]
92+
public void AddKnownIntSettingUsingName()
93+
{
94+
_settings.Add("MaxAgents", 42);
95+
Assert.That(_settings.GetSetting("MaxAgents"), Is.EqualTo(42));
96+
}
97+
98+
[Test]
99+
public void AddKnownintSettingUsingNameThrowsWithWrongType()
100+
{
101+
Assert.That(() => _settings.Add("MaxAgents", "Bad"), Throws.ArgumentException);
102+
}
103+
104+
[Test]
105+
public void AddUnknownStringSettingUsingName()
106+
{
107+
_settings.Add("MySetting", "Something");
108+
Assert.That(_settings.GetSetting("MySetting"), Is.EqualTo("Something"));
109+
}
110+
111+
[Test]
112+
public void AddUnknownBoolSettingUsingName()
113+
{
114+
_settings.Add("MySetting", true);
115+
Assert.That(_settings.GetSetting("MySetting"), Is.True);
116+
}
117+
118+
[Test]
119+
public void AddUnknownIntSettingUsingName()
120+
{
121+
_settings.Add("MySetting", 999);
122+
Assert.That(_settings.GetSetting("MySetting"), Is.EqualTo(999));
123+
}
124+
125+
[Test]
126+
public void CanRemoveSettingUsingSettingDefinition()
127+
{
128+
var setting = SettingDefinitions.MaxAgents.WithValue(20);
129+
_settings.Add(setting);
130+
Assert.That(_settings.HasSetting("MaxAgents"));
131+
_settings.Remove(SettingDefinitions.MaxAgents);
132+
Assert.That(_settings.HasSetting("MaxAgents"), Is.False);
133+
}
134+
135+
[Test]
136+
public void CanRemoveSettingUsingName()
137+
{
138+
var setting = SettingDefinitions.MaxAgents.WithValue(20);
139+
_settings.Add(setting);
140+
Assert.That(_settings.HasSetting("MaxAgents"));
141+
_settings.Remove("MaxAgents");
142+
Assert.That(_settings.HasSetting("MaxAgents"), Is.False);
143+
}
144+
145+
// Adding special types is NYI
146+
//[Test]
147+
//public void AddKnownSpecialSettingUsingName()
148+
//{
149+
// var dict = new Dictionary<string, string>();
150+
// _settings.Add("TestParametersDictionary", dict);
151+
// Assert.That(_settings.GetSetting("TestParametersDictionary"), Is.SameAs(dict));
152+
//}
153+
}
154+
}

0 commit comments

Comments
 (0)