Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValuesAttribute uses an outdated version of TestCaseAttribute's type coercion logic #2638

Closed
jnm2 opened this issue Dec 20, 2017 · 3 comments

Comments

@jnm2
Copy link
Contributor

jnm2 commented Dec 20, 2017

As part of fixing #2630, I copied many tests from TestCaseAttributeTests to ValueAttributeTests and had to remove them again because they failed.

[TestCase(1)]
public void CanConvertIntToNullableLong(long? x)
{
Assert.That(x.HasValue);
Assert.That(x.Value, Is.EqualTo(1));
}

When translated to ValuesAttribute:

[Test]
public void CanConvertIntToNullableLong([Values(1)] long? x)
{
    Assert.That(x.HasValue);
    Assert.That(x.Value, Is.EqualTo(1));
}

Gives:

System.ArgumentException : Object of type 'System.Int32' cannot be converted to type 'System.Nullable`1[System.Int64]'.
   at System.RuntimeType.CheckValue(Object value, Binder binder, CultureInfo culture, BindingFlags invokeAttr)
   at System.Reflection.MethodBase.CheckArguments(Object[] parameters, Binder binder, BindingFlags invokeAttr, CultureInfo culture, Signature sig)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture, Boolean skipVisibilityChecks)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at NUnit.Framework.Internal.Reflect.InvokeMethod(MethodInfo method, Object fixture, Object[] args)

Before @ChrisMaddock brought #2104 to my attention, I had already fixed this one: https://github.com/nunit/nunit/compare/jnm2/values_type_coercion

I'm just going to leave this here until I have time to dig into #2104. #2104 looks huge and I don't want to jeopardize it, though perhaps it could be done in granular chunks.

Please comment.

@jnm2 jnm2 self-assigned this Dec 20, 2017
@ChrisMaddock
Copy link
Member

I agree with everything you've said. Let's discuss #2104 over on #2104. 😄

@stevenaw
Copy link
Member

I think this was incidentally fixed when resolving #3053

@stevenaw
Copy link
Member

stevenaw commented Feb 6, 2021

I'm closing this issue. I believe it was fixed via #3053 and covered via a test:

public void CanConvertIntsToNullableLong([Values(5, int.MaxValue)]long? x)

@stevenaw stevenaw closed this as completed Feb 6, 2021
@mikkelbu mikkelbu added this to the Closed Without Action milestone Feb 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants