Make dotnet build work without having NetFX installed

[joaopgrassi]

• Added a reference to Microsoft.NETFramework.ReferenceAssemblies
• Small re-organization on .csproj file to easily identify the supported target frameworks
• Add a custom property TargetIsNetFx to easily identify when netfx is being used.

Tested on WSL (Ubuntu 18.04):

dotnet build

 Restore completed in 16.67 ms for /mnt/d/github/NSubstitute/src/NSubstitute/NSubstitute.csproj.
  NSubstitute -> /mnt/d/github/NSubstitute/bin/Debug/NSubstitute/net45/NSubstitute.dll
  NSubstitute -> /mnt/d/github/NSubstitute/bin/Debug/NSubstitute/net46/NSubstitute.dll
  NSubstitute -> /mnt/d/github/NSubstitute/bin/Debug/NSubstitute/netstandard2.0/NSubstitute.dll
  NSubstitute -> /mnt/d/github/NSubstitute/bin/Debug/NSubstitute/netstandard1.3/NSubstitute.dll

Build succeeded.
    0 Warning(s)                                                                                                                                                                                                     0 Error(s)

dotnet build -f net45

 Restore completed in 16.82 ms for /mnt/d/github/NSubstitute/src/NSubstitute/NSubstitute.csproj.
  NSubstitute -> /mnt/d/github/NSubstitute/bin/Debug/NSubstitute/net45/NSubstitute.dll

Build succeeded.
    0 Warning(s)                                                                                                                                                                                                     0 Error(s)

dotnet build -f net46

 Restore completed in 16.84 ms for /mnt/d/github/NSubstitute/src/NSubstitute/NSubstitute.csproj.
  NSubstitute -> /mnt/d/github/NSubstitute/bin/Debug/NSubstitute/net46/NSubstitute.dll

Build succeeded.
    0 Warning(s)                                                                                                                                                                                                     0 Error(s)

As a side note: I think it would be valuable to introuce a Directory.Build.props file in order to improve the overall organization of the csprojs files. I took a quick look and I saw there's some repetition in a few things, like:

'$(TargetFramework)' == 'net45' OR '$(TargetFramework)'=='net46'" or <LangVersion>latest</LangVersion>. These properties can all be defined once in the top-level .props file and all projects would get these values. This way, the individual csproj is even smaller and focused on what they should be focused on. I would be glad to work on it, in case you think it's relevant 😊

Closes #335

[dtchepak]

Hi @joaopgrassi ,

This is awesome! Thanks! Tested on Mac and compiles main project. 👍

I've sent a PR to add a commit that applies this change to the other csproj files. Unfortunately running ./build/build.sh fails for netfx targets for a few tests, but it runs for .net core app targets so I think it a big step forward!

If you're happy with that PR, please add it and then I'll merge them both into the main repo.
The Directory.Build.props stuff looks great too, so if you're happy to work on that as well that would be great! 😄

PS: If you've got any ideas for fixing the build.sh failures please let me know! I think it's just some odd mismatch with the reference assemblies, so maybe we can just #if TargetIsNetFx them. For me it currently gives me errors like:

  X ShouldCorrectlyConfigureProperty [< 1ms]
  Error Message:
   Castle.DynamicProxy.InvalidProxyConstructorArgumentsException : Can not instantiate proxy of class: TypeWithMissingSpecialAttributes.
Could not find a parameterless constructor.
  Stack Trace:
    at Castle.DynamicProxy.ProxyGenerator.CreateClassProxyInstance (System.Type proxyType, System.Collections.Generic.List`1[T] proxyArguments, System.Type classToProxy, System.Object[] constructorArguments) [0x0009a] in <1ffeacebfa574008a73b2273fdaf6e88>:0
...

[joaopgrassi]

Hey @dtchepak

Great! I didn't want to touch the test projects before getting the green light. I can gladly add your changes to this PR as well. Is that what you meant here?:

If you're happy with that PR, please add it and then I'll merge them both into the main repo.

I also notice that the project NSubstitute.Specs.csproj is not in the new SDK Style, and I think it didn't show up when I opened the solution file. Maybe another PR to fix it?

PS: If you've got any ideas for fixing the build.sh failures please let me know!

Sure! I will take a look when I get home and see what I can find.

[dtchepak]

@joaopgrassi

I can gladly add your changes to this PR as well. Is that what you meant here?

Yes, it was just re-applying your change to the other csproj files so I didn't want to make repeat work for you when I could just push up the commit. :)

The NSubstitute.Specs.csproj is obsolete so don't include that. It hasn't been deleted as there are a couple of tests I'd like to port over to the main test project. I've delayed this for years though so I'll probably end up giving up and deleting it at some point, but today is not that day. 😂

[joaopgrassi]

Got it. I'll update this later today with the changes on the test projects :)

It hasn't been deleted as there are a couple of tests I'd like to port over to the main test project.

If you can pinpoint me to these tests, I can work on porting them :). Also, I will work then on the props files suggestion. I guess it makes sense to create issues for these two things?

[dtchepak]

@joaopgrassi Merged, thanks! 👍

If you can pinpoint me to these tests, I can work on porting them :).

Thanks for the offer! I'll take a look through when I get a chance.

Also, I will work then on the props files suggestion. I guess it makes sense to create issues for these two things?

Sounds good. 👍 Thanks for the help. 😄

[joaopgrassi]

@dtchepak pushed again now with the additions also for the test projects. It should be good to go now.

Also: I took a brief look at the error while running the tests on linux (using mono). I also got the same as you mentioned.

Out of curiosity I did this: var ctor = typeBuilder.DefineDefaultConstructor(MethodAttributes.Public); inside GenerateTypeWithMissingSpecialNameAttributes() then I got a different issue:

Error Message: System.Reflection.TargetInvocationException : Exception has been thrown by the target of an invocation.
  ----> NSubstitute.Exceptions.CouldNotRaiseEventException : 
Make sure you are using Raise.Event() as part of an event subscription on a substitute.
For example:
        mySub.Clicked += Raise.Event();

If you substituted for a class rather than an interface, check that the event on your substitute is virtual/abstract.   
Events on classes cannot be raised if they are not declared virtual or abstract.

Note that the source of the problem may be prior to where this exception was thrown (possibly in a previous test!). For example:
        var notASub = new Button();
        notASub.Clicked += Raise.Event(); // <-- Problem here. This is not a substitute.
        var sub = Substitute.For<IController>();
        sub.Load(); // <-- Exception thrown here. NSubstitute thinks the earlier Raise.Event() was meant for this call.

Since the tests pass on .NET Core, and we don't run them on NetFX on CI builds, I'm not sure if we should investigate more. Maybe just skipping them as you said?

[dtchepak]

Yeh I think we'll just need to skip them for netfx on non-Windows. Maybe with a comment on the PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" to revisit these tests once the reference assemblies get updated?