Integrate Omnisharp in VSCode Testing API

[PascalSenn]

Integrate Omnisharp in VSCode Testing API

• MultiTarget support
• Multi Test Framework Support (currently only supports Xunit)
• Run Tests
• Debug Tests
• Cover Tests
• Wire up the code lens with the new testing provider
• Print the output of the test only in the test console
• Remove the old behaviour of forcing the user to see the test output
• What do we do with parallel testing outputs, could we extend O# with a id of the session?
• Investigate partial test results from O# (for performance reasons a whole assembly is executed at the time, it would be nice to still be able to see the individual tests pass, we basically need the assembly name, the target framework and the fullqualified name to map the results back)
• Organize assemblies in tree in folder structure
• Concurrent Load the test projects

Known issues

• Target Frameworks cannot be executed in parallel (need to change the batching algorithm)
• Inline updates to sometimes not work (could be related to target frameworks)
• Namespace normalization does not work if assembly name is different from root namespace
• use csproj as identitfier for tests (run test request)

Closes #5020

Screenshot

[dnfadmin]

All CLA requirements met.

[PascalSenn]

There could be a option for this. Like a glob pattern or something similar.
This avoid loading every project on startup

[JoeRobich]

We could also consider a new O# request to get the list of Test projects.

[filipw]

or we could add the project path to the test discovery response https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.DotNetTest/Models/DiscoverTestsResponse.cs

[nohwnd]

@filipw Is there already any code that marks projects as test projects (e.g. based on package references, or name)?

[JoeRobich]

Thanks for starting this work. I need to pull it down and give it a whirl.

[JoeRobich]

We could also consider a new O# request to get the list of Test projects.

[JoeRobich]

@nohwnd @vzarytovskii Please take a look

[PascalSenn]

@JoeRobich should we remove the old code lense and wire up the testingProvider with it?

[PascalSenn]

@JoeRobich also, can we upgrade the vscode dependency further? they have added more features to the newer versions of the test controller

[JoeRobich]

@JoeRobich should we remove the old code lense and wire up the testingProvider with it?

I didn't realize that the testing api also provided Run and Debug code lens. We definitely want to move to the out of the box support.

also, can we upgrade the vscode dependency further? they have added more features to the newer versions of the test controller

We can't use any APIs still under proposal but feel free to move us up to whichever version you require.

[retterbot]

Empty duplicate assembly folder in test explorer (on Windows?) both in list and tree. Was a Windows only issue and is now fixed.

[JoeRobich]

@PascalSenn Before reviewing I wanted to get a sense of whether you felt this was in a good place to merge the initial experience. Anything on your TODO list that can't be deferred? Anything in the known issues that are truly blockers for this functionality?

[PascalSenn]

@JoeRobich I dont think there are any blockers in the implementation. I use the test explorer daily and it works quite well.
I left the code lens in, as i think it's still nice to have them, as the default behavior of VSCode is a bit weird.
There are a couple of things that are Omnisharp related, but nothing major.
I guess it's also easier to open issues, when there is a way to provide a reproduction

[nohwnd]

I had a look and it looks great.

[JoeRobich]

@PascalSenn I gave this branch a try locally against dotnet-format and although it did discover tests cases, test discovery did not return CodeFilePath or LineNumber information.

Steps:

1. Clone dotnet/format
2. Run restore.sh (or Restore.cmd)
3. Open folder in VS Code

I tried with both the Full Framework and net6 OmniSharp, but got the same results.

@nohwnd The discovery returned the Source of each TestCase as the path to the dotnet-format.UnitTests.dll in the artifact folder. Is there a way to have discovery run against the project source?

[PascalSenn]

@JoeRobich interesting, i do have this in some cases too. I cannot really understand why this happens. Everytime i think i found out what the problem is, i prove my assumption wrong the next minute.

One of the files in this namespace is corrupt and with this corrupts the whole namespace. I tracked this issue down on one of our solution, and once i isolate the file, all the other tests are discovered correctly. When i start moving the file around it will work again.
I just tried it out again, and now it works outside of the namespace but not inside of the namespace. It is weird.

This was one of the omnisharp bugs I mention before, i didnt realized it was common.

E.g.
namespace ChilliCream.Cloud.IdentityServer.Hosts.Tests.Isolated;

