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

Support .NET Core #313

Merged
merged 37 commits into from
May 3, 2017
Merged

Support .NET Core #313

merged 37 commits into from
May 3, 2017

Conversation

rprouse
Copy link
Member

@rprouse rprouse commented Apr 5, 2017

Do Not Merge

Fixes #297

Starting a PR so that people can easily find the most recent AppVeyor NuGet feed builds for testing. This produces a NuGet package. It currently does not produce a VSIX. I have also up'd the version to 4.0 because these are major changes. It isn't inline with our original plans for 4.0 which included running NUnit 2.x tests, so feel free to object to the version.

These changes require that the new dotnet CLI and the .NET SDK to be installed and only builds in Visual Studio 2017.

Work still to do,

  • Re-add .NET 3.5 support
  • Get the AppVeyor build working
  • Re-add the VSIX project and figure out how to multi-target it
  • Figure out why dotnet test is not working
  • Enable the VSTest tests in build.cake
  • Add .NET Core projects to the demo projects
  • Full CLR tests are no longer being discovered

@CharliePoole
Copy link
Contributor

My concern would be running tests targeting 2.0, 3.5, 4.0 under those runtimes. Current adapter does that. It looks like this will not be available, although I may be wrong. Unfortunately, it's not something the unit tests tell us.

@rprouse
Copy link
Member Author

rprouse commented Apr 9, 2017

Well, that was a lot of checkins getting this building and producing a NuGet package artifact, but this is now one step closer to being ready. I have added a list of remaining tasks that need to be dealt with before this PR is merged. The one I could use the most help with is figuring out why the dotnet test command isn't working.

@rprouse
Copy link
Member Author

rprouse commented Apr 18, 2017

dotnet test is now working at the command line and I've been able to run .NET Core and full .NET tests from VSTest, in Visual Studio and using dotnet test. Some additional work to do then we should be able to review this in earnest.

@brantburnett
Copy link

@rprouse

In your list of test projects above using different frameworks, I didn't see anything mentioning multi-targeting. I think that an NUnit test project that targets multi frameworks should probably be supported. If I'm writing a library that uses multi-targeting, I'd like the tests to run against each target as well.

In my initial testing, it does appear to work when run with "dotnet test". I just took a NUnit test project and changed from <TargetFramework>netcoreapp1.0</TargetFramework> to <TargetFrameworks>netcoreapp1.0;net46</TargetFrameworks>.

It's not working in VS, but I'm pretty sure VS is known to not support multi-targeting tests. So not much you can do about that.

Anyway, I just thought you might want to add this use case to your set of tests.

Thanks,
Brant

@DustinCampbell
Copy link

@brantburnett: In my experience, reordering the target frameworks so that net46 is first will allow them to run in VS.

@bernardbr
Copy link

bernardbr commented Apr 28, 2017

@brantburnett I use this when I need create an interpolable class library, for example, that will be run in both frameworks (NetCore and NetFramework).

But in my opinion, when do you need test different frameworks the best practice is create 2 projects with different target framework and run tests from there...

@rprouse
Copy link
Member Author

rprouse commented May 3, 2017

@OsirisTerje has finished his code review via email. Thank you @OsirisTerje.

Based on our conversations, we decided to revert back to version 3.8.0-alpha1. We both thought it would be confusing to have an NUnit 3 adapter with version 4.0, so decided to stick with our original plan of reserving 4.0 for when we merge the two adapter projects. There are also no major changes to the full .NET version of the adapter and it is not a breaking change, so according to semver, we probably shouldn't increment the major number yet.

If anyone else would like to review this, please do so today. I plan on merging this evening and doing an alpha release to NuGet. Before that, I am going to pull the recent changes and give them one last test and start writing some documentation on how to use this in .NET Core projects.

@rprouse rprouse merged commit 85a09d9 into master May 3, 2017
@rprouse rprouse deleted the issue-297 branch May 3, 2017 23:52
@bernardbr
Copy link

Thank you very much @rprouse !

@CurlyFire
Copy link

Thank you all for adding this feature!

@mikebeaton
Copy link

Hi, This is brilliant, many thanks!

I've just successfully run a set of tests on a project which dual targets netcoreapp1.1 and net45. It runs all the tests fine (256 tests on net45, 153 on netcoreapp1.1), and from a very quick look it supports debugging the selected tests etc. just as before too.

I did note that if I am using multi-targetting with, e.g. <TargetFrameworks>net45;netcoreapp1.1</TargetFrameworks> then the Visual Studio test explorer only seems to show and run the tests as for the first listed target?

I am on Visual Studio Professional 2017 15.0.0.0+26228.9

Also, I was previously building and targeting this project against net40 upwards, but it looks as if the dependency on Microsoft.NET.Test.Sdk v15.0.0 means that the lowest .NET Framework version I can test this way is net45, because if I try to target net40 I get:

Package Microsoft.TestPlatform.TestHost 15.0.0 is not compatible with net40 (.NETFramework,Version=v4.0). Package Microsoft.TestPlatform.TestHost 15.0.0 supports:
  - net45 (.NETFramework,Version=v4.5)
  - netstandard1.5 (.NETStandard,Version=v1.5)

@rprouse
Copy link
Member Author

rprouse commented May 5, 2017

@mikebeaton that does seem to be an unfortunate limitation of multi-targeting with the new CSPROJ format. AFAIK, Microsoft.NET.Test.Sdk is required to run tests against the new project format. Our adapter does work with older frameworks, but only using the old CSPROJ format.

image

@rprouse
Copy link
Member Author

rprouse commented May 5, 2017

For anyone who wants to start working with the new test adapter, I have written a blog post covering it's usage. Once everything settles down, I will migrate the core info into the docs, but for now, instructions are at http://www.alteridem.net/2017/05/04/test-net-core-nunit-vs2017/

@mikebeaton
Copy link

Found another slight quirk (I saw this last night, but thought it might help if I replicate it and report on it this morning).

If I have the 3.7 VSIX and the 3.8-alpha NuGet package both installed (I do realise this is a non-supported configuration, although I suppose it's quite easy to get there by mistake...!) then, when I test a netcoreapp1.1 target, it uses the 3.8 test engine (

------ Discover test started ------
NUnit Adapter 3.8.0.0: Test discovery starting
NUnit Adapter 3.8.0.0: Test discovery complete
========== Discover test finished: 137 found (0:00:01.1684465) ==========
------ Run test started ------
NUnit Adapter 3.8.0.0: Test execution started
Running all tests in C:\Projects\...\net.core.shared\tests\bin\Debug\netcoreapp1.1\...Tests.dll
NUnit3TestExecutor converted 137 of 137 NUnit test cases
NUnit Adapter 3.8.0.0: Test execution complete
========== Run test finished: 137 run (0:00:14.4474454) ==========

), but if I test a net45 target, it uses the 3.7 test engine (

------ Run test started ------
NUnit Adapter 3.7.0.0: Test execution started
Running all tests in C:\Projects\...\net.core.shared\tests\bin\Debug\net45\...Tests.dll
NUnit3TestExecutor converted 256 of 256 NUnit test cases
NUnit Adapter 3.7.0.0: Test execution complete
========== Run test finished: 256 run (0:00:23.7608298) ==========

).

These are each from exactly the same multi-targetted solution, using new format .csproj files throughout, so I'm not qute sure what's going on!

@mikebeaton
Copy link

mikebeaton commented May 5, 2017

@rprouse I think after all the hard (and very much appreciated!) work, you may have posted the wrong link above (?!), should be http://www.alteridem.net/2017/05/04/test-net-core-nunit-vs2017/ ? :)

@rprouse
Copy link
Member Author

rprouse commented May 5, 2017

@mikebeaton thanks for pointing out the bad link. I've fixed it in my comment.

@mikebeaton
Copy link

(Hi. I was still trying to understand what happened as reported in my comment 3 above this. I think if I'm understanding it correctly, then it is that NUnit 3.7 can support new .csproj files, as long as Microsoft.NET.Test.Sdk 15.0.0 is installed, but not for .NET Core? Sorry - I know this is not a bug or an issue as such!)

@rprouse
Copy link
Member Author

rprouse commented May 8, 2017

@mikebeaton when Visual Studio attempts to find tests in a project, it asks each installed VSIX and NuGet adapter in turn if they can run the tests and I believe it stops asking when it finds an adapter that works.

So, it looks like VS asks the VSIX first. For .NET Core tests, the 3.7 VSIX will not run the tests, so it moves on until the 3.8 NuGet package runs them. For .NET 4.5, the 3.7 VSIX can run them so it does and the 3.8 never gets a chance.

Unfortunately, VS doesn't see a NuGet adapter as newer than the installed VSIX. To VS, they are different things.

@OsirisTerje
Copy link
Member

I can confirm this, wrote about it back in 2013, https://blogs.msdn.microsoft.com/visualstudioalm/2013/06/11/part-3-unit-testing-with-traits-and-code-coverage-in-visual-studio-2012-using-the-tfs-build-and-the-new-nuget-adapter-approach/ saying

"One thing to be aware of is that the VSIX adapter takes precedence over the NuGet adapter, if you have both installed. Even if you update the NuGet adapter to a later version than the VSIX, the VSIX will be used. That means you might run on an earlier version of the adapter, whereas TFS Build and developers who don’t have the VSIX will use the newer NuGet version. "

Neither MSTest2 nor XUnit delivers their adapters as VSIX, and my guess (un-informed and unconfirmed) is VSIX test adapters are not going to get much support from MS moving forward. The recommended approach should be to add the nuget adapters to all solutions and uninstall the vsix from all devs machines.

@mikebeaton
Copy link

Dear both, Many thanks for confirmation of this. For me, it wasn't so much that the VSIX gets precedence, I can see from an engineering point of view why VS might not be expected to know that that the VSIX and NuGet versions are the same thing, but rather I wasn't expecting to see 3.7 running tests from new format .csproj files at all! :)

@rprouse
Copy link
Member Author

rprouse commented May 9, 2017

@mikebeaton other than the way that we package it, the full .NET version of the adapter in the alpha is the same as the 3.7 adapter, so it isn't too surprising that it works.

@mikebeaton
Copy link

Yes! It was more the parsing of the csproj file that I was surprised by, not the executing of the tests in .NET Framework - I think I'd got confused and thought that the issue being addressed by the update was support for the new csproj format as well as support for Core, but obviously not. :) Thanks again for all this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants