Add generic combinatorial attributes

[siewers]

I'd like to propose a combinatorial attribute equivalent to the xUnit ClassDataAttribute allowing reuse of the same data source values across multiple (combinatorial) tests.

I've implemented and tested the following:

public class CombinatorialClassDataAttribute : Attribute
{
    public CombinatorialClassDataAttribute(Type valuesSourceType, params object[]? args)
    {
        this.Values = GetValues(valuesSourceType, args);
    }

    /// <inheritdoc />
    public object?[] Values { get; }

    private static object?[] GetValues(Type valuesSourceType, object[] args)
    {
        var valuesSource = Activator.CreateInstance(
            valuesSourceType,
            BindingFlags.CreateInstance | BindingFlags.OptionalParamBinding,
            null,
            args,
            CultureInfo.InvariantCulture) as IEnumerable;

        if (valuesSource is null)
        {
            throw new InvalidOperationException($"The values source must be convertible to {typeof(IEnumerable)}.");
        }

        if (valuesSource is IEnumerable<object[]> enumerableOfObjects)
        {
            // This also supports types deriving from TheoryData and TheoryData<T>, which are IEnumerable<object[]>
            return enumerableOfObjects.SelectMany(objects => objects).Cast<object?>().ToArray();
        }

        return valuesSource.Cast<object?>().ToArray();
    }
}

In .NET Standard 2.0, there's the option to use generic attributes, which I've implemented like this:

#if NETSTANDARD2_0_OR_GREATER
public class CombinatorialClassDataAttribute<TValueSource> : CombinatorialClassDataAttribute
    where TValueSource : class, IEnumerable
{
    public CombinatorialClassDataAttribute(params object[]? args)
        : base(typeof(TValueSource), args)
    {
    }
}
#endif

This will allow users to write a test like this:

public record MyTestData(int Number, string Text);

 // This can be anything implementing IEnumerable returning the expected type, using TheoryData<T> is just more convenient
public class MySharedTestData : TheoryData<MyTestData>
{
    public MySharedTestData(int seed = 0)
    {
        Add(new(Number: 42 + seed, Text: "Lorem");
        Add(new(Number: 123 + seed, Text": "Ipsum");
    }
}

public class MyTests
{
    [Theory, CombinatorialData]
    public void TestMyData([CombinatorialClassData(typeof(MySharedTestData)] MyTestData testCase /* add other combinations of combinatorial data */)
    {
        // Assert things with testCase
    }

    [Theory, CombinatorialData]
    public void TestMyDataWithGenericAttribute([CombinatorialClassData<MySharedTestData>] MyTestData testCase /* add other combinations of combinatorial data */)
    {
        // Assert things with testCase
    }

    [Theory, CombinatorialData]
    public void TestMyDataWithGenericAttributeAndSeed([CombinatorialClassData<MySharedTestData>(/* seed */ 42)] MyTestData testCase /* add other combinations of combinatorial data */)
    {
        // Assert things with testCase
    }
}

I think this could be a valuable addition to this great library.

[siewers]

In my implementation, I also introduced the following interface:

/// <summary>
/// Defines a value provider that provides values for a parameter on a test method.
/// </summary>
public interface ICombinatorialValuesProvider
{
    /// <summary>
    /// Gets the values that should be passed to this parameter on the test method.
    /// </summary>
    /// <value>An array of values.</value>
    object?[] Values { get; }
}

This reduces the implementation in ValuesUtilities.GetValuesFor to this:

internal static IEnumerable<object?> GetValuesFor(ParameterInfo parameter)
{
    Requires.NotNull(parameter, nameof(parameter));

    ICombinatorialValuesProvider? valuesSource = parameter.GetCustomAttributes().OfType<ICombinatorialValuesProvider>().SingleOrDefault();
    if (valuesSource is not null)
    {
        return valuesSource.Values;
    }

    {
        CombinatorialMemberDataAttribute? attribute = parameter.GetCustomAttribute<CombinatorialMemberDataAttribute>();
        if (attribute is not null)
        {
            return attribute.GetValues(parameter);
        }
    }

    return GetValuesFor(parameter.ParameterType);
}

Technically, the interface could define GetValuesFor(ParameterInfo parameter) instead, which would remove the need for special handling of CombinatorialMemberDataAttribute. It would allow for potential future flexibility if more attributes are implemented.

[siewers]

Related to #94

[siewers]

The existing CombinatorialValuesAttribute could potentially also be used for this, but the alignment with existing xUnit naming is likely easier to understand.
There is already the CombinatorialMemberDataAttribute that aligns with xUnit, so introducing CombinatorialClassDataAttribute makes sense to me.

Warning: Ramblings ahead...

As I see it, allowing users to create a single class containing shared values used across tests makes sense.
The alternative, using member data, requires the attribute to reference the member's name of a class, which can be avoided if a single class, implementing IEnumerable (or derivatives), can be constructed instead.

Using the values attribute, I imagine something like this:

public void Test([CombinatorialValues(typeof(MySharedTestData), nameof(MySharedTestData.Values)] SharedTestData testData)

And a new generic version (as this issue is about):

public void Test([CombinatorialValues<MySharedTestData>(nameof(MySharedTestData.Values))] SharedTestData testData)
// Alternatively
public void Test([CombinatorialValues<MySharedTestData>("Values")] SharedTestData testData)

An overload to CombinatorialValuesAttribute only accepting would need to be introduced:

public CombinatorialValues(Type valuesSource){} // New
public CombinatorialValues(params object?[]? values){} // Existing

And the generic version

public class CombinatorialValues<T> : CombinatorialValues
    where T : class, IEnumerable
{
    public CombinatorialValues()
        : base(typeof(T))
    {
    }
}

However, this might be confusing or even impossible to use since the attribute constructor already accepts params object?[]? values, meaning any user wanting to pass an actual type as a value will be impossible, e.g.:

public void Test[CombinatorialValues(typeof(SomeTypeToTest)] Type t) // This will resolve the new constructor

I know this scenario makes no sense combinatorially, so if this is preferred, the params constructor should throw if only a single Type argument is passed (I guess). I'm just a bit skeptical of that behavior, if I'm debugging tests that accepts types:

public void Test([CombinatorialValues(typeof(Type1), typeof(Type2))] Type someType)
{
    // I'm getting Type1 and Type2 as expected
}
public void Test([CombinatorialValues(typeof(Type1))] Type someType)
{
    // This change will break that behavior and throw since the type is now being treated as a value source that doesn't implement IEnumerable (or it might, who knows)
}

As a final idea, the existing CombinatorialValuesAttribute could remain unaltered and only introduce a new generic version. The downside is that this would prevent users who cannot use generic attributes from using this functionality.
It would also cause inconsistency between the behavior of the two, identically named, attributes.

[AArnott]

@siewers I approve of your CombinatorialClassDataAttribute proposal.
Refactoring with your new ICombinatorialValuesProvider also looks good.

If you can submit pull requests for these soon, we can get all your ideas in together in the next release.
Thank you for your well thought out suggestions.

[siewers]

@AArnott, I've submitted the PR. I have to review my comments above since the PR is based on some of the thoughts we haven't agreed on yet.
Please have a look and let me know what should change.