Add explicit generic type specification to TestCase #4620

stevenaw · 2024-02-15T13:10:10Z

Adds support for explicitly specifying a generic type for a test case method. I've chosen to keep it simple and let the runtime itself ensure that cases like this work or not:

[TestCase("2", TypeArgs = new[] { typeof(int) })]
public void ExplicitTypeArgsWithIncompatibleParameters<T>(T input)

I'd like to hear thoughts on if we wanted to allow the framework to eagerly detect and mark these as NonRunnable. My own suggestion is to propose we defer it to another PR

stevenaw · 2024-02-15T13:12:03Z

global.json

@@ -2,6 +2,6 @@
  "sdk": {
    "version": "8.0.100",
    "allowPrerelease": false,
-    "rollForward": "disable"
+    "rollForward": "latestFeature"


Unrelated to the issue at hand, but I needed to change this to be able to build under the following configuration:

You can set "rollForward": "latestPatch" to allow 8.0.1xx.
Now it also allows 8.0.2xx and MS is has in the past broken things when adding new "features". (6.0.300)
But I haven't found anything breaking with 8.0.200

Thanks! I hadn't realized there was a precedent there.

stevenaw · 2024-02-15T22:18:33Z

src/NUnitFramework/framework/Attributes/TestCaseAttribute.cs

@@ -247,6 +247,11 @@ public bool Explicit
        /// </summary>
        public string? ExcludePlatform { get; set; }

+        /// <summary>
+        /// Comma-delimited list of platforms to run the test for


Note to self: Fix this copy/paste error

Yes, copy it from TestFixtureAttribute's TypeArgs property instead.

manfred-brands

See comments requesting a practical example where current code without TypeArgs won't work, but it does now.

manfred-brands · 2024-02-16T00:17:11Z

src/NUnitFramework/framework/Attributes/TestCaseAttribute.cs

@@ -247,6 +247,11 @@ public bool Explicit
        /// </summary>
        public string? ExcludePlatform { get; set; }

+        /// <summary>
+        /// Comma-delimited list of platforms to run the test for


Yes, copy it from TestFixtureAttribute's TypeArgs property instead.

manfred-brands · 2024-02-16T00:22:32Z

src/NUnitFramework/testdata/TestCaseAttributeFixture.cs

@@ -114,5 +114,10 @@ public void MethodWithExcludeRuntime(int num)
        public void MethodWithArrayArguments(object o)
        {
        }
+
+        [TestCase("doesn't work", TypeArgs = new[] { typeof(int) })]


I feel an Analyzer rule request coming up.

True, analyzers are a great validator for what would otherwise be runtime checks. What do you think about also having a generic attribute to extend this on modern (NET8+) runtimes after we get the framework targeting that TFM?

Do we really need to wait for .NET8? It is a C#11 feature and it seems to work in .NET6.0 but fails in .NET Framework. with a runtime error: Generic types are not valid

/// <summary> /// Marks a method as a parameterized test suite and provides arguments for each test case. /// </summary> [AttributeUsage(AttributeTargets.Method, AllowMultiple = true, Inherited = false)] public class TestCaseAttribute<T> : TestCaseAttribute { /// <summary> /// Construct a TestCaseAttribute with a list of arguments. /// </summary> public TestCaseAttribute(T argument) : base(new object?[] { argument }) { TypeArgs = new[] { typeof(T) }; } }

And equivalent version with 2, 3, 4, etc. parameters.

With a usage :

[TestCase(2, TypeArgs = new[] { typeof(double) }, ExpectedResult = typeof(double))] [TestCase<double>(2, ExpectedResult = typeof(double))] public Type GenericMethodAndParameterWithExplicitOrImplicitTyping<T>(T input) => typeof(T);

manfred-brands · 2024-02-16T00:24:22Z

src/NUnitFramework/tests/Attributes/TestCaseAttributeTests.cs

+            Assert.That(typeof(T), Is.EqualTo(typeof(int)));
+        }
+
+        [TestCase(2, TypeArgs = new[] { typeof(int) })]


Doesn't this already without having to add TypeArgs as the TryGetTypeArguments determines the types from the actual parameters?

Maybe add a [TestCase(2)] as well to proof it still works.

src/NUnitFramework/tests/Attributes/TestCaseAttributeTests.cs

stevenaw · 2024-02-18T00:25:10Z

Thanks for syncing your changes here @OsirisTerje

@manfred-brands as I've been revisiting the test scenarios, I've realized it's likely very minimal extra work to make this work for TestCaseSource - something Charlie had also originally advocated for in the issue.
I have had a few more pockets of time than expected, so I think the next push will likely include support for TestCaseSourceAttribute as well.

stevenaw · 2024-02-18T02:00:28Z

src/NUnitFramework/framework/Internal/TestCaseParameters.cs

+        /// If not set explicitly, the generic types will be inferred
+        /// based on the test case parameters.
+        /// </summary>
+        public Type[]? TypeArgs { get; set; } = null;


I was surprised at just how little was required to make it work for TestCaseSource after TestCase was done. If I had made this public originally instead of private then it would've "just worked"

src/NUnitFramework/tests/Attributes/TestCaseSourceTests.cs

manfred-brands

Some of the tests are wrong and asserting failures that are the result of that instead of what really should be happening.

src/NUnitFramework/testdata/TestCaseSourceAttributeFixture.cs

manfred-brands · 2024-02-18T03:39:04Z

src/NUnitFramework/tests/Attributes/TestCaseSourceTests.cs

