AArnott / Xunit.Combinatorial

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

Deriving from CombinatorialValuesAttribute #13

Closed mwpowellhtx closed 6 years ago

mwpowellhtx commented 6 years ago

Hello, I am trying to derive from CVA along these lines, but the Visual Studio visual runner is not finding the test cases:

public class SocketAddressFamilyCombinatorialValuesAttribute : CombinatorialValuesAttribute
{
    private static readonly object[] Families;

    static SocketAddressFamilyCombinatorialValuesAttribute()
    {
        Families = new object[]
        {
            (ushort) SocketAddressFamily.Unspecified,
            (ushort) SocketAddressFamily.InProcess,
            (ushort) SocketAddressFamily.InterProcess,
            (ushort) SocketAddressFamily.InetIPv4,
            (ushort) SocketAddressFamily.InetIPv6,
            (ushort) SocketAddressFamily.ZeroTier,
            (ushort) 0xfeed,
            (ushort) 0xfade,
        };
    }

    public SocketAddressFamilyCombinatorialValuesAttribute()
        : base(Families.ToArray())
    {
    }
}

I am trying to statically initialize the values (once), then they should be informing the ctor, correct?

And in my test case, looks like this:

[Theory]
[CombinatorialData]
public void ThatAddressViewTransitionsWithFamily(
    [SocketAddressFamilyCombinatorialValues] ushort a
    , [SocketAddressFamilyCombinatorialValues] ushort b)
{
    // ...
}
mwpowellhtx commented 6 years ago

My error is:

2017.11.02 20:31:26.537   ERROR JetBrains.Util.InternalErrorException: Unexpected: Unable to find any matching test cases
   at JetBrains.ReSharper.UnitTestRunner.Xunit.TestRunner.Run(XunitTestAssemblyTask assemblyTask)
mwpowellhtx commented 6 years ago

For giggles, I wondered if I just use the CombinatorialValuesAttribute itself, to no avail:

[Theory]
[CombinatorialData]
public void ThatAddressViewTransitionsWithFamily(
    [CombinatorialValues((ushort) InetIPv6, (ushort) InetIPv4)] ushort a
    , [CombinatorialValues((ushort) InProcess, (ushort) InterProcess)] ushort b)
{
  //...
}
AArnott commented 6 years ago

I think a derived attribute type won't work because this library specifically looks for just the one class.

Given that your last attempt failed, I think something else is broken in your runner (or perhaps it's just a mismatch of versions), since that looks like the basic use case.

Can you share what version of xunit, xunit.runner.visualstudio, this project, etc. you're using? Are you running from VS or the command line?

mwpowellhtx commented 6 years ago

@AArnott As far as I know, latest versions of all.

So, I take it xUnit isn't looking for "assignable from" type relationships? Obviously so, given the line of code.

Commonly this is done from an interface perspective, but base classes are also useful.

AArnott commented 6 years ago

So this isn't a problem. I just merged in a test and sample that proves it works.

mwpowellhtx commented 6 years ago

@AArnott Yes, the VS runner is not discovering attributes. So hence we're spinning our wheels on the issue.

Edit: And, AFAIK, I'm running the latest versions of the xunit suite of assemblies, etc, in VS2015.

mwpowellhtx commented 6 years ago

@AArnott I maintain that the line needing to be addressed is this one:

var valuesAttribute = parameter.GetCustomAttribute<CombinatorialValuesAttribute>();

For the simple reason that you look for only one type of attribute. I think which the suggested approach addresses the issue.

internal static IEnumerable<object> GetValuesFor(ParameterInfo parameter)
{
    var valuesAttrib = parameter.GetCustomAttributes(true)
        .SingleOrDefault(a => a is CombinatorialValuesAttribute);

    return valuesAttrib != null
        ? ((CombinatorialValuesAttribute) valuesAttrib).Values
        : GetValuesFor(parameter.ParameterType);
}

I see that your tests invoke something called GetValues, but does this ever cover the GetValuesFor method in question in quite the same manner?

// ...
IEnumerable<object[]> actual = att.GetData(testhelperMethodInfo);
// ...

Any further attribution, i.e. AttributeUsageAttribute, should be strictly unnecessary IMO; assuming that it is properly inherited. At least I was not receiving any messages along those lines when I tried.

mwpowellhtx commented 6 years ago

@AArnott Until this is properly addressed, Xunit is a non-starter, at least for this purpose. I'll continue running with Nunit for the time being since at least they've correctly supported the issue.

AArnott commented 6 years ago

Yes, the test I added covers the code in question and proves that GetCustomAttribute<T> returns attributes that derive from T. Also, the sample I added shows the Test Explorer populate and running the test method with all the values supplied by the derived attribute. I'm using VS 2017, BTW.

You're free to use NUnit instead if that's working for you. I'm satisfied at this point that there's nothing I can or need to fix here. If you have more information of what might need to change, feel free to share.

mwpowellhtx commented 6 years ago

@AArnott The problem statement is clear, as is the suggested correction.

Documentation for GetCustomAttribute<T> mentions nothing concerning derived types. That looks for just the specified type T, nothing more, nothing less. So your assertion concerning derived types is just wrong. You need to examine the range of Custom Attributes accordingly and decide that the attribute is of the type, which includes derived types. I provided one usage, but you're free to use which ever GetCustomAttributes is most appropriate.

It's just a shame that there would need to be a couple of combinatorial xunit extensions in the ecosystem.

AArnott commented 6 years ago

If the docs mention nothing about derived types, I don't think we can deduce that that surely affirms that it doesn't work with derived types.

If you're prepared to introduce a second instance of such an xunit extension, I would expect you would consider actually cloning this project locally first to see that in fact the test I added for verifying your claim actually shows that the derived type is recognized, executed, and produces the result you want. Changing the code as you are asking for would make no difference.

Check out this screenshot that proves it works. You'll see code coverage for just the one test shows GetCustomAttribute behaves the way I describe, and that the Test Explorer is in fact populated with all the test cases you'd expect given the sample I added.

image

I hope this will satisfy you that if it's not working for you that the problem lies elsewhere. But if not, I'm done spending time on this anyway.

mwpowellhtx commented 6 years ago

@AArnott Have another look at my solution; GetCustomAttributes, not GetCustomAttribute.

I've spent more than enough time on this black hole.