Skip to content

Feature/mtp test adapter 2803 - #3229

Open
sheddy123 wants to merge 114 commits into
dotnet:masterfrom
sheddy123:feature/mtp-test-adapter-2803
Open

Feature/mtp test adapter 2803#3229
sheddy123 wants to merge 114 commits into
dotnet:masterfrom
sheddy123:feature/mtp-test-adapter-2803

Conversation

@sheddy123

Copy link
Copy Markdown
Contributor

#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.

Added a new guide for running benchmarks with Microsoft.Testing.Platform (MTP), covering setup, usage, and caveats. Updated the table of contents to include the new page and added a note to the VSTest docs about the MTP adapter option.
Added InternalsVisibleTo attribute in AssemblyInfo.cs to expose internal members to the BenchmarkDotNet.TestAdapter.TestingPlatform assembly, ensuring it uses the same public key as other related assemblies.
Deleted the internal static method GetUnrandomizedJobDisplayInfo from BenchmarkCaseExtensions.cs. This method handled normalization of job display info by removing randomness from job IDs for consistent benchmark referencing. No other code changes were made.
Introduced BenchmarkCaseIdentityExtensions with GetUnrandomizedJobDisplayInfo to normalize Job DisplayInfo by removing random ID components. This ensures consistent benchmark identification across processes for test adapters.
Introduce GetBenchmarksFromAssembly to extract benchmarks from an already loaded Assembly. Refactor existing logic to use this method, improving code reuse and enabling benchmark retrieval from both loaded assemblies and file paths.
Add MSBuild props to enable TestingPlatform integration, set defaults for `dotnet test` compatibility, disable parallel TFM runs, and auto-register BenchmarkDotNet builder hook.
Introduced AsyncWorkQueue, an internal sealed class in BenchmarkDotNet.TestAdapter.TestingPlatform. It enables ordered, thread-safe queuing of asynchronous work items, allowing synchronous producers and asynchronous consumers. Utilizes ConcurrentQueue and SemaphoreSlim, supports completion signaling, and implements IDisposable for resource cleanup.
Created a new .csproj targeting netstandard2.0 for the TestingPlatform adapter. Configured project metadata, packaging, and references. Integrated Microsoft.Testing.Platform.MSBuild and BenchmarkDotNet, and linked shared source files for benchmark enumeration. Set IsTestingPlatformApplication to false to avoid test app behavior.
Introduced BenchmarkDotNetExtension class implementing IExtension to provide extension metadata and enablement status for Microsoft.Testing.Platform integration.
Introduced BenchmarkEventProcessor to process BenchmarkDotNet events and translate them into test node updates for the testing platform. Handles validation errors, build results, benchmark execution, and ensures all benchmarks have published results. Includes logic for error aggregation, output formatting, and timing information.
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.
Introduced the internal sealed class BenchmarkTestNode to encapsulate immutable BenchmarkCase data for Microsoft.Testing.Platform integration. This includes stable UID generation, display name and path construction, property management, and support for test filtering and message bus conversion.
Introduced OutputDeviceLogger class implementing ILogger to forward BenchmarkDotNet logs to the platform output device. Handles log kinds, buffers lines, and asynchronously displays output to ensure build progress and results are visible in test run output.
Introduce TestApplicationBuilderExtensions with AddBenchmarkDotNet methods for integrating BenchmarkDotNet benchmarks into Microsoft.Testing.Platform. Includes overloads for entry assembly and specific assemblies, null checks, test framework registration, and tree node filter service support.
Introduced a static TestingPlatformBuilderHook class in the BenchmarkDotNet.TestAdapter.TestingPlatform namespace. This class provides an AddExtensions method to register BenchmarkDotNet with the test application builder, intended for use by generated code and hidden from IntelliSense.
Added a "test" section to global.json to specify "Microsoft.Testing.Platform" as the test runner. This configures the project to use the designated testing platform.
Introduce BenchmarkDotNet.IntegrationTests.TestingPlatform.csproj targeting net10.0 as an executable. The project includes assembly metadata, references BenchmarkDotNet.TestAdapter.TestingPlatform, manually imports its build props, and uses shared common.props and common.targets for build configuration.
Introduced SampleBenchmarks class in BenchmarkDotNet.IntegrationTests.TestingPlatform. Defines Add and Multiply benchmarks with parameterized Size, categorized as "Fast" and "Slow". Uses a custom FastConfig to run benchmarks in-process with a single dry iteration for quick end-to-end testing.
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.
Explicitly set BenchmarkDotNet.TestAdapter.TestingPlatform and BenchmarkDotNet.IntegrationTests.TestingPlatform to not build in the Debug configuration by adding <Build Solution="Debug|*" Project="false" /> in BenchmarkDotNet.slnx. No other changes made.
@timcassell

