Support for params keyword in parameterized test fixtures

[oliwennell]

Below is an example where the params keyword can be used in v2 to eliminate the need to have one constructor/TestFixtureSetUp/OneTimeSetUp for each TestFixture

    [TestFixture(1, 1)]
    [TestFixture(3, 1, 2)]
    [TestFixture(6, 1, 2, 3)]
    class Test
    {
        private readonly int total;
        private readonly int[] integers;

        public Test(int total, params int[] integers)
        {
            this.total = total;
            this.integers = integers;
        }

        [Test]
        public void Can_add_numbers()
        {
            Assert.That(integers.Sum(), Is.EqualTo(total));
        }
    }

Whilst this kind of test works in v2, when run in v3 (tried in version 3.2.1 of console runner) it seems an exception is thrown within NUnit:

1) Invalid : ConsoleApplication1.Test(1,1)
No suitable constructor was found

2) Invalid : ConsoleApplication1.Test(3,1,2)
No suitable constructor was found

3) Invalid : ConsoleApplication1.Test(6,1,2,3)
No suitable constructor was found

Related discussion:
https://groups.google.com/forum/#!topic/nunit-discuss/iPuVKU6bFws

[CharliePoole]

I called this a bug although it could arguably be considered a new feature. In any case it fits with our plans to make test fixture parameters work similarly to test case parameters, so I gave it high priority.

[CharliePoole]

It wouldn't be an easy fix except that the code already exists for methods and should be able to be ported.

[rvignesh89]

@CharliePoole I'd like to help with this issue. I looked at the code to figure out where the change is. Here is what I've found. Let me know if I'm looking in the right direction.

As you mentioned that this functionality exists in test case I started with the search there and found this in TestCaseAttribute.GetParametersForTestCase() class.

if (lastParameterType.IsArray && lastParameter.IsDefined<ParamArrayAttribute>(false))
{
   ...
}

My guess is that there should be a similar checking done at TypeWrapper.GetConstructor()?

public ConstructorInfo GetConstructor(Type[] argTypes)
{
    return Type.GetConstructors()
        .Where(c => c.GetParameters().ParametersMatch(argTypes))
         .FirstOrDefault();
}

[ChrisMaddock]

@rvignesh89 This issue is closely tied to a PR currently in progress - #1850. I imagine how that implementation is made will affect what's necessary here, might just be worth holding on till that ones been discussed, before doing too much work here. 🙂

[rvignesh89]

@ChrisMaddock Just caught up with the other issue thread. Seems like multiple Attributes have this problem. Thanks for letting me know! I'll wait and see how this is fixed 👍

[BelgiAmir]

@CharliePoole - I noticed that this issue is still not resolved and I have looked at this issue the last couple of days. I have managed to get some progress done and I would like your opinion:

It seem that the problem can't be resolved with modification to TypwWrapper GetConstructor method, since the Invoke method that is applied by the Reflection class will throw an exception, even if the correct Ctor is returned. This is because the compiler considers the params parameter as one parameter of type of an array, and will say that there is a count mismatch.

I thought of couple of possible solutions:

1. Use the Activator. CreateInstance seems to do the trick, it is supported on all platforms except dot net standard which seems reasonable
2. I could change the call to the invoke method on the ctor in a way that splits the object[] passed into the parameters preceeding the params, and an array which contains all parameters which are part of the params. This would be more cumbersome and would require a few more lines of code, but should work on all platforms.

What do you think?

[CharliePoole]

It should use similar code to that which we use for params args passed to test methods. Essentially, we analyze the provided args and transform the final args into a single one of the correct array type. This is similar to your second option.

That code is currently found as part of TestCaseAttribute. It can probably be reused with some changes. It's OK to create some duplication of code here - we have some plans that will result in removing it at a later point.

[ChrisMaddock]

See #2268 for a more recent update on this issue.

[Parshudar]

Hi. I'm interested in helping out with this issue. I see this issue is of high priority and is still open. If this issue is unassigned, I would like to start working on a solution for this

[jnm2]

@Parshudar Hello! We'd be happy for the help. Let us know if you're still interested and we'll assign to you.

[rnemeth90]

Is anyone working on this? If not, I'd like to.

[stevenaw]

Welcome @rnemeth90 ! Apologies for the late reply. It looks like this may've stalled out with a previous contributor. You are more than welcome to work on this.

[Shiney]

I've created a PR for this one, only really solved this problem directly rather than sorting out sharing code with the code that calls this for test methods.

[CharliePoole]

@Shiney IMO, that's cool! We're told to "remove duplication." That means we have to create it first. :-)

[OsirisTerje]

@Shiney Could you add information on this to the documentation for the Testfixture ?

You find the source for the docs here: https://github.com/nunit/docs/blob/master/docs/articles/nunit/writing-tests/attributes/testfixture.md

The code snippet referred to is here: https://github.com/nunit/docs/blob/master/docs/snippets/Snippets.NUnit/Attributes/TestFixtureAttributeExamples.cs

Add the params as a new way of doing the same, thus saving on constructors.

[Shiney]

Okay I've done this

[OsirisTerje]

I see the code, but not any changes to the markdown file, did you miss to include it ?

[Shiney]

Whoops ahng on

…

On Sun, Oct 15, 2023 at 3:19 PM Terje Sandstrom ***@***.***> wrote: I see the code, but not any changes to the markdown file, did you miss to include it ? — Reply to this email directly, view it on GitHub <#1459 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHTNWV2MAOVXWIOIISH3XLX7PWGVAVCNFSM4CB6WVL2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCNZWGM2DAMZZHEYQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>