TheoryAttribute does not work with generic methods about nunit HOT 16 OPEN

Generic allows to use one test method for different types (see sample below).
Sample: Use one method TestGenericForArbitraryArray for double and int arrays, instead of 2 methods TestDoubleForArbitraryArray and TestIntForArbitraryArray for each type.

    [TestFixture]
    public class TheorySampleTests
    {
        [Datapoint]
        public double[,] Array2X2Double = new double[,] { { 1, 0 }, { 0, 1 } };
        [Datapoint]
        public int[,] Array2X2Int = new int[,] { { 1, 0 }, { 0, 1 } };

        [Theory]
        public void TestDoubleForArbitraryArray(double[,] array)
        {
            Console.WriteLine("TestDoubleForArbitraryArray()");
            // ...
        }
        [Theory]
        public void TestIntForArbitraryArray(int[,] array)
        {
            Console.WriteLine("TestIntForArbitraryArray()");
            // ...
        } 

        [Theory]
        public void TestGenericForArbitraryArray<T>(T[,] array)
        {
            Console.WriteLine("TestGenericForArbitraryArray()");
            // ...
        } 
    }

from nunit.

@constructor-igor This seems like a logical workaround. The OP wanted the generic method to pick up all data that could possibly satisfy the generic method. I'm not sure that's really feasible here. I think it would probably work in a generic fixture, where T would actually mean a specific type.

from nunit.

@CharliePoole, please, could you help me to understand how can I help in the issue (status:confirm)?

from nunit.

The first step would be to verify that the failure actually occurs. As I commented a while back, it makes sense that NUnit can't find an open generic type. Next would be to create some tests to show exactly what does and doesn't work. You got a good start at that. I think your first and second methods work, but the third does not find any data. Right?

A further step would be to see what happens in a generic fixture, where the method itself uses T but is no longer generic. I am guessing that it would work and would find the specific int or double data.

If we can lay it all out, then we know it's truly a bug. If you want to work on it, we will have to decide how much to fix and how much to simply document.

Is that enought to help you get started?

from nunit.

original test can be reproduced in nunit v3 Beta1

    [TestFixture]
    public class TheorySampleTests
    {
        [Datapoint]
        public double[] ArrayDouble = { 0, 1, 2, 3 };
        [Datapoint]
        public int[] ArrayInt = { 0, 1, 2, 3 };

        [Theory]
        public void TestDoubleForArbitraryArray(double[] array)
        {
            Console.WriteLine("TestDoubleForArbitraryArray()");
            TestGenericForArbitraryArray(array);
        }
        [Theory]
        public void TestIntForArbitraryArray(int[] array)
        {
            Console.WriteLine("TestIntForArbitraryArray()");
            TestGenericForArbitraryArray(array);
        }

        [Theory]
        public void TestGenericForArbitraryArray<T>(T[] array)
        {
            Console.WriteLine("TestGenericForArbitraryArray()");
            // ...
        }
    }

Test Name:  TestGenericForArbitraryArray
Test FullName:  NUnit_v3_samples.TheorySampleTests.TestGenericForArbitraryArray
Test Source:    \NUnit_v3_samples\TheorySampleTests.cs : line 58
Test Outcome:   Failed
Test Duration:  0:00:00.001

Result Message: No arguments were provided

from nunit.

Found next workaround: base class with generic test method and number (per type) of test classes

    public class TheorySampleTestsBasic<T>
    {
        [Theory]
        public void TestGenericForArbitraryArray(T[] array)
        {
            Console.WriteLine("TestGenericForArbitraryArray()");
        }
    }
    [TestFixture]
    public class TheorySampleTestsChildDouble : TheorySampleTestsBasic<double>
    {
        [Datapoint]
        public double[] ArrayDouble1 = { 0, 1, 2, 3 };
        [Datapoint]
        public double[] ArrayDouble2 = { 4, 5, 6, 7 };
    }
    [TestFixture]
    public class TheorySampleTestsChildInt : TheorySampleTestsBasic<int>
    {
        [Datapoint]
        public int[] ArrayDouble1 = { 0, 1, 2, 3 };
        [Datapoint]
        public int[] ArrayDouble2 = { 4, 5, 6, 7 };
    }

from nunit.

What if you make the generic class a generic fixture, eliminating the derived classes and moving all the datapoints into the one class? The generic fixture could have [TestFixture(typeof(int))] and [TestFixture(typeof(double))] attributes.

from nunit.

ok.
Next way checked: seems works correctly.

    [TestFixture(typeof(int))]
    [TestFixture(typeof(double))]
    public class TheorySampleTestsGeneric<T>
    {
        [Datapoint]
        public double[] ArrayDouble1 = { 0, 1, 2, 3 };
        [Datapoint]
        public double[] ArrayDouble2 = { 4, 5, 6, 7 };
        [Datapoint]
        public int[] ArrayInt = { 0, 1, 2, 3 };
        [Theory]
        public void TestGenericForArbitraryArray(T[] array)
        {
            Console.WriteLine("TestGenericForArbitraryArray()");
        }
    }

by the way, Resharper v8 doesn't work correct for the use case:

from nunit.

Yes, so the bug is really that it doesn't work for Generic methods. When the class is generic, then that method is no longer a generic because T has already been resolved to int or double. I changed the title accordingly.

from nunit.

So do you think we should "fix" this or just document it?

For the original example to work, NUnit would have to find all Datapoints of any type and try to use them for the method. That seems a bit extreme.

Another option would be to give Theory an optional type argument, so that the theory was instantiated multiple times, just as we do for a TestFixture. IOW, we would have [Theory(typeof(int))] and [Theory(typeof(double))] in this case. Is that desirable?

Finally, as I say, we could just document what works and what doesn't on the TheoryAttribute page.

from nunit.

I guess, the issue should be supported ("fixed").
In my opinion, if compiler can understand and call generic method (see sample below), then developer expects same behavior in "Theory" attribute.

    [TestFixture]
    public class TheorySampleTests
    {
        [Datapoint]
        public double[] ArrayDouble = { 0, 1, 2, 3 };
        [Datapoint]
        public int[] ArrayInt = { 0, 1, 2, 3 };

        [Theory]
        public void TestDoubleForArbitraryArray(double[] array)
        {
            Console.WriteLine("TestDoubleForArbitraryArray()");
            TestGenericForArbitraryArray(array);
        }
        [Theory]
        public void TestIntForArbitraryArray(int[] array)
        {
            Console.WriteLine("TestIntForArbitraryArray()");
            TestGenericForArbitraryArray(array);
        }

        private void TestGenericForArbitraryArray<T>(T[] array)
        {
            Console.WriteLine("TestGenericForArbitraryArray()");
        }
    }

from nunit.

Thanks for checking this out. I think we need to figure out just what to support here, so I'm flagging it as status:design. We use that label when we don't want anyone to just start coding, without first having an agreed-upon specification. In this case, I think we have choices ranging from most complete implementation down to no implementation at all:

1. NUnit could search for data points of every type that can be used to instantiate the theory. It would have to honor any constraints on the type parameters, which would be new code for us. This approach would be challenging but definitely possible. The main drawback is that it might make generic Theories less useful. The user could not define datapoints in the class of any type that should not be used for this Theory. Requires some thought and some sample code.
2. We could give TheoryAttribute a type parameter and have the user specify exactly what instances of the Theory should be tested. In our example, we would use [Theory(typeof(int))] and [Theory(typeof(double))] to explicitly cause the theory to be used twice. This would probably be a somewhat easier implementation and we would be treating TheoryAttribute in the same way we currently handle TestFixtureAttribute.
3. We could simply document the limitation. Rather than generic Theories, users should use a generic class to contain the Theory.

These issues are not mutually exclusive. We also could:

1. Do both 1 and 2.
2. Do 3 first, then follow up with 1, 2 or 4.

This issue interacts with #150 and with the fix already made for #377. We should review it in light of those issues as well.

We can discuss which way to go here. Once we set a direction, we should post it here and remove the design status.

from nunit.

The documentation has been updated to indicate that generic methods may not be used and explaining the workaround using a generic fixture: https://github.com/nunit/nunit/wiki/Theory-Attribute

I'm moving this issue out of the beta 2 milestone so we can decide which way to go in the next release.

from nunit.

Looking at the options, I lean towards giving TheoryAttribute a Type argument. Finding all possible types that can be used seems like a step too far in making decisions for the user. Just as the programmer writes [TestFixture(typeof(int))] to determine how a generic fixture will be instantiated, they can write [Theory(typeof(int))] for a generic theory. The above example would then be handled like this:

    [TestFixture]
    public class TheorySampleTests
    {
        [Datapoint]
        public double[] ArrayDouble1 = { 0, 1, 2, 3 };
        [Datapoint]
        public double[] ArrayDouble2 = { 4, 5, 6, 7 };
        [Datapoint]
        public int[] ArrayInt = { 0, 1, 2, 3 };

        [Theory(typeof(int))]
        [Theory(typeof(double))]
        public void TestGenericForArbitraryArray(T[] array)
        {
            Console.WriteLine("TestGenericForArbitraryArray()");
        }
    }

@nunit/core-team What do you think of this approach versus the other? Should this stay in the 3.0 release as a feature? What priority?

from nunit.

I like this alternative. I would be fine with leaving it in 3.0, but I think that it should be low priority for the release. I think we should be concentrating on core-vision type issues for the 3.0 release so we can get it out there and widely used, and then start adding features.

from nunit.

OK, I'm making it low and leaving it in the 3.0 milestone. It could just as well end up pushed to 'future'.

from nunit.