Looking to implement NUnit 2.6.x feature for MBUnit to NUnit Conversion

Right now, our plans don't include a 2.6.4 release. OTOH, if you want to
work on this issue, I'll be glad to review the changes and integrate them
into the source, where it would be available to anyone who builds from
source and would be in any release we eventually decided to put out. It's
more likely, however, that we would port your changes to NUnit 3.0 for
release.

Charlie

On Wed, Oct 30, 2013 at 7:56 AM, Danil Flores <
<email address hidden>> wrote:

> New question #238356 on NUnit Framework:
> https://answers.launchpad.net/nunit-3.0/+question/238356
>
> NUnit Team,
>
> As you may know, MBUnit is officially dead in the water and no longer
> maintained. Because of this, my company has decided to dedicate some
> resources to converting our existing MBUnit tests to NUnit and to use that
> going forward. I've been tasked with writing an automated tool to do this
> conversion since we have hundreds of thousands of test cases written with
> MBunit. My automated tool is able to do the job fine converting the MBUnit
> test patterns to their equivalents in NUnit, however, I've run into one
> snag in the way the [Row] attribute behaves vs the way the [TestCase]
> attribute behaves in it's handling of nullable types.
>
> I'm having the same issue as the poster of the bug
> https://bugs.launchpad.net/nunit-3.0/+bug/461721. We have many tests that
> are passing around nullable types from rows and were working before with
> MBUnit. I've been looking at the NUnit 2.6.3 source code and have found a
> way to implement this feature myself, but would really like to work with
> you guys if at all possible to get it into perhaps NUnit 2.6.4. So far this
> is the only snag I've run into with NUnit and the rest of the conversion
> has been going pretty smoothly.
>
> If I can get a working solution to this in the NUnit 2.6.x code base,
> would you guys be able to code review it, provide feedback, and eventually
> release it? Or if there is an existing work-around for it, would you guys
> please let me know?
>
>
> Thanks in advance,
> Danil M. Flores
> Ultimate Software - Software Engineer
>
> --
> You received this question notification because you are a member of
> NUnit Core Developers, which is an answer contact for NUnit Framework.
>
> _______________________________________________
> Mailing list: https://launchpad.net/~nunit-core
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~nunit-core
> More help : https://help.launchpad.net/ListHelp
>

Thanks for the quick response! That definitely works for me, I'll try to do some more testing around this and let you know once I have something tangible. One more question. Who maintains the Resharper integration for NUnit, you guys or the R# team? I've basically got this working with the NUnit GUI and console runner, but since the R# plugin has it's own method of generating test cases, if a test case is as follows:

[Test]
[TestCase(1, 2, 3)]
public void SomeTest(decimal? first, decimal? second, decimal? third) { ... }

Then the R# runner would generate two test cases:

SomeTest(1, 2, 3) <- State is inconclusive
SomeTest(1m, 2m, 3m) <- Actually runs

A full solution would probably have to take this into account in their test case builder as well, however, this isn't really a deal-breaker for us. We may just use it as is for now and count on the support in NUnit 3 for nullable rows.

R# uses - or says they use - NUnit itself to run tests. If so, it should
work the same. However, I think they may do some preprocessing to decide
what is a test.

For NUnit, I'd expect it to use the [TestCase] and ignore the [Test] in
your example, since [Test] essentially means "this is a test" but does not
generate any test cases in our current usage.

Maybe it would be a good idea - if we want to integrate this eventually -
for you to send me a note with some description of how you intend to fix
this. Would you treat a nullable argument the same as an optional argument?
My first assumption is that they are two different things.

Charlie

On Thu, Oct 31, 2013 at 7:11 AM, Danil Flores <
<email address hidden>> wrote:

> Question #238356 on NUnit Framework changed:
> https://answers.launchpad.net/nunit-3.0/+question/238356
>
> Danil Flores posted a new comment:
> Thanks for the quick response! That definitely works for me, I'll try to
> do some more testing around this and let you know once I have something
> tangible. One more question. Who maintains the Resharper integration for
> NUnit, you guys or the R# team? I've basically got this working with the
> NUnit GUI and console runner, but since the R# plugin has it's own
> method of generating test cases, if a test case is as follows:
>
> [Test]
> [TestCase(1, 2, 3)]
> public void SomeTest(decimal? first, decimal? second, decimal? third) {
> ... }
>
> Then the R# runner would generate two test cases:
>
> SomeTest(1, 2, 3) <- State is inconclusive
> SomeTest(1m, 2m, 3m) <- Actually runs
>
> A full solution would probably have to take this into account in their
> test case builder as well, however, this isn't really a deal-breaker for
> us. We may just use it as is for now and count on the support in NUnit 3
> for nullable rows.
>
> --
> You received this question notification because you are a member of
> NUnit Core Developers, which is an answer contact for NUnit Framework.
>
> _______________________________________________
> Mailing list: https://launchpad.net/~nunit-core
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~nunit-core
> More help : https://help.launchpad.net/ListHelp
>

The change itself is rather simple. There are two places in the code that use Convert.ChangeType(dataValue, targetMethodParameterType, ...); to attempt to convert convertible values into the actual type a parameter expects.

Convert.ChangeType(...) crashes when the type is a nullable of a type, so basically the algorithm is as follows:

