Feature/mtp test adapter 2803

[sheddy123]

#2803
@timcassell

Introduce BenchmarkTestFramework to integrate BenchmarkDotNet with Microsoft.Testing.Platform, enabling benchmark discovery and execution. Implements session management, filtering, event processing, and output routing. Handles test node updates and cancellation, with support for experimental platform features.
Added BenchmarkDotNet.TestAdapter.TestingPlatform and BenchmarkDotNet.IntegrationTests.TestingPlatform projects to the solution. Updated MonoBenchmarks and SharedDiagnosers integration test projects to only build for the solution in Debug configuration.

[timcassell]

Let's name it BenchmarkDotNet.TestingPlatform.

Updated MonoBenchmarks and SharedDiagnosers integration test projects to only build for the solution in Debug configuration.

Why? We run tests in Release configuration.

[filzrev]

I though GetUid logics should be implemented on BenchmarkDotNet core project side.
Because --filter-uid option is useful for normal benchmark exe project without MTP.

I've implemented MSTest based UID generation logics on #3227.
Is it able to confirm these logics can be shared with TestAdapter?

[sheddy123]

This has been noted and taken into consideration. I have done the fix

[filzrev]

#3227 is merged to master.
So GUID based UID generator is available.

BenchmarkDotNet/src/BenchmarkDotNet/Helpers/UniqueIDGenerator.cs

Line 16 in b6d2177

public static string FromBenchmarkCase(BenchmarkCase benchmarkCase)

[filzrev]

Following generic benchmarks are not shown correctly on VS Test Explorer.

    [InProcess]
    [GenericTypeArguments(typeof(int))]
    [GenericTypeArguments(typeof(int?))]
    [GenericTypeArguments(typeof(int[]))]
    [GenericTypeArguments(typeof(int?[]))]
    [GenericTypeArguments(typeof(int[,]))]
    [GenericTypeArguments(typeof(int?[,]))]
    public class GenericTypeBenchmarks<T>
    {
        [Benchmark]
        public void Benchmark() { }
    }

I though TestMethodIdentifier's property require ECMA-335 compliant type names.
https://learn.microsoft.com/en/dotnet/api/microsoft.testing.platform.extensions.messages.testmethodidentifierproperty

xUnit.net example.
https://github.com/xunit/xunit/blob/rel/4.0.0/src/xunit.v3.common/Extensions/ReflectionExtensions.cs#L171

[sheddy123]

@filzrev , @timcassell, this is up for review. Thanks

[timcassell]

Reviewed the MTP adapter glue. The shared-code extraction (GetUnrandomizedJobDisplayInfo → BenchmarkCaseIdentityExtensions, the BenchmarkEnumerator split, FullNameProvider.GetBenchmarkUid) looks clean. Two correctness issues in BenchmarkTestFramework, plus a couple of minor notes.

1. A faulting queued work item orphans the benchmark run and disposes the semaphore under it

src/BenchmarkDotNet.TestingPlatform/BenchmarkTestFramework.cs:108–157

The background runTask (BenchmarkRunner.Run) keeps calling workQueue.Enqueue from the event-processor/logger callbacks for the whole run, while the main thread awaits workQueue.DrainAsync(). DrainAsync (AsyncWorkQueue.cs:52) awaits each item and does not catch exceptions. If any queued item throws — e.g. MessageBus.PublishAsync or outputDevice.DisplayAsync faulting as the host bus tears down — the exception propagates out of RunAsync, so await runTask (line 157) is never reached and using var workQueue (line 108) disposes the SemaphoreSlim. The still-running runTask then hits Enqueue → available.Release() on the disposed semaphore, throwing ObjectDisposedException on BenchmarkDotNet's run thread, while the benchmark run itself continues orphaned and its Task fault goes unobserved.

Suggest wrapping the drain so runTask is driven to completion (or cancelled) before the workQueue is disposed.

2. A duplicate UID aborts the entire run request instead of failing one test

src/BenchmarkDotNet.TestingPlatform/BenchmarkTestFramework.cs:104

