microsoft / testfx

MSTest framework and adapter
MIT License
714 stars 253 forks source link

Microsoft.Testing.Platform for xUnit #3478

Closed bradwilson closed 1 week ago

bradwilson commented 1 month ago

The approach xUnit.net v3 is taking for supporting Microsoft.Testing.Platform (since it already produces executables) is to conditionally use the "create TestApplication.CreateBuilderAsync and build it" approach when it understands that it's time to run in that way, vs. letting it run with our entry point.

Our current strategy is to:

There are two branches right now that perform this work in concert with one another:

https://github.com/xunit/xunit/tree/microsoft-testing-platform https://github.com/xunit/visualstudio.xunit/tree/microsoft-testing-platform

My question is about the documentation & support behind the hook. Right now, through a combination of decompilation of Microsoft.Testing.Extensions.VSTestBridge (which we are not using) and inspecting the logs from dotnet test when using that, it appears that dotnet test is utilizing a command line option that looks like --internal-msbuild-node <argument>.

Based on this comment, this is currently an undocumented and unsupported way to get dotnet test to work:

This is the reason why the option starts with --internal-, options that start with it are hidden internal.

So my question is:

When will the interaction between Microsoft.Testing.Platform test frameworks and dotnet test become documented and supported? If this isn't the way we should be doing it, then what is the officially supported way to enable dotnet test for Microsoft.Testing.Platform test framework implementations that are not based on Microsoft.Testing.Extensions.VSTestBridge?

bradwilson commented 1 month ago

@MarcoRossignoli @drognanar

MarcoRossignoli commented 1 month ago

My question is about the documentation & support behind the hook. Right now, through a combination of decompilation of Microsoft.Testing.Extensions.VSTestBridge (which we are not using) and inspecting the logs from dotnet test when using that, it appears that dotnet test is utilizing a command line option that looks like --internal-msbuild-node .

Let's start to describe what's inside the Microsoft.Testing.Platform.MSBuild: It's not a mandatory package, it's used to:

In this case we didn't do any update to the sdk we simply rely on the current support. The task in this case will run the application as-is connecting through a pipe with a custom internal protocol. It means that we're skipping the VSTest infra.

NB. We're working on a new version for dotnet test to skip completely MSBuild and start directly the tests module in server mode(for now with pipe binary protocol, not jsonRpc). This will solve the problem of unclear and confusing UX and giving us a new model for execution like dotnet test --testsmodule /**/*Tests.dll to support scenario where users build on one machine move to another one and run tests. Let's say we're working to avoid the needs to run through MSBuild because it imposes some UX and runtime limitation and make complex the evolutions (i.e. pre and post actions, post assets elaboration etc...).

The target is defined here https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Platform.MSBuild/buildMultiTargeting/Microsoft.Testing.Platform.MSBuild.targets#L71

The testing framework supports extension and as you know they're linked at build-time. So this task takes also a specific Items collection and generates a call to an extension provided method passing the application builder and the entry point args. So to "plug" the "compile time" registration (vs manual registration where user can manually write the code inside the Main) an extension MUST provide a prop with the registration method. The task will emit the code in the autogenerated main. The sample schema for the Items is for instance the build-time registration of the Trx extension https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Extensions.TrxReport/buildMultiTargeting/Microsoft.Testing.Extensions.TrxReport.props

Thanks to the TestingPlatformBuilderHook we know how to emit the call, we use the TypeFullName provided by the extension and we "append" a method on it (.AddExtensions(builder, args);).

== Why you see internal-msbuild-node <argument> as args?

Because the Microsoft.Testing.Platform.MSBuild it's nothing special than an extension relying on the built-in extensibility point and so it's injecting an IDataConsumer https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Platform.MSBuild/TestPlatformExtensions/MSBuildExtensions.cs using the static registration mechanism https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Platform.MSBuild/buildMultiTargeting/Microsoft.Testing.Platform.MSBuild.props. That pipe is used to transfer the information needed by the InvokeTestingPlatformTask using a custom consumer https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Platform.MSBuild/TestPlatformExtensions/MSBuildConsumer.cs#L65.

So Microsoft.Testing.Platform.MSBuild is not a mandatory package to run tests it's shipping some support to help users to write less code generating some code. This package has got nothing to do with the Microsoft.Testing.Extensions.VSTestBridge. The Microsoft.Testing.Extensions.VSTestBridge is trasparent and is plugged under the test framework adapter interface https://github.com/microsoft/testfx/blob/main/docs/testingplatform/itestframework.md. This is used to have 0 update for old VSTest adapters. The future goal would be remove it and avoid deps on VSTest infrastructure. Clearly the jurney won't be quick, the new testing platform has got a complete different extensibility and runtime model so extensions like collectors, loggers are no more supported, and for that we need to fill the gap with new extensions like we did here https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-platform-extensions.

From versioning perspective Microsoft.Testing.Extensions.VSTestBridge and Microsoft.Testing.Platform.MSBuild depend on the single core platform with 0 dependency Microsoft.Testing.Platform. This means that at built-time nuget will resolve the version, giving us (if we play good with semver) pretty stable and deterministic/testable platform+extensions configuration, a pain point of VSTest.

Microsoft.Testing.Platform.MSBuild is a transitive dep so the targets will flow up. The issue I see with the custom main with the if is that we need to take the control of program.cs if you want to give to xUnit access to the extensions. We emit the full program.cs with the registration using the MSBuild Items above. This is what we codegen:

//------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by Microsoft.Testing.Platform.MSBuild
// </auto-generated>
//------------------------------------------------------------------------------

[global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
internal sealed class TestingPlatformEntryPoint
{
    public static async global::System.Threading.Tasks.Task<int> Main(string[] args)
    {
        global::Microsoft.Testing.Platform.Builder.ITestApplicationBuilder builder = await global::Microsoft.Testing.Platform.Builder.TestApplication.CreateBuilderAsync(args);
        Microsoft.Testing.Platform.MSBuild.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.Testing.Extensions.Telemetry.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.VisualStudio.TestTools.UnitTesting.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.Testing.Extensions.CodeCoverage.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.Testing.Extensions.TrxReport.TestingPlatformBuilderHook.AddExtensions(builder, args);
        using (global::Microsoft.Testing.Platform.Builder.ITestApplication app = await builder.BuildAsync())
        {
            return await app.RunAsync();
        }
    }
}

Collecting from MSBuild

image

So I think that if you want to plug to the new testing platform you have to opt-out your one and give us the control on the starting point or extensions won't work.

Another possible issue I see is that the new testing platform support custom command line arguments so if you want your users use your v3 options at command line you should provide the extension for it. Here the documentation https://github.com/microsoft/testfx/blob/main/docs/testingplatform/icommandlineoptionsprovider.md with a sample here https://github.com/microsoft/testfx/blob/main/docs/testingplatform/Source/TestingPlatformExplorer/TestingFramework/TestingFramework.CommandLineOptions.cs

MarcoRossignoli commented 1 month ago

If this isn't the way we should be doing it, then what is the officially supported way to enable dotnet test for Microsoft.Testing.Platform test framework implementations that are not based on Microsoft.Testing.Extensions.VSTestBridge?

Finally if you plug to the Microsoft.Testing.Platform you will have built-in support to all the "protocol" model we will ship for dotnet test and VS/VS Code transparently. The server mode is developed, maintained and evolved inside the core Microsoft.Testing.Platform.

You could have an msbuild property like <UseTestingPlatform> and if true you ref Microsoft.Testing.Platform and Microsoft.Testing.Platform.MSBuild same version and opting out your codegen. In this way users can opt-in the new testing platform for what you've already implemented the framework and we take control of the entry-point and extensions(if users will add some). In that way you will have everything out of the box.

Another suggestion maybe for next loop is to have all this inside an MSBuild sdk. It helps a lot with clean opt-in/out without giving to users a way to break the config https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-mstest-sdk source here https://github.com/microsoft/testfx/tree/main/src/Package/MSTest.Sdk

bradwilson commented 1 month ago

So Microsoft.Testing.Platform.MSBuild is not a mandatory package to run tests

When users are asking us to support Microsoft.Testing.Platform (primarily because they've been told there are performance benefits), I don't think this statement is true, because they will expect dotnet test to run with Microsoft.Testing.Platform if we claim to support it.

Another way to phrase this is: if any unit test framework (ignore xUnit.net v3 the moment) only supported Microsoft.Testing.Platform and took no other dependencies, how would users be able to see that benefit? There would be no support in dotnet test and no support in Test Explorer. The only way they'd "see" it is if they tried to directly run the test project itself.

If that's the intended goal, to be able to run directly run a project and have it be faster than dotnet test, we already meet that goal with our v3 design, without needing Microsoft.Testing.Platform.

I personally believe that from a user perspective they're going to expect that we use Microsoft.Testing.Platform for dotnet test and Test Explorer in order to consider this feature complete. There would be zero benefit for us shipping an "only Microsoft.Testing.Platform" feature today, because all we'd end up doing is adding your layer of indirection around a discovery and execution system that already exists, and by definition is faster when we don't involve you. (That assumes that we let you control the entry point.) Do you agree?

This is used to have 0 update for old VSTest adapters. The future goal would be remove it and avoid deps on VSTest infrastructure.

We're ready now. 😄

Our current split plan right now with xunit.runner.visualstudio is that we'll continue to use TPv2 for v1 and v2 unit tests, and Microsoft.Testing.Platform for v3 unit tests. I have isolated the logic to "turn on" Microsoft.Testing.Platform for v3 tests into this package, but there's no reason for that to be true today. My original prototype just enabled it straight away for all v3 test projects without needing that separate package. I'm honestly torn on which is better, but for sure because of the way NuGet dependencies work in my current plan, all v3 test projects will end up getting linked against Microsoft.Testing.Platform whether they end up bringing in xunit.runner.visualstudio or not, simply because there's no other way for me to conditionally add that reference based on whether the user has a v3 test project. NuGet's package dependency logic is "always on" or "always off": your dependencies are in your .nuspec, no way around that. And we don't want to split v1/v2 users off to one package and v3 users off to a different package. Besides, there appears to be zero downside to our users having that dependency when it isn't used, precisely because you've designed it for zero upstream dependencies.

The issue I see with the custom main with the if is that we need to take the control of program.cs if you want to give to xUnit access to the extensions. We emit the full program.cs with the registration using the MSBuild Items above.

You've botched the dependency tree, though. Microsoft.Testing.Platform.MSBuild requires references, based on that auto-generated code, that you haven't brought along with you. You're assuming they'll come from users who are referencing Microsoft.Testing.Extensions.VSTestBridge. Right now you probably assume that those two packages always live together, but that's not true for me. That's the reason that I don't have any of the extension registrations in my code that you do, other than the one from Microsoft.Testing.Platform.MSBuild, because I haven't taken any of those other dependencies: https://github.com/xunit/xunit/blob/cccc2b35f58bf2503b63b5ab380d20f511006621/src/xunit.v3.runner.testingplatform/TestPlatformTestFramework.cs#L227-L232

We can't unilaterally give you control of our entry point, though. That's out of the question, because our users need to be able to run tests outside of dotnet test and Test Explorer. Not only do we have the interactive scenario (where a user just runs the test project) where we need strong control over the UX, there's also the scenario where our multi-assembly runners (like xunit.runner.visualstudio and xunit.v3.runner.console) need to be able to launch and control v3 unit test projects, and that is simply not possible if we let you own the command line. I looked at your command line infrastructure and it's not sufficient for the kinds of parsing and validation we do. We also need strong control over StdOut when we're running in what I called "automated" mode, which is how v3 unit test projects are controlled by our multi-assembly runners. The only output permitted on StdOut is one-per-line JSON serialization of our runner message model.

So I think that if you want to plug to the new testing platform you have to opt-out your one and give us the control on the starting point or extensions won't work.

The two questions I have here aren't around the core of what we're doing right now in that code snippet above, but:

  1. How stable is this extension list over time? Would it be possible for us to discover that list at build time without using your injected entry point, like maybe you inject a function at a well known namespace/class/method that could be called by us at the right time to do extension registration? That's basically 90% of your auto-gen'd code, but in a way that we could consume; the other 10% is understanding what command line switches tell us we need to delegate control to you for dotnet test and Test Explorer to function (like --server).
  2. How many dependencies do you force into us as a result?

You speak above about the zero-dependency Microsoft.Testing.Platform, but those extensions aren't in Microsoft.Testing.Platform. They're spread among a bunch of other NuGet packages, which do have real dependency impacts directly on our end users. The top level dependency list from Microsoft.Testing.Extensions.VSTestBridge 1.3.2 is:

Digging into those, you can see more upstream dependencies of things that unit test users very well may be using, and therefore have potential places to conflict when different versions are needed that end up not being compatible.

Bottom Line

Is the "we must own your entry point" a 100% requirement that you are immovable from? If so, then it sounds like we will never support Microsoft.Testing.Platform, because it is a 100% requirement on our side that we own our entry point.

We'd prefer a model where we work with you to find a way so that you can evolve Microsoft.Testing.Platform while we still own our own command line UX. An ideal solution would be for us to be able to just update NuGet package versions and it "just works", but if that means occasionally updating our entry point to accommodate new extensions and the evolution of your command line requirements, I'm fine with that. We can move forward with that strategy, assuming that things get documented appropriately. Right now having to dig everything out of decompiled binaries and log file isn't really a tenable solution. I understand that that's because you're still in an experimental place (at least as it pertains to dotnet test; I expect we're going to have a similar conversation in my other issue related to Test Explorer). But right now, I know that you would like for us to support Microsoft.Testing.Platform, and our users would like the presumed performance benefits when using dotnet test and Test Explorer. It just seems like it's too early for us to be able to do that officially, because we are unable to use the VSTestBridge (where it seems like the heavy investment went in order to get things bootstrapped went).

If the entry point ownership is 100% no compromise, then please say that plainly so that I can close this issue and point users here when they ask why we won't support Microsoft.Testing.Platform.

MarcoRossignoli commented 1 month ago

I personally believe that from a user perspective they're going to expect that we use Microsoft.Testing.Platform for dotnet test and Test Explorer in order to consider this feature complete. There would be zero benefit for us shipping an "only Microsoft.Testing.Platform" feature today, because all we'd end up doing is adding your layer of indirection around a discovery and execution system that already exists, and by definition is faster when we don't involve you. (That assumes that we let you control the entry point.) Do you agree?

We're towards that goal but we added already some benefits that are currently used.

First step was have the new platform with all the needed extensibility point. This already solved some problem for some team, they're running in CI with dotnet run (with AzureCLI@2 for instance) or manually with scripts. Others are using F5 in VS/VS Code working in a focus way without the TestExplorer (they setup the filter in the launcher and they debug in that way) for tests that are hard to run or failing inside VSTest host. We shipped some interesting extension like Retry that now is portable in every OS/.NET version, this is another already used scenario, where the user friendliness of dotnet test is less important than the feature. Plus allowing to run aot or with custom configuration like in fan out or without the needs to have the sdk on the machine are anyway appreciated additions. We're close to have Test Explorer using directly the testing application in server mode (we're dogfooding it and we're adding the last needed feature to have the parity with VSTest).

So the Microsoft.Testing.Platform.MSBuild currently is giving the same performance as dotnet run but with the known gesture. And we're working to drop it in favor of the new dotnet test version that will talk directly with the test application like Test Explorer without the needs of the MSBuild infra, this should solve the old confusing UX output. This needs some time.

We're ready now. 😄

I saw our branch, happy to see that the documentation and the sample helped to quickly integrate. On our side we need a bit more time to "adapt" to the rest of the environment and tooling.

NuGet's package dependency logic is "always on" or "always off": your dependencies are in your .nuspec, no way around that.

We solved that problem with the MSBuild SDK there you can conditionally opt-in out set of different packages. We switch completely the implementation of the engine when we go AOT. Preserving the UseVSTest for users that wants to opt-in old set of packages.

You've botched the dependency tree, though. Microsoft.Testing.Platform.MSBuild requires references, based on that auto-generated code, that you haven't brought along with you. You're assuming they'll come from users who are referencing Microsoft.Testing.Extensions.VSTestBridge

What you mean? Microsoft.Testing.Platform.MSBuild depends only on the core plat to insert the extension needed for dotnet test (IDataConsumer) https://www.nuget.org/packages/Microsoft.Testing.Platform.MSBuild#dependencies-body-tab it doesn't need Microsoft.Testing.Extensions.VSTestBridge. It generates the entry point and the configuration but you can opt-out it.

I looked at your command line infrastructure and it's not sufficient for the kinds of parsing and validation we do.

What's missing?

Not only do we have the interactive scenario (where a user just runs the test project) where we need strong control over the UX

We ship by default with our console UX to report the result and the errors but we don't capture the output in any way, if you do Console.WriteLine it will end in console. If you need the complete control on the UX(we call it OutputDevice) we can open this extension (closed because we were waiting to understand if can be interesting for someone) https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Platform/OutputDevice/IPlatformOutputDevice.cs and you can substitute completely the UX. The new testing platform can run headless. @nohwnd is working to make the built-in one better at the moment https://github.com/microsoft/testfx/pull/3292.

How stable is this extension list over time? Would it be possible for us to discover that list at build time without using your injected entry point, like maybe you inject a function at a well known namespace/class/method that could be called by us at the right time to do extension registration? That's basically 90% of your auto-gen'd code, but in a way that we could consume; the other 10% is understanding what command line switches tell us we need to delegate control to you for dotnet test and Test Explorer to function (like --server).

This is not super clear as question, at the moment we're doing what you said, we use MSBuild prop Items that we codegen at build-time as I've explained above(https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Platform.MSBuild/buildMultiTargeting/Microsoft.Testing.Platform.MSBuild.props). Extension will be shipped by us and by users and if they provide the correct Item inside the prop they will register themselves without the need of any change inside the program.cs. Otherwise users can disable the codegen and do it manually with the classic extension method.

How many dependencies do you force into us as a result?

For a test adapter the only dependency is Microsoft.Testing.Platform. Microsoft.Testing.Platform.MSBuild is needed today to inject the entry point with the custom extension registration that you can do by your own or providing a clear Program.cs and to plug dotnet test using MSBuild that you can do by your own using the current infrastructure. When TextExplorer and the new dotnet test will be completed the usage of Microsoft.Testing.Platform.MSBuild can be opted-out and used only to insert the entry point and transform the configuration file. Anyway these 2 extension are pretty light Microsoft.Testing.Platform takes no deps and Microsoft.Testing.Platform.MSBuild takes Microsoft.Testing.Platform. So we should not pollute or interfere in any way.

Microsoft.Testing.Extensions.Telemetry is used for telemetry, useful to understand what we can deprecate, here all the info collected https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-platform-telemetry, but optional. Microsoft.Testing.Extensions.TrxReport.Abstractions: it's part of the Trx generator extension. The new testing platform is shipped with a new capability system that allows extensions to talk and provide information to other extension. This lib export the object model for some custom property to push in the message bus to make the trx complete (trx is based on class/method names and the default list of properties shipped with the core lib doesn't containt specific extension information). So it's optional, good to have to produce a good trx, the deps is without any deps but again the core one. Here the "properties" exposed by the trx extension https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Extensions.TrxReport.Abstractions/TrxReportProperties.cs

After users can add extensions and extensions can take dependencies our of our control.

You speak above about the zero-dependency Microsoft.Testing.Platform, but those extensions aren't in Microsoft.Testing.Platform. They're spread among a bunch of other NuGet packages, which do have real dependency impacts directly on our end users. The top level dependency list from Microsoft.Testing.Extensions.VSTestBridge 1.3.2 is:

You don't need Microsoft.Testing.Extensions.VSTestBridge if you drop the old VSTest support. For what I see you didn't use it in xunit v3 so you should not see it in the list of deps. We use it in MSTest because unfortunately we're not yet ready to drop the object model of VSTest that's down the code and we don't have separation between engine objects and VSTest objects.

Is the "we must own your entry point" a 100% requirement that you are immovable from? If so, then it sounds like we will never support Microsoft.Testing.Platform, because it is a 100% requirement on our side that we own our entry point.

If you copy our way to at compile time register the dependency you can take it. It's explained above, if you codegen the registration by your own it's fine we don't need the entry point. You need to flow the options to our builder for server mode (for dotnet test and TestExplorer)

