Test all properties with single test function In writing unit tests for c# functions, there is a good case made for testing properties. In testing those properties, I felt there should be a few goals.


Provide a wide range of inputs
Test additional features like PropertyChanged
Don't write a ton of very repetitive test code


So I came up with this function for a specific class (although it could even be generalized with a templated method for all classes) that essentially gives a property name, a value to set the property to, the expected value to receive out, and a list of PropertyChanged names that should be fired as a result. This is accomplished by taking advantage of MSTestv2's DataTestMethod and DataRow attributes.

Looking for feedback on the quality/durability/usefulness of this code.

[TestMethod()]
[DataTestMethod]
[DataRow("Name", "A normal name", "A normal name", new string[] { "Name" })]
[DataRow("Name", null, null, new string[] { "Name" })]
[DataRow("Name", "", "", new string[] { "Name" })]
[DataRow("Name", "A normal name", "A normal name", new string[] { "Name" })]
[DataRow("Name", "A really very super duper long string to test the overall allowed length", "A really very super duper long string to test the overall allowed length", new string[] { "Name" })]
[DataRow("Address", 0, 0, new string[] { "Address", "AbsAddress"})]
[DataRow("Address", -1, null, new string[] { "Address", "AbsAddress" })]
[DataRow("Address", 300, 300, new string[] { "Address", "AbsAddress" })]
[DataRow("Address", int.MaxValue, int.MaxValue, new string[] { "Address", "AbsAddress" })]
public void ConfigItem_AllProperties_Test(string propName, object set, object expected, string[] propChangedNames)
{
    var item = new ConfigItem();
    var propsChanged = new Dictionary&lt;string, bool&gt;();
    foreach (var s in propChangedNames)
    {
        propsChanged.Add(s, false);
    }
    item.PropertyChanged += (sender, args) =&gt;
    {
        if (propsChanged.ContainsKey(args.PropertyName))
        {
            propsChanged[args.PropertyName] = true;
        }
    };
    typeof(ConfigItem).GetProperty(propName).SetValue(item, set);
    object actual = typeof(ConfigItem).GetProperty(propName).GetValue(item);
    Assert.AreEqual(expected, actual);
    foreach (var pair in propsChanged)
    {
        Assert.IsTrue(pair.Value, pair.Key);
    }
}
too vague &amp; hiding test cases


[DataRow("Name", "A normal name", "A normal name", new string[] { "Name" })]
[DataRow("Name", null, null, new string[] { "Name" })]
[DataRow("Name", "", "", new string[] { "Name" })]
[DataRow("Name", "A normal name", "A normal name", new string[] { "Name" })]
[DataRow("Name", "A really very super duper long string to test the overall allowed length", "A really very super duper long string to test the overall allowed length", new string[] { "Name" })]



Tests should be testing very specific things. This test however isn't doing that. I looks like it's not simply testing the PropertyChanged event. If it was, then a single DataRow would be enough to tell whether the event fires or not.

It seems to also test some conditions that might prevent it from firering. Such tests should be done separately because here it's not clear what conditions you are testing. 



too many hardcoded strings

Technically I find it's ok but to my taste there are too many hardcoded strings. I wouldn't say anything if it was just data but since they are property names I don't like them being strings.

You could use nameof to replace them with something more dynamic but then the test would not be very readable anymore.



extend Assert &amp; use expressions

Instead it'd be a better idea to write an extension for the Assert.That property that you could use for testing the event in this way:

Assert.That.PropertyChanged(
    new Customer(), 
    x =&gt; x.FirstName = "John", 
    x =&gt; x.FirstName,
    x =&gt; x.FullName
);


where the signature of the method is:

public static void PropertyChanged&lt;T&gt;(
    this Assert assert, 
    T obj, 
    Action&lt;T&gt; setProperty,
    params Expression&lt;Func&lt;T&gt;&gt;[] changedProperties
)
{
    ...
}


now you can do the reflection magic here.



This is just an idea but you could of course have a couple of such extensions. One very gneric and another one that e.g. assumes that the property being set always fires an event for it.



confusing naming


foreach (var s in propChangedNames)



If you must give variables single letter names then let them at least be derived from the main name. Here p or pcn would be appropriate. s does not stand for anything. The only exception is x in lambda expressions which stands for current-item.



misses the point


  although it could even be generalized with a templated method for all classes


Then do this because otherwise you failed to reach the third goal which is:


  Don't write a ton of very repetitive test code