Copy link
Copy Markdown
Collaborator

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.

Comment thread src/BenchmarkDotNet.TestAdapter.TestingPlatform/BenchmarkTestNode.cs Outdated
- Correct NuGet package and namespace in documentation
- Add GetBenchmarkUid for stable benchmark identification
- Change namespace in BenchmarkCaseIdentityExtensions
- Update InternalsVisibleTo for TestingPlatform assembly
Deleted all source, project, and props files from BenchmarkDotNet.TestAdapter.TestingPlatform. This removes all implementation and integration for running benchmarks as tests via Microsoft.Testing.Platform, including test discovery, execution, and result processing logic.
Add BenchmarkDotNet.TestingPlatform.props to enable seamless integration with Microsoft.Testing.Platform. This includes setting required properties for Testing Platform application behavior, ensuring `dotnet test` compatibility on older SDKs, disabling parallel test execution for multi-targeted projects by default, and registering BenchmarkDotNet as a builder hook.
Introduced AsyncWorkQueue in BenchmarkDotNet.TestingPlatform to enable thread-safe, ordered queuing of asynchronous work items. Supports synchronous enqueuing, asynchronous draining, completion signaling, and resource disposal using ConcurrentQueue and SemaphoreSlim.
Introduce a new project to integrate BenchmarkDotNet with Microsoft.Testing.Platform, enabling benchmarks to be discovered and executed as tests. Implements extension identification, test framework, event processing, test node representation, and output logging. Provides builder extensions for easy registration and an MSBuild hook for automatic integration. Updates project configuration for packaging and dependencies.
Updated BenchmarkDotNet.IntegrationTests.TestingPlatform.csproj to reference BenchmarkDotNet.TestingPlatform instead of BenchmarkDotNet.TestAdapter.TestingPlatform. Adjusted both the ProjectReference and Import paths accordingly.
GenericBenchmarkType now has an IsUnreadable property to indicate types rejected before reading benchmarks. Added Unreadable static method and updated constructor to accept isUnreadable. Failed static method now sets isUnreadable to false. GenericBenchmarksBuilder uses BuildRunnableBenchmarks to return all built and rejected types. BuildGenericsIfNeeded returns Unreadable for types with missing arguments. Added XML docs for new property and method.
Added TestingPlatformServerModeSession for JSON-RPC server mode integration, simulating VS/VS Code test host interactions. Expanded TestingPlatformAdapterTests to cover async-disposable parameter handling and server mode parameter lifecycle. Improved test node state tracking and request/response synchronization.
Added AsyncDisposableProbe for benchmarking async-disposables and tracking their disposal, writing counts on process exit. Updated Tracked in DisposableProbe.cs to use a private number field, added isDisposed flag, throw on access after disposal, updated ToString(), and set isDisposed in Dispose().
Added tests to ensure types with unreadable [Config] attributes
are properly reported and do not silently fail benchmark
discovery. Updated GenericBuilderTests and TypeFilterTests with
assertions and new test classes to simulate config instantiation
failures.
Refactored TypeFilter.cs to introduce AddRunnable, which adds runnable benchmark types and logs errors for types with unreadable attributes. Replaced LINQ-based addition with explicit error reporting, enhancing clarity on excluded types and improving diagnostics during benchmark discovery.
Previously, all unsuccessful builds were reported as validation errors, including those that were unreadable and already handled by the TypeFilter. The updated LINQ query now excludes builds that are both unsuccessful and unreadable, preventing duplicate error reporting for unreadable types.
Introduce ParameterValueLifetime to manage benchmark parameter lifetimes, preventing premature disposal in server mode. Update BenchmarkTestFramework and related classes to use this for tracking and disposing parameter values. Improve benchmark state handling in BenchmarkEventProcessor and avoid publishing empty source file locations. Update service registration and filtering logic for safety and correctness.
Introduce DisposeUnusedAsync in ParameterValueDisposer to support asynchronous disposal of unused benchmark parameter values (IDisposable and IAsyncDisposable). Refactor disposal logic to operate on ParameterInstance objects for correct semantics. Overload GetBenchmarksFromAssembly to accept a custom disposal action, enhancing flexibility. Maintain reference-based equality for parameter value tracking. Synchronous DisposeUnused now wraps the async method using BenchmarkSynchronizationContext.

