-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add initial support for Microsoft.Testing.Platform #403
Conversation
samples/NuGet.Config
Outdated
<clear /> | ||
<add key="api.nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" /> | ||
<add key="feedz.io/xunit/xunit" value="https://f.feedz.io/xunit/xunit/nuget/index.json" protocolVersion="3" /> | ||
<add key="local" value="C:\src\visualstudio.xunit\artifacts\packages\" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be kept, this is just a temp change for you to test locally (you'll need to update the path as you know).
public static void AddXunit(this ITestApplicationBuilder testApplicationBuilder, Func<IEnumerable<Assembly>> getTestAssemblies) | ||
{ | ||
XunitExtension extension = new(); | ||
testApplicationBuilder.AddRunSettingsService(extension); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This provides the runsettings support.
{ | ||
XunitExtension extension = new(); | ||
testApplicationBuilder.AddRunSettingsService(extension); | ||
testApplicationBuilder.AddTestCaseFilterService(extension); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This provides support for the vstest way of filtering tests.
DO NOT CHANGE THE GUID, IT'S A WELL KNOWN EXTENSION POINT AND THIS EXTENSION NEEDS TO BE REGISTERED AT THE END | ||
WE HAVE CODE INSIDE THE TASK 'TestingPlatformEntryPoint' TO ENSURE THE ORDER OF THE REGISTRATION BASED ON THIS GUID | ||
--> | ||
<TestingPlatformBuilderHook Include="17E773D9-071C-4C66-97DD-57A450BDB027" Condition=" $(GenerateTestingPlatformEntryPoint) == 'true' " > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allows Microsoft.Testing.Platform.MSBuild
to detect the hook to call so we can auto-generate the entry point.
@@ -1,11 +1,24 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<Project ToolsVersion="12.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | |||
<PropertyGroup> | |||
<EnableXunitRunner Condition=" '$(EnableXunitRunner)' == '' ">false</EnableXunitRunner> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let you decide on the name, this is just an example.
@@ -18,8 +18,15 @@ | |||
<dependencies> | |||
<group targetFramework="net462"> | |||
<dependency id="Microsoft.TestPlatform.ObjectModel" version="$MicrosoftTestPlatformObjectModelVersion$" /> | |||
<dependency id="Microsoft.Testing.Extensions.Telemetry" version="$MicrosoftTestingPlatformVersion$" /> | |||
<dependency id="Microsoft.Testing.Extensions.VSTestBridge" version="$MicrosoftTestingPlatformVersion$" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is mixing tabs/spaces, you'll need to configure your editor to show whistespaces
@Evangelink When building the sample, I get a compile failure that looks like some kind of missing assembly/package reference:
If I set |
Looks like it might be package reference related. Converting this: <PackageReference Include="xunit.runner.visualstudio" Version="2.5.8-pre.2">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference> to this: <PackageReference Include="xunit.runner.visualstudio" Version="2.5.8-pre.2" /> fixes the issue. So it looks like setting developmentDependency is probably wrong for .nuspec now. Next question to answer is: Will NuGet pick up changes to this during package upgrade and update the PackageReference appropriately? |
All three of these variants did not upgrade the package correctly:
That's pretty disappointing. That means we're on a failure path here unless we can somehow convince people to uninstall and then reinstall the package. 😞 |
Also, it looks like you're setting a value in the .targets file ( |
What are the plans for |
Why are .NET Framework tests run in x86 by default if the test project is compiled as AnyCPU?
|
Hey @bradwilson, Sorry for the delay in my replies, I am off and had some trouble finding some time to get back here. I'll resume work on 11th so will be fully available for sure at this stage.
I assume that this is telling to msbuild that all deps are only compile time deps and not runtime ones but for the runner we do need the platform + adapter to be available at runtime. I see you have created a ticket on NuGet, I will try to ping some people so we can see how we can unblock this. @baronfel would you have some info or contact about that?
As the name implies this is explicitly asking for the vstest mode. This is equivalent to Our goal is to be able to deprecate
We are not doing any change on our side, this seems to be MSBuild defaults that changed between netfx and netcore. |
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<!-- This GUID comes from Microsoft.Testing.Platform, do not modify --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GUID is actually just any random GUID but it needs to be unique across the various extensions registered through the hook.
@MarcoRossignoli just pointed out this ticket dotnet/sdk#3492 that confirms this is the "default" behavior and this is a change linked to the sdk style project. |
No worries, this isn't a rush situation. :) |
I ask because I use |
@bradwilson .NET 9 preview 2 includes a rework of the test-reporting infrastructure that does show testing progress, as well as ordered test result reporting directly in MSBuild. It's built on a tiny bit of a hack while the MSBuild team works on a proper progress/ephemeral message API for the MSBuild logging infrastructure, after which we should be able to use those new message types to implement nice CLI progress reporting and such. So it's coming, we're working on it :) |
We definitely want to work on and improve the interactive mode. As Chet is saying we have already started some work and we are also exploring some paths in parallel so we can decide what's best and most evolutive |
@Evangelink @MarcoRossignoli @baronfel Where is the source for |
Is the planned IPC for support inside Visual Studio/VS Code also going to be named pipes? If so, what is the equivalent of Why are all the types in |
I'm just going to start opening issues with these questions so they're not buried here, and others can contribute (and perhaps get the benefit of the answers). First set of issues: microsoft/testfx#2536 ("FailedTestNodeStateProperty should include the ability to report without an exception") |
FYI, I have a parallel prototype effort to integrate |
@bradwilson Please let me know what you would need from us so we can move forward this PR. |
@Evangelink I have at least two problems before this could be merged:
|
I can definitely do something on the first one (closed source deps) and I'll see what I can do for the NuGet part but as you know it's on a different team so I'm not sure about it 😕 |
I have other unanswered questions:
|
Another question:
|
People can start testing in "form factors" that are not supported by
No, through the VSTest bridge package, you get support for both VSTest and the new mode. By default, running In addition they can run the exe directly, use
v3 is out of scope of this PR, we should discuss how we want to address it. Our var testApplicationBuilder = await TestApplication.CreateBuilderAsync(args);
// Register the testing framework
testApplicationBuilder.AddTestingFramework(XXX);
using var testApplication = await testApplicationBuilder.BuildAsync();
return await testApplication.RunAsync();
Correct. This is not documented as it's not a public extension point the same way as VSTest isn't documenting its integration with
We expose a MSBuild property that lets TE knows this is using the protocol. There is an option to enable in your VS settings to enable the support of the new mode. |
Happy to share we got green light about opens sourcing the required components. I'll work on that on the upcoming days and release a preview version to use here (I expect final version to be available around beginning of July). For the NuGet part, I am looking at the right contacts to help drive it forward. |
Then what is the purpose of the |
Where is this documented? |
It's my intention to migrate to the |
It's a safetynet if users don't want an exe to be produced. This is also the variable that commands having some properties set or not:
This is also controlling whether the test adapter dll should be only copied to the output or referenced: https://github.com/xunit/visualstudio.xunit/pull/403/files#diff-6311d61396af0339e07d9ec7f69921244c42e31ac28456dd51256f343fa2e0edR14-R29
It's not documented this is some internal implementation detail between us and Test Explorer. The same goes for some of VSTest configurations.
I saw that main was already merged as v3 hence my change of target to the |
I'm going to be honest: answers like this aren't great, and your team has given me this answer a LOT about the new system, including a months old promise to document the JSON-RPC protocol. Answers like this make me feel like I don't want xUnit.net to be the guinea pig in a system you have not finished designing, much less documenting, including a seeming indefinite reliance on the old object model by way of the adapter being the only supported way to use your system.
Combine this with the above, and I don't believe I'm comfortable adopting this yet. It, for all intents and purposes, leads me down a path that might not easily--if ever--support our v3 design. |
cc @drognanar who owns that part
Outside of the protocol who is still being developed, we are running our new platform internally for 1y and half without major issue and publicly since 6 months.
I disagree, we are putting a lot of personal time to improve documentation whether technical https://github.com/microsoft/testfx/blob/main/docs/testingplatform/Index.md or functional https://learn.microsoft.com/dotnet/core/testing/unit-testing-platform-intro
If you are talking about VSTest object model. You can definitely cut it out and that's our target goal. Sadly we need to ensure backward compatibility for our users. On your side, and I guess this is what you will do with v3 you can definitely cut it out. If you are talking about using our new platform package as client for your v3, what's your issue here? Are you not taking any dependency to anything? The LSP like protocol is definitely a WIP and not a direct priority at the moment as there is a lightweight alternative by using the core platform.
I am pretty confused honestly. You have 2 drastically different versions (different enough that they could be 2 products). I am telling you we have been able to validate the v2 for some time and that we are confident this will be working. For the v3, AFAIK, it's still a beta/preview that we haven't looked at. I have absolutely no problem working with you to find how we can integrate with the v3. As we discussed in past, if your constraint is to have zero dependency on anything we ship and rely only on the protocol then I can only say that this is not a priority for the current semester. We can totally discuss with you and management how to plan for the next one. |
How will that work when the entire design appears to be based on the adapter from the TPv2 Object Model? For example, how would one opt into TPvNext as a unit test framework that is not currently using the TPv2 Object Model?
I agree that v2 and v3 have very fundamentally different designs, and the fact that you have validated v2 doesn't help me with v3, and I'm not willing to yet adopt something that might be a dead end for me with v2.
I don't know if that's the only way I can get what I want, because we haven't yet engaged on what it means to support v3 yet. Let me see if I can make my position clearer. You list these issues as the ones that may concern our users:
We do not support any of these form factors. Native AOT may be something we eventually support, but that support would come at an undetermined point in the future.
I don't know what these issues are, but they're also potentially sidestepped simply by our v3 architecture. Are you aware of any issues raised here against xUnit.net that would've been caused by these TPv2 problems? I have also raised concerns with the reduction in interactive usability for our users with Finally, the question of how v3 gets supported needs to be resolved. In short, I only see this as a step backward for my users, and a potential dead end for myself. I have (well founded, I think) concerns that if I were to merge this for v2, your team would go silent again when asked how the system would work against our v3 design. Today, I have v3 working against TPLv2, which means that trying to merge this will potentially become a blocker for me getting a v3-compatible build of |
Your point is clear I think. Moving forward by closing as you don't see any added value. |
I'm open to helping you work out design issues to support v3 (and/or any test framework which comes already in executable format). |
Fixes #402