But right now, I know that you would like for us to support Microsoft.Testing.Platform, and our users would like the presumed performance benefits when using dotnet test and Test Explorer. It just seems like it's too early for us to be able to do that officially, because we are unable to use the VSTestBridge (where it seems like the heavy investment went in order to get things bootstrapped went).

Our goal is to drop the VSTestBridge so you're one step ahead. If you want to wait till we will have complete the new dotnet test and the TestExplorer to complete the integration I think it's understandable. We need to understand how to handle the command line issue. If you try a --help you'll see some options and extension are also registering custom options. I think that if you want to take the entry is doable for the compile-time extension registration so that problem I think it's solved(here how we're doing and the Items spec https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Platform.MSBuild/Tasks/TestingPlatformEntryPointTask.cs#L45 and we can help there too given that we spec the MSBuild Items), we provide the server mode for dotnet test and TestExplorer and we can fill the gap in case something is missing so also that part should be fine, the only tricky part is the command line.

bradwilson commented 1 month ago

What you mean? Microsoft.Testing.Platform.MSBuild depends only on the core plat to insert the extension needed for dotnet test (IDataConsumer) https://www.nuget.org/packages/Microsoft.Testing.Platform.MSBuild#dependencies-body-tab it doesn't need Microsoft.Testing.Extensions.VSTestBridge. It generates the entry point and the configuration but you can opt-out it.

Your claim was that you auto-generate this entry point (which, BTW, I cannot duplicate):

[global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
internal sealed class TestingPlatformEntryPoint
{
    public static async global::System.Threading.Tasks.Task<int> Main(string[] args)
    {
        global::Microsoft.Testing.Platform.Builder.ITestApplicationBuilder builder = await global::Microsoft.Testing.Platform.Builder.TestApplication.CreateBuilderAsync(args);
        Microsoft.Testing.Platform.MSBuild.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.Testing.Extensions.Telemetry.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.VisualStudio.TestTools.UnitTesting.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.Testing.Extensions.CodeCoverage.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.Testing.Extensions.TrxReport.TestingPlatformBuilderHook.AddExtensions(builder, args);
        using (global::Microsoft.Testing.Platform.Builder.ITestApplication app = await builder.BuildAsync())
        {
            return await app.RunAsync();
        }
    }
}

My counterpoint was that several of these lines would not compile without additional references; specifically:

        Microsoft.Testing.Extensions.Telemetry.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.VisualStudio.TestTools.UnitTesting.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.Testing.Extensions.CodeCoverage.TestingPlatformBuilderHook.AddExtensions(builder, args);
        Microsoft.Testing.Extensions.TrxReport.TestingPlatformBuilderHook.AddExtensions(builder, args);

Given that I tried to repro this, and saw only this as your auto-generated entry point, I'm confused by what you claimed previously:

image

Note that this is exactly what we're doing as well:

image

(The extra Register call is us registering ourselves as a test framework.)

So I'm not sure what/how we go from what I'm seeing today to what you claimed above. Is your claim above an idealized goal for the future? Is the auto-generation dependent on the user adding references to NuGet packages that include register-able things (for lack of a better term)? Are you sniffing the TestingPlatformBuilderHook item groups from the NuGet packages to decide how to generate the entry point? If so, that's something I could do as well, and that would always keep us in sync. Users could add the packages they want and we'll make sure they get registered. Is that how it works today?

What's missing?

We ship by default with our console UX to report the result and the errors but we don't capture the output in any way, if you do Console.WriteLine it will end in console.

I'll take your word for it if you say that you don't ever output anything to the console. My experience so far has been with dotnet test, but those headers and the VSTest style output may be a result of running via dotnet test rather than running directly.

You need to flow the options to our builder for server mode (for dotnet test and TestExplorer)

Does this mean I only have to look for you to pass --server, at which point I just hand you all the args and go? That's effectively what I'm doing today (except that I'm also looking for --internal-msbuild-node, but if we remove Microsoft.Testing.Platform.MSBuild and the temporary dotnet test overloads, then I could stop looking for that).

If you want to wait till we will have complete the new dotnet test and the TestExplorer to complete the integration I think it's understandable.

If there's value in me providing something in the meantime that people could opt into that would work, then I'd be happy to do that (as I presume it would also help you all do testing and validation of the proposed usage).

Today that looks like me including Microsoft.Testing.Platform.MSBuild and recognizing --internal-msbuild-node to offer that transitional dotnet test support, which I'm okay with if that's what people want.

Also today, as #3479 illustrates, I have no idea whether I'm being used via TPLv2 or Microsoft.Testing.Platform inside of Visual Studio. I do know the tests are being run, but there is no visible way to know which path they're taking (short of giving myself environment debugger hacks to watch and see which path things take), but I'm happy to have that discussion over in that issue, if it's appropriate.

I feel like, based on this conversation, the appropriate path is probably:

Given the non-intrusive nature of this (basically, I'm just sniffing for --server and users bring along one dependency they won't care about), I have no problem doing this now and not waiting. I just want to get clarification on my question about how your auto-generation is triggered today, and make sure I can do something equivalent on my end.

bradwilson commented 1 month ago

I do see one problem right now with my current thought:

<TestingPlatformBuilderHook Include="98058041-B5B6-4A75-9834-58E6DF796A22" Condition=" '$(GenerateTestingPlatformEntryPoint)' == 'True' " >

We obviously have to set GenerateTestingPlatformEntryPoint to false so you won't generate the entry point, which means I can't sniff to TestingPlatformBuilderHook entries to do the dynamic registration.

Is there any reason this should actually be conditional? What's the harm in all of your extensions just always having TestingPlatformBuilderHook MSBuild items? It's just a couple strings. Unless you make this stuff always on, nobody else will be able to generate an entry point but you.

(That said, I could make it work today, because removing Microsoft.Testing.Platform.MSBuild turns off your entry point generation anyway, but it does sound like eventually you're going to move that somewhere else and then we'll be stuck.)

bradwilson commented 1 month ago

I have updated the microsoft-testing-platform branch in the core framework, and I'm throwing the one from xunit.runner.visualstudio away, since it's not necessary in the current proposed model.

If you want to take a look at the changeset and see if this aligns with what you'd expect: https://github.com/xunit/xunit/commit/0073f0bc4b7522e81ac206a2b5ccd17ef99b23cf

This is a project that just references xunit.v3 (albeit one built from my branch):

image

It references the Microsoft.Testing.Extensions.TrxReport NuGet package, so this is what the auto-generated entry point looks like:

// <auto-generated> This file has been auto generated by xunit.v3.core. </auto-generated>

[global::System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
public class XunitAutoGeneratedEntryPoint
{
    public static int Main(string[] args)
    {
        if (args.Length > 0 && args[0] == "--server")
            return global::Xunit.Runner.InProc.SystemConsole.TestingPlatform.TestPlatformTestFramework.RunAsync(args, typeof(Microsoft.Testing.Extensions.TrxReport.TestingPlatformBuilderHook)).GetAwaiter().GetResult();
        else
            return global::Xunit.Runner.InProc.SystemConsole.ConsoleRunner.Run(args).GetAwaiter().GetResult();
    }
}

I followed the pattern of the code you linked to that ensures that the TrxReport extension is always registered last.

bradwilson commented 1 month ago

I followed the pattern of the code you linked to that ensures that the TrxReport extension is always registered last.

I feel like maybe an optional priority might be better than hard coding this logic.

MarcoRossignoli commented 1 month ago

So I'm not sure what/how we go from what I'm seeing today to what you claimed above. Is your claim above an idealized goal for the future? Is the auto-generation dependent on the user adding references to NuGet packages that include register-able things (for lack of a better term)? Are you sniffing the TestingPlatformBuilderHook item groups from the NuGet packages to decide how to generate the entry point? If so, that's something I could do as well, and that would always keep us in sync. Users could add the packages they want and we'll make sure they get registered. Is that how it works today?

Yes we're "sniffing the TestingPlatformBuilderHook" so the lines you see inside the codegen depends on the extension packages added.

I feel like maybe an optional priority might be better than hard coding this logic.

I agree, we discussed and for first version we decided to embed it. But it's on the table.

How can I make the -? output look like this? Also note: there are two command line options that aren't based on dashes: you can specify : to set the randomization seed and you can specify a path to the configuration file. This was done to match the command line option in xunit.v3.runner.console which allows you to specify : [] [: []...].

I think that is not fundamental have 100% same UX for the help. Under the new(in-progress) "dotnet test --help" for instance we aggregate all the options and we notify the users if some tests module has got different options (usually brought by extensions) because at runtime for that specific test module with the missing extension if will fail for invalid option. I mean the UX will be for sure different.

So I think that you can map all your options under the testing plat and for the option without a "-some" add a new one i.e. --config ... --seed ... only when is running in testing platform mode. Despite your UX looks really well organized I think that at least for the start have a plain list where users can read and maybe look at the documentation if something is not clear is a good starting point. Usually once users understand the option they won't change it for some time. I mean the important thing I see is that users can in some way opt-in/out all the xunit feature.

PS. It's not so bad idea give an extensibility point where test framework author can write the custom help, I'll open a ticket for it. cc: @Evangelink

Is there a hook that exists for me, before you ask to discover or execute, that I could inspect those command line options and set up things like the reporter?

In your ITestFramework implementation you can get thought the IServiceProvider the ICommandLineOptions and query the command line and apply the options. For instance https://github.com/microsoft/testfx/blob/main/docs/testingplatform/Source/TestingPlatformExplorer/TestingFramework/TestingFramework.cs#L47

How would I validate something complex like the -maxThreads option?

The ICommandLineOptionsProvider allows you to validate single options and global configuration at the end for instance https://github.com/microsoft/testfx/blob/main/docs/testingplatform/Source/TestingPlatformExplorer/TestingFramework/TestingFramework.CommandLineOptions.cs

Can I use single-dashes for long names, which has historically been how our console runner works? Or will I be forced to change all my command line options that my users have a decade of muscle memory (and build scripts)?

At the moment we support -- and - also if we advice for the -- in the documentation and in the help.

I'll take your word for it if you say that you don't ever output anything to the console. My experience so far has been with dotnet test, but those headers and the VSTest style output may be a result of running via dotnet test rather than running directly.

We give a way to personalize the banner implementing the IBannerMessageOwnerCapability and at the moment we ship our built-in output console. But we can if you need add an option that will allow you to have headless runtime and you can write your own UX. If it's a requirement for you open a ticket for it.

Does this mean I only have to look for you to pass --server, at which point I just hand you all the args and go? That's effectively what I'm doing today (except that I'm also looking for --internal-msbuild-node, but if we remove Microsoft.Testing.Platform.MSBuild and the temporary dotnet test overloads, then I could stop looking for that).

Yes, in--server modes (jsonRpc for VS and pipe binary for dotnet test) we expect to invoke the tests module passing --server --client-port xx --client-host xxxx (VS create the socket at the moment) or --server --dotnet-test-pipe xxxx (but you don't have to take care of it we could change in future at the moment the options for server mode are hidden and used only by tools) to decide which protocol we want to use...so if you do an index of inside the args[] and you find this parameter and it's not one of yours you could "forward" to the testing platform workflow. And anyway if this is not enough we can add an hidden option (we support hidden options) like --internal-run-testingplatform and you can check that one or something specific.

Today that looks like me including Microsoft.Testing.Platform.MSBuild and recognizing --internal-msbuild-node to offer that transitional dotnet test support, which I'm okay with if that's what people want.

True current dotnet integration that rely on the current(old) MSBuild infra can be recognized using --internal-msbuild-node . NB --internal- prefix is reserved.

Also today, as https://github.com/microsoft/testfx/issues/3479 illustrates, I have no idea whether I'm being used via TPLv2 or Microsoft.Testing.Platform inside of Visual Studio. I do know the tests are being run, but there is no visible way to know which path they're taking (short of giving myself environment debugger hacks to watch and see which path things take), but I'm happy to have that discussion over in that issue, if it's appropriate.

VS decide if start using the server mode (start process passing args) looking at the ProjectCapability named TestingPlatformServer. We set it inside the Microsoft.Testing.Platform.MSBuild at the moment https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Platform.MSBuild/buildMultiTargeting/Microsoft.Testing.Platform.MSBuild.targets#L11-L16 if that is found and the preview feature(not yet public) is enabled the TestExplorer will use the jsonRpc protocol and will skip VSTest infra. Again, if you accept for now to link Microsoft.Testing.Platform.MSBuild and Microsoft.Testing.Platform that are deps free we can "incrementally" provide the glue and when we will be there we can drop the Microsoft.Testing.Platform.MSBuild. The reason for the split of the two lib is because we want to be able to "always" in every situation run tests, to have always the plan b. You can link only Microsoft.Testing.Platform and we can run the test, the services provided by msbuild can be done manually(entry point, config) and in some scenario dotnet test is not available at all so your can anyway run your tests.

Remove our dependency on Microsoft.Testing.Platform.MSBuild and accept that dotnet test for now will still be done via TPLv2 Enable Microsoft.Testing.Platform in v3 test projects, always, and remove the necessity of xunit.runner.visualstudio (which will remain how you enable TPLv2) Trust that when I have both Microsoft.Testing.Platform and TPLv2 support (because almost all users will still want to reference xunit.runner.visualstudio, at least until such time that realistically zero users will need the support of a TPLv2 runner), you all are picking up that we support M.T.P and using us that way.

I think you could keep both deps Microsoft.Testing.Platform.MSBuild and Microsoft.Testing.Platform for v3, when you're running with Tpv2 we're using reflection/scan files to load the adapter and you're "entering" in your code ignoring the 2 dll (Microsoft.Testing.Platform.MSBuild.dll and Microsoft.Testing.Platform), isn't it? We ship both libs aligned always so if they're harmful(if not 2 small file inside the bin) update them to take new stuff should be easy upgrade. Users can opt-in/out the integration with dotnet test using the TestingPlatformDotnetTestSupport. But up to you if you want to start without the current dotnet test support. The good part is that the msbuild package allows you also to plug VS thanks to the TestingPlatformServer. Let's say that we could think to have the same setup we have in MSTest but you don't have the VSTestBridge and the others deps that we ship by-default (extensions like codecoverage, trx and telemetry).

Is there any reason this should actually be conditional? What's the harm in all of your extensions just always having TestingPlatformBuilderHook MSBuild items? It's just a couple strings. Unless you make this stuff always on, nobody else will be able to generate an entry point but you.

The reason is to not pollute MSBuild with unused items. We didn't expect the needs to provide the same items outside our codege. But it's fine for me remove the conditional and let you sniff.

If you want to take a look at the changeset and see if this aligns with what you'd expect: https://github.com/xunit/xunit/commit/0073f0bc4b7522e81ac206a2b5ccd17ef99b23cf

I'll check and let you know soon. Thanks for the effort.

MarcoRossignoli commented 1 month ago

I was wondering if to solve the issue with the codegen for the extension we can do something simpler where you can keep the Microsoft.Testing.Platform.MSBuild and actually you opt-out our entry point but we always emit a full registration method kinda like:

Microsoft.Testing.Platform.TestingPlatformBuilderHook.AddExtensions(builder,args);

So you can simply emit in your code and we take care of the dirty sniff.

And we update our codegen to call the same method for symmetry. In this way we can evolve the extensions infra without the need to bother you too much and you don't have to implement the logic by your own.

bradwilson commented 1 month ago

I think that is not fundamental have 100% same UX for the help. Under the new(in-progress) "dotnet test --help" for instance we aggregate all the options and we notify the users if some tests module has got different options (usually brought by extensions) because at runtime for that specific test module with the missing extension if will fail for invalid option. I mean the UX will be for sure different.

I'm not talking about what the user sees with dotnet test --help, but rather what the user sees with dotnet run -- -?, though I understand if we were using you to output the latter, then what we registered in terms of options would impact the former, and that potentially complicates things.

At the moment I think of dotnet test as falling into the "third party multi-assembly runner" pattern (just like Test Explorer or Resharper or NCrunch or CodeRush) rather than the dotnet run "first party single-assembly runner" experience. I don't expect multi-assembly runners to have our same UX, but I would like our single-assembly runner experience to be as near to identical in UX as our multi-assembly runner experience (i.e., xunit.v3.runner.console). For ease of migration, I don't want to radically change that UX from v2 to v3 (not to mention I quite like our UX here, for the most part).

I think that you can map all your options under the testing plat and for the option without a "-some" add a new one i.e. --config ... --seed ... only when is running in testing platform mode.

Agreed.

The interesting problem with seed, specifically, is that it's a per-assembly value. I don't think there's any way to run dotnet test and have it run multiple assemblies (and/or multiple target frameworks, which do end up with unique seed values), but then allow the user to say "the seed for assembly A in net472 is 123, but the seed for assembly B in net6.0 is 456". That's probably a degenerate case that doesn't need to be dealt with, though: if the user is asking to run a specific seed, then they're trying to repro a problem with a specific assembly.

A related question: how will these command line options be surfaced into Test Explorer, if at all? To use the seed example, would I be able to set a seed for a specific test assembly (at a specific target framework) with some UI gesture in Test Explorer?

We give a way to personalize the banner implementing the IBannerMessageOwnerCapability and at the moment we ship our built-in output console. But we can if you need add an option that will allow you to have headless runtime and you can write your own UX. If it's a requirement for you open a ticket for it.

For the moment, since we're trying to achieve the ability for me to still own my entry point, I don't think we need to open any other issues. I'd personally prefer for me to continue to own my entry point, at which point it becomes unnecessary for me to own the banner or anything else like that (since it would only be seen in the case of the user running dotnet test, and not in the case of the user directly running the assembly or running it via xunit.v3.runner.console).

so if you do an index of inside the args[]

This is a simple change for me to make (rather than assuming it's first).

if this is not enough we can add an hidden option

Personally I'm fine with --server, because it doesn't match the pattern we use for command line options anyways (we converted from /option to -option in 2.x for cross platform reasons, and we did not adopt the --option pattern).

I'm also fine if it turns out that some other test framework wants this hidden option, we can look for that hidden option instead.

VS decide if start using the server mode (start process passing args) looking at the ProjectCapability named TestingPlatformServer. [...] the preview feature(not yet public) is enabled the TestExplorer will use the jsonRpc protocol and will skip VSTest infra

You should be able to test this with my branch right now. All I do is run ./build packages in the main branch of xunit/xunit and it'll make *-dev packages. You use nuget.config to point at the /artifacts/packages folder in the clone to pull the package locally.

I also use a function that I wrote that cleans out -dev packages from the NuGet cache before trying to pick up a newly build package:

function Clean-DevPackages {
    $packageCache = $ENV:NUGET_PACKAGES
    if ($null -eq $packageCache) {
        $packageCache = [System.IO.Path]::Combine($home, ".nuget", "packages")
    }
    if ($null -ne $packageCache) {
        Get-ChildItem ([System.IO.Path]::Combine($packageCache, "xunit.*")) | ForEach-Object {
            Get-ChildItem ([System.IO.Path]::Combine($_.FullName, "*-dev")) | ForEach-Object {
                Write-Host $_.FullName -ForegroundColor Yellow
                Remove-Item -Recurse -Force $_
            }
        }
    }
}

Alternatively, if you can point me to instructions on enabling this preview Test Explorer feature (assuming I can do so), I could test this integration myself.

up to you if you want to start without the current dotnet test support

I agree that shipping what I have right now to end users doesn't buy them anything, because (a) there's no dotnet test support (because we're incompatible with Microsoft.Testing.Platform.MSBuild), and (b) there's no Test Explorer support (because you haven't shipped that publicly yet).

I'd be willing to ship this as-is to help your team use us as a second implementation (that does not use VSTestBridge) to verify that everything has been documented and working from the core protocol/API perspective.

Obviously it requires either implementing my or your suggestion on how to work around my problems today with needing GenerateTestingPlatformEntryPoint set to true then causing our auto-generated entry points to collide. I would also, for the time being, need to continue looking for --internal-msbuild-node, at least until dotnet test support is moved over to using --server.

the others deps that we ship by-default (extensions like codecoverage, trx and telemetry)

Part of the end user documentation of this experimental feature could be to not only say "you need to turn on TestingPlatformDotnetTestSupport" but also "the VSTest team recommends you enable these extensions" (or even "here are the known extensions today, and what registering them gives you", which is something I'm not 100% clear on myself in all cases, since there are more extensions available than just those default 3).

I could give them a simple copy/paste block to add those three defaults, which is a sneaky way to ensure they were using a version that's compatible (if we end up needing versions without the conditional on GenerateTestingPlatformEntryPoint). If it turns out that we just shift the requirements over into newer versions of Microsoft.Testing.Platform and/or Microsoft.Testing.Platform.MSBuild, then so much the easier for the end user, because those references would already be coming via xunit.v3.core.

Later on, when this becomes non-experimental, it would make sense for us to update our project templates for v3 to include these extensions by default (much like the v2 templates today include coverlet, as a way to document "here's how you enable this" in a way that users can opt out of by simply deleting the dependencies they don't care about).

How permanent is TestingPlatformDotnetTestSupport? Is this something that you envision existing even after your first official dotnet test support is shipped, or does it exist today because you're still evolving all that support and consider it experimental?

you opt-out our entry point but we always emit a full registration method kinda like: [...] In this way we can evolve the extensions infra without the need to bother you too much and you don't have to implement the logic by your own.

I think is is better than my suggestion, because it leaves the ownership of the discovery and ordering (and whatever else you might need in the future) to you, rather than duplicated by me. You can auto-generate this static method in a well known namespace & class, and I could push a reference to it (as a lambda) into my existing compiled TestPlatformTestFramework entry point rather than the array of typeofs. That means I can get rid of my code-gen-as-MSBuild-task and go back to a simple template.

bradwilson commented 1 month ago

I've updated my branch, and with this update, I can use dotnet test (if you ignore the entry point conflict warning, which thankfully we win):

image

If you don't set TestingPlatformDotnetTestSupport then of course we get failures related to a lack of TPv2 support (for that you'd need to bring in xunit.runner.visualstudio and Microsoft.NET.Test.Sdk), but this seems like we're pretty much in alignment now and just in need of the auto-generated registration method instead of entry point and this will be ready for users to try out. 🤞🏼

MarcoRossignoli commented 1 month ago

I'm not talking about what the user sees with dotnet test --help, but rather what the user sees with dotnet run -- -?, though I understand if we were using you to output the latter, then what we registered in terms of options would impact the former, and that potentially complicates things.

We can add the alias --? to our --help I think it's fine. https://github.com/microsoft/testfx/issues/3501

At the moment I think of dotnet test as falling into the "third party multi-assembly runner" pattern (just like Test Explorer or Resharper or NCrunch or CodeRush) rather than the dotnet run "first party single-assembly runner" experience. I don't expect multi-assembly runners to have our same UX, but I would like our single-assembly runner experience to be as near to identical in UX as our multi-assembly runner experience (i.e., xunit.v3.runner.console). For ease of migration, I don't want to radically change that UX from v2 to v3 (not to mention I quite like our UX here, for the most part).

Ok I think make sense.

The interesting problem with seed, specifically, is that it's a per-assembly value. I don't think there's any way to run dotnet test and have it run multiple assemblies (and/or multiple target frameworks, which do end up with unique seed values), but then allow the user to say "the seed for assembly A in net472 is 123, but the seed for assembly B in net6.0 is 456". That's probably a degenerate case that doesn't need to be dealt with, though: if the user is asking to run a specific seed, then they're trying to repro a problem with a specific assembly.

True, we forward all param to all underneath test modules, if you output the seed on console with the output users can repro or am I out of track?

A related question: how will these command line options be surfaced into Test Explorer, if at all? To use the seed example, would I be able to set a seed for a specific test assembly (at a specific target framework) with some UI gesture in Test Explorer?

At the moment we have fixed way to startup the modules in Test Explorer, but we discussed the concept of profile file (TBD if use VS or VS Code or custom launcher format) where users can create a profile file in the project self and decide which one to run, but this needs some UX updates. cc: @drognanar

For the moment, since we're trying to achieve the ability for me to still own my entry point, I don't think we need to open any other issues. I'd personally prefer for me to continue to own my entry point, at which point it becomes unnecessary for me to own the banner or anything else like that (since it would only be seen in the case of the user running dotnet test, and not in the case of the user directly running the assembly or running it via xunit.v3.runner.console).

I wonder if you want to support also our runner without dotnet test in case users doesn't have it in their machine and they want to use testing platform extension, I dunno something like --use-testing-platform or something like that. But up to you if you want to wait users feedback for that.

Personally I'm fine with --server, because it doesn't match the pattern we use for command line options anyways (we converted from /option to -option in 2.x for cross platform reasons, and we did not adopt the --option pattern). I'm also fine if it turns out that some other test framework wants this hidden option, we can look for that hidden option instead

Ok let's start with --server

Alternatively, if you can point me to instructions on enabling this preview Test Explorer feature (assuming I can do so), I could test this integration myself.

I don't think it's shipped in public yet so I need to test it by my own, @drognanar can you confirm that's only internal for now?

You should be able to test this with my branch right now. All I do is run ./build packages in the main branch of xunit/xunit and it'll make *-dev packages. You use nuget.config to point at the /artifacts/packages folder in the clone to pull the package locally.

I did it quickly yesterday the clone I got some sec issue with 9 preview so I need to add a global.json I suppose with 8. Have you tried to build with 9 preview?

I'd be willing to ship this as-is to help your team use us as a second implementation (that does not use VSTestBridge) to verify that everything has been documented and working from the core protocol/API perspective.

That's great help for use you're not using the bridge so we can fold the difference at protocol level inside the Test Explorer(VSTestBridge is adding some custom vstest.* info for VS that we expect to fold).

Obviously it requires either implementing my or your suggestion on how to work around my problems today with needing GenerateTestingPlatformEntryPoint set to true then causing our auto-generated entry points to collide. I would also, for the time being, need to continue looking for --internal-msbuild-node, at least until dotnet test support is moved over to using --server. @Evangelink will work on the idea to emit a file with the static method for the "sniffed" static registration soon, so you can set our generation to false and emit our "registration method". I agree also on the current logic to indexof--internal-msbuild-node`.

Part of the end user documentation of this experimental feature could be to not only say "you need to turn on TestingPlatformDotnetTestSupport" but also "the VSTest team recommends you enable these extensions" (or even "here are the known extensions today, and what registering them gives you", which is something I'm not 100% clear on myself in all cases, since there are more extensions available than just those default 3).

You can point that part of the docs to our extension list where there's the description and the setup https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-platform-extensions in that case you can avoid to add any extension by default users can start adding manually in their csproj. After you can decide if implement an SDK(imo great clarity for users, they see their csproj completely empty and with MSBuild knobs you can really change everything without any confusion on what packages needs to be added or remove or needs of complex porting documentation, but it's my opinion).

I could give them a simple copy/paste block to add those three defaults, which is a sneaky way to ensure they were using a version that's compatible (if we end up needing versions without the conditional on GenerateTestingPlatformEntryPoint). If it turns out that we just shift the requirements over into newer versions of Microsoft.Testing.Platform and/or Microsoft.Testing.Platform.MSBuild, then so much the easier for the end user, because those references would already be coming via xunit.v3.core.

We will remove the conditional and offer the extension registration you can track this label https://github.com/microsoft/testfx/issues?q=is%3Aissue+is%3Aopen+label%3Axunit

Later on, when this becomes non-experimental, it would make sense for us to update our project templates for v3 to include these extensions by default (much like the v2 templates today include coverlet, as a way to document "here's how you enable this" in a way that users can opt out of by simply deleting the dependencies they don't care about).

Up to you

How permanent is TestingPlatformDotnetTestSupport? Is this something that you envision existing even after your first official dotnet test support is shipped, or does it exist today because you're still evolving all that support and consider it experimental?

I'd says that they will coexist for at least 3/6(worst case) months after our dotnet test release. The new test will be opted in using a global.json for the whole solution and in future it should become the "default" when enough users will move out VSTest. But as you know it's a long run. So before to "deprecate" the current "MSBuild" dotnet test we need feedbacks and we cannot plan with strict ETA so I prefer be pessimistic.

I think is is better than my suggestion, because it leaves the ownership of the discovery and ordering (and whatever else you might need in the future) to you, rather than duplicated by me. You can auto-generate this static method in a well known namespace & class, and I could push a reference to it (as a lambda) into my existing compiled TestPlatformTestFramework entry point rather than the array of typeofs. That means I can get rid of my code-gen-as-MSBuild-task and go back to a simple template.

We have discussed with @Evangelink today about it https://github.com/microsoft/testfx/issues/3494

MarcoRossignoli commented 1 month ago

If you don't set TestingPlatformDotnetTestSupport then of course we get failures related to a lack of TPv2 support (for that you'd need to bring in xunit.runner.visualstudio and Microsoft.NET.Test.Sdk), but this seems like we're pretty much in alignment now and just in need of the auto-generated registration method instead of entry point and this will be ready for users to try out. 🤞🏼

I think that the idea to drop VSBridge/VSTest for v3 will be also useful for us to understand if something is missing to "deprecate" VSTest for .NET. We don't have the luxury to drop it with MSTest yet and xunit is great help to understand what we really need to deprecate the glorious but a bit tired VSTest.

bradwilson commented 1 month ago

I wonder if you want to support also our runner without dotnet test in case users doesn't have it in their machine and they want to use testing platform extension, I dunno something like --use-testing-platform or something like that.

Yep, this is a good idea. Right now our only way to "push" into your command line parser is by picking up on --server (or --internal-msbuild-node), which does not give them the opportunity to use you appropriately.

In order to implement this and make it usable, we'll also need to make sure we register our command line options (we don't do that yet).

Have you tried to build with 9 preview?

No, I generally try to keep my machine free of preview builds, and I also try not to be overly prescriptive with global.json for local development. The CI machine installs 8 and we use a global.json that we swap in there for stricter control in that environment.

Since our minimum platform right now is .NET 6, the only reason we'd push to .NET SDK 8 is to take advantage of newer compiler features. I'll fire up a Linux machine with a .NET SDK 9 preview and see what the breaking changes are.

MarcoRossignoli commented 1 month ago

--use-testing-platform

A cleaner idea could be also to allow users to specify in their project <UseMicrosoftTestingPlatformRunner> and you emit a simple static class with a boolean that you can check in your else branch inside the entry point, so it's more "natural" for users to select the mode. Clearly the order of evaluation is always --server else xunitrunner or testing platform.

So they can opt-in it globally using directory props.

bradwilson commented 1 month ago

Done: https://github.com/xunit/xunit/commit/b7806edddcd6df3f94d23236b92ecc0deed607c5

image

bradwilson commented 1 month ago

I did a logic flip: instead of checking for --server and then running me if not, I check for -automated (or our response file indicator @@) and then run you if not. This is so that we can still run with our multi-assembly runners:

image

MarcoRossignoli commented 1 month ago

I did a logic flip: instead of checking for --server and then running me if not, I check for -automated (or our response file indicator @@) and then run you if not. This is so that we can still run with our multi-assembly runners:

Great, one detail that should not affect the entry point but I share so we can co-think about it, the testing platform is written to be Single module deploy it means that to support single file/aot with extensions we cannot have a build with "more than one file" in these formats. This means that for instance if a user register a native aot out of process extension we "restart" ourself passing the original parameter plus some more parameter to handle the information between the the test host controller(out of process that can observe the process that hosts the run of the tests) and the test host(where the adapter run the tests).

I think that this should not affect anything anyway you will forward to the correct branch due to the fact that we don't "add" -automated or @@. I wonder anyway if in future it's possible that someone will ship some extension with --automated command line and we will clash. For now I think we can go as-is, but we need to sleep a bit on it.

MarcoRossignoli commented 1 month ago

FYI you can build against our dev feed till we will reach a good integration so we can verify the progress https://github.com/microsoft/testfx/blob/main/docs/preview.md

bradwilson commented 1 month ago

I wonder anyway if in future it's possible that someone will ship some extension with --automated command line and we will clash.

At the moment we look for exactly -automated and --automated wouldn't match. All the calls through from the multi-assembly runners strictly use the @@ path because the out-of-process launcher always creates a response file: https://github.com/xunit/xunit/blob/d1331155eba56ca99b4c25fe318eeba2c37a77ad/src/xunit.v3.runner.utility/Frameworks/v3/Xunit3.cs#L373

I included -automated on the idea that something other than our runner utility code might be trying to automate things, although that's purely hypothetical at this point so early in the lifetime of v3, and I wouldn't want to break those automations. We do document -automated but we don't automate the response file pattern; I don't want to particularly mandate the response file pattern, because that's less convenient from a scripting perspective.

I'll have to think about whether there is concern about the -automated clash, and whether it's worth adding something guaranteed not to conflict (like something with xunit in the name, or a GUID, or something like that).

to support single file/aot with extensions we cannot have a build with "more than one file" in these formats

At the moment we are not planning to support AOT. I've had some discussions about what would be required to make this happen but it is not anywhere near the top of the backlog.

MarcoRossignoli commented 1 month ago

I'll have to think about whether there is concern about the -automated clash, and whether it's worth adding something guaranteed not to conflict (like something with xunit in the name, or a GUID, or something like that).

Ok let's sleep on it

At the moment we are not planning to support AOT. I've had some discussions about what would be required to make this happen but it is not anywhere near the top of the backlog.

Ack

MarcoRossignoli commented 1 month ago

@bradwilson we did the update and looks good, I did some test with your branch

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <UseMicrosoftTestingPlatformRunner>true</UseMicrosoftTestingPlatformRunner>
    <GenerateTestingPlatformEntryPoint>false</GenerateTestingPlatformEntryPoint>
    <TestingPlatformDotnetTestSupport>true</TestingPlatformDotnetTestSupport>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Testing.Extensions.TrxReport" Version="1.4.0-preview.24414.8" />
    <PackageReference Include="Microsoft.Testing.Platform.MSBuild" Version="1.4.0-preview.24414.8" />
    <PackageReference Include="xunit.v3.core" Version="0.2.0-pre.85-dev" />
  </ItemGroup>

</Project>
<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <config>
    <add key="globalPackagesFolder" value=".packages" />
  </config>
  <packageSources>
    <clear />
    <add key="localxunit" value="C:\git\xunit\artifacts\packages" />
    <add key="dotnet9" value="https://api.nuget.org/v3/index.json" />
    <add key="test-tools" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/test-tools/nuget/v3/index.json" />
  </packageSources>
</configuration>
using Xunit;

namespace xunitta
{
    public class Test
    {
        [Fact]
        public void T()
        {
            // SelfRegisteredExtensions.AddSelfRegisteredExtensions(null, null);
        }
    }
}

dotnet test works

You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  xunitta succeeded (0.3s) → bin\Debug\net8.0\xunitta.dll
  xunitta test succeeded (0.5s)

Test summary: total: 1, failed: 0, succeeded: 1, skipped: 0, duration: 0.5s
Build succeeded in 1.6s

Also the SelfRegisteredExtensions.AddSelfRegisteredExtensions is emitted like you can see from the commented code in the test and from the binlog (I've added trx for instance).

image

-? works too

dotnet run -- -?
.NET Testing Platform v1.4.0-preview.24414.8+d73f672e05 (UTC 8/14/2024) [x64 - net8]

Usage xunitta.exe [option providers] [extension option providers]

Execute a .NET Test Application.

Options:
  --diagnostic                             Enable the diagnostic logging. The default log level is 'Trace'. The file will be written in the output directory with the name log_[MMddHHssfff].diag
  --diagnostic-filelogger-synchronouswrite Force the built-in file logger to write the log synchronously. Useful for scenario where you don't want to lose any log (i.e. in case of crash). Note that this is slowing down the test execution.
  --diagnostic-output-directory            Output directory of the diagnostic logging, if not specified the file will be generated inside the default 'TestResults' directory.
  --diagnostic-output-fileprefix           Prefix for the log file name that will replace '[log]_.'
  --diagnostic-verbosity                   Define the level of the verbosity for the --diagnostic. The available values are 'Trace', 'Debug', 'Information', 'Warning', 'Error', and 'Critical'
  --exit-on-process-exit                   Exit the test process if dependent process exits. PID must be provided.
  --help                                   Show the command line help.
  --ignore-exit-code                       Do not report non successful exit value for specific exit codes (e.g. '--ignore-exit-code 8;9' ignore exit code 8 and 9 and will return 0 in these case)
  --info                                   Display .NET test application information.
  --list-tests                             List available tests.
  --minimum-expected-tests                 Specifies the minimum number of tests that are expected to run.
  --results-directory                      The directory where the test results are going to be placed. If the specified directory doesn't exist, it's created. The default is TestResults in the directory that contains the test application.

Extension options:
  --report-trx          Enable generating TRX report
  --report-trx-filename The name of the generated TRX report

And you injection of extensions too (to update with SelfRegisteredExtensions.AddSelfRegisteredExtensions)

image

Final sample with TRX

dotnet run -- --report-trx
.NET Testing Platform v1.4.0-preview.24414.8+d73f672e05 (UTC 8/14/2024) [x64 - net8]

xUnit.net v3 In-Process Runner v0.2.0-pre.85-dev+b7806edddc (64-bit .NET 8.0.8)
  Discovering: xunitta
  Discovered:  xunitta
  Starting:    xunitta
  Finished:    xunitta
=== TEST EXECUTION SUMMARY ===
   xunitta  Total: 1, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0, Time: 0.076s

The test framework 'xUnit.net v3 Microsoft.Testing.Platform test framework' with UID '30ea7c6e-dd24-4152-a360-1387158cd41d' does not support the 'ITrxReportCapability' leading to missing or incomplete TRX reports

  In process file artifacts produced:
    - bin\Debug\net8.0\TestResults\2024-08-14_15_56_00.024.trx

Test run summary: Passed! - bin\Debug\net8.0\xunitta.dll (net8|x64)
  total: 1
  failed: 0
  succeeded: 1
  skipped: 0
  duration: 0ms

Trx warning is correct, the testing platform is VSTest agnostic but the old trx uses some VSTest concept so with new capabilities model we can "add" the missing information implementing the expected ITrxReportCapability Here some doc on the capability system

Here is how in the bridge we fill the extra trx information https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Extensions.VSTestBridge/ObjectModel/ObjectModelConverters.cs#L135-L158 trx is based on the concept of class names, the new testing platform allows framework based on any concept(functions etc...) so we don't have an expected property for the class name used by the trx. The idea is that extensions can ship their object model that adapters can use to "collaborate" in a typed and clear way.

Here the implementation of the Trx capability https://github.com/microsoft/testfx/blob/main/src/Platform/Microsoft.Testing.Extensions.VSTestBridge/Capabilities/VSTestBridgeExtensionBaseCapabilities.cs

I'll test soon the interaction with VS.

bradwilson commented 1 month ago

That's great! Thanks! I'll get the branch updated.

Do you have a release date planned for 1.4.0 yet?

bradwilson commented 4 weeks ago

It looks like all the code generation is in Microsoft.Testing.Platform.MSBuild, and not in Microsoft.Testing.Platform as I'd expected. (By default, I only take a reference to just the Platform.)

Is there a reason it's not in Microsoft.Testing.Platform? I thought the .MSBuild package was a temporary one that would be going away, and is only there to support the "in-between" dotnet test experience.

MarcoRossignoli commented 4 weeks ago

Do you have a release date planned for 1.4.0 yet?

@Evangelink can answer to this one, or release a preview maybe.

Is there a reason it's not in Microsoft.Testing.Platform? I thought the .MSBuild package was a temporary one that would be going away, and is only there to support the "in-between" dotnet test experience.

Because we want to have always a way to run tests, for instance users could create manually the entry point and don't use at all MSBuild services(msbuid tasks). So the idea is to have the minimal deps for the core lib Microsoft.Testing.Platform.dll. The Microsoft.Testing.Platform.MSBuild takes dep to the Microsoft.Testing.Platform and nothing else and contains MSBuild Tasks helpers(configuration, entrypoint, current dotnet infra) and some extensions(at the moment the infra pipe to run dotnet test), but is "not mandatory to run tests"(you can run manually with script or executing it without dotnet test) and so we decided to have clean modules infra (minimize the deps size for the main core lib). Anyway like for the Microsoft.Testing.Platform packages Microsoft.Testing.Platform.MSBuild is always shipped together(same version) and won't take any dependencies, you can see these 2 lib as prefixed by System. When the new dotnet test that will work directly with Microsoft.Testing.Platform will be shipped the msbuild task for the integration with the current MSBuild infra won't be used anymore, but our codegen entrypoint and configuration transformer will be used(in your case only the configuration file transformer, the new testing platform supports new json config file, we take the file at project level called testingplatformconfiguration.json and we move to the bin with the name [app].testingplatformconfiguration.json like the [app].runtimeconfiguration.json, @Evangelink here rename the file today https://github.com/microsoft/testfx/pull/3553 to testconfig.json). Sorry if I haven't been clear on this point. But you can see these two as 1 lib

https://www.nuget.org/packages/Microsoft.Testing.Platform no deps [System.]Microsoft.Testing.Platform https://www.nuget.org/packages/Microsoft.Testing.Platform.MSBuild [System.]Microsoft.Testing.Platform.MSBuild deps to [System.]Microsoft.Testing.Platform

bradwilson commented 4 weeks ago

Because we want to have always a way to run tests, for instance users could create manually the entry point and don't use at all MSBuild services(msbuid tasks).

Unfortunately what it means is, for me to support Microsoft.Testing.Platform, you're forcing me to require Microsoft.Testing.Platform.MSBuild because that's the only way I can generate my entry point for MTP.

I get why you think the split makes sense for you, but it definitely doesn't make sense for me. 😞 Or, to phrase it another way, it's impossible for me to use one without the other.

Given that, what's the scenario for any test framework (new or existing) to use Microsoft.Testing.Platform without Microsoft.Testing.Platform.MSBuild, if the latter is absolutely required for the entry point to make everything functional?

MarcoRossignoli commented 4 weeks ago

Given that, what's the scenario for any test framework (new or existing) to use Microsoft.Testing.Platform without Microsoft.Testing.Platform.MSBuild, if the latter is absolutely required for the entry point to make everything functional?

For instance one scenario that we already tested is to run tests inside a physical device(phone, iot) with a custom aot test framework. There you cannot codegen or have particular extension (problem with filesystem and you cannot restart yourself, the concept of process is not the same as in a general purpose OS), but you can connect back to some tcp port and report tests result. In this scenario the "template" cannot be injected(or is not a standard one) and is usually a template where the starting point is manually crafted because you need the correct app model. In these case we won't take the complexity of codegen for sure but we let the respective teams create the correct template.

If you don't want to have it in case of UseMicrosoftTestingPlatformRunner=false you can add a condition to the Microsoft.Testing.Platform.MSBuild package in the target and opt it out. In that case you keep only the Microsoft.Testing.Platform link because you need to build the entry point.

Also the fact that by design they won't never take external deps I think that are harmful deps the only technical difference is that the code is in 2 lib instead of 1 so in your bin in case of UseMicrosoftTestingPlatformRunner=true you'll have(with 0 extensions registered ) Microsoft.Testing.Platform.dll and Microsoft.Testing.Platform.MSBuild in case of false only Microsoft.Testing.Platform.dll.

If you want to completely opt-out and don't have nothing in case of UseMicrosoftTestingPlatformRunner=false you could have the new testing framework in another lib that takes the two deps under the same condition. In that case your bin will be Microsoft.Testing.* free if users will use only xunit runner.

bradwilson commented 4 weeks ago

I don't think it's a good idea to tie UseMicrosoftTestingPlatformRunner=true to whether new protocol versions of dotnet test and Test Explorer can be supported. Those are two orthogonal concerns in my mind. Setting UseMicrosoftTestingPlatformRunner=true is about changing the default command line UX when invoking dotnet run on the test project, not about enabling or disabling TPvNext.

Given that, and how uncomfortable the disconnect feels if I try to push all the TPvNext logic into xunit.runner.visualstudio, it feels like I have no choice but to just give both dependencies to all users, even those who may never use dotnet test or Test Explorer.

As you say, it shouldn't be onerous, though there is the slight possibility of things getting "out of sync" based on the expectations of Microsoft.Testing.Platform.MSBuild being at one version (the one we force them into) vs. whatever extensions they choose to add. It'll really be on your team to ensure that there isn't a version drift problem if, say, we bring Microsoft.Testing.Platform.MSBuild 1.4.0 and they bring some later versions of extensions that wouldn't be supported by Microsoft.Testing.Platform.MSBuild's code generation (and if the support answer is to tell people to take a newer Microsoft.Testing.Platform.MSBuild version, ensuring that our dependency on what gets auto-generated isn't broken as a result).

MarcoRossignoli commented 4 weeks ago

I don't think it's a good idea to tie UseMicrosoftTestingPlatformRunner=true to whether new protocol versions of dotnet test and Test Explorer can be supported. Those are two orthogonal concerns in my mind. Setting UseMicrosoftTestingPlatformRunner=true is about changing the default command line UX, not about enabling or disabling TPvNext.

It's not clear this one, onboard Microsoft.Testing.Platform.dll ensure the support to dotnet test and the test explorer without UseMicrosoftTestingPlatformRunner=true if won't work in that scenario(at least for now or when we think we can ship and "advertise" stable binary/jsonrcp protocol).

As you say, it shouldn't be onerous, though there is the slight possibility of things getting "out of sync" based on the expectations of Microsoft.Testing.Platform.MSBuild being at one version (the one we force them into) vs. whatever extensions they choose to add. It'll really be on your team to ensure that there isn't a version drift problem if, say, we bring Microsoft.Testing.Platform.MSBuild 1.4.0 and they bring some later versions of extensions that wouldn't be supported by Microsoft.Testing.Platform.MSBuild's code generation (and if the support answer is to tell people to take a newer Microsoft.Testing.Platform.MSBuild version, ensuring that our dependency on what gets auto-generated isn't broken as a result).

Microsoft.Testing.Platform.MSBuild and Microsoft.Testing.Platform are build, test and ship together. Also all other extensions are build tested and shipped together(at least our own). If we "break" the MSBuild "sniffing" logic is up to us announce with a sem ver major breaking change, this is true also for ourself it can happen and it's I think a normal chance.

Given that, and how uncomfortable the disconnect feels if I try to push all the TPvNext logic into xunit.runner.visualstudio, it feels like I have no choice but to just give both dependencies to all users, even those who may never use dotnet test or Test Explorer.

So you mean that you want to provide both "entry" point to VSTest and to the new TestingPlatform having the old object model in the middle inside xunit.runner.visualstudio? And if users will set UseMicrosoftTestingPlatformRunner=true they will use the new testing infra? If you don't want to drop the VSTest dependecies the @Evangelink strategy described in the old PR using the VSTestBridge is the easiest path to start with. I was thinking you wanted to drop VSTest for v3 but it's up to you, if you want to start with the compatibility layer. Now you understood the new architecture and why we have a VSTestBridge, you can decide what to use. MSTest and nUnit are using the VSTestBridge with the pros and cons you know.

bradwilson commented 4 weeks ago

It's not clear this one, onboard Microsoft.Testing.Platform.dll ensure the support to dotnet test and the test explorer without UseMicrosoftTestingPlatformRunner=true if won't work in that scenario(at least for now or when we think we can ship and "advertise" stable binary/jsonrcp protocol).

Why not? I'm looking for --server and calling you when I see it (also --internal-msbuild-node which one day will be able to go away). This works fine today: if you dotnet run us, you see our command line UX, but you can run with the new in-between dotnet test just fine:

image

How do you anticipate breaking this in the future?

So you mean that you want to provide both "entry" point to VSTest and to the new TestingPlatform having the old object model in the middle inside xunit.runner.visualstudio?

No. I think I may have confused you.

There are two ways I could support TPvNext: either directly (what the current prototype does) with no additional dependencies, or indirectly (what my old prototype did) that's only enabled when you add xunit.runner.visualstudio. I didn't like the split in the old prototype for several technical reasons, so I prefer the current prototype where TPvNext is just always supported without any additional dependencies. If you import xunit.v3 (more specifically, xunit.v3.core), you get TPvNext. Period.

What you don't get out of the box by default is the TPvNext command line. dotnet run is our UX, not yours. It'll work with dotnet test and Test Explorer even though our command line UX is the default precisely because we're argument sniffing for --server to know when to call you vs. us.

The purpose of UseMicrosoftTestingPlatformRunner=true is solely to switch the default command line UX from us to you. In my mind this is going to be a very infrequently used feature that will be chosen when people want to use your extensions from the command line, but are NOT invoking the test project via dotnet test or Test Explorer (and instead invoking their test project via dotnet run). I don't know if anybody will ever do this, but I'm adding the switch in case such a person comes along at some point. 😂

So, let's talk about timelines.

Today: if I were to ship my prototype today, then users will not see any changes. Using Test Explorer (because the new version isn't ready yet) and dotnet test (because the "in-between" experience is gated behind setting TestingPlatformDotnetTestSupport=true) will both require a reference to a 3.x.y build of xunit.runner.visualstudio and compatibility for both will be based on TPv2 APIs. Users who are "in the know" can set TestingPlatformDotnetTestSupport=true and get the new dotnet test, but there is no way to opt into Test Explorer TPvNext yet because it hasn't shipped publicly yet.

Some point in the future: you ship final versions of TPvNext enabled dotnet test and Test Explorer, and their usage is triggered off of the presence of <ProjectCapability Include="TestingPlatformServer" /> (which we are setting now). xUnit.net v3 test projects will support both dotnet test and Test Explorer out-of-the-box, and the reference to xunit.runner.visualstudio becomes superfluous (or perhaps present just for runners that are based on TPv2, like I assume Rider/Resharper/CodeRush will stick to). dotnet test and Test Explorer will ignore whether we support TPv2 or not, and just know they can send us --server and we'll do the right thing.

Hopefully this makes my view of everything clearer. Please let me know if I've gotten something wrong.

MarcoRossignoli commented 4 weeks ago

Why not? I'm looking for --server and calling you when I see it (also --internal-msbuild-node which one day will be able to go away). This works fine today: if you dotnet run us, you see our command line UX, but you can run with the new in-between dotnet test just fine:

Ah ok got it you meant to use UseMicrosoftTestingPlatformRunner only to affect the dotnet run or direct execution using our runner.

How do you anticipate breaking this in the future?

I don't think we will break it, VS/dotnet test(new one) will always use --server so I think I got your idea and I think it's fine.

If you import xunit.v3 (more specifically, xunit.v3.core), you get TPvNext. Period.

Ok this is what I've understood.

The purpose of UseMicrosoftTestingPlatformRunner=true is solely to switch the default command line UX from us to you. In my mind this is going to be a very infrequently used feature that will be chosen when people want to use your extensions from the command line, but are NOT invoking the test project via dotnet test or Test Explorer (and instead invoking their test project via dotnet run). I don't know if anybody will ever do this, but I'm adding the switch in case such a person comes along at some point. 😂

Got it

Today: if I were to ship my prototype today, then users will not see any changes. Using Test Explorer (because the new version isn't ready yet) and dotnet test (because the "in-between" experience is gated behind setting TestingPlatformDotnetTestSupport=true) will both require a reference to a 3.x.y build of xunit.runner.visualstudio and compatibility for both will be based on TPv2 APIs. Users who are "in the know" can set TestingPlatformDotnetTestSupport=true and get the new dotnet test, but there is no way to opt into Test Explorer TPvNext yet because it hasn't shipped publicly yet. Some point in the future: you ship final versions of TPvNext enabled dotnet test and Test Explorer, and their usage is triggered off of the presence of (which we are setting now). xUnit.net v3 test projects will support both dotnet test and Test Explorer out-of-the-box, and the reference to xunit.runner.visualstudio becomes superfluous (or perhaps present just for runners that are based on TPv2, like I assume Rider/Resharper/CodeRush will stick to). dotnet test and Test Explorer will ignore whether we support TPv2 or not, and just know they can send us --server and we'll do the right thing. Hopefully this makes my view of everything clearer. Please let me know if I've gotten something wrong.

Got it, in the case of "but there is no way to opt into Test Explorer TPvNext yet because it hasn't shipped publicly yet." they'll transparently fallback to the TPv2 api. I think your plan is correct.

So you'll take Microsoft.Testing.Platform.MSBuild together with Microsoft.Testing.Platform but you'll ship v3 with the back compatibility through xunit.runner.visualstudio for now. So you don't drop yet xunit.runner.visualstudio till our side is fully completed and in "parity" with the support of the old Tpv2 from the tooling (test explorer and dotnet test) perspective, am I aligned?

(or perhaps present just for runners that are based on TPv2, like I assume Rider/Resharper/CodeRush will stick to).

I think your strategy make sense also for the support of other runners that are using Tpv2 api and will gradually move in future.

MarcoRossignoli commented 4 weeks ago

As soon as we have this part of the preview done we need to discuss another important thing, filtering. VSTestBridge add a --filter parameter that is used like in the Tpv2(we relay on the old object model). In the new testing platform we worked to have a new treenote filtering system(that can handle flat list of nodes too). To fold all filtering systems for all adapters in one and support complex tests structure(tree).

Here you can read the spec https://github.com/microsoft/testfx/blob/main/docs/mstest-runner-graphqueryfiltering/graph-query-filtering.md , we opened the experimental extension after the discussion with @thomhurst that's helping a lot with the dogfooding of the new features https://github.com/microsoft/testfx/issues/2462 https://github.com/microsoft/testfx/pull/3052 But this is matter for another "issue".

MarcoRossignoli commented 4 weeks ago

Do you have a release date planned for 1.4.0 yet?

Is it fine to you if for now we ship preview packages? Do you accept it as your deps? Until the first official release with these new features.

Evangelink commented 4 weeks ago

Do you have a release date planned for 1.4.0 yet?

It should be around early to mid September

thomhurst commented 4 weeks ago

Do you have a release date planned for 1.4.0 yet?

Is it fine to you if for now we ship preview packages? Do you accept it as your deps? Until the first official release with these new features.

Slightly off topic but I'd like to release an initial version of my framework soon, but I want IDEs to support it. Functionally the new framework has been great from my experience, but no one is going to want to adopt it if there isn't tooling support available.

Since I'm not using the VSTest bridge I don't get any of the existing test explorer functionality.

Is the new test explorer still in preview in VS? I haven't even tried VSCode, does that support it? And then there's Rider, have you collaborated with that team at all?

Evangelink commented 4 weeks ago

Is the new test explorer still in preview in VS? I haven't even tried VSCode, does that support it?

Yeah collaboration with the VS team is relatively slow but we should be taking over implementation of the feature soon. This will indeed work for Test Explorers of both VS and VS Code.

And then there's Rider, have you collaborated with that team at all?

We need to start discussion but we would really like to have something where we are sure we can cover all our needs before asking them to implement a different protocol and realize we are missing things.

thomhurst commented 4 weeks ago

I guess if you're making breaking changes and planning 2.0 (as I've seen in your issue list) then maybe it makes sense to wait for that. I just hope I can release something soon haha

bradwilson commented 4 weeks ago

So you'll take Microsoft.Testing.Platform.MSBuild together with Microsoft.Testing.Platform but you'll ship v3 with the back compatibility through xunit.runner.visualstudio for now. So you don't drop yet xunit.runner.visualstudio till our side is fully completed and in "parity" with the support of the old Tpv2 from the tooling (test explorer and dotnet test) perspective, am I aligned?

Exactly right. xunit.runner.visualstudio becomes just a vehicle for supporting TPv2, and if at some point there are no more TPv2 runners in the world, it can become deprecated. I suspect that will take a few years at least for all the third parties to catch up (and for people to stop using older IDEs; we are still supporting VS2019 today).

bradwilson commented 4 weeks ago

Is it fine to you if for now we ship preview packages? Do you accept it as your deps? Until the first official release with these new features.

I'm fine with the preview packages right now for the prototype, and once 1.4.0 ships officially in September then I can merge the prototype work into the main branch.

bradwilson commented 4 weeks ago

Okay, I've updated my prototype branch for the 1.4.0 prerelease: https://github.com/xunit/xunit/commit/0e44853abed0a2b067175920f00679853f69f83c

MarcoRossignoli commented 4 weeks ago

As soon as we have this part of the preview done we need to discuss another important thing, filtering.

Related to filtering you can also expose your runner options as first step, I see from the -h that you've already custom filtering options.

dotnet run -- -?
...
Filtering (optional, choose one or more)
If more than one filter type is specified, cross-filter type filters act as an AND operation

  -class "name"        : run all methods in a given test class (should be fully specified;
                       : i.e., 'MyNamespace.MyClass' or 'MyNamespace.MyClass+InnerClass')
                       :   if specified more than once, acts as an OR operation
  -class- "name"       : do not run any methods in a given test class (should be fully specified;
                       : i.e., 'MyNamespace.MyClass' or 'MyNamespace.MyClass+InnerClass')
                       :   if specified more than once, acts as an AND operation
  -method "name"       : run a given test method (can be fully specified or use a wildcard;
                       : i.e., 'MyNamespace.MyClass.MyTestMethod' or '*.MyTestMethod')
                       :   if specified more than once, acts as an OR operation
  -method- "name"      : do not run a given test method (can be fully specified or use a wildcard;
                       : i.e., 'MyNamespace.MyClass.MyTestMethod' or '*.MyTestMethod')
                       :   if specified more than once, acts as an AND operation
  -namespace "name"    : run all methods in a given namespace (i.e., 'MyNamespace.MySubNamespace')
                       :   if specified more than once, acts as an OR operation
  -namespace- "name"   : do not run any methods in a given namespace (i.e., 'MyNamespace.MySubNamespace')
                       :   if specified more than once, acts as an AND operation
  -trait "name=value"  : only run tests with matching name/value traits
                       :   if specified more than once, acts as an OR operation
  -trait- "name=value" : do not run tests with matching name/value traits
                       :   if specified more than once, acts as an AND operation
...
MarcoRossignoli commented 4 weeks ago

So I did a check on the branch and

To use new testing platform runner dotnet run or direct executable.

<UseMicrosoftTestingPlatformRunner>true</UseMicrosoftTestingPlatformRunner>

Plug the support for the current dotnet test (https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-platform-integration-dotnet-test#dotnet-test-integration)

<TestingPlatformDotnetTestSupport>true</TestingPlatformDotnetTestSupport>

Extension are ok, the missing part are only the mapping between the options for what I see.

MarcoRossignoli commented 4 weeks ago

@bradwilson I was doing some test with VS and --list-tests and discovery is not working, I noticed that no ITestCaseDiscovered are sent to the TestPlatformDiscoveryMessageSink.OnMessage so the push in the bus is not fired.

Evangelink commented 4 weeks ago

@bradwilson @thomhurst Platform 1.4.0-preview.24415.5 is published to NuGet. Let us know if anything is missing or wrong.

bradwilson commented 4 weeks ago

@bradwilson I was doing some test with VS and --list-tests and discovery is not working, I noticed that no ITestCaseDiscovered are sent to the TestPlatformDiscoveryMessageSink.OnMessage so the push in the bus is not fired.

Thanks, I hadn't tested list yet (wasn't possible really until I added UseMicrosoftTestingPlatformRunner). I also haven't pushed any command line options over into the M.T.P runner yet either.

bradwilson commented 4 weeks ago

@bradwilson @thomhurst Platform 1.4.0-preview.24415.5 is published to NuGet. Let us know if anything is missing or wrong.

(Sorry if this is a dup notifcation, I prepared my response in advance and accidentally sent it before I had run all the smoke tests 😂)

I successfully smoked test with all of C#/F#/VB without issue, for both dotnet test and the M.T.P native runner (not sure what y'all call that thing that surfaces from dotnet run). 👍🏼

bradwilson commented 4 weeks ago

I'm enabling my ability to use your runner from within the xunit.v3 test projects themselves, and I see two oddities. First, it says the target framework is net489; second, the one-liner for project status shows a different test count than the final results.

image

Edit: I noticed that the line with the check mark disappears immediately when I have removed all my console output:

https://github.com/user-attachments/assets/21ee7d40-2b31-42ee-90bc-001b05052572

bradwilson commented 4 weeks ago

Also, is there a better way for me to get our output in a place that users can find it, other than Console.WriteLine? It would be good for it to show up in the logs but not when normally running. Is there a logging API that I should take advantage of?