1. First check if the target type is Nullable using Nullable.GetUnderlyingType(targetType) which just returns null if the item is not nullable.
2. There's a section of if statements that determine if the value is convertible to something by comparing the parameter type to the proposed value type. If the item is nullable, compare to the underlying type instead.
3. Call Convert.ChangeType(...) on either the nullabletype if not null or the target type.
4. If the target type is nullable, create a NullableConverter for it and convert the value from step 3. into a nullable of it.

Now I realize there's some conditional compilation symbols that I'd still need to add to make sure that the assembly can still be compiled for CLR 2.0, but here's the jist of it from NUnitCore\core\builders\TestCaseParameterProvider.cs in NUnit v2

private static void PerformSpecialConversions(object[] arglist, ParameterInfo[] parameters)
{
    // Rest of code is omitted for brevity, this starts on line 151 in TestCaseParameterProvider.cs
    Type nullableType = Nullable.GetUnderlyingType(targetType);
    bool convert = false;
    bool isNullable = nullableType != null;
    Type typeToCompare = nullableType ?? targetType;

    if (typeToCompare == typeof(short) || typeToCompare == typeof(byte) || typeToCompare == typeof(sbyte))
           convert = arg is int;
    else if (typeToCompare == typeof(decimal))
           convert = arg is double || arg is string || arg is int;
    else if (typeToCompare == typeof(DateTime) || typeToCompare == typeof(TimeSpan))
           convert = arg is string;

    if (convert)
    {
          arglist[i] = Convert.ChangeType(arg, nullableType ?? targetType, System.Globalization.CultureInfo.InvariantCulture);

          if (isNullable)
          {
                 NullableConverter converter = new NullableConverter(targetType);
                 arglist[i] = converter.ConvertFrom(arglist[i]);
          }
    }
}

There is a similar change to NUnitTestCaseBuilder as well to ensure that the types match up. Those were the only two areas that I found were doing the conversion, but feel free to let me know if I've missed something. Do you think this approach is the best way to go?

(By the way, not sure how this comment thread treats whitespace, so I apologize if the code snippet comes out poorly formatted).

Thanks!
Danil M. Flores

It looks like a good approach to me.

It looks as if you won't even be called unless the correct number of
arguments are provided, which is good. We may want to deal with default
arguments at some point in the future, but for now it's better to keep that
a separate issue.

Charlie

On Thu, Oct 31, 2013 at 11:31 AM, Danil Flores <
<email address hidden>> wrote:

> Question #238356 on NUnit Framework changed:
> https://answers.launchpad.net/nunit-3.0/+question/238356
>
> Danil Flores posted a new comment:
> The change itself is rather simple. There are two places in the code
> that use Convert.ChangeType(dataValue, targetMethodParameterType, ...);
> to attempt to convert convertible values into the actual type a
> parameter expects.
>
> Convert.ChangeType(...) crashes when the type is a nullable of a type,
> so basically the algorithm is as follows:
>
> 1. First check if the target type is Nullable using
> Nullable.GetUnderlyingType(targetType) which just returns null if the item
> is not nullable.
> 2. There's a section of if statements that determine if the value is
> convertible to something by comparing the parameter type to the proposed
> value type. If the item is nullable, compare to the underlying type instead.
> 3. Call Convert.ChangeType(...) on either the nullabletype if not null or
> the target type.
> 4. If the target type is nullable, create a NullableConverter for it and
> convert the value from step 3. into a nullable of it.
>
> Now I realize there's some conditional compilation symbols that I'd
> still need to add to make sure that the assembly can still be compiled
> for CLR 2.0, but here's the jist of it from
> NUnitCore\core\builders\TestCaseParameterProvider.cs in NUnit v2
>
> private static void PerformSpecialConversions(object[] arglist,
> ParameterInfo[] parameters)
> {
> // Rest of code is omitted for brevity, this starts on line 151 in
> TestCaseParameterProvider.cs
> Type nullableType = Nullable.GetUnderlyingType(targetType);
> bool convert = false;
> bool isNullable = nullableType != null;
> Type typeToCompare = nullableType ?? targetType;
>
> if (typeToCompare == typeof(short) || typeToCompare == typeof(byte) ||
> typeToCompare == typeof(sbyte))
> convert = arg is int;
> else if (typeToCompare == typeof(decimal))
> convert = arg is double || arg is string || arg is int;
> else if (typeToCompare == typeof(DateTime) || typeToCompare ==
> typeof(TimeSpan))
> convert = arg is string;
>
> if (convert)
> {
> arglist[i] = Convert.ChangeType(arg, nullableType ?? targetType,
> System.Globalization.CultureInfo.InvariantCulture);
>
> if (isNullable)
> {
> NullableConverter converter = new
> NullableConverter(targetType);
> arglist[i] = converter.ConvertFrom(arglist[i]);
> }
> }
> }
>
> There is a similar change to NUnitTestCaseBuilder as well to ensure that
> the types match up. Those were the only two areas that I found were
> doing the conversion, but feel free to let me know if I've missed
> something. Do you think this approach is the best way to go?
>
> (By the way, not sure how this comment thread treats whitespace, so I
> apologize if the code snippet comes out poorly formatted).
>
>
> Thanks!
> Danil M. Flores
>
> --
> You received this question notification because you are a member of
> NUnit Core Developers, which is an answer contact for NUnit Framework.
>
> _______________________________________________
> Mailing list: https://launchpad.net/~nunit-core
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~nunit-core
> More help : https://help.launchpad.net/ListHelp
>