Switch to Verify for API approval test

[bording]

This switches a different approval library for the API approval test. Verify doesn't require special project settings to work in all scenarios like ApprovalTests does.

Fixes #815

[michaelklishin]

Thank you. On MacOS, the .NET 4.6.1 version fails with

dotnet test projects/Unit --filter "FullyQualifiedName~RabbitMQ.Client.Unit.APIApproval"                                                                                                                                Test run for /Users/antares/Development/RabbitMQ/umbrella.git/deps/rabbitmq_dotnet_client/projects/Unit/bin/Debug/net461/Unit.dll(.NETFramework,Version=v4.6.1)
Microsoft (R) Test Execution Command Line Tool Version 16.3.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

A total of 1 test files matched the specified pattern.
  X Approve [382ms]
  Error Message:
   System.TypeInitializationException : The type initializer for 'Verify.SharedVerifySettings' threw an exception.
  ----> System.TypeInitializationException : The type initializer for 'Verify.Namer' threw an exception.
  ----> System.Exception : Could not resolve runtime for 'Mono 6.8.0.105 (tarball Tue Feb  4 20:29:35 GMT 2020)'.
  Stack Trace:
    at RabbitMQ.Client.Unit.APIApproval.Approve () [0x00032] in <0736cef46ef144f48e43a2dea3523e23>:0
  at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
  at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0006a] in <b6f643ae6ccb410db69142d5eaf9b9c1>:0
--TypeInitializationException
  at (wrapper managed-to-native) System.Object.__icall_wrapper_mono_generic_class_init(intptr)
  at Verify.SharedVerifySettings..cctor () [0x0000a] in <b6c99d0af76340b597c1067a1db4155d>:0
--Exception
  at Verify.Namer.GetRuntimeAndVersion () [0x00064] in <b6c99d0af76340b597c1067a1db4155d>:0
  at Verify.Namer..cctor () [0x00000] in <b6c99d0af76340b597c1067a1db4155d>:0

Test Run Failed.
Total tests: 1
     Failed: 1

while the .NET Core 3.1 target passes. Should we make this approval test specific to .NET Core to avoid such issues? It would still serve its purpose.

[bording]

Thank you. On MacOS, the .NET 4.6.1 version fails with

You really shouldn't be trying to run the 4.6.1 tests on non-Windows OSes. That will try to run them with mono, and who knows if that will work.

[michaelklishin]

Fair enough. I have added a pragma to skip this test. If we run into more Mono-specific issues we can consider doing this for all tests. I'm not sure what would be the best way to do that with NUnit, though.

[bording]

Please don't skip the test on 4.6.1, it's important it runs on both.

You just need to not run the 4.6.1 tests when you're not on Windows.

[michaelklishin]

@bording OK, how do I do that? dotnet test currently runs them against both targets.

[bording]

Should we make this approval test specific to .NET Core to avoid such issues? It would still serve its purpose.

Since there are separate assemblies for each target framework, it's actually possible that there could be unintentional differences in the public API surface, for example through #defines being introduced. That's why it's important to run the test against both assemblies to make sure they are the same.

@bording OK, how do I do that? dotnet test currently runs them against both targets.

dotnet test -f netcoreapp3.1

[michaelklishin]

@bording I will skip the test only on Mono then. Adding -f netcoreapp3.1 is something our team can keep in mind but we need to keep dotnet test passing [when there are no actual behavior changes) for external contributors. How does that sound?

[bording]

@bording I will skip the test only on Mono then

Why do you care about mono at all at this point? It's not clear to me why you'd even be attempting to run any tests on mono now.

Are you saying that you have been running the test suite with mono previously?

[lukebakken]

Are you saying that you have been running the test suite with mono previously?

Yes, prior to all of the changes for 5.2.0 / 6.0.0 when net451 was one of the target frameworks. But now, Mono should not be used anywhere (@michaelklishin). I'll double-check on my OS X machine. I think we can remove anything Mono-specific in the codebase. I'll open a PR for that.

[michaelklishin]

I use .NET Core on MacOS. Windows is not the primary development environment for our team (although we obvious test on it).

How can I run tests on .NET 4.6.1 on MacOS?

[bording]

How can I run tests on .NET 4.6.1 on MacOS?

That's the point I'm trying to make. There is no compelling reason to try and run .NET Framework tests on non-Windows operating systems now that .NET Core exists.

[michaelklishin]

OK, then it makes sense to exclude them and only run against .NET Core (which has been the case until we went multi-target recently?).

[bording]

OK, then it makes sense to exclude them and only run against .NET Core (which has been the case until we went multi-target recently?).

What do you mean by exclude them? You still need to run them on Windows.

What problem are you trying to solve exactly? It's not clear to me what you're trying to do.

[michaelklishin]

dotnet test currently executes tests twice on MacOS, once for every target. It would be nice to only have a single .NET Core run everywhere other than Windows.

[bording]

That's why you should use the -f parameter I pointed out. You can add that to a script if you don't want to type it all the time.

[bording]

Actually, that's already in the run-test.sh script.

[lukebakken]

Thanks for pointing that out @bording

@michaelklishin we can expand run-test.sh to pass on arguments to dotnet test to allow for filtering or what-not

[michaelklishin]

I see, so I have to use -f all the time. That's fine, no need to modify run-test.sh. I have amended the examples to use -f so that they do the same thing on all platforms. Hopefully .NET Core alone should provide sufficiently good coverage most of the time.