AArnott / Xunit.Combinatorial

Adds combinatorial and pairwise testing capability to Xunit tests
Other
180 stars 16 forks source link

CombinatorialMemberData cannot be of type IEnumerable<object[]> #60

Closed siewers closed 3 weeks ago

siewers commented 1 year ago

The CombinatorialMemberDataAttribute requires the member to return an enumerable which is not an IEnumerable<object[]>. This breaks compatibility with the Xunit MemberData attribute, which requires the member to return exactly an IEnumerable<object[]>.

We should figure out how to support the same member data type as Xunit support, so we can use the same member data for both MemberData and CombinatorialMemberData.

I think I might have broken that in the latest prerelease version.

siewers commented 1 year ago

Right now I have to do something like this, which is a bit annoying.

public static IEnumerable<string> GetValues 
{
    get 
    { 
        yield "Foo";
        yield "Bar";
        yield "Baz";
    } 
}

public static IEnumerable<object[]> GetValuesXunit 
{ 
    get { return GetValues.Select(x => new object[] { x }); }
}

[Theory]
[MemberData(nameof(GetValuesXunit))]
public void TestWithMemberData(string value)
{
}

public void TestWithCombinatorialData(
    [CombinatorialMemberData(nameof(GetValues))] string value, 
    [CombinatorialValues(1, 2, 3)] int number)
{
}
siewers commented 1 year ago

When I think about it, I'm not sure this is even possible to solve. It also seems like a rather odd constraint in xUnit. It should be able to verify any IEnumerable<T> to match the method signature.

siewers commented 1 year ago

Perhaps using TheoryData<T> could fix it, but currently CombinatorialMemberData doesn't understand that.

AArnott commented 1 year ago

I don't see how these two would be compatible anyway. CombinatorialMemberDataAttribute takes values for an individual parameter, whereas MemberDataAttribute is applied to the whole method and therefore takes values for all the parameters. They are fundamentally different data and applied to different places. What's more, the whole point of CombinatorialMemberDataAttribute is to apply to just one parameter so that the set of test cases can be generated by the library, whereas MemberDataAttribute requires you to generate the test cases yourself.

So I'd say this is By Design.

siewers commented 1 year ago

What do you think about supporting TheoryData<T>? That should be doable and would probably support both scenarios 🤔

AArnott commented 1 year ago

I don't know anything about TheoryData<T>. If it's something you're proposing, I'd need more of a spec to comment on it. A new issue would be appropriate for that.

siewers commented 1 year ago

Yeah, it's also not very well documented, if at all. I haven't found any official documentation about it, but Andrew Lock has a nice blog post about it.

I'm a bit reluctant whether it should be supported, given the feature itself is so poorly documented. I guess it wouldn't hurt and my best guess is that the xUnit maintainers simply just don't have a lot of documentation at all.

If you think it's worth supporting, I'd be happy to create a PR for it, I just don't want to waste my time if you think the idea is pointless 🙂

AArnott commented 1 year ago

Cool. But I still need to understand how that would be used and apply to this library. Thank you for volunteering to contribute to the library. I'd be interested to read sample code and a brief explanation of its behavior as it would apply to this library (no need to make it actually work yet).

silkfire commented 7 months ago

@AArnott

Supporting 1-dimensional TheoryData (i.e. only TheoryData<T> with one type parameter) would be very convenient.

The conversion from object[][] to IEnumerable<T> goes as follows:

return new MyTheoryData().Select(d => (MyType)d.Single());
siewers commented 6 months ago

@silkfire

As part of #95 I'm working on supporting a part of this. If the idea is approved, I intend to support TheoryData (or basically IEnumerable<object[]>) on the CombinatorialMemberDataAttribute as well, which will also get a generic counterpart.

siewers commented 6 months ago

It's not technically impossible to support IEnumerable<object[]> since that is intended to map to multiple parameters in a test method. TheoryData, however, maps to a single type, which is exactly what will make tests a lot easier to read, write and maintain. With TheoryData the type can be whatever you want it to be, so it should be flexible enough to support most cases.

The idea is to allow this:


public class Tests
{
    public record MyTestCase(int Number, string Text);

    public static readonly TheoryData<MyTestCase> MyTestCases = new(
        new MyTestCase(1, "Foo"),
        new MyTestCase(2, "Bar")
    );

    [Theory, CombinatorialData]
    public void TestMyTestCases([CombinatorialMemberData(nameof(MyTestCases))] MyTestCase testCase, bool flag)
    {
        /*
            This will give you:
            testCase(1, "Foo"), true
            testCase(1, "Foo"), false
            testCase(2, "Bar"), true
            testCase(2, "Bar"), false
        */
    }
}
siewers commented 6 months ago

It can also only support TheoryData<T> (with a single generic parameter, since multiple generic parameters act similarly to IEnumerable<object[]> that maps to multiple method arguments.

AArnott commented 6 months ago

Reactivating due to recent activity. I'll review the proposal and get back to you soon.

siewers commented 5 months ago

@AArnott have you had time to review the proposal yet?

AArnott commented 3 weeks ago

Reviewing this now. Sorry it took so long. I still don't understand what value you're adding with your proposal. You most recent code snippet that demonstrates what you want to do is great, but I can accomplish the same with no changes to the library after changing very little in your snippet:

public class Tests
{
    public record MyTestCase(int Number, string Text);

    public static readonly IEnumerable<MyTestCase> MyTestCases = [
        new MyTestCase(1, "Foo"),
        new MyTestCase(2, "Bar")
    ];

    [Theory, CombinatorialData]
    public void TestMyTestCases([CombinatorialMemberData(nameof(MyTestCases))] MyTestCase testCase, bool flag)
    {
        /*
            This will give you:
            testCase(1, "Foo"), true
            testCase(1, "Foo"), false
            testCase(2, "Bar"), true
            testCase(2, "Bar"), false
        */
    }
}

So why then is your version so much better?

silkfire commented 3 weeks ago

Will this add support for TheoryData or how is this different from #95?

siewers commented 3 weeks ago

@AArnott The difference is that using TheoryData<T> allows the same data source to be used as theory data in other tests. This can be very convenient for 1-dimensional theories that should also be used for combinatorial tests. xUnit does not support typed IEnumerables, but only accepts IEnumerable<object[]>. Additionally, the later versions of xUnit analyzers even warn about the usage of IEnumerable<object[]> and recommend using TheoryData<T> instead.

Does that make sense? :)

siewers commented 3 weeks ago

@silkfire #95 was more about introducing generic combinatorial attributes. It's not directly related, but my work on that added more functionality which could be very useful (I think).