@timcassell timcassell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up on the latest push. The IAsyncDisposable gap, the per-value exception handling, the Skipped/Failed state and the SourceCodeFile guard all look closed; six things on the new code, inline.

🤖 Reviewed with Claude Code

Comment thread src/BenchmarkDotNet/Running/TypeFilter.cs
Comment thread src/BenchmarkDotNet/Validators/GenericBenchmarksValidator.cs Outdated
Comment thread src/BenchmarkDotNet.TestAdapter/TestingPlatform/ParameterValueLifetime.cs Outdated
Comment thread src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestFramework.cs Outdated
Comment thread src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestFramework.cs Outdated
Comment thread src/BenchmarkDotNet.TestAdapter/TestingPlatform/ParameterValueLifetime.cs Outdated
Updated the XML documentation for the IsUnreadable property to clarify its meaning. The new comment explains that types rejected due to incompatible [GenericTypeArguments] still declare benchmarks, so "no benchmarks were found" should not be used for them. It also specifies responsibility for explaining dropped types. No code logic was changed.
…marks

Refactored to build runnable benchmark types once per assembly and reuse the result, improving efficiency and clarity. Updated logic to check for runnable benchmarks using assemblyTypes.Any with IsSuccess or IsUnreadable. Enhanced comments to clarify reporting of unreadable types and the validator's role.
Refactor parameter value disposal logic to track usage across requests, preventing premature disposal and memory leaks, especially in server mode scenarios. BenchmarkEventProcessor now exposes ParameterValuesDisposed. Update filter matching to handle unknown types gracefully and warn users. Add WarnAboutUnrecognisedFilterAsync to BenchmarkTestFramework. Update comments and docs to clarify new strategy.
Added tests for parameter value disposal in TestingPlatformAdapterTests.cs, including cases for per-read sources and BenchmarkDotNet validation failures. Updated TestingPlatformServerModeSession.cs to support an optional discoverAgain parameter, enabling a second discovery after test runs to simulate IDE refresh. Modified Run and DiscoverThenRun methods to handle discoverAgain, and enhanced test infrastructure for better real-world simulation.
Added tests to ensure proper logging when benchmark types or assemblies have unreadable attributes. Introduced a helper to emit assemblies with unreadable benchmark types for testing.
Previously, only unsuccessful and readable benchmarks were reported as validation errors. Now, the logic includes all unsuccessful benchmarks, ensuring unreadable types are also reported. This change improves error reporting for scenarios without TypeFilter, such as BenchmarkRunner.Run<T>() and some test adapters.
Introduced FreshValueProbe in BenchmarkDotNet.IntegrationTests.TestingPlatform to benchmark parameter sources that yield new resources on each read. Tracks reads, creations, and disposals of Fresh disposable instances, and writes reports to a file per read and at process exit. Includes FastConfig for in-process dry job execution.

@timcassell timcassell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up on the latest push. The lifetime rework does close the value-loss windows for the sequential discover→run→discover case (traced against the new FreshValueProbe for both cached and per-read sources), and the TypeFilter rework reads as behaviour-preserving apart from the intended change. Seven things inline, two worth acting on before merge.

🤖 Reviewed with Claude Code

Comment thread src/BenchmarkDotNet.TestAdapter/TestingPlatform/ParameterValueLifetime.cs Outdated
Comment thread src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkEventProcessor.cs Outdated
Comment thread src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestFramework.cs Outdated
Comment thread src/BenchmarkDotNet/Validators/GenericBenchmarksValidator.cs
Comment thread tests/BenchmarkDotNet.IntegrationTests/TestingPlatformAdapterTests.cs Outdated
- Use ExceptionDispatchInfo to capture/rethrow exceptions during async disposal, ensuring cleanup and event signaling always occur
- Update artifacts cleanup log message for consistency
- Prevent duplicate ValidationError entries by tracking errors in a HashSet during validation
Added AnAssemblyWideValidationWarningIsReportedOncePerNode test to ensure assembly-wide validation warnings are reported once per node. Updated ServerNode in TestingPlatformServerModeSession.cs to include StandardOutput property and populated it from node JSON data.
Refactored parameter value lifetime management by introducing a RequestScope class to encapsulate parameter values per request, improving isolation and handling of overlapping requests. Moved tracking, hiding, and completion logic into RequestScope and updated BenchmarkTestFramework to use the new scope. Updated disposal logic to ensure correct cleanup and clarified comments. Improved handling of unrecognized filters: discovery lists all benchmarks, run requests reject unsupported filters.