but in case of
namespace ChilliCream.Cloud.IdentityServer.Hosts.Tests;

I tried to isolate this issue in a test project, but i do not manage to reproduce it in a clean project.

[nohwnd]

The discovery returned the Source of each TestCase as the path to the dotnet-format.UnitTests.dll in the artifact folder. Is there a way to have discovery run against the project source?

TL;DR: Source property is the test container, which for nunit, mstest and xunit is the dll. If you want code source file information you need to look at CodeFilePath and LineNumber. For this to work you also must use non-embedded pdbs, which you don't in dotnet-format, so you won't see that info from your dll.

@JoeRobich There is not, at least not currently (see below). Test platform delegates the work of discovering tests to an adapter and test framework (in this case xunit (framework) and xunit.visualstudio.runner (adapter)), which finds tests by looking for methods with an appropriate attribute (for XUnit it is methods in public classes with Theory, or Fact attribute).

But if you are running under an IDE (design mode) or force including source information via runsettings, then PDBs are used to get source file name and line number for the given test method. Omnisharp is running in design mode, but test platform does not support embedded PDBs. Your dotnet-format test project is using embedded PDBs, and so we fail to read the info because it cannot find the pdb file.

I tried it with the latest versions of test platform and it still does not work. And I don't think we have a plan to support embedded pdbs yet.

---

There is also code based discovery that works against the actual source, which is what is used by Test Explorer in VS to lookup tests from unbuilt code, but that functionality is not present in test platform and so it is not available to omnisharp-roslyn.

[nohwnd]

We can probably ship embedded support in Test Platform 17.2, but if you can get the .cs file info from other places then do it, because not every project will update to the latest net.test.sdk, or to the preview net7 SDK. microsoft/vstest#3454

[JoeRobich]

We can probably ship embedded support in Test Platform 17.2, but if you can get the .cs file info from other places then do it, because not every project will update to the latest net.test.sdk, or to the preview net7 SDK. microsoft/vstest#3454

Thanks @nohwnd! I didn't knowingly choose embedded pdbs in dotnet-format, so they may be the default for repos that use Arcade.

[JoeRobich]

@PascalSenn Would you be able to merge master into your branch? The package-lock changes make it difficult to do using GH.

[PascalSenn]

@JoeRobich updated the branch

[Igorgro]

Any updates on this?

[PhilParisot]

@PascalSenn do you have any updates for us?

[PhilParisot]

I really appreciate all your work on this, its a sorely needed feature.

[PascalSenn]

@PhilParisot I merged master back to the branch and was waiting for a review.

[PhilParisot]

@JoeRobich @filipw Can we put a priority on this pull request pretty please? Sorry to be a bother but it would help my team out quite a bit.

[JoeRobich]

@PascalSenn @PhilParisot Sorry for the delay and thanks for all the work on this. As you can imagine there has been a lot of internal discussions about how to move forward with the C# extension.

We have decided not to take this PR into the VS Code extension itself. We think this functionality would be better suited to going into the OmniSharp-Roslyn repo where it can have direct access to the .NET testing apis and where it can be used by editors other than VS Code. We would expect this functionality to light up in VS Code as part of our move to using OmniSharp LSP.

[PhilParisot]

Thanks @JoeRobich that sounds like a plan.

[sgbench]

@JoeRobich Is there an issue/PR for this yet at the feature's new home? (omnisharp-roslyn/csharp-language-server-protocol)

[JoeRobich]

@sgbench I did not find an open issue for this feature request. The request handlers and response models will need to be added to csharp-language-server-protocol. However, the feature itself will be implemented in omnisharp-rolyn.

[sgbench]

@JoeRobich I don't see any test-specific requests/responses in the LSP spec, so I imagine omnisharp-roslyn could use existing features of the language server to discover and publish tests to the VS Code testing API. In other words, I don't think this feature depends on any updates to csharp-language-server-protocol, and someone familiar with omnisharp-roslyn could start implementing it today.

[loligans]

is there an open issue to track this work in the omnisharp-roslyn repo? I'd like to subscribe to updates over there. @JoeRobich @sgbench @PhilParisot

[Tantol]

I would also like to sign up (track progress). Any updates on this topic?

[heartacker]

hei hei, M$ add this feature to c# dev Kit, Which is closed source
👯