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

[filzrev]

It looks like generics are still not displayed as expected.

[sheddy123]

@filzrev does the latest change suffice

[timcassell]

Thanks for this — the adapter itself works. Built Debug and Release (0 warnings), unit suite 1057 passed, and the new MTP project discovers and runs 9/9 with filters behaving, including through a real packed .nupkg following testadapter.md steps 1–5. The design is sound; what blocks merge is the MSBuild layer.

1. The IsTestingPlatformApplication opt-outs never fire for package consumers (build/BenchmarkDotNet.TestAdapter.targets:16-17). Both conditions require the property to be empty, but MTP's buildTransitive targets are imported first and already default it to true (confirmed via -pp ordering and -getProperty). Against the packed nupkg: dotnet test hard-fails for the documented VSTest opt-in on .NET 10+ SDKs; warning CS8892 fires for every non-default configuration, which is an error under TreatWarningsAsErrors; and testadapter.md:157's GenerateProgramFile=false claim is false. Making the opt-out authoritative fixes all three:

<IsTestingPlatformApplication Condition="'$(BenchmarkDotNetUseVSTest)' == 'true' or '$(GenerateProgramFile)' == 'false'">false</IsTestingPlatformApplication>

The repo can't catch this because the samples and the new project <Import> the props/targets by file path from the csproj body — before nuget.g.targets, the opposite order from a real consumer. Same trap as #3186.

2. Existing VSTest users silently lose their entry point (build/BenchmarkDotNet.TestAdapter.props:16). GenerateBDNEntryPoint now requires BenchmarkDotNetUseVSTest=true, so the setup documented before this PR no longer gets EntryPoint.cs. Reproduced: Microsoft.NET.Test.Sdk's stub Main wins and the exe prints nothing and exits 0, where it used to run BenchmarkSwitcher.

Both are invisible to this repo's own build, and the new project has no assertions and is only ever built — coverage that consumes the built .nupkg would catch them.

3. The global.json has no effect where it sits

tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/global.json sets test.runner: Microsoft.Testing.Platform, which is what makes dotnet test drive an MTP app instead of VSTest on .NET 10+ SDKs. It's resolved from the working directory upward, not relative to the project, so it only applies when dotnet test is invoked from that folder — and nothing in the repo does that. UnitTestRunner passes full project paths and runs from the repo root; the coverage workflow names project directories; and run-tests-selected.yaml, which does set working-directory to the project, takes a fixed type: choice list that doesn't include this project (and this PR doesn't add it).

So either delete the file, or add tests/BenchmarkDotNet.IntegrationTests.TestingPlatform to that workflow's project options — which is what would make it load-bearing. Two notes if you wire the project into CI:

• dotnet test from the repo root fails, on the project or the solution: error : Testing with VSTest target is no longer supported by Microsoft.Testing.Platform on .NET 10 SDK and later. Not a CI path today, but a normal thing for a contributor or IDE to do.
• Adding it to UnitTestRunner will fail as-is: that runner passes a full project path with no WorkingDirectory, so it executes from the repo root and won't find the file. Either set WorkingDirectory on the Cake DotNetTestSettings for this project, or skip dotnet test and exec the built exe directly — the latter works today, filters included.

Two fixes that look obvious but don't work, so nobody burns time on them:

• Moving global.json to the repo root. It wouldn't pin the SDK (no sdk section), but test.runner is repo-wide, and BDN's suites are xunit 2.9.3 + Microsoft.NET.Test.Sdk. With it in place, dotnet test refuses them outright: "The following test projects are using VSTest test runner", exit 1. A repo picks one runner, and BDN's is VSTest.
• Opting out per project. Neither IsTestProject=false nor TestingPlatformDotnetTestSupport=true suppresses it, because the error comes from MTP's own target, gated only on IsTestingPlatformApplication and the SDK version:

<Target Name="_MTPBeforeVSTest" BeforeTargets="VSTest">
  <Error Text="Testing with VSTest target is no longer supported…"
         Condition="'$(IsTestingPlatformApplication)'=='true' AND '$(TargetFramework)'!='' AND '$(_SupportsGlobalJsonTestRunner)'=='true'" />

It fires even when the VSTest target would be a complete no-op for that project, which is arguably worth an upstream issue against Microsoft.Testing.Platform.MSBuild.

Smaller

• BenchmarkTestFramework.cs:174 — cancelled runs leave InProgress nodes with no terminal state, so IDEs pin them at "running".
• BenchmarkEventProcessor.cs:57 — OnBuildComplete runs concurrently per build partition and mutates plain Dictionary/HashSet on the failure path.
• BenchmarkTestNode.BuildPath escapes / but not the [/] it inserts around the job, so an exact-path tree filter can't match a node; three comments say --filter for what is --treenode-filter; AssemblyInfo.cs's only diff is a stray trailing space.

Core changes are behaviour-neutral — FullNameProvider only adds a method, so summaries, exports and baselines are unchanged, and UIDs are stable across discovery and execution.

The description mentions making MonoBenchmarks/SharedDiagnosers Debug-only — that's not in the diff; worth correcting.

Reviewed by Claude (Opus 5), posted by @timcassell.

[sheddy123]

@timcassell / @filzrev the last 4 issues raised have been reviewed and addressed:

1. Mutator wiping categories - fixed in ImmutableConfigBuilder: the job's own categories are captured before Apply and merged back after. I didn't use IgnoreOnApply as suggested, UnfreezeCopyCore is built on ApplyCore, so it would drop categories on every WithXxx call, not just mutator application (which is why IdCharacteristic is special-cased there). I tried it; it broke TheSameCategoryIsNotAddedTwice. There's now a test pinning that invariant.
2. Meta.Categories = null guard moved into MetaMode, the funnel all three entry points share, so the property setter and AddCategories are covered too.
3. WithCategory(null) null categories now rejected instead of stored.
4. Missing attribute and CLI option added [JobCategoryFilter] and --jobCategories, both tested end to end.

[timcassell]

Reviewed b6d217780..eb40140d5.

The adapter code holds up. The concurrency reasoning checks out against the runner rather than just the comments: OnBuildComplete really is raised from the parallel build tasks (BenchmarkRunnerClean.cs:436) and really is the only concurrent callback, since BDN swaps in NullLogger whenever there is more than one partition (BenchmarkRunnerClean.cs:421) — so buildCompleteGate covers the right surface and the unsynchronised StringBuilder in OutputDeviceLogger is safe. The channel drain, linked cancellation and TryComplete in the inner finally are all correct. The histogram block in AppendMeasurementSummary is verbatim from the existing VSTestEventProcessor, so no new risk there.

Two things worth raising.

1. The new integration project asserts nothing, and never builds a benchmark.

All three probes pin Job.Dry.WithToolchain(InProcessEmitToolchain.Default), so the build path is never taken — which leaves OnBuildComplete, the one callback that needed the new locking and that maps build failures onto failed tests, with zero coverage. The project has no assertions at all; CI runs dotnet test against it and the only thing proven is that a run does not crash. Nothing pins uid stability across the discover/run process boundary, the Description-over-method-name display naming, --treenode-filter matching, or the collision report — which is the actual new logic. A single out-of-process job would cover the build path.

2. Neither in-repo consumer exercises the import order BenchmarkDotNet.TestAdapter.targets is built around.

The IsTestingPlatformApplication overwrite is justified in-comment by "a real package consumer imports [Microsoft.Testing.Platform.MSBuild's targets] before this file". But the samples project and the new test project both <Import> inside the csproj body, which MSBuild evaluates before nuget.g.targets (imported at the end via Sdk.targets). In-repo, BDN's targets therefore land before MTP.MSBuild's — the reverse of the packaged order. It probably still works either way if MTP's default is conditioned on empty, but the ordering the comment reasons about is never actually tested. Same delivery-path split as #3186; a pack-and-restore smoke test would close it.

Minor: BenchmarkTestNode.Escape replaces / with \/ but leaves existing backslashes alone, so a parameter whose ToString() contains a literal \/ is indistinguishable from an escaped separator in the tree path.

Confirmed fixed since the last round: global.json is scoped to the test project rather than the repo root, and GetUniqueId now comes from the core public extension added in #3227, with the adapter's duplicate removed.

_{Reviewed by Claude Opus 5 via Claude Code.}

[sheddy123]

@timcassell the issues have been addressed