+            var exception = Assert.Throws<NUnitException>(() => test.Method!.Invoke(test.Parent, test.Arguments));
+
+            Assert.That(exception.InnerException, Has.Message.EqualTo("Object does not match target type."));


This is wrong. The test now fails because test.Parent is of type ParameterizedMethodSuite and not TestCaseSourceAttributeFixture
The error talks about target type which is the this argument, not about parameters.
You have to call TestBuilder.RunTest and check the results which shows that the test fails.
However, in case of mismatch of type arguments, the test is marked as non-runnable as you don't have to run it at al.

Sorry, maybe there's been a misunderstanding on my end. I like your suggestion to use RunTest() as it avoids the need to explicitly check exception status. I'll push some changes shortly.

However, in case of mismatch of type arguments, the test is marked as non-runnable as you don't have to run it at al.

I may still not be understanding this suggestion. Are you saying that mismatched values and generic types should fail earlier than test execution time? If so, I think I'm having a hard time reconciling that with "let the (dotnet) runtime sort it out" when TypeArgs are explicitly passed.

Are you saying that mismatched values and generic types should fail earlier than test execution time

Yes. If NUnit cannot determine the TypeArgs. See NUnitTestCaseBuilder. If NUnit cannot determine the TypeArguments to use, the test is not runnable.

"let the (dotnet) runtime sort it out" when TypeArgs are explicitly passed.

If TypeArgs is passed, the above TryGetTypeArguments is not called and NUnit first try to do some conversion in TypeHelper.ConvertArgumentList but after that passes it on to the runtime. The conversions done by NUnit are the ones normally done by the compiler when passing an int to a double parameter.

src/NUnitFramework/tests/Attributes/TestCaseSourceTests.cs

src/NUnitFramework/tests/Attributes/TestCaseAttributeTests.cs

…ine if it fails to run

manfred-brands

Changes and tests look fine now.
I'll leave it up to you if you want to add the Generic TestCaseAttributes

stevenaw · 2024-02-19T14:35:09Z

Thanks @manfred-brands !
That's great to hear it'll also work with NET 6. I'd thought it needed runtime support but happy to see that may not be the case.
If you're alright with it, I'd like to tackle generic attribute support in a separate issue rather than rushing into this one in a potentially half-baked manner.

I'm going to merge this now for release -related timing (FYI @OsirisTerje ) and will create those follow up issues later today

stevenaw commented Feb 15, 2024

View reviewed changes

Initial implementation of TestCase with TypeArgs

93355d8

stevenaw force-pushed the 1215-generic-test-case branch from 39e9e95 to 93355d8 Compare February 15, 2024 13:18

stevenaw commented Feb 15, 2024

View reviewed changes

manfred-brands requested changes Feb 16, 2024

View reviewed changes

Merge branch 'master' into 1215-generic-test-case

9a5dcf0

stevenaw added 2 commits February 17, 2024 19:59

Respond to feedback

4d37b4c

TestCaseSource support + tests

9343574

stevenaw commented Feb 18, 2024

View reviewed changes

src/NUnitFramework/tests/Attributes/TestCaseSourceTests.cs Show resolved Hide resolved

Make it fail when TypeArgs omitted

83fe073

manfred-brands requested changes Feb 18, 2024

View reviewed changes

Fix type issues in expected failure test. Rely on nunit API to determ…

74467e1

…ine if it fails to run

manfred-brands approved these changes Feb 19, 2024

View reviewed changes

stevenaw merged commit 829e304 into nunit:master Feb 19, 2024
5 checks passed

stevenaw mentioned this pull request Feb 19, 2024

Generic TestCase and TestCaseParameter support #4632

Open

stevenaw mentioned this pull request Feb 29, 2024

How to make TestCaseGenericAttribute and TestCaseGenericSourceAttribute? #3580

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add explicit generic type specification to TestCase #4620

Add explicit generic type specification to TestCase #4620

stevenaw commented Feb 15, 2024 •

edited

stevenaw Feb 15, 2024

manfred-brands Feb 16, 2024

stevenaw Feb 17, 2024

stevenaw Feb 15, 2024

manfred-brands Feb 16, 2024

manfred-brands left a comment

manfred-brands Feb 16, 2024

manfred-brands Feb 16, 2024

stevenaw Feb 18, 2024 •

edited

manfred-brands Feb 19, 2024

manfred-brands Feb 16, 2024 •

edited

stevenaw commented Feb 18, 2024 •

edited

stevenaw Feb 18, 2024

manfred-brands left a comment

manfred-brands Feb 18, 2024

stevenaw Feb 18, 2024 •

edited

manfred-brands Feb 19, 2024

manfred-brands left a comment

stevenaw commented Feb 19, 2024

		var exception = Assert.Throws<NUnitException>(() => test.Method!.Invoke(test.Parent, test.Arguments));

		Assert.That(exception.InnerException, Has.Message.EqualTo("Object does not match target type."));

Add explicit generic type specification to TestCase #4620

Add explicit generic type specification to TestCase #4620

Conversation

stevenaw commented Feb 15, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manfred-brands left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenaw Feb 18, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manfred-brands Feb 16, 2024 • edited

Choose a reason for hiding this comment

stevenaw commented Feb 18, 2024 • edited

Choose a reason for hiding this comment

manfred-brands left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenaw Feb 18, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manfred-brands left a comment

Choose a reason for hiding this comment

stevenaw commented Feb 19, 2024

stevenaw commented Feb 15, 2024 •

edited

stevenaw Feb 18, 2024 •

edited

manfred-brands Feb 16, 2024 •

edited

stevenaw commented Feb 18, 2024 •

edited

stevenaw Feb 18, 2024 •

edited