Refactor how test case metadata is obtained.

[mrvoorhe]

We are updating our Unity.Linker.Tests project to target net5. This means our NUnit tests will run on net5. The catch is, we will still be running our linker on assemblies compiled against mono based profiles. This is the first time there has been a difference between the target framework of the unit tests and the test cases.

This presents us with a problem. The TestCaseCollector is running in Mono.Linker.Tests (which will be targeting net5 for us) this means that it's reference to Mono.Linker.Tests.Cases is also on net5. This leads to TestCaseMetadataProvider giving back attribute information from a NETCOREAPP compiled assembly. Causing our tests to run with the NETCOREAPP behavior and that's not what we want.

This PR does a few things to make the test framework more flexible to this new need.

• Move compilation related methods from TestCaseMetadataProvider to TestCaseCompilationMetadataProvider.

• TestCaseCompilationMetadataProvider provides metadata from the solution compiled test case assembly. This is the only build available to bootstrap the test case building process. This means that attributes related to this metadata should not use #if's to control when they are defined.

• TestCaseMetadataProvider - Will contain linker related metadata and test framework asserting metadata. These attributes can continue to use #if's. This metadata provider will now obtain it's information from the expectations version of the test case assembly after it has been compiled in the sandbox.

• A new [TestCaseRequirements] attribute has been added. This allows a test case to declare the requirements that must be met by the driving test framework in order for the test to run.

• [IgnoreTestCase] usages inside #if's have been changed to use [TestCaseRequirements]

• TestRunCharacteristics.SupportsDefaultInterfaceMethods was added and default interface methods were placed behind this instead of NETCOREAPP. This allows us to turn these tests on since our mono supports default interface methods now.

• TestRunCharacteristics.SupportsStaticInterfaceMethods was added and static interface methods were placed behind this. This is consistent with SupportsDefaultInterfaceMethods and leaves the door open to us turning these on if we ever support this.

[marek-safar]

I don't like this mode. Can such tests be fixed or files excluded in your build?

[mrvoorhe]

Good point. Technically, it would be nice we could run the tests that are currently using this.

I was able to get these tests running. It took 2 changes.

1. I hardcoded some hack logic in our UnityTestCaseCompiler to tweak the references when we see illink.dll appear. I don't see a clean way to handle this. I can live with the hack in our test framework.

2. I need to change Driver.FindStep in order to get SubStepDispatcherUsage passing. It tries to add a step before MarkStep, however, we have a UnityMarkStep which inherits from MarkStep. I thought a nice way to deal with this would be to change FindStep to look like.

		static IStep FindStep (Pipeline pipeline, string name)
		{
			foreach (IStep step in pipeline.GetSteps ()) {
				Type t = step.GetType ();
				while (t.BaseType != null) {
					if (t.Name == name)
						return step;
					t = t.BaseType;
				}
			}

			return null;
		}

Are you OK with that? If so, I can update the PR with the necessary changes.

Alternatively, I can make FindStep virtual and then override it in our linker.

[marek-safar]

Alternatively, I can make FindStep virtual and then override it in our linker.

That's my preference too

[mrvoorhe]

Done.

[marek-safar]

Does --exclude-feature have value for you? If not I'd suggest to delete the tests

[mrvoorhe]

We still use it for etw and com.

[marek-safar]

Why is #if needed?

[mrvoorhe]

It looks like the #if NETCOREAPP was added because this code wouldn't compile against .NET Framework. There are types such as IDynamicInterfaceCastable that don't exist.

We haven't pulled this test into our fork yet. And it looks like removing all of the #ifs in this file wouldn't cause us any issues if we did have this test.

Is it OK if we leave these #ifs in the PR and you can remove them later if you don't want them? I don't care either way. I mostly want to avoid having this PR held up longer.

[marek-safar]

I don't think this has much to do with DIM as it's simply API available since net5. Would changing the define to NET_5_0 work for you?

[mrvoorhe]

I was thinking about this more last night and you're right, this is really specific to net5, not DIM. I'll fix it.

[mrvoorhe]

I changed it back to NETCOREAPP instead of NET_5_0.

[mrvoorhe]

@marek-safar should be good to go.