matches.ToDictionary(match => match.Node.Uid, match => match.Node) throws ArgumentException ("An item with the same key has already been added") on a duplicate UID. UIDs come from FullNameProvider.GetBenchmarkUid = class.method(params) [job], where the params are user-stringified values. Two distinct [Arguments]/[ParamsSource] values whose ToString() collapses to the same text (or the same value under two jobs with identical unrandomized DisplayInfo) will abort the whole run request with an unhandled exception rather than reporting the tests. Note the discovery path (DiscoverAsync, lines 84–95) has no equivalent guard and would instead publish two nodes with the same TestNodeUid.

Minor

• Cancellation drain (BenchmarkTestFramework.cs:118 / OutputDeviceLogger.cs:62): the already-cancelled cancellationToken is passed to outputDevice.DisplayAsync, so queued log writes throw OperationCanceledException inside DrainAsync before the remaining items drain. Close to the intended "OCE on cancel" behavior, but combined with finding 1 the resulting runTask fault is unobserved.
• Cosmetic (FullNameProvider.cs:76, 82–83): trailing whitespace on [PublicAPI] and a double blank line.

---

Review by Claude (Claude Code), posted on Tim Cassell's behalf.

[filzrev]

Is it required to keep VSTest based TestAdapter?

I think it's better just update the test adapter to use MTP, rather than creating a separate package.
as commented at #2803 (comment)

It might be cause following limitations by dropping VSTest-based TestAdapter.

• It can't mix VSTest-based test projects with MTP based benchmark project in single solution.
• It's not works on old platform that don't support MTP (e.g. Visual Studio 2019)

Though, It can reduce maintenance cost by dropping VSTest based TestAdapter.
Because TestAdapter feature is mainly for test/debugging purpose.

[sheddy123]

@filzrev you're right that this is what @timcassell asked for in #2803, so I should settle it before going further.

One thing worth separating, though, is that "one package" and "drop VSTest" aren't the same decision. A single package can serve both protocols; that's what MSTest does (EnableMSTestRunner flips the same adapter package into MTP mode), and MS even ships Microsoft.Testing.Extensions.VSTestBridge for frameworks that want one implementation to cover both. BDN wouldn't need the bridge since both implementations already exist here; they'd just ship together, gated by a property. That keeps VS2019 and mixed VSTest/MTP solutions working while still being one package.

Merging into BenchmarkDotNet.TestAdapter is mostly repackaging on my side the framework/node/event-processor code moves over intact; what changes is the csproj, the build props, and choosing which entry point to generate.

I will be happy to do it. @timcassell @filzrev , which do you want: 1. two packages or 2. one package supporting both protocols? I'll rework to whichever you pick.

[timcassell]

"Drop vstest" was implied by my original comment, however if we can have both without too much hassle, I think it's fine. The vstest adapter is considered feature complete afaik, so maintenance should only be a matter of keeping up with any core API changes. I'm not too concerned about 1 vs 2 packages, whatever seems better for user consumption for minimal confusion. If you go with 1 package, the new platform should be the default.

[sheddy123]

Thanks for the response @timcassell
I'll go with 1 package, MTP as the default, since making people choose between two packages is exactly the confusion worth avoiding, and VSTest stays available behind an opt-in property for VS2019 and mixed VSTest/MTP solutions.
A user adding BenchmarkDotNet.TestAdapter shouldn't have to first learn what VSTest and MTP are to choose between two packages

However, one migration detail I want to flag before I do it: the package currently generates a BenchmarkSwitcher.FromAssembly(...).Run(args) entry point (entrypoints/EntryPoint.cs), and in MTP mode Microsoft.Testing.Platform.MSBuild generates its own instead. So on upgrade, dotnet run on an existing benchmark project changes behaviour. I am thinking of accepting it and document it in the changelog since it is simpler and matches "new platform is the default"

[timcassell]

However, one migration detail I want to flag before I do it: the package currently generates a BenchmarkSwitcher.FromAssembly(...).Run(args) entry point (entrypoints/EntryPoint.cs), and in MTP mode Microsoft.Testing.Platform.MSBuild generates its own instead.

Right, that's why the current vstest adapter disables that (see BenchmarkDotNet.TestAdapter.props);

So on upgrade, dotnet run on an existing benchmark project changes behaviour. I am thinking of accepting it and document it in the changelog since it is simpler and matches "new platform is the default"

We're already shipping lots of breaking changes in 0.16, so it's fine.