@timcassell timcassell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything from the last round looks closed: the per-request RequestScope holds up under overlapping requests (discovery-during-run, run-during-run, cached and per-read sources), HasEnumerated covers the request that dies before it enumerates, the validation dedup collapses exactly the case-less assembly-wide duplicates, and the filter fallback is split the right way. Three small things inline.

🤖 Reviewed with Claude Code

public Task BeforeRunAsync(CancellationToken cancellationToken) => Task.CompletedTask;

/// <inheritdoc />
public async Task AfterRunAsync(int exitCode, CancellationToken cancellationToken)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scopes refactor dropped the exit-time safety net. This used to drain held.Concat(inFlight), with the comment that "a value that somehow is [in flight] would otherwise be leaked for good"; now that a request's values live in a RequestScope the lifetime object has no reference to, nothing takes its place.

Server mode, client sends exit (or the IDE cancels) while a run request's finally has not run yet: AfterRunAsync drains held, then the request's CompleteAsync sets held = enumerated, and those values are never disposed — the #1383 finalizer case this class exists to prevent.

Registering the scope in BeginRequest, removing it in CompleteAsync, and draining the still-live ones here restores the guarantee the old Concat gave.

// spend the machine's next hour benchmarking the whole assembly instead of the subset that was asked
// for, and a warning on the output device is not something an IDE is bound to surface. Refusing the
// request keeps the filter visible and costs nothing but a re-run once it is supported.
if (enumeration.UnrecognisedFilter is { } unrecognisedFilter)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither half of the split is covered: nothing tests the discovery warning and nothing tests this refusal. Since the branch is unreachable with MTP 2.3.3 — NopFilter, TestNodeUidListFilter and TreeNodeFilter are the only implementations, and there's no composite — a regression here would be silent. A fake ITestExecutionFilter fed through the framework pins both paths.

Worth checking what the platform sees when it does fire: this escapes ExecuteRequestAsync after its finally has already called context.Complete(), and no test node has been published, so the request is reported finished before the failure is observed and an IDE gets a run with no per-test feedback. Publishing the nodes as failed (or erroring before Complete) would say it in a place the user can read.

"Probe.Identity",
Timeout);

Assert.True(ran.Count >= 4, $"Expected several benchmark types to run, but {ran.Count} node(s) did.");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ran.Count counts nodes, but the message says "benchmark types" and the dedup under test only does anything when two or more BenchmarkRunInfos are validated. "Probe.Identity" currently selects 8 nodes across 4 types, so the test does fail without the fix — but a later probe change leaving 4+ nodes on a single type would make it pass without exercising the dedup at all.

Asserting on distinct type prefixes (>= 2 distinct types) keeps it honest regardless of how the probes evolve.

Comment thread src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs Outdated
Refactored post-benchmark cleanup to remove ExceptionDispatchInfo usage. Cleanup and event notification now occur in a finally block, guaranteeing artifact removal and stage completion even if disposal throws, simplifying error handling and ensuring consistent resource management.
@sheddy123

Copy link
Copy Markdown
Contributor Author

@timcassell the changes have been effected

@timcassell

Copy link
Copy Markdown
Collaborator

@timcassell the changes have been effected

You missed 3 comments.

Add InternalsVisibleTo for BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals using BenchmarkDotNetInfo.PublicKey. This allows direct testing of Microsoft.Testing.Platform internals, supporting scenarios like test execution filters and incomplete application requests not accessible via a real test host. Added comments to clarify intent.
Refactored BenchmarkTestFramework to mark benchmarks as failed with clear messages for unrecognized filters, enhancing IDE feedback. Updated ParameterValueLifetime to track live request scopes, ensuring all parameter values are disposed properly and preventing resource leaks. Added comments to clarify resource management and error handling changes.
- Added project reference to BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals.
- Introduced `InternalsProbe` constant in `TestingPlatformAdapterTests.cs`.
- Improved test to count distinct benchmark types.
- Added tests for unrecognized filters and parameter disposal.
- Implemented `RunInternalsProbe` helper and `InternalsReport` record for probe execution and parsing.
Introduce a new project to test BenchmarkDotNet's Microsoft.Testing.Platform integration. Includes benchmarks for resource cleanup and filtering, a platform stub for simulation, and a test orchestrator. Adds project file and documentation detailing test scenarios not possible from a real test host.
Added BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals project to the solution file for improved test coverage and integration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants