diff --git a/.github/workflows/run-tests-selected.yaml b/.github/workflows/run-tests-selected.yaml index 0e2f436810..e4e8be56fe 100644 --- a/.github/workflows/run-tests-selected.yaml +++ b/.github/workflows/run-tests-selected.yaml @@ -26,6 +26,10 @@ on: - tests/BenchmarkDotNet.IntegrationTests - tests/BenchmarkDotNet.IntegrationTests.ManualRunning - tests/BenchmarkDotNet.IntegrationTests.ManualRunning.MultipleFrameworks + # This one is a Microsoft.Testing.Platform application. It carries the `global.json` which routes + # `dotnet test` to the platform, and that file is resolved from the working directory, which this + # workflow sets to the project. + - tests/BenchmarkDotNet.IntegrationTests.TestingPlatform - samples/BenchmarkDotNet.Samples framework: type: choice diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index b3ea72e473..abc15b37dd 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -185,6 +185,11 @@ jobs: - uses: actions/checkout@v7 - name: Run task 'pack' run: ./build.cmd pack + # Nothing in the solution consumes BenchmarkDotNet.TestAdapter as a package, so nothing else exercises the + # order in which NuGet imports its build files relative to Microsoft.Testing.Platform.MSBuild's. + - name: Smoke test the packed BenchmarkDotNet.TestAdapter + shell: pwsh + run: ./build/smoke-tests/test-adapter-consumer.ps1 spellcheck-docs: runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 4226473653..ba9270b557 100644 --- a/.gitignore +++ b/.gitignore @@ -67,6 +67,9 @@ Resource.designer.cs # Tests TestResults +# The smoke test restores BenchmarkDotNet.TestAdapter into a packages folder of its own, see its .csproj +build/smoke-tests/packages/ + ## Mac OS # General diff --git a/BenchmarkDotNet.slnx b/BenchmarkDotNet.slnx index f9557849db..26a5ea5a5d 100644 --- a/BenchmarkDotNet.slnx +++ b/BenchmarkDotNet.slnx @@ -44,6 +44,10 @@ + + + + diff --git a/build/cSpell.json b/build/cSpell.json index 0e3a002ab1..e21fa024aa 100644 --- a/build/cSpell.json +++ b/build/cSpell.json @@ -35,6 +35,7 @@ "vsprofiler", "vstest", "Tailcall", + "testadapter", "toolchains", "unmanaged" ], diff --git a/build/smoke-tests/TestAdapterConsumer/ConsumedBenchmark.cs b/build/smoke-tests/TestAdapterConsumer/ConsumedBenchmark.cs new file mode 100644 index 0000000000..fc74f1e53e --- /dev/null +++ b/build/smoke-tests/TestAdapterConsumer/ConsumedBenchmark.cs @@ -0,0 +1,22 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace TestAdapterConsumer; + +/// +/// A single benchmark, enough for the smoke test to check that the packaged adapter turns this project into a +/// Microsoft.Testing.Platform application that can list it. +/// +[Config(typeof(FastConfig))] +public class ConsumedBenchmark +{ + [Benchmark] + public int Add() => 1 + 1; + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } +} diff --git a/build/smoke-tests/TestAdapterConsumer/TestAdapterConsumer.csproj b/build/smoke-tests/TestAdapterConsumer/TestAdapterConsumer.csproj new file mode 100644 index 0000000000..d1fb804f5e --- /dev/null +++ b/build/smoke-tests/TestAdapterConsumer/TestAdapterConsumer.csproj @@ -0,0 +1,43 @@ + + + + + + net10.0 + Exe + enable + enable + + $(MSBuildThisFileDirectory)..\..\..\artifacts + + $(MSBuildThisFileDirectory)..\packages\ + + + + + + + + diff --git a/build/smoke-tests/test-adapter-consumer.ps1 b/build/smoke-tests/test-adapter-consumer.ps1 new file mode 100644 index 0000000000..8865365f1f --- /dev/null +++ b/build/smoke-tests/test-adapter-consumer.ps1 @@ -0,0 +1,147 @@ +#!/usr/bin/env pwsh + +<# +.SYNOPSIS + Smoke tests the packed BenchmarkDotNet.TestAdapter against a project that consumes it as a NuGet package. + +.DESCRIPTION + Everything in this repository that uses the adapter's build files imports them by path from the project file, + which MSBuild evaluates before nuget.g.targets. A package consumer gets the opposite order: NuGet imports + Microsoft.Testing.Platform.MSBuild's targets, which default IsTestingPlatformApplication to true, and only then + the adapter's, which have to overwrite that default for the opt-outs to work. Nothing in the solution can + reproduce that order, so this restores the real package and asserts on how the properties resolve. + + Run `build.cmd pack` first, so that the packages exist. + +.PARAMETER ArtifactsDirectory + The directory `build.cmd pack` wrote the packages to. + +.PARAMETER Configuration + The configuration to build the consuming project in. +#> + +[CmdletBinding()] +param( + [string] $ArtifactsDirectory = [System.IO.Path]::Combine($PSScriptRoot, '..', '..', 'artifacts'), + [string] $Configuration = 'Release' +) + +$ErrorActionPreference = 'Stop' + +# Invoke-Dotnet checks $LASTEXITCODE itself, and prints the log before it throws. Leaving this on would make a failing +# `dotnet` throw at the call itself on PowerShell Core - which is what the workflow runs - so the log would never be +# printed and a broken restore or build would report nothing at all. +$PSNativeCommandUseErrorActionPreference = $false + +$project = [System.IO.Path]::Combine($PSScriptRoot, 'TestAdapterConsumer', 'TestAdapterConsumer.csproj') +$targetFramework = 'net10.0' + +# $IsWindows only exists on PowerShell Core, where it is the only way to tell; Windows PowerShell is Windows by definition. +$onWindows = ($null -eq $IsWindows) -or $IsWindows + +# build.cmd installs the SDK the repository is pinned to into .dotnet, and only puts it on PATH for its own run. +$dotnet = [System.IO.Path]::Combine($PSScriptRoot, '..', '..', '.dotnet', $(if ($onWindows) { 'dotnet.exe' } else { 'dotnet' })) +if (-not (Test-Path $dotnet)) { + $dotnet = 'dotnet' +} + +if (-not (Test-Path -LiteralPath $ArtifactsDirectory)) { + throw "The artifacts directory '$ArtifactsDirectory' does not exist. Run 'build.cmd pack' first." +} + +# Both the package this reads the version from and the source the restore below resolves it through, so it has to be +# the same absolute path in both: a relative one would otherwise be resolved against the consuming project. +$ArtifactsDirectory = (Resolve-Path -LiteralPath $ArtifactsDirectory).ProviderPath + +# `build.cmd pack` never cleans, so the folder can hold several versions. The newest is the one that was just packed, +# which is the one worth smoke testing. +$packages = @(Get-ChildItem -Path $ArtifactsDirectory -Filter 'BenchmarkDotNet.TestAdapter.*.nupkg' | + Where-Object { $_.Name -notlike '*.symbols.nupkg' } | + Sort-Object -Property LastWriteTime -Descending) + +if ($packages.Count -eq 0) { + throw "No BenchmarkDotNet.TestAdapter package was found in '$ArtifactsDirectory'. Run 'build.cmd pack' first." +} + +$package = $packages[0] + +if ($packages.Count -gt 1) { + Write-Output "'$ArtifactsDirectory' holds $($packages.Count) BenchmarkDotNet.TestAdapter packages, taking the most recently written one." +} + +$version = $package.BaseName -replace '^BenchmarkDotNet\.TestAdapter\.', '' +Write-Output "Consuming BenchmarkDotNet.TestAdapter $version from $ArtifactsDirectory" + +function Invoke-Dotnet { + param([Parameter(ValueFromRemainingArguments = $true)] [string[]] $Arguments) + + $output = & $dotnet @Arguments 2>&1 | Out-String + + if ($LASTEXITCODE -ne 0) { + Write-Output $output + throw "'dotnet $($Arguments -join ' ')' failed with exit code $LASTEXITCODE." + } + + return $output +} + +function Assert-Property { + param( + [string] $Name, + [string] $Expected, + [string[]] $With = @() + ) + + $arguments = @($project, '-nologo', '-tl:off', "-p:BenchmarkDotNetVersion=$version", "-p:Configuration=$Configuration") + $With + @("-getProperty:$Name") + $actual = (Invoke-Dotnet msbuild @arguments).Trim() + + $description = if ($With.Count -eq 0) { 'by default' } else { "with $($With -join ' ')" } + + if ($actual -ne $Expected) { + throw "Expected $Name to be '$Expected' $description, but it was '$actual'." + } + + Write-Output " OK: $Name is '$Expected' $description" +} + +# The project restores into this folder rather than into the global one, see its .csproj. NuGet never re-extracts a +# version it already has, and the version does not change between runs, so the packages this repository produces are +# dropped before the restore; everything else in there is an ordinary cache and is left alone. +$packagesDirectory = [System.IO.Path]::Combine($PSScriptRoot, 'packages') +if (Test-Path -LiteralPath $packagesDirectory) { + Get-ChildItem -Path $packagesDirectory -Directory -Filter 'benchmarkdotnet*' | Remove-Item -Recurse -Force +} + +Write-Output '##[group]Restoring the consuming project' +# The project assigns RestoreAdditionalProjectSources too, but a global property wins over that assignment, which is +# what makes a custom -ArtifactsDirectory restore from the folder the version was read off. Only the restore needs it: +# nuget.g.props bakes the result in for every later invocation. +Invoke-Dotnet restore $project "-p:BenchmarkDotNetVersion=$version" ` + "-p:RestoreAdditionalProjectSources=$ArtifactsDirectory" '-tl:off' | Write-Output +Write-Output '##[endgroup]' + +Write-Output 'Checking how the packaged build files resolve the test platform:' + +# Microsoft.Testing.Platform is the default, and the adapter leaves the entry point to it. +Assert-Property -Name 'IsTestingPlatformApplication' -Expected 'true' +Assert-Property -Name 'GenerateProgramFile' -Expected 'false' + +# The two opt-outs have to win over the default Microsoft.Testing.Platform.MSBuild sets in its own targets, which a +# package consumer imports before the adapter's. +Assert-Property -Name 'IsTestingPlatformApplication' -Expected 'false' -With '-p:BenchmarkDotNetUseVSTest=true' +Assert-Property -Name 'IsTestingPlatformApplication' -Expected 'false' -With '-p:GenerateProgramFile=false' + +Write-Output '##[group]Building the consuming project' +Invoke-Dotnet build $project '--no-restore' '-c' $Configuration "-p:BenchmarkDotNetVersion=$version" '-tl:off' | Write-Output +Write-Output '##[endgroup]' + +Write-Output 'Listing the benchmarks through the entry point Microsoft.Testing.Platform generated:' +$application = [System.IO.Path]::Combine($PSScriptRoot, 'TestAdapterConsumer', 'bin', $Configuration, $targetFramework, 'TestAdapterConsumer.dll') +$listed = Invoke-Dotnet $application '--list-tests' '--no-ansi' +Write-Output $listed + +if ($listed -notmatch 'TestAdapterConsumer\.ConsumedBenchmark\.Add') { + throw 'The packaged adapter did not list the benchmark of the consuming project.' +} + +Write-Output 'The packaged BenchmarkDotNet.TestAdapter behaves as expected.' diff --git a/docs/articles/features/testadapter.md b/docs/articles/features/testadapter.md new file mode 100644 index 0000000000..6f039328bb --- /dev/null +++ b/docs/articles/features/testadapter.md @@ -0,0 +1,217 @@ +--- +uid: docs.testadapter +name: Running benchmarks as tests +--- + +# Running benchmarks as tests + +`BenchmarkDotNet.TestAdapter` lets your IDE and `dotnet test` discover and execute your benchmarks the way they do + unit tests. +This provides an alternative user experience to running benchmarks with the CLI + and may be preferable for those who like their IDE's test integrations that they may have used when running unit tests. + +Below is an example of running some benchmarks from the BenchmarkDotNet samples project in Visual Studio's Test Explorer. + +![](../../images/vs-testexplorer-demo.png) + +The adapter supports two test platforms: + +* [Microsoft.Testing.Platform](https://learn.microsoft.com/dotnet/core/testing/microsoft-testing-platform-intro) (MTP), + the platform that `dotnet test` and modern IDE integrations are moving to. **This is the default.** +* [VSTest](xref:docs.vstest), for tooling without Microsoft.Testing.Platform support (such as Visual Studio 2019) + and for solutions that mix benchmark projects with VSTest based test projects. + +The difference that matters most is *where your benchmarks run*: + +* With **VSTest**, an external `testhost` process loads your benchmark assembly and the adapter reflects into it. +* With **Microsoft.Testing.Platform**, your benchmark project *is* the test host. + There is no separate host process, so BenchmarkDotNet behaves exactly as it does when you run the app from the CLI, + and the adapter does not need the child `AppDomain` that the VSTest adapter uses to load your assemblies correctly. + +## Caveats and things to know + +* **The benchmark measurements may be affected by the test host and your IDE!** + If you want accurate measurements, + it is still recommended to run benchmarks through the CLI without other processes impacting performance. + The measurements remain useful during development when comparing different approaches. +* **The adapter will not display or execute benchmarks if optimizations are disabled.** + Please ensure you are compiling in Release mode or with `Optimize` set to true. + Using an `InProcess` toolchain will let you run your benchmarks with optimizations disabled + and will let you attach the debugger as well. +* **The adapter will not call your application's entry point.** + If you use the entry point to customize how your benchmarks are run, + you will need to do this through other means such as an assembly-level `IConfigSource`, + as shown in [Setting a default configuration](xref:docs.vstest#setting-a-default-configuration). +* **The adapter will generate an entry point for you automatically.** + The generated entry point starts the test application. + See [Keeping your own entry point](#keeping-your-own-entry-point) if your project already has one. + +## Getting started + +* **Step 1.** Install the NuGet package. + Only one package is needed; it brings in `Microsoft.Testing.Platform` and the MSBuild integration for you: + +```xml + + + +``` + +* **Step 2.** Make sure the project is an executable and does not define its own entry point. + Microsoft.Testing.Platform applications are executables, and the package generates the entry point for you. + Here is a complete `.csproj` based on the default Console Application template: + +```xml + + + + Exe + net10.0 + enable + enable + + + + + + + +``` + +> [!NOTE] +> The name of your project file must match the name of the produced assembly. +> This is a general BenchmarkDotNet requirement: it rebuilds your project to run benchmarks out of process. + +* **Step 3.** Opt into the Microsoft.Testing.Platform mode of `dotnet test`. + On the .NET 10 SDK and later this is required, because `dotnet test` runs in VSTest mode by default. + Add a `global.json` next to your solution: + +```json +{ + "test": { + "runner": "Microsoft.Testing.Platform" + } +} +``` + + On the .NET 9 SDK and earlier this step is not needed: + the package sets `TestingPlatformDotnetTestSupport` for you, which routes `dotnet test` to the platform. + +* **Step 4.** Switch to the `Release` configuration. + As mentioned above, the adapter does not discover or run benchmarks with optimizations disabled (by design). + +* **Step 5.** Build and run. + +```console +dotnet test -c Release +``` + + You can also run the produced executable directly, which is the same thing without going through MSBuild: + +```console +dotnet run -c Release +``` + +If this doesn't work for you, don't hesitate to file [a new GitHub issue](https://github.com/dotnet/BenchmarkDotNet/issues/new). + +## Listing and filtering benchmarks + +The benchmark project is a normal Microsoft.Testing.Platform application, so it accepts the platform's options. +Run it with `--help` to see all of them; the ones you are most likely to want are: + +```console +# List the benchmarks without running them. +dotnet run -c Release -- --list-tests + +# Run every benchmark of a class. +dotnet run -c Release -- --treenode-filter "/*/*/MyBenchmarks/*" + +# Run every benchmark of a category. +dotnet run -c Release -- --treenode-filter "/*/*/*/*[Category=Fast]" +``` + +The tree node filter path is `////`, + and the categories of a benchmark are exposed as a `Category` trait that the filter can match on. +The last level ends with the job between brackets, as in `MyBenchmark(Size: 1) [Dry]`. + +The characters the filter itself gives a meaning to are percent encoded in that path: + `/`, which separates the levels, is written `%2F`; + `[` and `]`, which delimit a property filter, are written `%5B` and `%5D`; + and a literal `%` is written `%25`. +So a parameter value of `a/b` is spelled `a%2Fb` in a filter, and the `[Dry]` above is spelled `%5BDry%5D`. +The parentheses around the parameters are not encoded - escape them with a backslash in the filter instead. +This affects the filter only; the name the benchmark is displayed under is unchanged. + +A benchmark is displayed under its `[Benchmark(Description = "...")]` when it has one, and under the name of its + method otherwise, followed by its parameters. + +## Keeping your own entry point + +The generated entry point starts the test application, which means it replaces the `BenchmarkSwitcher` entry point that + a benchmark project normally has. +There are two ways to keep your own. + +To keep a plain `BenchmarkSwitcher` entry point, tell the adapter that the project generates its own: + +```xml + + + false + +``` + +The project is then a normal console application again, and no test platform integration is set up for it. + +To keep an entry point *and* the test integration, start the test application yourself: + +```xml + + true + false + +``` + +```csharp +using BenchmarkDotNet.TestAdapter.TestingPlatform; +using Microsoft.Testing.Platform.Builder; + +public static class Program +{ + public static async Task Main(string[] args) + { + var builder = await TestApplication.CreateBuilderAsync(args); + builder.AddBenchmarkDotNet(); + using var app = await builder.BuildAsync(); + return await app.RunAsync(); + } +} +``` + +From there you are free to decide when to start the test application and when to hand over to `BenchmarkSwitcher`, + for example by looking at the arguments your CI passes. + +## Using VSTest instead + +Set `BenchmarkDotNetUseVSTest` and add the VSTest host package: + +```xml + + true + + + + + + +``` + +See [Running with VSTest](xref:docs.vstest) for the details, including the IDE settings that VSTest integration needs. + +## Viewing the results + +The full BenchmarkDotNet output, including the summary table that compares benchmarks with each other, + is written to the test run output. + +In addition, each individual benchmark reports its own output, containing a histogram and various statistics for that + single benchmark case. +Depending on your IDE, this is shown when selecting the test after running it. diff --git a/docs/articles/features/toc.yml b/docs/articles/features/toc.yml index b456ed66be..db72e5c3e2 100644 --- a/docs/articles/features/toc.yml +++ b/docs/articles/features/toc.yml @@ -16,5 +16,7 @@ href: event-pipe-profiler.md - name: VSProfiler href: vsprofiler.md +- name: Benchmarks as tests + href: testadapter.md - name: VSTest href: vstest.md \ No newline at end of file diff --git a/docs/articles/features/vstest.md b/docs/articles/features/vstest.md index 3e94901bdd..2e0b3368d3 100644 --- a/docs/articles/features/vstest.md +++ b/docs/articles/features/vstest.md @@ -5,6 +5,11 @@ name: Running with VSTest # Running with VSTest +> [!NOTE] +> `BenchmarkDotNet.TestAdapter` runs your benchmarks through +> [Microsoft.Testing.Platform](xref:docs.testadapter) by default, which is the platform that succeeds VSTest. +> VSTest is opt-in, as described below. + BenchmarkDotNet supports discovering and executing benchmarks through VSTest. This provides an alternative user experience to running benchmarks with the CLI and may be preferable for those who like their IDE's VSTest integrations that they may have used when running unit tests. @@ -54,7 +59,17 @@ In addition, we can still make use of this boolean output to indicate You need to install two packages into your benchmark project: * `BenchmarkDotNet.TestAdapter`: Implements the VSTest protocol for BenchmarkDotNet * `Microsoft.NET.Test.Sdk`: Includes all the pieces needed for the VSTest host to run and load the VSTest adapter. -* **Step 2.** Make sure that the entry point is configured correctly. +* **Step 2.** Ask the adapter for VSTest. + `BenchmarkDotNet.TestAdapter` uses [Microsoft.Testing.Platform](xref:docs.testadapter) unless you set + `BenchmarkDotNetUseVSTest` in your project file: + +```xml + + true + +``` + +* **Step 3.** Make sure that the entry point is configured correctly. As mentioned in the caveats section, `BenchmarkDotNet.TestAdapter` will generate an entry point for you automatically. So, if you have an entry point already, you will either need to delete it or set `GenerateProgramFile` to `false` in your project file to continue using your existing one. @@ -68,6 +83,8 @@ In addition, we can still make use of this boolean output to indicate net8.0 enable enable + + true false @@ -80,7 +97,7 @@ In addition, we can still make use of this boolean output to indicate ``` -* **Step 3.** Make sure that your IDE supports VSTest integration. +* **Step 4.** Make sure that your IDE supports VSTest integration. In Visual Studio, everything works out of the box. In Rider/R#, the VSTest integration might need to be activated: * Go to the "Unit Testing" settings page. @@ -88,9 +105,9 @@ In addition, we can still make use of this boolean output to indicate * R#: Extensions -> ReSharper -> Options -> Tools -> Unit Testing -> Test Frameworks -> VSTest * Make sure that the "Enable VSTest adapter support" checkbox is checked. In recent versions of Rider, this may be enabled by default. -* **Step 4.** Switch to the `Release` configuration. +* **Step 5.** Switch to the `Release` configuration. As mentioned above, the TestAdapter is not able to discover and run benchmarks with optimizations disabled (by design). -* **Step 5.** Build the project. +* **Step 6.** Build the project. In order to discover the benchmarks, the VSTest adapter needs to be able to find the assembly. Once you build the project, you should observe the discovered benchmarks in your IDE's Unit Test Explorer. diff --git a/samples/BenchmarkDotNet.Samples.FSharp/BenchmarkDotNet.Samples.FSharp.fsproj b/samples/BenchmarkDotNet.Samples.FSharp/BenchmarkDotNet.Samples.FSharp.fsproj index eeee8b90f2..fd51c0e8bf 100644 --- a/samples/BenchmarkDotNet.Samples.FSharp/BenchmarkDotNet.Samples.FSharp.fsproj +++ b/samples/BenchmarkDotNet.Samples.FSharp/BenchmarkDotNet.Samples.FSharp.fsproj @@ -25,5 +25,13 @@ + + + + diff --git a/samples/BenchmarkDotNet.Samples/BenchmarkDotNet.Samples.csproj b/samples/BenchmarkDotNet.Samples/BenchmarkDotNet.Samples.csproj index 2dcbfaf472..c52a4d580c 100644 --- a/samples/BenchmarkDotNet.Samples/BenchmarkDotNet.Samples.csproj +++ b/samples/BenchmarkDotNet.Samples/BenchmarkDotNet.Samples.csproj @@ -38,5 +38,13 @@ + + + + diff --git a/src/BenchmarkDotNet.TestAdapter/BenchmarkDotNet.TestAdapter.csproj b/src/BenchmarkDotNet.TestAdapter/BenchmarkDotNet.TestAdapter.csproj index 3deb70b1cd..6d61995d42 100644 --- a/src/BenchmarkDotNet.TestAdapter/BenchmarkDotNet.TestAdapter.csproj +++ b/src/BenchmarkDotNet.TestAdapter/BenchmarkDotNet.TestAdapter.csproj @@ -5,8 +5,16 @@ BenchmarkDotNet.TestAdapter BenchmarkDotNet.TestAdapter BenchmarkDotNet.TestAdapter + Runs BenchmarkDotNet benchmarks as tests, through Microsoft.Testing.Platform or VSTest README.md True + + + false @@ -20,6 +28,13 @@ + + + + + @@ -28,7 +43,17 @@ - + + + diff --git a/src/BenchmarkDotNet.TestAdapter/BenchmarkEnumerator.cs b/src/BenchmarkDotNet.TestAdapter/BenchmarkEnumerator.cs index e8cbc4f130..bb5604e8bf 100644 --- a/src/BenchmarkDotNet.TestAdapter/BenchmarkEnumerator.cs +++ b/src/BenchmarkDotNet.TestAdapter/BenchmarkEnumerator.cs @@ -48,29 +48,57 @@ public static BenchmarkRunInfo[] GetBenchmarksFromAssemblyPath(string assemblyPa }; #endif - var assembly = Assembly.LoadFrom(assemblyPath); + return GetBenchmarksFromAssembly(Assembly.LoadFrom(assemblyPath)); + } - var isDebugAssembly = assembly.IsJitOptimizationDisabled() ?? false; + /// + /// Returns all the BenchmarkRunInfo objects from an already loaded assembly. + /// + /// The assembly of the benchmark project. + /// The benchmarks inside the assembly. + public static BenchmarkRunInfo[] GetBenchmarksFromAssembly(Assembly assembly) + => GetBenchmarksFromAssembly(assembly, ParameterValueDisposer.DisposeUnused); - return GenericBenchmarksBuilder.GetRunnableBenchmarks(assembly.GetRunnableBenchmarks()) - .Select(type => - { - var benchmarkRunInfo = BenchmarkConverter.TypeToBenchmarks(type); - if (isDebugAssembly) - { - // If the assembly is a debug assembly, then only display them if they will run in-process - // This will allow people to debug their benchmarks using VSTest if they wish. - benchmarkRunInfo = new BenchmarkRunInfo( - benchmarkRunInfo.BenchmarksCases.Where(c => c.GetToolchain().IsInProcess).ToArray(), - benchmarkRunInfo.Type, - benchmarkRunInfo.Config, - benchmarkRunInfo.CompositeInProcessDiagnoser); - } + /// + /// Returns all the BenchmarkRunInfo objects from an already loaded assembly. + /// + /// The assembly of the benchmark project. + /// + /// What to do with the parameter values of the benchmarks that are hidden here, given everything the assembly + /// declares and the benchmarks that are kept. Disposing them right away is only right when nothing will + /// enumerate the assembly again: a host that serves several requests from one process hides the same + /// benchmarks every time, and a cached source hands back the same values, so it takes the disposal over. + /// + /// The benchmarks inside the assembly. + internal static BenchmarkRunInfo[] GetBenchmarksFromAssembly( + Assembly assembly, + Action, IEnumerable> disposeHidden) + { + var all = GenericBenchmarksBuilder.GetRunnableBenchmarks(assembly.GetRunnableBenchmarks()) + .Select(type => BenchmarkConverter.TypeToBenchmarks(type)) + .ToArray(); - return benchmarkRunInfo; - }) + if (!(assembly.IsJitOptimizationDisabled() ?? false)) + return all.Where(runInfo => runInfo.BenchmarksCases.Length > 0).ToArray(); + + // If the assembly is a debug assembly, then only display the benchmarks that will run in-process. This + // will allow people to debug their benchmarks from a test runner if they wish. + var runnable = all + .Select(runInfo => new BenchmarkRunInfo( + runInfo.BenchmarksCases.Where(c => c.GetToolchain().IsInProcess).ToArray(), + runInfo.Type, + runInfo.Config, + runInfo.CompositeInProcessDiagnoser)) .Where(runInfo => runInfo.BenchmarksCases.Length > 0) .ToArray(); + + // BenchmarkConverter has already constructed every parameter value by now, and a case hidden here is never + // handed to BenchmarkDotNet by either adapter, so nothing downstream will ever dispose what is dropped. + disposeHidden( + all.SelectMany(runInfo => runInfo.BenchmarksCases), + runnable.SelectMany(runInfo => runInfo.BenchmarksCases)); + + return runnable; } } } diff --git a/src/BenchmarkDotNet.TestAdapter/ParameterValueDisposer.cs b/src/BenchmarkDotNet.TestAdapter/ParameterValueDisposer.cs new file mode 100644 index 0000000000..899f891e13 --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/ParameterValueDisposer.cs @@ -0,0 +1,76 @@ +using BenchmarkDotNet.Engines; +using BenchmarkDotNet.Helpers; +using BenchmarkDotNet.Parameters; +using BenchmarkDotNet.Running; +using System.Runtime.CompilerServices; + +namespace BenchmarkDotNet.TestAdapter +{ + /// + /// Disposes the parameter values of the benchmarks that were enumerated but will not be run. + /// + /// + /// Enumerating an assembly instantiates the values of every [Params], [ParamsSource] and [ArgumentsSource], and + /// BenchmarkDotNet only disposes the ones belonging to the benchmarks it was handed. A value with a locking + /// finalizer hangs the runtime when it is left to the finalizer thread instead, see dotnet/BenchmarkDotNet#1383, + /// which is what makes this worse than an ordinary leak. + /// + internal static class ParameterValueDisposer + { + /// + internal static void DisposeUnused(IEnumerable enumerated, IEnumerable retained) + { + using var context = BenchmarkSynchronizationContext.CreateAndSetCurrent(); + context.ExecuteUntilComplete(DisposeUnusedAsync(enumerated, retained)); + } + + /// + /// Disposes every value that the enumerated benchmarks own and the retained ones do not. + /// + /// + /// The values are matched by reference instead of being disposed case by case, because BenchmarkConverter + /// gives the same ParameterInstance to every job and every argument set of a benchmark: disposing a dropped + /// case wholesale would take down values that a benchmark which is about to run still owns. Disposal goes + /// through ParameterInstance so that a value which is only IAsyncDisposable is disposed as well. + /// + /// Everything the assembly declares. + /// The benchmarks that are kept, if any. + internal static ValueTask DisposeUnusedAsync(IEnumerable enumerated, IEnumerable retained) + { + var unused = new Dictionary(ReferenceComparer.Instance); + + foreach (var parameter in GetDisposableParameters(enumerated)) + unused[parameter.Value!] = parameter; + + foreach (var parameter in GetDisposableParameters(retained)) + unused.Remove(parameter.Value!); + + return unused.Values.DisposeAllAsync(); + } + + /// + /// Compares values by reference, so that a parameter value which overrides Equals is still disposed once per + /// instance. + /// + internal static IEqualityComparer ByReference => ReferenceComparer.Instance; + + /// + /// Gets the parameters of the given benchmarks whose value needs disposing. + /// + /// The benchmarks to read the parameters of. + /// The parameters holding a disposable value. + internal static IEnumerable GetDisposableParameters(IEnumerable benchmarkCases) + => benchmarkCases + .SelectMany(benchmarkCase => benchmarkCase.Parameters.Items) + .Where(parameter => parameter.Value is IDisposable or IAsyncDisposable); + + private sealed class ReferenceComparer : IEqualityComparer + { + public static readonly ReferenceComparer Instance = new ReferenceComparer(); + + public new bool Equals(object? x, object? y) => ReferenceEquals(x, y); + + public int GetHashCode(object obj) => RuntimeHelpers.GetHashCode(obj); + } + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/Properties/AssemblyInfo.cs b/src/BenchmarkDotNet.TestAdapter/Properties/AssemblyInfo.cs new file mode 100644 index 0000000000..5b0bbc532c --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/Properties/AssemblyInfo.cs @@ -0,0 +1,7 @@ +using BenchmarkDotNet.Properties; +using System.Runtime.CompilerServices; + +// Drives the adapter's Microsoft.Testing.Platform types directly, to cover what a real test host cannot reach: a +// test execution filter no platform this was built against can produce, and the end of an application whose request +// never completed. See that project's README. +[assembly: InternalsVisibleTo("BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals,PublicKey=" + BenchmarkDotNetInfo.PublicKey)] diff --git a/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkDotNetExtension.cs b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkDotNetExtension.cs new file mode 100644 index 0000000000..6982414812 --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkDotNetExtension.cs @@ -0,0 +1,34 @@ +using Microsoft.Testing.Platform.Extensions; + +namespace BenchmarkDotNet.TestAdapter.TestingPlatform +{ + /// + /// Identifies BenchmarkDotNet to Microsoft.Testing.Platform. + /// + /// + /// Several platform services, such as the tree node filter behind --treenode-filter, are registered on behalf of an + /// extension rather than of the test framework itself, so the identity lives in its own type. + /// + internal sealed class BenchmarkDotNetExtension : IExtension + { + /// + /// The uid shared by every extension this package registers. + /// + public const string ExtensionUid = "BenchmarkDotNet.TestAdapter"; + + /// + public string Uid => ExtensionUid; + + /// + public string Version => typeof(BenchmarkDotNetExtension).Assembly.GetName().Version?.ToString() ?? "0.0.0"; + + /// + public string DisplayName => "BenchmarkDotNet"; + + /// + public string Description => "Runs BenchmarkDotNet benchmarks as tests."; + + /// + public Task IsEnabledAsync() => Task.FromResult(true); + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkEventProcessor.cs b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkEventProcessor.cs new file mode 100644 index 0000000000..aaa64f506a --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkEventProcessor.cs @@ -0,0 +1,223 @@ +using BenchmarkDotNet.EventProcessors; +using BenchmarkDotNet.Exporters; +using BenchmarkDotNet.Extensions; +using BenchmarkDotNet.Reports; +using BenchmarkDotNet.Running; +using BenchmarkDotNet.Toolchains.Results; +using BenchmarkDotNet.Validators; +using Microsoft.Testing.Platform.Extensions.Messages; +using Perfolizer.Mathematics.Histograms; +using System.Diagnostics; +using System.Globalization; +using System.Text; + +namespace BenchmarkDotNet.TestAdapter.TestingPlatform +{ + + /// + /// Translates the BenchmarkDotNet run into the stream of test node updates the platform expects. + /// + internal sealed class BenchmarkEventProcessor : EventProcessor + { + private readonly IReadOnlyDictionary nodes; + private readonly Action publish; + private readonly Stopwatch runTimerStopwatch = new(); + private readonly Dictionary pendingResults = []; + private readonly HashSet publishedResults = []; + + // BenchmarkDotNet builds the partitions in parallel and raises OnBuildComplete from each of the build tasks, + // so that callback can run on several threads at once. The others cannot: every build is awaited before the + // first benchmark runs, and the benchmarks themselves run one after another. + private readonly object buildCompleteGate = new(); + + // Written on BenchmarkDotNet's thread, read on the request's once the run is over. + private volatile bool parameterValuesDisposed; + + public BenchmarkEventProcessor(IReadOnlyDictionary nodes, Action publish) + { + this.nodes = nodes; + this.publish = publish; + } + + /// + /// Gets whether BenchmarkDotNet disposed the parameter values of the benchmarks it was handed. + /// + /// + /// BenchmarkRunnerClean disposes them in the finally of its run stage, which is where OnEndRunStage is + /// raised from - after the disposal has been through every value, whether the run completed, threw, was + /// cancelled, or the disposal itself threw. It never gets there when a critical validation error makes it + /// return before the run stage, and nothing disposes the values then, so whoever handed them over has to. + /// + public bool ParameterValuesDisposed => parameterValuesDisposed; + + public override void OnEndRunStage() => parameterValuesDisposed = true; + + public override void OnValidationError(ValidationError validationError) + { + // If the error is not linked to a benchmark case, then set the error on all benchmarks. + var affected = validationError.BenchmarkCase == null + ? nodes.Values + : [nodes[validationError.BenchmarkCase.GetUniqueId()]]; + + foreach (var node in affected) + { + var pending = GetOrCreatePendingResult(node); + + if (validationError.IsCritical) + { + // The result is not published yet, in case there are more validation errors to append. + pending.ErrorMessages.Add(validationError.Message); + } + else + { + pending.Output.AppendLine($"WARNING: {validationError.Message}"); + } + } + } + + public override void OnBuildComplete(BuildPartition buildPartition, BuildResult buildResult) + { + // Only build failures need to be reported, successful builds are followed by a run. + if (buildResult.IsBuildSuccess) + return; + + lock (buildCompleteGate) + { + foreach (var benchmarkBuildInfo in buildPartition.Benchmarks) + { + var node = nodes[benchmarkBuildInfo.BenchmarkCase.GetUniqueId()]; + var pending = GetOrCreatePendingResult(node); + + if (buildResult.GenerateException != null) + pending.ErrorMessages.Add($"// Generate Exception: {buildResult.GenerateException.Message}"); + else if (buildResult.TryToExplainFailureReason(buildPartition.GetInProcessDiagnoserHandlerTypes(), out string? reason)) + pending.ErrorMessages.Add($"// Build Error: {reason}"); + else if (buildResult.ErrorMessage != null) + pending.ErrorMessages.Add($"// Build Error: {buildResult.ErrorMessage}"); + + // A benchmark that failed to build will never run, so the result can be published immediately. + publish(node.ToTestNode(InProgressTestNodeStateProperty.CachedInstance)); + PublishResult(node, pending, new FailedTestNodeStateProperty(pending.GetErrorMessage() ?? "The benchmark failed to build.")); + } + } + } + + public override void OnStartRunBenchmark(BenchmarkCase benchmarkCase) + { + var node = nodes[benchmarkCase.GetUniqueId()]; + var pending = GetOrCreatePendingResult(node); + pending.StartTime = DateTimeOffset.UtcNow; + + publish(node.ToTestNode(InProgressTestNodeStateProperty.CachedInstance)); + runTimerStopwatch.Restart(); + } + + public override void OnEndRunBenchmark(BenchmarkCase benchmarkCase, BenchmarkReport report) + { + var node = nodes[benchmarkCase.GetUniqueId()]; + var pending = GetOrCreatePendingResult(node); + pending.Duration = runTimerStopwatch.Elapsed; + pending.EndTime = DateTimeOffset.UtcNow; + + AppendMeasurementSummary(pending.Output, report); + + TestNodeStateProperty state = report.Success && pending.ErrorMessages.Count == 0 + ? PassedTestNodeStateProperty.CachedInstance + : new FailedTestNodeStateProperty(pending.GetErrorMessage() ?? "The benchmark did not complete successfully."); + + PublishResult(node, pending, state); + } + + /// + /// Publishes a result for every benchmark that was scheduled to run but never reported one, which happens when + /// a critical validation error stopped it or when BenchmarkDotNet never reached it. + /// + public void PublishOutstandingResults() + { + foreach (var node in nodes.Values) + { + if (publishedResults.Contains(node.Uid)) + continue; + + var pending = GetOrCreatePendingResult(node); + + // A benchmark that reported a start and never an end did not finish: the run was torn down under it + // by something this processor was never told about, such as BenchmarkRunnerClean throwing. Only a + // benchmark that never started at all was really never run. + TestNodeStateProperty state = (pending.GetErrorMessage(), pending.StartTime) switch + { + (string errorMessage, _) => new FailedTestNodeStateProperty(errorMessage), + (null, not null) => new FailedTestNodeStateProperty( + "The benchmark started but never reported a result, so the run did not complete."), + (null, null) => SkippedTestNodeStateProperty.CachedInstance + }; + + publish(node.ToTestNode(InProgressTestNodeStateProperty.CachedInstance)); + PublishResult(node, pending, state); + } + } + + private void PublishResult(BenchmarkTestNode node, PendingResult pending, TestNodeStateProperty state) + { + var properties = new List(); + + if (pending.StartTime is { } startTime) + { + var duration = pending.Duration ?? TimeSpan.Zero; + properties.Add(new TimingProperty(new TimingInfo(startTime, pending.EndTime ?? startTime + duration, duration))); + } + + if (pending.Output.Length > 0) + properties.Add(new StandardOutputProperty(pending.Output.ToString())); + + publish(node.ToTestNode(state, properties.ToArray())); + publishedResults.Add(node.Uid); + } + + private PendingResult GetOrCreatePendingResult(BenchmarkTestNode node) + { + if (!pendingResults.TryGetValue(node.Uid, out var pending)) + { + pending = new PendingResult(); + pendingResults[node.Uid] = pending; + } + + return pending; + } + + private static void AppendMeasurementSummary(StringBuilder output, BenchmarkReport report) + { + var resultRuns = report.GetResultRuns(); + if (resultRuns.Count == 0) + return; + + output.AppendLine(report.BenchmarkCase.DisplayInfo); + output.AppendLine($"Runtime = {report.GetRuntimeInfo()}; GC = {report.GetGcInfo()}"); + + var statistics = resultRuns.GetStatistics(); + var cultureInfo = CultureInfo.InvariantCulture; + var formatter = statistics.CreateNanosecondFormatter(cultureInfo); + + var histogram = HistogramBuilder.Adaptive.Build(statistics.Sample.Values); + output.AppendLine("-------------------- Histogram --------------------"); + output.AppendLine(histogram.ToString(formatter)); + output.AppendLine("---------------------------------------------------"); + output.AppendLine(statistics.ToString(cultureInfo, formatter, calcHistogram: false)); + } + + private sealed class PendingResult + { + public List ErrorMessages { get; } = []; + + public StringBuilder Output { get; } = new(); + + public DateTimeOffset? StartTime { get; set; } + + public DateTimeOffset? EndTime { get; set; } + + public TimeSpan? Duration { get; set; } + + public string? GetErrorMessage() => ErrorMessages.Count == 0 ? null : string.Join("\n", ErrorMessages); + } + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestFramework.cs b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestFramework.cs new file mode 100644 index 0000000000..452d95b6f6 --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestFramework.cs @@ -0,0 +1,494 @@ +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Loggers; +using BenchmarkDotNet.Running; +using Microsoft.Testing.Platform.Capabilities.TestFramework; +using Microsoft.Testing.Platform.Extensions.Messages; +using Microsoft.Testing.Platform.Extensions.OutputDevice; +using Microsoft.Testing.Platform.Extensions.TestFramework; +using Microsoft.Testing.Platform.OutputDevice; +using Microsoft.Testing.Platform.Requests; +using Microsoft.Testing.Platform.Services; +using Microsoft.Testing.Platform.TestHost; +using System.Reflection; +using System.Runtime.ExceptionServices; +using System.Threading.Channels; + +namespace BenchmarkDotNet.TestAdapter.TestingPlatform +{ + /// + /// Discovers and executes the benchmarks of the running assembly through Microsoft.Testing.Platform. + /// + internal sealed class BenchmarkTestFramework : ITestFramework, IDataProducer, IOutputDeviceDataProducer + { + private readonly BenchmarkDotNetExtension extension = new(); + private readonly IServiceProvider serviceProvider; + private readonly Assembly assembly; + private readonly ParameterValueLifetime parameterValues; + + public BenchmarkTestFramework( + ITestFrameworkCapabilities capabilities, + IServiceProvider serviceProvider, + Assembly assembly, + ParameterValueLifetime parameterValues) + { + Capabilities = capabilities; + this.serviceProvider = serviceProvider; + this.assembly = assembly; + this.parameterValues = parameterValues; + } + + /// + public string Uid => extension.Uid; + + /// + public string Version => extension.Version; + + /// + public string DisplayName => extension.DisplayName; + + /// + public string Description => extension.Description; + + /// + public Type[] DataTypesProduced => [typeof(TestNodeUpdateMessage)]; + + /// + /// Gets the capabilities the framework was registered with. + /// + public ITestFrameworkCapabilities Capabilities { get; } + + /// + public Task IsEnabledAsync() => extension.IsEnabledAsync(); + + /// + public Task CreateTestSessionAsync(CreateTestSessionContext context) + => Task.FromResult(new CreateTestSessionResult { IsSuccess = true }); + + /// + public Task CloseTestSessionAsync(CloseTestSessionContext context) + => Task.FromResult(new CloseTestSessionResult { IsSuccess = true }); + + /// + public async Task ExecuteRequestAsync(ExecuteRequestContext context) + { + try + { + switch (context.Request) + { + case DiscoverTestExecutionRequest discoverRequest: + await DiscoverAsync(discoverRequest, context).ConfigureAwait(false); + break; + case RunTestExecutionRequest runRequest: + await RunAsync(runRequest, context).ConfigureAwait(false); + break; + } + } + finally + { + context.Complete(); + } + } + + private async Task DiscoverAsync(DiscoverTestExecutionRequest request, ExecuteRequestContext context) + { + // The enumeration sits inside the try: it records the values it creates as it goes, and completing the + // request is what hands them over, so nothing that throws between the two can lose them. + var parameterValueScope = parameterValues.BeginRequest(); + + try + { + var enumeration = GetMatchingBenchmarks(request.Filter, parameterValueScope); + + // Discovery runs nothing, so a filter this adapter does not know costs nothing but a wrong list, and + // reporting every benchmark beats reporting none. The run path refuses it instead. + await WarnAboutUnrecognisedFilterAsync(enumeration, context.CancellationToken).ConfigureAwait(false); + + foreach (var benchmarks in enumeration.Matches) + { + context.CancellationToken.ThrowIfCancellationRequested(); + + // Exactly one node per uid: publishing a colliding uid twice would leave the platform with two + // nodes it cannot tell apart. The collision itself is reported when the benchmarks are run. + var message = new TestNodeUpdateMessage( + request.Session.SessionUid, + benchmarks[0].Node.ToTestNode(DiscoveredTestNodeStateProperty.CachedInstance)); + + await context.MessageBus.PublishAsync(this, message).ConfigureAwait(false); + } + } + finally + { + // Discovery runs nothing, so BenchmarkDotNet disposed nothing. The values are not disposed here + // either: under server mode a run request follows in this very process, see ParameterValueLifetime. + await parameterValueScope.CompleteAsync([]).ConfigureAwait(false); + } + } + + private async Task RunAsync(RunTestExecutionRequest request, ExecuteRequestContext context) + { + var sessionUid = request.Session.SessionUid; + var cancellationToken = context.CancellationToken; + + // Declared ahead of the try, because the finally needs both to say what BenchmarkDotNet disposed - and the + // event processor is created out here too, so that a run which threw after BenchmarkDotNet had already + // disposed the values, as one does when publishing fails, still has that on record. + var runnable = new List(); + BenchmarkEventProcessor? eventProcessor = null; + var parameterValueScope = parameterValues.BeginRequest(); + + // BenchmarkDotNet reports its progress through synchronous callbacks (EventProcessor and ILogger) while + // the message bus and the output device are asynchronous. Blocking on those from inside a callback risks + // deadlocking against the synchronization context BenchmarkDotNet installs while it runs, so the callbacks + // write to this channel and the drain in RunAsync does the awaiting. Synchronous continuations are left + // off, so that a write can never end up publishing on BenchmarkDotNet's own thread. + var workQueue = Channel.CreateUnbounded>(new UnboundedChannelOptions + { + SingleReader = true, + AllowSynchronousContinuations = false + }); + + try + { + var enumeration = GetMatchingBenchmarks(request.Filter, parameterValueScope); + + // Unlike discovery, a run cannot treat a filter it does not know as matching everything: that would + // 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) + { + await RefuseUnrecognisedFilterAsync(context, sessionUid, enumeration, unrecognisedFilter).ConfigureAwait(false); + return; + } + + foreach (var benchmarks in enumeration.Matches) + { + if (benchmarks.Count == 1) + runnable.Add(benchmarks[0]); + else + await PublishCollisionAsync(context, sessionUid, benchmarks).ConfigureAwait(false); + } + + if (runnable.Count == 0) + return; + + eventProcessor = new BenchmarkEventProcessor( + runnable.ToDictionary(match => match.Node.Uid, match => match.Node), + testNode => + { + var message = new TestNodeUpdateMessage(sessionUid, testNode); + workQueue.Writer.TryWrite(() => context.MessageBus.PublishAsync(this, message)); + }); + + await RunAsync(context, runnable, eventProcessor, workQueue, cancellationToken).ConfigureAwait(false); + } + finally + { + // A benchmark that was filtered out or that collided was never handed to BenchmarkDotNet, so nothing + // else disposes its values; and the ones that were handed over are only disposed by BenchmarkDotNet + // once its run stage began, not when it bailed out on a critical validation error. As in + // DiscoverAsync, nothing is disposed here that a later request could still run. + var ranCases = eventProcessor is { ParameterValuesDisposed: true } + ? runnable.Select(match => match.Node.BenchmarkCase) + : []; + + await parameterValueScope.CompleteAsync(ranCases).ConfigureAwait(false); + } + } + + /// + /// Runs the given benchmarks through BenchmarkDotNet, publishing their results as they are produced. + /// + private async Task RunAsync( + ExecuteRequestContext context, + List runnable, + BenchmarkEventProcessor eventProcessor, + Channel> workQueue, + CancellationToken cancellationToken) + { + // A failure while publishing has to stop the benchmarks as well, otherwise the run would carry on with + // nobody listening to it. + using var runCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + + // BenchmarkDotNet's own console output is replaced so that everything goes through the output device, + // which keeps it in the right place when the platform runs in server mode or inside an IDE. + var logger = new OutputDeviceLogger(serviceProvider.GetOutputDevice(), this, workQueue.Writer, cancellationToken); + + var runInfos = runnable + .GroupBy(match => match.RunInfo) + .Select(group => new BenchmarkRunInfo( + group.Select(match => match.Node.BenchmarkCase).ToArray(), + group.Key.Type, + group.Key.Config + .AddEventProcessor(eventProcessor) + .AddLogger(logger) + .RemoveLoggersOfType() + .CreateImmutableConfig(), + group.Key.CompositeInProcessDiagnoser)) + .ToArray(); + + // BenchmarkDotNet blocks the calling thread for the whole run, so it gets a thread of its own and the + // queued messages are published from here as they are produced. + var runTask = Task.Run( + () => + { + try + { + BenchmarkRunner.Run(runInfos, runCancellation.Token); + } + finally + { + try + { + // Benchmarks that never reported a result still need one, unless the run was cancelled. + // Two things make the cancelled run the exception: + // + // * The platform's contract for a cancelled request is an OperationCanceledException, + // not a terminal state per node. CancelledTestNodeStateProperty is obsolete for + // exactly this reason, so a node left in progress is the shape it asks for. + // BenchmarkDotNet rethrows the cancellation, and awaiting the run task below surfaces + // it out of the request. + // * The token is also cancelled when publishing itself failed. Nothing drains the queue + // at that point, so results published here would be dropped anyway. + if (!runCancellation.IsCancellationRequested) + eventProcessor.PublishOutstandingResults(); + + logger.Flush(); + } + finally + { + // The drain only ends once the queue is completed, so this has to happen no matter what + // else went wrong. + workQueue.Writer.TryComplete(); + } + } + }, + CancellationToken.None); + + ExceptionDispatchInfo? drainFailure = null; + try + { + await DrainAsync(workQueue.Reader).ConfigureAwait(false); + } + catch (Exception exception) + { + // Nothing consumes the queue anymore, so the run has to be stopped rather than left orphaned. This is + // also the path a cancelled run takes, since the queued writes are handed the platform's token. + drainFailure = ExceptionDispatchInfo.Capture(exception); + runCancellation.Cancel(); + } + + try + { + // The run has to be over before the request completes, otherwise it would carry on in the background + // and its failure would go unobserved. + await runTask.ConfigureAwait(false); + } + catch when (drainFailure != null) + { + // The run was stopped because publishing failed, so that failure is the one worth reporting. + } + + drainFailure?.Throw(); + } + + /// + /// Refuses a run whose filter this adapter does not know, reporting every benchmark it could have selected as + /// failed. + /// + /// + /// Throwing instead would leave the platform with nothing per benchmark to show: the request is completed by + /// the finally in before the exception is observed, so an IDE gets a run + /// that finished with no feedback on any test. A failed node per benchmark says it where the user is looking. + /// + private async Task RefuseUnrecognisedFilterAsync( + ExecuteRequestContext context, + SessionUid sessionUid, + Enumeration enumeration, + Type unrecognisedFilter) + { + var error = + $"BenchmarkDotNet.TestAdapter does not support the '{unrecognisedFilter.FullName}' test execution " + + "filter, so it cannot tell which benchmarks this run asked for, and it will not run every benchmark " + + "of the assembly in its place. Please report this at " + + "https://github.com/dotnet/BenchmarkDotNet/issues."; + + foreach (var benchmarks in enumeration.Matches) + { + context.CancellationToken.ThrowIfCancellationRequested(); + + var node = benchmarks[0].Node; + + await context.MessageBus.PublishAsync( + this, + new TestNodeUpdateMessage(sessionUid, node.ToTestNode(InProgressTestNodeStateProperty.CachedInstance))).ConfigureAwait(false); + + await context.MessageBus.PublishAsync( + this, + new TestNodeUpdateMessage(sessionUid, node.ToTestNode(new FailedTestNodeStateProperty(error)))).ConfigureAwait(false); + } + } + + /// + /// Tells the user that the request's filter is one this adapter does not know, and is being treated as + /// matching everything. + /// + private Task WarnAboutUnrecognisedFilterAsync(Enumeration enumeration, CancellationToken cancellationToken) + { + if (enumeration.UnrecognisedFilter is not { } filterType) + return Task.CompletedTask; + + var warning = new WarningMessageOutputDeviceData( + $"BenchmarkDotNet.TestAdapter does not recognise the '{filterType.FullName}' test execution filter " + + "and is treating it as matching every benchmark. Please report this at " + + "https://github.com/dotnet/BenchmarkDotNet/issues."); + + return serviceProvider.GetOutputDevice().DisplayAsync(this, warning, cancellationToken); + } + + /// + /// Runs the queued work items in order, until the queue is completed and empty. + /// + /// The reader of the work queue. + private static async Task DrainAsync(ChannelReader> reader) + { + while (await reader.WaitToReadAsync().ConfigureAwait(false)) + { + while (reader.TryRead(out var work)) + await work().ConfigureAwait(false); + } + } + + /// + /// Reports benchmarks that share a uid as a single failed test. + /// + /// + /// The platform identifies test nodes by uid, so benchmarks that produce the same one cannot be reported + /// separately. Failing them keeps the rest of the run going, which is more useful than aborting the request. + /// + private async Task PublishCollisionAsync(ExecuteRequestContext context, SessionUid sessionUid, List collision) + { + var node = collision[0].Node; + + // The colliding benchmarks share every string the uid is built from, so the method names are the only + // thing left that can tell them apart. They are the same name when it is the parameters that collide. + var methodNames = string.Join(", ", collision + .Select(match => match.Node.BenchmarkCase.Descriptor.WorkloadMethod.Name) + .Distinct(StringComparer.Ordinal)); + var error = + $"{collision.Count} benchmarks are identified as '{node.Uid}' and cannot be told apart, so none of " + + $"them were run: {methodNames}. The identity is built from the type, the benchmark name " + + "([Benchmark(Description = \"...\")] when set, the method name otherwise), the job, and the string " + + "representation of the parameters. Give the colliding benchmarks distinct descriptions, distinct " + + "jobs, or distinct parameter ToString() results."; + + await context.MessageBus.PublishAsync( + this, + new TestNodeUpdateMessage(sessionUid, node.ToTestNode(InProgressTestNodeStateProperty.CachedInstance))).ConfigureAwait(false); + + await context.MessageBus.PublishAsync( + this, + new TestNodeUpdateMessage(sessionUid, node.ToTestNode(new FailedTestNodeStateProperty(error)))).ConfigureAwait(false); + } + + /// + /// Enumerates the benchmarks of the assembly and keeps the ones the request asked for. + /// + /// The filter of the request. + /// The scope the values this creates are recorded in. + /// + /// The matching benchmarks in enumeration order and grouped by uid. A group holding more than one benchmark + /// is a uid collision. + /// + private Enumeration GetMatchingBenchmarks(ITestExecutionFilter filter, ParameterValueLifetime.RequestScope parameterValueScope) + { + var (matches, unrecognisedFilter) = CreateMatcher(filter); + var matchingGroups = new List>(); + var matchesByUid = new Dictionary>(StringComparer.Ordinal); + var runInfos = BenchmarkEnumerator.GetBenchmarksFromAssembly(assembly, parameterValueScope.TrackHidden); + + // Recorded before anything else is done with them, so that whatever throws from here on - a node that + // cannot be built, a message that cannot be published - cannot lose them. + parameterValueScope.Track(runInfos.SelectMany(runInfo => runInfo.BenchmarksCases)); + + foreach (var runInfo in runInfos) + { + // The job only earns a place in the display name when the benchmark actually runs under several jobs. + // This is computed before filtering so that a benchmark keeps the same name however it was selected. + var includeJobInName = runInfo.BenchmarksCases.Select(c => c.Job.DisplayInfo).Distinct().Count() > 1; + + foreach (var benchmarkCase in runInfo.BenchmarksCases) + { + var node = BenchmarkTestNode.Create(benchmarkCase, includeJobInName); + if (!matches(node)) + continue; + + if (!matchesByUid.TryGetValue(node.Uid, out var sameUid)) + { + sameUid = new List(); + matchesByUid.Add(node.Uid, sameUid); + matchingGroups.Add(sameUid); + } + + sameUid.Add(new Match(runInfo, node)); + } + } + + return new Enumeration(matchingGroups, unrecognisedFilter); + } + +#pragma warning disable TPEXP // The tree node filter is still marked as experimental by the platform. + private static (Func Matches, Type? UnrecognisedFilter) CreateMatcher(ITestExecutionFilter filter) => filter switch + { + TestNodeUidListFilter uidListFilter => (node => uidListFilter.TestNodeUids.Any(uid => uid.Value == node.Uid), null), + TreeNodeFilter treeNodeFilter => (node => treeNodeFilter.MatchesFilter(node.Path, node.GetFilterableProperties()), null), + NopFilter => (_ => true, null), + + // ITestExecutionFilter is a public extension point, and a consumer can resolve a newer platform than + // this was built against, so a filter this does not know is bound to turn up one day. It matches + // everything and is reported as unrecognised; what that costs differs between the two requests, so what + // to do about it is left to each of them - discovery lists the lot and says so, a run refuses. + _ => (_ => true, filter.GetType()) + }; +#pragma warning restore TPEXP + + /// + /// The result of enumerating the assembly for a request. + /// + private sealed class Enumeration + { + public Enumeration(List> matches, Type? unrecognisedFilter) + { + Matches = matches; + UnrecognisedFilter = unrecognisedFilter; + } + + /// + /// Gets the type of the request's filter when it is one this adapter does not know, and was therefore + /// taken to match every benchmark. + /// + public Type? UnrecognisedFilter { get; } + + /// + /// Gets the benchmarks the request asked for, grouped by uid. + /// + public List> Matches { get; } + } + + /// + /// A benchmark that matched the request, together with the run info it belongs to. + /// + private sealed class Match + { + public Match(BenchmarkRunInfo runInfo, BenchmarkTestNode node) + { + RunInfo = runInfo; + Node = node; + } + + public BenchmarkRunInfo RunInfo { get; } + + public BenchmarkTestNode Node { get; } + } + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestNode.cs b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestNode.cs new file mode 100644 index 0000000000..b65f820cf2 --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/BenchmarkTestNode.cs @@ -0,0 +1,198 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Exporters; +using BenchmarkDotNet.Extensions; +using BenchmarkDotNet.Running; +using Microsoft.Testing.Platform.Extensions.Messages; +using System.Reflection; +using System.Text; + +namespace BenchmarkDotNet.TestAdapter.TestingPlatform +{ + /// + /// The Microsoft.Testing.Platform view of a single . + /// + /// + /// A carries mutable state (the property bag holds the current outcome), so a fresh node is + /// created for every message published on the bus. This class holds the parts that never change. + /// + internal sealed class BenchmarkTestNode + { + private readonly IProperty[] staticProperties; + + private BenchmarkTestNode(BenchmarkCase benchmarkCase, string uid, string displayName, string path, IProperty[] staticProperties) + { + BenchmarkCase = benchmarkCase; + Uid = uid; + DisplayName = displayName; + Path = path; + this.staticProperties = staticProperties; + } + + /// + /// Gets the benchmark this node represents. + /// + public BenchmarkCase BenchmarkCase { get; } + + /// + /// Gets the stable identifier of the node. It has to be identical in the discovery and the execution phase, + /// which may happen in different processes. + /// + public string Uid { get; } + + /// + /// Gets the name shown by test runners. + /// + public string DisplayName { get; } + + /// + /// Gets the '/' separated path used by . + /// + public string Path { get; } + + /// + /// Creates the node for a benchmark case. + /// + /// The benchmark case to describe. + /// + /// Whether the display name should be suffixed with the job, which is only useful when the benchmark runs + /// under more than one job. + /// + /// The created node. + public static BenchmarkTestNode Create(BenchmarkCase benchmarkCase, bool includeJobInName) + { + var benchmarkMethod = benchmarkCase.Descriptor.WorkloadMethod; + var type = benchmarkCase.Descriptor.Type; + var fullClassName = type.GetCorrectCSharpTypeName(prefixWithGlobal: false); + var parametrizedMethodName = FullNameProvider.GetMethodName(benchmarkCase); + var jobDisplayInfo = benchmarkCase.GetUnrandomizedJobDisplayInfo(); + + // The uid is the hash BenchmarkDotNet itself uses (and reports through `--list json`), so that a benchmark + // keeps the same identity across processes and across tools. The job is only part of the display name + // when it actually adds information. + var uid = benchmarkCase.GetUniqueId(); + + // Microsoft.Testing.Platform keeps the display name and the identity apart, so the name is free to be the + // [Benchmark(Description = ...)] the author chose, spelled the way it was written: Descriptor's + // WorkloadMethodDisplayInfo is the console table form, which quotes a description containing a space or a + // bracket so that BenchmarkDotNet's own --filter can delimit it, and an IDE label does not want that. It + // is still the fallback, so that a hand-built Descriptor with no attribute keeps its name. The path keeps + // the method name, so that a filter still matches what --filter matches. + var benchmarkAttribute = benchmarkMethod.ResolveAttribute(); + var benchmarkName = string.IsNullOrEmpty(benchmarkAttribute?.Description) + ? benchmarkCase.Descriptor.WorkloadMethodDisplayInfo + : benchmarkAttribute!.Description!; + var displayMethodName = FullNameProvider.GetMethodDisplayName(benchmarkCase, benchmarkName); + var displayName = $"{fullClassName}.{displayMethodName}" + (includeJobInName ? $" [{jobDisplayInfo}]" : ""); + + var properties = new List + { + new TestMethodIdentifierProperty( + type.Assembly.FullName, + type.Namespace ?? string.Empty, + GetEcmaTypeName(type), + benchmarkMethod.Name, + benchmarkMethod.IsGenericMethodDefinition ? benchmarkMethod.GetGenericArguments().Length : 0, + benchmarkMethod.GetParameters().Select(p => p.ParameterType.FullName ?? p.ParameterType.Name).ToArray(), + benchmarkMethod.ReturnType.FullName ?? benchmarkMethod.ReturnType.Name), + }; + + // SourceCodeFile is a non-nullable string that a [CallerFilePath] fills in, so it is empty rather than + // null on an attribute built without caller information - by an analyzer, or by hand. Publishing a + // location of "" at line 0 would send an IDE to a file that does not exist. + if (benchmarkAttribute != null && !string.IsNullOrEmpty(benchmarkAttribute.SourceCodeFile)) + { + // BenchmarkAttribute captures the line of the attribute itself, and the platform expects 0-based lines. + var line = Math.Max(0, benchmarkAttribute.SourceCodeLineNumber - 1); + var position = new LinePosition(line, 0); + properties.Add(new TestFileLocationProperty(benchmarkAttribute.SourceCodeFile, new LinePositionSpan(position, position))); + } + + // The categories come from the descriptor rather than from DefaultCategoryDiscoverer, because + // BenchmarkConverter has already resolved them through the config's ICategoryDiscoverer. Rediscovering + // them here would hide the categories of a custom discoverer from --treenode-filter, even though + // BenchmarkDotNet's own --anyCategories and the summary do see them. + foreach (var category in benchmarkCase.Descriptor.Categories) + properties.Add(new TestMetadataProperty("Category", category)); + + var path = BuildPath(type.Assembly, type.Namespace, fullClassName, parametrizedMethodName, jobDisplayInfo); + + return new BenchmarkTestNode(benchmarkCase, uid, displayName, path, properties.ToArray()); + } + + /// + /// Creates a message-bus ready node in the given state. + /// + /// The state of the benchmark, e.g. discovered, passed or failed. + /// Any additional properties, such as timing or captured output. + /// The created test node. + public TestNode ToTestNode(TestNodeStateProperty state, params IProperty[] extraProperties) + { + var properties = new PropertyBag(staticProperties); + properties.Add(state); + foreach (var property in extraProperties) + properties.Add(property); + + return new TestNode + { + Uid = new TestNodeUid(Uid), + DisplayName = DisplayName, + Properties = properties + }; + } + + /// + /// Gets the properties a can match against, + /// which is what makes `--treenode-filter "/*/*/*/*[Category=Fast]"` work. + /// + /// The filterable properties. + public PropertyBag GetFilterableProperties() => new PropertyBag(staticProperties); + + /// + /// Gets the name of a type in the form Microsoft.Testing.Platform documents for + /// , which is the ECMA-335 one rather than the C# one. + /// + /// + /// A generic type is named after its arity, `GenericProbe`1`, and its type arguments are no part of the name - + /// they belong to the display name, which carries them. A nested type is qualified by its declaring types, + /// separated by '+'; the namespace is left out, because the property carries it separately. + /// + /// The type declaring the benchmark. + /// The ECMA-335 name of the type. + private static string GetEcmaTypeName(Type type) + { + // Type.Name is already the arity form, for an open and for a closed generic type alike. + var name = type.Name; + + for (var declaringType = type.DeclaringType; declaringType != null; declaringType = declaringType.DeclaringType) + name = declaringType.Name + "+" + name; + + return name; + } + + private static string BuildPath(Assembly assembly, string? @namespace, string fullClassName, string methodName, string jobDisplayInfo) + { + // The convention followed by the other test frameworks is ////. + var className = @namespace == null || !fullClassName.StartsWith(@namespace + ".", StringComparison.Ordinal) + ? fullClassName + : fullClassName.Substring(@namespace.Length + 1); + + return new StringBuilder() + .Append('/').Append(Escape(assembly.GetName().Name)) + .Append('/').Append(Escape(@namespace ?? string.Empty)) + .Append('/').Append(Escape(className)) + .Append('/').Append(Escape($"{methodName} [{jobDisplayInfo}]")) + .ToString(); + } + + // Benchmark parameters are stringified user values, and the leaf ends in the job between brackets, so a + // segment can contain the characters TreeNodeFilter gives a meaning to. None of them can be escaped into a + // segment: Microsoft.Testing.Platform splits the path on every '/' without ever unescaping it, and + // TreeNodeFilter rejects a filter whose segment contains one, so a raw '/' would both deepen the tree and + // leave the benchmark unmatchable; '[' and ']' delimit a property filter, so a filter spelling a leaf out in + // full would have its ' [Dry]' parsed as one instead of matched. Percent encoding keeps the path four levels + // deep and every segment addressable, at the price of a filter having to spell those characters as '%2F', + // '%5B' and '%5D'. + private static string Escape(string segment) + => segment.Replace("%", "%25").Replace("/", "%2F").Replace("[", "%5B").Replace("]", "%5D"); + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/TestingPlatform/OutputDeviceLogger.cs b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/OutputDeviceLogger.cs new file mode 100644 index 0000000000..a2d1b0d2b4 --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/OutputDeviceLogger.cs @@ -0,0 +1,78 @@ +using BenchmarkDotNet.Loggers; +using Microsoft.Testing.Platform.Extensions.OutputDevice; +using Microsoft.Testing.Platform.OutputDevice; +using System.Text; +using System.Threading.Channels; + +namespace BenchmarkDotNet.TestAdapter.TestingPlatform +{ + /// + /// Forwards the BenchmarkDotNet log to the platform output device, so that build progress and the result summary + /// show up in the test run output. + /// + internal sealed class OutputDeviceLogger : ILogger + { + private readonly IOutputDevice outputDevice; + private readonly IOutputDeviceDataProducer producer; + private readonly ChannelWriter> workQueue; + private readonly CancellationToken cancellationToken; + private readonly StringBuilder currentLine = new(); + private LogKind currentLineKind = LogKind.Default; + + public OutputDeviceLogger( + IOutputDevice outputDevice, + IOutputDeviceDataProducer producer, + ChannelWriter> workQueue, + CancellationToken cancellationToken) + { + this.outputDevice = outputDevice; + this.producer = producer; + this.workQueue = workQueue; + this.cancellationToken = cancellationToken; + } + + public string Id => nameof(OutputDeviceLogger); + + public int Priority => 0; + + public void Write(LogKind logKind, string text) + { + currentLine.Append(text); + + // Assume that if any part of the line is an error or a warning, the whole line is. + // The kind is reset when the line is flushed. + if (logKind == LogKind.Error || (logKind == LogKind.Warning && currentLineKind != LogKind.Error)) + currentLineKind = logKind; + } + + public void WriteLine() + { + var text = currentLine.ToString(); + var kind = currentLineKind; + + currentLine.Clear(); + currentLineKind = LogKind.Default; + + IOutputDeviceData data = kind switch + { + LogKind.Error => new ErrorMessageOutputDeviceData(text), + LogKind.Warning => new WarningMessageOutputDeviceData(text), + _ => new TextOutputDeviceData(text) + }; + + workQueue.TryWrite(() => outputDevice.DisplayAsync(producer, data, cancellationToken)); + } + + public void WriteLine(LogKind logKind, string text) + { + Write(logKind, text); + WriteLine(); + } + + public void Flush() + { + if (currentLine.Length > 0) + WriteLine(); + } + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/TestingPlatform/ParameterValueLifetime.cs b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/ParameterValueLifetime.cs new file mode 100644 index 0000000000..29fa031b86 --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/ParameterValueLifetime.cs @@ -0,0 +1,232 @@ +using BenchmarkDotNet.Helpers; +using BenchmarkDotNet.Parameters; +using BenchmarkDotNet.Running; +using Microsoft.Testing.Platform.Extensions.TestHost; + +namespace BenchmarkDotNet.TestAdapter.TestingPlatform +{ + /// + /// Owns the parameter values that the requests of a test application enumerate and do not run, and disposes them + /// once no request can hand them back again. + /// + /// + /// + /// Disposing a value at the end of the request that enumerated it is wrong under server mode, which is how Visual + /// Studio and the VS Code Test Explorer drive the platform: one process serves a discovery request and the run + /// requests that follow it, every request enumerates the assembly again, and a [ParamsSource] backed by a cached + /// collection - a static field, a property over a readonly array - hands back the very same objects. Discovery + /// would dispose the values the run is about to execute against. + /// + /// + /// Holding every value until the application ends is wrong the other way round: a source that constructs per + /// read - yield return new FileStream(...), the common shape - produces fresh objects on every request, + /// none of which a later request can reuse, and a long session would pile them up. The two are told apart by + /// what the next request enumerates: a value that comes back is cached and stays, a value that does not is gone + /// for good and is disposed then. The same rule bounds what is remembered about the values BenchmarkDotNet + /// disposed itself. + /// + /// + /// Each request collects into a of its own, so that requests the platform chooses to + /// overlap cannot take each other's values down; only completing a request touches what is held. + /// + /// + /// The platform builds a per request but this extension only once, which is + /// why the values live here. + /// + /// + internal sealed class ParameterValueLifetime : ITestHostApplicationLifetime + { + private readonly BenchmarkDotNetExtension extension = new(); + + private readonly object gate = new(); + + // Keyed by the value rather than by the ParameterInstance, because BenchmarkConverter hands the same value to + // every job and every argument set of a benchmark, and it is to be disposed once. + // + // What the last completed request enumerated, and which of those BenchmarkDotNet has disposed itself because + // it ran them. + private Dictionary held = new(ParameterValueDisposer.ByReference); + private readonly HashSet disposedByBenchmarkDotNet = new(ParameterValueDisposer.ByReference); + + // The scopes of the requests that have not completed yet. A request hands its values over by completing, so + // without this the values of one that never got there - the client sent `exit`, or the IDE cancelled, while + // the request was still in flight - would be reachable from nothing by the time the application ends. + private readonly HashSet live = []; + + /// + public string Uid => extension.Uid + ".ParameterValueLifetime"; + + /// + public string Version => extension.Version; + + /// + public string DisplayName => extension.DisplayName; + + /// + public string Description => extension.Description; + + /// + public Task IsEnabledAsync() => extension.IsEnabledAsync(); + + /// + /// Starts collecting the parameter values of one request. + /// + /// The scope to record that request's values in, and to complete when it is over. + public RequestScope BeginRequest() + { + var request = new RequestScope(this); + + lock (gate) + live.Add(request); + + return request; + } + + /// + public Task BeforeRunAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + /// + public async Task AfterRunAsync(int exitCode, CancellationToken cancellationToken) + { + List unused; + + lock (gate) + { + // Nothing can ask for any of these again. That includes the values of a request still in flight: the + // application is going away, so its scope will never be completed, and these are the only reference + // left to them - a value left to the finalizer instead is the dotnet/BenchmarkDotNet#1383 hang. + unused = held + .Concat(live.SelectMany(request => request.Enumerated)) + .Where(pair => !disposedByBenchmarkDotNet.Contains(pair.Key)) + .GroupBy(pair => pair.Key, ParameterValueDisposer.ByReference) + .Select(group => group.First().Value) + .ToList(); + + held.Clear(); + live.Clear(); + disposedByBenchmarkDotNet.Clear(); + } + + await unused.DisposeAllAsync().ConfigureAwait(false); + } + + private async ValueTask CompleteAsync(RequestScope request, IEnumerable ranCases) + { + List gone = []; + + lock (gate) + { + // Handed over, whichever way this goes: what happens to these values is decided here and now, so the + // exit-time sweep must not find them a second time. + live.Remove(request); + + var enumerated = request.Enumerated; + + if (!request.HasEnumerated) + { + // The request never reached the end of its enumeration - the assembly failed to load, a source + // threw partway - so its absences say nothing about what a source would hand back, and what is + // held has to stay. Whatever it did manage to create joins it, rather than being lost. + foreach (var pair in enumerated) + held[pair.Key] = pair.Value; + + return; + } + + // Recorded before anything is chosen for disposal, so that a value BenchmarkDotNet has just disposed + // can never also be a candidate here. + foreach (var parameter in ParameterValueDisposer.GetDisposableParameters(ranCases)) + disposedByBenchmarkDotNet.Add(parameter.Value!); + + // A value the last completed request enumerated and this one did not comes from a source that + // constructs per read: no request can hand it back again, so it goes now rather than at exit. A value + // that came back is cached, and stays until nothing can ask for it. + gone = held + .Where(pair => !enumerated.ContainsKey(pair.Key) && !disposedByBenchmarkDotNet.Contains(pair.Key)) + .Select(pair => pair.Value) + .ToList(); + + // Only the values that keep coming back need remembering as already disposed; a fresh one that was + // run is gone with its request. + disposedByBenchmarkDotNet.RemoveWhere(value => !enumerated.ContainsKey(value)); + + held = enumerated; + } + + await gone.DisposeAllAsync().ConfigureAwait(false); + } + + /// + /// The parameter values one request enumerated. + /// + internal sealed class RequestScope + { + private readonly ParameterValueLifetime owner; + + internal RequestScope(ParameterValueLifetime owner) => this.owner = owner; + + /// + /// Gets the values this request enumerated, keyed by the value itself. + /// + internal Dictionary Enumerated { get; } = new(ParameterValueDisposer.ByReference); + + /// + /// Gets whether the request got as far as enumerating the assembly. It tells "this request enumerated + /// nothing" apart from "this request never got to enumerate", which is the difference between concluding + /// that a source no longer hands a value back and having asked it nothing at all. + /// + internal bool HasEnumerated { get; private set; } + + /// + /// Records the values of the benchmarks the enumeration returned, and marks the enumeration as reached. + /// + /// The benchmarks the enumeration returned. + public void Track(IEnumerable enumeratedCases) + { + lock (owner.gate) + { + foreach (var parameter in ParameterValueDisposer.GetDisposableParameters(enumeratedCases)) + Enumerated[parameter.Value!] = parameter; + + HasEnumerated = true; + } + } + + /// + /// Records the values of the benchmarks that the enumeration hid, which no request will ever be handed. + /// + /// + /// Called from inside the enumeration, which may still throw afterwards, so it deliberately does not mark + /// the enumeration as reached: being kept by the enumeration is not the same as being run, and the kept + /// values are recorded by once the enumeration has returned. + /// + /// Everything the assembly declares. + /// The benchmarks the enumeration returned. + public void TrackHidden(IEnumerable enumeratedCases, IEnumerable keptCases) + { + lock (owner.gate) + { + var kept = new HashSet( + ParameterValueDisposer.GetDisposableParameters(keptCases).Select(parameter => parameter.Value!), + ParameterValueDisposer.ByReference); + + foreach (var parameter in ParameterValueDisposer.GetDisposableParameters(enumeratedCases)) + { + if (!kept.Contains(parameter.Value!)) + Enumerated[parameter.Value!] = parameter; + } + } + } + + /// + /// Completes the request: disposes the values of the last completed request that this one did not + /// enumerate again, and keeps the rest for the next one. + /// + /// + /// The benchmarks whose values BenchmarkDotNet disposed itself, which it does once its run stage began - + /// and not at all when it bailed out before that, on a critical validation error. + /// + public ValueTask CompleteAsync(IEnumerable ranCases) => owner.CompleteAsync(this, ranCases); + } + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/TestingPlatform/TestApplicationBuilderExtensions.cs b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/TestApplicationBuilderExtensions.cs new file mode 100644 index 0000000000..ea2f997fa3 --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/TestApplicationBuilderExtensions.cs @@ -0,0 +1,56 @@ +using Microsoft.Testing.Platform.Builder; +using Microsoft.Testing.Platform.Capabilities.TestFramework; +using Microsoft.Testing.Platform.Helpers; +using System.Reflection; + +namespace BenchmarkDotNet.TestAdapter.TestingPlatform +{ + /// + /// Extensions for registering BenchmarkDotNet with a Microsoft.Testing.Platform application. + /// + public static class TestApplicationBuilderExtensions + { + /// + /// Registers BenchmarkDotNet as the test framework, so that the benchmarks of the entry assembly are exposed + /// as tests. + /// + /// The builder of the test application. + /// The same builder, so that calls can be chained. + public static ITestApplicationBuilder AddBenchmarkDotNet(this ITestApplicationBuilder builder) + => builder.AddBenchmarkDotNet( + Assembly.GetEntryAssembly() ?? throw new InvalidOperationException( + "There is no entry assembly to look for benchmarks in. Use the overload that takes an assembly.")); + + /// + /// Registers BenchmarkDotNet as the test framework, so that the benchmarks of the given assembly are exposed + /// as tests. + /// + /// The builder of the test application. + /// The assembly to look for benchmarks in. + /// The same builder, so that calls can be chained. + public static ITestApplicationBuilder AddBenchmarkDotNet(this ITestApplicationBuilder builder, Assembly assembly) + { + if (builder == null) + throw new ArgumentNullException(nameof(builder)); + if (assembly == null) + throw new ArgumentNullException(nameof(assembly)); + + // The platform builds a test framework per request but this once, which is what makes it able to hold + // the parameter values of one request until the whole application is done. See ParameterValueLifetime. + var parameterValues = new ParameterValueLifetime(); + + builder.RegisterTestFramework( + _ => new TestFrameworkCapabilities(), + (capabilities, serviceProvider) => new BenchmarkTestFramework(capabilities, serviceProvider, assembly, parameterValues)); + + builder.TestHost.AddTestHostApplicationLifetime(_ => parameterValues); + + // Opts into the tree node filter, which is what backs `--treenode-filter "/*/*/MyBenchmarks/*"`. +#pragma warning disable TPEXP // The tree node filter is still marked as experimental by the platform. + builder.AddTreeNodeFilterService(new BenchmarkDotNetExtension()); +#pragma warning restore TPEXP + + return builder; + } + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/TestingPlatform/TestingPlatformBuilderHook.cs b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/TestingPlatformBuilderHook.cs new file mode 100644 index 0000000000..bc7c7db03c --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/TestingPlatform/TestingPlatformBuilderHook.cs @@ -0,0 +1,26 @@ +using Microsoft.Testing.Platform.Builder; +using System.ComponentModel; + +namespace BenchmarkDotNet.TestAdapter.TestingPlatform +{ + /// + /// The hook Microsoft.Testing.Platform.MSBuild calls from the entry point it generates for the benchmark project. + /// + /// + /// This is wired up by the TestingPlatformBuilderHook item in BenchmarkDotNet.TestAdapter.targets. + /// It is public because the generated code lives in the benchmark assembly, but it is not meant to be called + /// directly; use + /// instead when writing an entry point by hand. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public static class TestingPlatformBuilderHook + { + /// + /// Registers BenchmarkDotNet with the test application being built. + /// + /// The builder of the test application. + /// The command line arguments of the process. Unused. + public static void AddExtensions(ITestApplicationBuilder builder, string[] arguments) + => builder.AddBenchmarkDotNet(); + } +} diff --git a/src/BenchmarkDotNet.TestAdapter/BenchmarkCaseExtensions.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/BenchmarkCaseExtensions.cs similarity index 77% rename from src/BenchmarkDotNet.TestAdapter/BenchmarkCaseExtensions.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/BenchmarkCaseExtensions.cs index 06e5782b6a..ab0121ad0c 100644 --- a/src/BenchmarkDotNet.TestAdapter/BenchmarkCaseExtensions.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/BenchmarkCaseExtensions.cs @@ -1,12 +1,11 @@ using BenchmarkDotNet.Attributes; -using BenchmarkDotNet.Characteristics; using BenchmarkDotNet.Exporters; using BenchmarkDotNet.Extensions; using BenchmarkDotNet.Running; using Microsoft.TestPlatform.AdapterUtilities; using Microsoft.VisualStudio.TestPlatform.ObjectModel; -namespace BenchmarkDotNet.TestAdapter +namespace BenchmarkDotNet.TestAdapter.VSTest { /// /// A set of extensions for BenchmarkCase to support converting to VSTest TestCase objects. @@ -65,26 +64,6 @@ internal static TestCase ToVsTestCase(this BenchmarkCase benchmarkCase, string a return vsTestCase; } - /// - /// If an ID is not provided, a random string is used for the ID. This method will identify if randomness was - /// used for the ID and return the Job's DisplayInfo with that randomness removed so that the same benchmark - /// can be referenced across multiple processes. - /// - /// The benchmark case. - /// The benchmark case' job's DisplayInfo without randomness. - internal static string GetUnrandomizedJobDisplayInfo(this BenchmarkCase benchmarkCase) - { - var jobDisplayInfo = benchmarkCase.Job.DisplayInfo; - if (!benchmarkCase.Job.HasValue(CharacteristicObject.IdCharacteristic) && - benchmarkCase.Job.ResolvedId.StartsWith("Job-", StringComparison.OrdinalIgnoreCase)) - { - // Replace Job-ABCDEF with Job - jobDisplayInfo = "Job" + jobDisplayInfo.Substring(benchmarkCase.Job.ResolvedId.Length); - } - - return jobDisplayInfo; - } - /// /// Gets an ID for a given BenchmarkCase that is uniquely identifiable from discovery to execution phase. /// diff --git a/src/BenchmarkDotNet.TestAdapter/BenchmarkExecutor.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/BenchmarkExecutor.cs similarity index 97% rename from src/BenchmarkDotNet.TestAdapter/BenchmarkExecutor.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/BenchmarkExecutor.cs index 8fea8f0c9d..214176ef3a 100644 --- a/src/BenchmarkDotNet.TestAdapter/BenchmarkExecutor.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/BenchmarkExecutor.cs @@ -1,10 +1,10 @@ using BenchmarkDotNet.Configs; using BenchmarkDotNet.Loggers; using BenchmarkDotNet.Running; -using BenchmarkDotNet.TestAdapter.Remoting; +using BenchmarkDotNet.TestAdapter.VSTest.Remoting; using Microsoft.VisualStudio.TestPlatform.ObjectModel; -namespace BenchmarkDotNet.TestAdapter +namespace BenchmarkDotNet.TestAdapter.VSTest { /// /// A class used for executing benchmarks diff --git a/src/BenchmarkDotNet.TestAdapter/Remoting/BenchmarkEnumeratorWrapper.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/BenchmarkEnumeratorWrapper.cs similarity index 96% rename from src/BenchmarkDotNet.TestAdapter/Remoting/BenchmarkEnumeratorWrapper.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/BenchmarkEnumeratorWrapper.cs index 9012328a1f..eb9a888896 100644 --- a/src/BenchmarkDotNet.TestAdapter/Remoting/BenchmarkEnumeratorWrapper.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/BenchmarkEnumeratorWrapper.cs @@ -1,4 +1,4 @@ -namespace BenchmarkDotNet.TestAdapter.Remoting +namespace BenchmarkDotNet.TestAdapter.VSTest.Remoting { /// /// A wrapper around the BenchmarkEnumerator for passing data across AppDomain boundaries. diff --git a/src/BenchmarkDotNet.TestAdapter/Remoting/BenchmarkExecutorWrapper.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/BenchmarkExecutorWrapper.cs similarity index 91% rename from src/BenchmarkDotNet.TestAdapter/Remoting/BenchmarkExecutorWrapper.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/BenchmarkExecutorWrapper.cs index 98b5b9a35f..9318769a8c 100644 --- a/src/BenchmarkDotNet.TestAdapter/Remoting/BenchmarkExecutorWrapper.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/BenchmarkExecutorWrapper.cs @@ -1,4 +1,4 @@ -namespace BenchmarkDotNet.TestAdapter.Remoting +namespace BenchmarkDotNet.TestAdapter.VSTest.Remoting { /// /// A wrapper around the BenchmarkExecutor that works across AppDomain boundaries. diff --git a/src/BenchmarkDotNet.TestAdapter/Remoting/MessageLoggerWrapper.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/MessageLoggerWrapper.cs similarity index 91% rename from src/BenchmarkDotNet.TestAdapter/Remoting/MessageLoggerWrapper.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/MessageLoggerWrapper.cs index 9d2bc6f3b3..5d04e17739 100644 --- a/src/BenchmarkDotNet.TestAdapter/Remoting/MessageLoggerWrapper.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/MessageLoggerWrapper.cs @@ -1,6 +1,6 @@ using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; -namespace BenchmarkDotNet.TestAdapter.Remoting +namespace BenchmarkDotNet.TestAdapter.VSTest.Remoting { /// /// A wrapper around an IMessageLogger that works across AppDomain boundaries. diff --git a/src/BenchmarkDotNet.TestAdapter/Remoting/SerializationHelpers.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/SerializationHelpers.cs similarity index 95% rename from src/BenchmarkDotNet.TestAdapter/Remoting/SerializationHelpers.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/SerializationHelpers.cs index 7a99a741bc..872530190e 100644 --- a/src/BenchmarkDotNet.TestAdapter/Remoting/SerializationHelpers.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/SerializationHelpers.cs @@ -1,6 +1,6 @@ using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities; -namespace BenchmarkDotNet.TestAdapter.Remoting +namespace BenchmarkDotNet.TestAdapter.VSTest.Remoting { /// /// A set of helper methods for serializing and deserializing the VSTest TestCases and TestReports. diff --git a/src/BenchmarkDotNet.TestAdapter/Remoting/TestExecutionRecorderWrapper.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/TestExecutionRecorderWrapper.cs similarity index 96% rename from src/BenchmarkDotNet.TestAdapter/Remoting/TestExecutionRecorderWrapper.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/TestExecutionRecorderWrapper.cs index 728039ade8..7625bd444a 100644 --- a/src/BenchmarkDotNet.TestAdapter/Remoting/TestExecutionRecorderWrapper.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/Remoting/TestExecutionRecorderWrapper.cs @@ -1,7 +1,7 @@ using Microsoft.VisualStudio.TestPlatform.ObjectModel; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter; -namespace BenchmarkDotNet.TestAdapter.Remoting +namespace BenchmarkDotNet.TestAdapter.VSTest.Remoting { /// /// A wrapper around the ITestExecutionRecorder which works across AppDomain boundaries. diff --git a/src/BenchmarkDotNet.TestAdapter/Utility/LoggerHelper.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/Utility/LoggerHelper.cs similarity index 97% rename from src/BenchmarkDotNet.TestAdapter/Utility/LoggerHelper.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/Utility/LoggerHelper.cs index ff06c93189..594beba206 100644 --- a/src/BenchmarkDotNet.TestAdapter/Utility/LoggerHelper.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/Utility/LoggerHelper.cs @@ -1,7 +1,7 @@ using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; using System.Diagnostics; -namespace BenchmarkDotNet.TestAdapter; +namespace BenchmarkDotNet.TestAdapter.VSTest; internal class LoggerHelper { diff --git a/src/BenchmarkDotNet.TestAdapter/Utility/TestCaseFilter.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/Utility/TestCaseFilter.cs similarity index 99% rename from src/BenchmarkDotNet.TestAdapter/Utility/TestCaseFilter.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/Utility/TestCaseFilter.cs index 47f9941e2e..457b07e98c 100644 --- a/src/BenchmarkDotNet.TestAdapter/Utility/TestCaseFilter.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/Utility/TestCaseFilter.cs @@ -3,7 +3,7 @@ using System.Reflection; using System.Runtime.ExceptionServices; -namespace BenchmarkDotNet.TestAdapter; +namespace BenchmarkDotNet.TestAdapter.VSTest; internal class TestCaseFilter { diff --git a/src/BenchmarkDotNet.TestAdapter/VSTestAdapter.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/VSTestAdapter.cs similarity index 99% rename from src/BenchmarkDotNet.TestAdapter/VSTestAdapter.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/VSTestAdapter.cs index ced77fae7a..6d4f5bb13b 100644 --- a/src/BenchmarkDotNet.TestAdapter/VSTestAdapter.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/VSTestAdapter.cs @@ -1,11 +1,11 @@ -using BenchmarkDotNet.TestAdapter.Remoting; +using BenchmarkDotNet.TestAdapter.VSTest.Remoting; using Microsoft.VisualStudio.TestPlatform.ObjectModel; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Adapter; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; using System.Diagnostics; using System.Reflection; -namespace BenchmarkDotNet.TestAdapter +namespace BenchmarkDotNet.TestAdapter.VSTest { /// /// Discovers and executes benchmarks using the VSTest protocol. diff --git a/src/BenchmarkDotNet.TestAdapter/VSTestEventProcessor.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/VSTestEventProcessor.cs similarity index 98% rename from src/BenchmarkDotNet.TestAdapter/VSTestEventProcessor.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/VSTestEventProcessor.cs index 8b4546b034..14570c748e 100644 --- a/src/BenchmarkDotNet.TestAdapter/VSTestEventProcessor.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/VSTestEventProcessor.cs @@ -2,7 +2,7 @@ using BenchmarkDotNet.Extensions; using BenchmarkDotNet.Reports; using BenchmarkDotNet.Running; -using BenchmarkDotNet.TestAdapter.Remoting; +using BenchmarkDotNet.TestAdapter.VSTest.Remoting; using BenchmarkDotNet.Toolchains.Results; using BenchmarkDotNet.Validators; using Microsoft.VisualStudio.TestPlatform.ObjectModel; @@ -11,7 +11,7 @@ using System.Globalization; using System.Text; -namespace BenchmarkDotNet.TestAdapter +namespace BenchmarkDotNet.TestAdapter.VSTest { /// /// An event processor which will pass on benchmark execution information to VSTest. diff --git a/src/BenchmarkDotNet.TestAdapter/VSTestLogger.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/VSTestLogger.cs similarity index 97% rename from src/BenchmarkDotNet.TestAdapter/VSTestLogger.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/VSTestLogger.cs index 03bf860cba..78b17a9f40 100644 --- a/src/BenchmarkDotNet.TestAdapter/VSTestLogger.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/VSTestLogger.cs @@ -2,7 +2,7 @@ using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; using System.Text; -namespace BenchmarkDotNet.TestAdapter +namespace BenchmarkDotNet.TestAdapter.VSTest { /// /// A class to send logs from BDN to the VSTest output log. diff --git a/src/BenchmarkDotNet.TestAdapter/VSTestProperties.cs b/src/BenchmarkDotNet.TestAdapter/VSTest/VSTestProperties.cs similarity index 94% rename from src/BenchmarkDotNet.TestAdapter/VSTestProperties.cs rename to src/BenchmarkDotNet.TestAdapter/VSTest/VSTestProperties.cs index ac039045f5..ae07b803f9 100644 --- a/src/BenchmarkDotNet.TestAdapter/VSTestProperties.cs +++ b/src/BenchmarkDotNet.TestAdapter/VSTest/VSTestProperties.cs @@ -1,6 +1,6 @@ using Microsoft.VisualStudio.TestPlatform.ObjectModel; -namespace BenchmarkDotNet.TestAdapter +namespace BenchmarkDotNet.TestAdapter.VSTest { /// /// A class that contains all the custom properties that can be set on VSTest TestCase and TestResults. diff --git a/src/BenchmarkDotNet.TestAdapter/build/BenchmarkDotNet.TestAdapter.props b/src/BenchmarkDotNet.TestAdapter/build/BenchmarkDotNet.TestAdapter.props index 7184842aba..5d1eabd4f6 100644 --- a/src/BenchmarkDotNet.TestAdapter/build/BenchmarkDotNet.TestAdapter.props +++ b/src/BenchmarkDotNet.TestAdapter/build/BenchmarkDotNet.TestAdapter.props @@ -1,15 +1,19 @@ - + $(MSBuildThisFileDirectory)..\entrypoints\ false - - + diff --git a/src/BenchmarkDotNet.TestAdapter/build/BenchmarkDotNet.TestAdapter.targets b/src/BenchmarkDotNet.TestAdapter/build/BenchmarkDotNet.TestAdapter.targets new file mode 100644 index 0000000000..331d980cfd --- /dev/null +++ b/src/BenchmarkDotNet.TestAdapter/build/BenchmarkDotNet.TestAdapter.targets @@ -0,0 +1,49 @@ + + + + + + false + true + + + true + + + false + + + + + + BenchmarkDotNet + BenchmarkDotNet.TestAdapter.TestingPlatform.TestingPlatformBuilderHook + + + diff --git a/src/BenchmarkDotNet/Exporters/FullNameProvider.cs b/src/BenchmarkDotNet/Exporters/FullNameProvider.cs index c8d32a20e0..f1eecd7396 100644 --- a/src/BenchmarkDotNet/Exporters/FullNameProvider.cs +++ b/src/BenchmarkDotNet/Exporters/FullNameProvider.cs @@ -99,6 +99,28 @@ internal static string GetMethodName(BenchmarkCase benchmarkCase) return name.ToString(); } + /// + /// Gets the method name to show to a user, which is the given name followed by the parameters. + /// + /// + /// The name is taken as an argument rather than read off the descriptor, because + /// is the console table form: it wraps a + /// description containing a space, a quote or a bracket in single quotes, so that BenchmarkDotNet's own + /// --filter can delimit it. A label shown by an IDE wants the description as it was written. + /// + /// The benchmark case. + /// The name of the benchmark, without its parameters. + /// The method name to display. + internal static string GetMethodDisplayName(BenchmarkCase benchmarkCase, string name) + { + var builder = new StringBuilder(name); + + if (benchmarkCase.HasParameters) + builder.Append(GetBenchmarkParameters(benchmarkCase.Descriptor.WorkloadMethod, benchmarkCase.Parameters)); + + return builder.ToString(); + } + private static string GetBenchmarkParameters(MethodInfo method, ParameterInstances benchmarkParameters) { var methodArguments = method.GetParameters(); diff --git a/src/BenchmarkDotNet/Extensions/BenchmarkCaseIdentityExtensions.cs b/src/BenchmarkDotNet/Extensions/BenchmarkCaseIdentityExtensions.cs new file mode 100644 index 0000000000..948ea1d7fe --- /dev/null +++ b/src/BenchmarkDotNet/Extensions/BenchmarkCaseIdentityExtensions.cs @@ -0,0 +1,32 @@ +using BenchmarkDotNet.Characteristics; +using BenchmarkDotNet.Running; + +namespace BenchmarkDotNet.Extensions +{ + /// + /// Helpers for deriving stable identities for a BenchmarkCase. + /// + internal static class BenchmarkCaseIdentityExtensions + { + /// + /// If an ID is not provided, a random string is used for the ID. This method will identify if randomness was + /// used for the ID and return the Job's DisplayInfo with that randomness removed so that the same benchmark + /// can be referenced across multiple processes. + /// + /// The benchmark case. + /// The benchmark case' job's DisplayInfo without randomness. + internal static string GetUnrandomizedJobDisplayInfo(this BenchmarkCase benchmarkCase) + { + var jobDisplayInfo = benchmarkCase.Job.DisplayInfo; + if (!benchmarkCase.Job.HasValue(CharacteristicObject.IdCharacteristic) && + benchmarkCase.Job.ResolvedId.StartsWith("Job-", StringComparison.OrdinalIgnoreCase)) + { + // Replace Job-ABCDEF with Job + jobDisplayInfo = "Job" + jobDisplayInfo.Substring(benchmarkCase.Job.ResolvedId.Length); + } + + return jobDisplayInfo; + } + } +} + diff --git a/src/BenchmarkDotNet/Helpers/GenericBenchmarkType.cs b/src/BenchmarkDotNet/Helpers/GenericBenchmarkType.cs new file mode 100644 index 0000000000..a597bf70d7 --- /dev/null +++ b/src/BenchmarkDotNet/Helpers/GenericBenchmarkType.cs @@ -0,0 +1,42 @@ +namespace BenchmarkDotNet.Helpers +{ + /// + /// A type that was considered for running benchmarks, and the reason it cannot be run when it cannot. + /// + internal readonly struct GenericBenchmarkType + { + private GenericBenchmarkType(Type type, string? error, bool isUnreadable) + { + Type = type; + Error = error; + IsUnreadable = isUnreadable; + } + + /// + /// Gets the type. It is the type the benchmarks are read from when , and the type that + /// was rejected otherwise. + /// + internal Type Type { get; } + + /// + /// Gets the reason the type cannot be run, or null when it can. + /// + internal string? Error { get; } + + /// + /// Gets whether the type was rejected before any of its benchmarks could be read, rather than because one set + /// of [GenericTypeArguments] did not fit it. Such a type still declares benchmarks, which is what "no + /// benchmarks were found" must not be said about, and whoever drops it when nothing else survives is the + /// only one left to say why. + /// + internal bool IsUnreadable { get; } + + internal bool IsSuccess => Error == null; + + internal static GenericBenchmarkType Runnable(Type type) => new GenericBenchmarkType(type, null, false); + + internal static GenericBenchmarkType Failed(Type type, string error) => new GenericBenchmarkType(type, error, false); + + internal static GenericBenchmarkType Unreadable(Type type, string error) => new GenericBenchmarkType(type, error, true); + } +} diff --git a/src/BenchmarkDotNet/Helpers/GenericBenchmarksBuilder.cs b/src/BenchmarkDotNet/Helpers/GenericBenchmarksBuilder.cs index 471087713c..ddffd86be9 100644 --- a/src/BenchmarkDotNet/Helpers/GenericBenchmarksBuilder.cs +++ b/src/BenchmarkDotNet/Helpers/GenericBenchmarksBuilder.cs @@ -6,26 +6,69 @@ namespace BenchmarkDotNet.Helpers internal static class GenericBenchmarksBuilder { internal static Type[] GetRunnableBenchmarks(IEnumerable types) - => types.Where(type => type.ContainsRunnableBenchmarks()) - .SelectMany(BuildGenericsIfNeeded) - .Where(x => x.isSuccess) - .Select(x => x.result) + => BuildRunnableBenchmarks(types) + .Where(x => x.IsSuccess) + .Select(x => x.Type) .ToArray(); - internal static IEnumerable<(bool isSuccess, Type result)> BuildGenericsIfNeeded(Type type) + /// + /// Builds the benchmark types of the given types, keeping the ones that could not be built so that a caller + /// with somewhere to report them can. + /// + /// The types to consider. + /// Every type that was built, and every one that was rejected. + internal static IEnumerable BuildRunnableBenchmarks(IEnumerable types) + => types.Where(type => type.ContainsRunnableBenchmarks()) + .SelectMany(BuildGenericsIfNeeded); + + internal static IEnumerable BuildGenericsIfNeeded(Type type) { - var typeArguments = type.GetCustomAttributes(true).OfType() - .Select(x => x.GenericTypeArguments) - .ToArray(); + if (!TryGetGenericTypeArguments(type, out var typeArguments, out var error)) + return [GenericBenchmarkType.Unreadable(type, error)]; - if (typeArguments.Any()) + if (typeArguments.Length > 0) return BuildGenericTypes(type, typeArguments); - return [(true, type)]; + return [GenericBenchmarkType.Runnable(type)]; + } + + /// + /// Reads the [GenericTypeArguments] of a type, if the attributes of that type can be read at all. + /// + /// + /// Reflection constructs every attribute of a type in order to hand any of them back, so a single attribute + /// whose constructor throws makes the whole read throw: [Config(typeof(SomeAbstractConfig))] fails inside + /// ConfigAttribute's own constructor, and there is no way to ask for the [GenericTypeArguments] alone. The + /// type is unusable once that happens - BenchmarkConverter would throw on the very same read - so it is + /// reported as a failure and dropped, rather than aborting the enumeration of every other benchmark in the + /// assembly. + /// + /// The type to read the attributes of. + /// The type argument sets the type is to be closed over. + /// The reason the attributes could not be read. + /// Whether the attributes could be read. + private static bool TryGetGenericTypeArguments(Type type, out Type[][] typeArguments, out string error) + { + try + { + typeArguments = type.GetCustomAttributes(true).OfType() + .Select(x => x.GenericTypeArguments) + .ToArray(); + error = string.Empty; + return true; + } + catch (Exception e) + { + typeArguments = []; + error = $"Type {type.Name} was ignored because its attributes could not be read: {e.Message}"; + return false; + } } - private static IEnumerable<(bool isSuccess, Type result)> BuildGenericTypes(Type type, IEnumerable typeArguments) - => typeArguments.Select(genericArg => (type.TryMakeGenericType(genericArg, out var builtType), builtType)); + private static IEnumerable BuildGenericTypes(Type type, IEnumerable typeArguments) + => typeArguments.Select(genericArg => type.TryMakeGenericType(genericArg, out var builtType) + ? GenericBenchmarkType.Runnable(builtType) + : GenericBenchmarkType.Failed(builtType, $"Generic type {builtType.Name} failed to build due to wrong type argument or arguments count, ignoring.")); private static bool TryMakeGenericType(this Type type, Type[] typeArguments, out Type result) { @@ -41,4 +84,4 @@ private static bool TryMakeGenericType(this Type type, Type[] typeArguments, out } } } -} \ No newline at end of file +} diff --git a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs index 775b02bfe5..30bc2a92df 100644 --- a/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs +++ b/src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs @@ -197,14 +197,23 @@ private static async ValueTask RunCore(BenchmarkRunInfo[] benchmarkRu // some benchmarks might be using parameters that have locking finalizers // so we need to dispose them after we are done running the benchmarks // see https://github.com/dotnet/BenchmarkDotNet/issues/1383 and https://github.com/dotnet/runtime/issues/314 for more - await benchmarkRunInfos.DisposeAllAsync().ConfigureAwait(); - - compositeLogger.WriteLineHeader("// * Artifacts cleanup *"); - Cleanup(compositeLogger, new HashSet(artifactsToCleanup.Distinct())); - compositeLogger.WriteLineInfo("Artifacts cleanup is finished"); - compositeLogger.Flush(); + // + // DisposeAllAsync goes through every value before it reports a failure, so one that threw leaves + // nothing else undisposed - and must not take the teardown down with it either: whoever is listening + // is told that the run stage ended whatever the disposal did. + try + { + await benchmarkRunInfos.DisposeAllAsync().ConfigureAwait(); + } + finally + { + compositeLogger.WriteLineHeader("// * Artifacts cleanup *"); + Cleanup(compositeLogger, new HashSet(artifactsToCleanup.Distinct())); + compositeLogger.WriteLineInfo("Artifacts cleanup is finished."); + compositeLogger.Flush(); - eventProcessor.OnEndRunStage(); + eventProcessor.OnEndRunStage(); + } } } @@ -404,6 +413,13 @@ private static async ValueTask> Validate(BenchmarkR { var errors = new List(); + // The validators run once per BenchmarkRunInfo, so one that looks at the whole assembly - as + // GenericBenchmarksValidator does - reports the same thing again for every type in it. PrintValidationErrors + // has always shown those once, but the event processors are handed every copy, and an error that names no + // benchmark case is fanned out by the test adapters to every node: N types then put N copies of the same + // warning on each of N nodes. The same error, is one error however many times it is raised. + var reported = new HashSet(); + foreach (var benchmark in benchmarks) { var validationParameters = new ValidationParameters(benchmark.BenchmarksCases, benchmark.Config); @@ -411,7 +427,8 @@ private static async ValueTask> Validate(BenchmarkR await foreach (var error in benchmark.Config.GetCompositeValidator().ValidateAsync(validationParameters).ConfigureAwait(cancellationToken)) #pragma warning restore CA2007 { - errors.Add(error); + if (reported.Add(error)) + errors.Add(error); } } diff --git a/src/BenchmarkDotNet/Running/TypeFilter.cs b/src/BenchmarkDotNet/Running/TypeFilter.cs index 25846ee252..eed7e591df 100644 --- a/src/BenchmarkDotNet/Running/TypeFilter.cs +++ b/src/BenchmarkDotNet/Running/TypeFilter.cs @@ -13,8 +13,16 @@ public static (bool allTypesValid, IReadOnlyList runnable) GetTypesWithRun { var validRunnableTypes = new List(); + // Built once: the guard below and the list at the end both need it. + var assemblyTypes = assemblies + .SelectMany(assembly => GenericBenchmarksBuilder.BuildRunnableBenchmarks(assembly.GetRunnableBenchmarks())) + .ToArray(); + bool hasRunnableTypeBenchmarks = types.Any(type => type.ContainsRunnableBenchmarks()); - bool hasRunnableAssemblyBenchmarks = assemblies.Any(assembly => GenericBenchmarksBuilder.GetRunnableBenchmarks(assembly.GetRunnableBenchmarks()).Length > 0); + + // A type whose attributes cannot be read counts as declaring benchmarks here - it does, and telling the + // user that no [Benchmark] was found would be wrong. It is reported below, in place of being run. + bool hasRunnableAssemblyBenchmarks = assemblyTypes.Any(built => built.IsSuccess || built.IsUnreadable); if (!hasRunnableTypeBenchmarks && !hasRunnableAssemblyBenchmarks) { @@ -39,11 +47,29 @@ public static (bool allTypesValid, IReadOnlyList runnable) GetTypesWithRun return (false, Array.Empty()); } + // A type whose attributes cannot be read at all - [Config(typeof(SomeAbstractConfig))], say, where the + // attribute throws while reflection constructs it - is dropped rather than allowed to abort the run, but + // that has to be said out loud. GenericBenchmarksValidator says so too, and on the paths that never come + // through here it is the only one that can, but it only runs once at least one benchmark survived: when + // the unreadable type was the only one, this is the one place left to say why nothing was found. Saying + // it twice on the way to a run is the lesser problem. A type that failed on its [GenericTypeArguments] + // is left to the validator alone, which is where that has always been reported. + void AddRunnable(IEnumerable built) + { + foreach (var candidate in built) + { + if (candidate.IsSuccess) + validRunnableTypes.Add(candidate.Type); + else if (candidate.IsUnreadable) + logger.WriteLineError(candidate.Error!); + } + } + foreach (var type in types) { if (type.ContainsRunnableBenchmarks()) { - validRunnableTypes.AddRange(GenericBenchmarksBuilder.BuildGenericsIfNeeded(type).Where(tuple => tuple.isSuccess).Select(tuple => tuple.result)); + AddRunnable(GenericBenchmarksBuilder.BuildGenericsIfNeeded(type)); } else { @@ -53,10 +79,7 @@ public static (bool allTypesValid, IReadOnlyList runnable) GetTypesWithRun } } - foreach (var assembly in assemblies) - { - validRunnableTypes.AddRange(GenericBenchmarksBuilder.GetRunnableBenchmarks(assembly.GetRunnableBenchmarks())); - } + AddRunnable(assemblyTypes); return (true, validRunnableTypes); } diff --git a/src/BenchmarkDotNet/Validators/GenericBenchmarksValidator.cs b/src/BenchmarkDotNet/Validators/GenericBenchmarksValidator.cs index be0522c138..1c5b7eb0d9 100644 --- a/src/BenchmarkDotNet/Validators/GenericBenchmarksValidator.cs +++ b/src/BenchmarkDotNet/Validators/GenericBenchmarksValidator.cs @@ -16,8 +16,10 @@ public IAsyncEnumerable ValidateAsync(ValidationParameters vali .Distinct() .SelectMany(assembly => assembly.GetRunnableBenchmarks()) .SelectMany(GenericBenchmarksBuilder.BuildGenericsIfNeeded) - .Where(result => !result.isSuccess) - .Select(result => new ValidationError(false, $"Generic type {result.result.Name} failed to build due to wrong type argument or arguments count, ignoring.")) + // An unreadable type is reported here as well as by TypeFilter: BenchmarkRunner.Run() and both + // test adapters never go through TypeFilter, so for them this is the only report there is. + .Where(built => !built.IsSuccess) + .Select(built => new ValidationError(false, built.Error!)) .ToAsyncEnumerable(); } } \ No newline at end of file diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures.csproj b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures.csproj new file mode 100644 index 0000000000..610c70c194 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures.csproj @@ -0,0 +1,32 @@ + + + + net10.0 + Exe + BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures + BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures + BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures + + + true + + + + + + + + + + + + diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/BuildFailureProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/BuildFailureProbe.cs new file mode 100644 index 0000000000..ba1708bdb9 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/BuildFailureProbe.cs @@ -0,0 +1,56 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Environments; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Loggers; +using BenchmarkDotNet.Running; +using BenchmarkDotNet.Toolchains; +using BenchmarkDotNet.Toolchains.Parameters; +using BenchmarkDotNet.Toolchains.Results; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures +{ + /// + /// A benchmark whose build always fails, so that the adapter has to turn the build failure into a failed test. + /// The failure is faked by the toolchain rather than by uncompilable code, which keeps it quick and keeps the + /// error message the test asserts on under this file's control. + /// + [Config(typeof(FailingBuildConfig))] + public class BuildFailureProbe + { + internal const string ErrorMessage = "The build of this benchmark always fails, on purpose."; + + [Benchmark] + public int Add() => 1 + 1; + + private class FailingBuildConfig : ManualConfig + { + public FailingBuildConfig() + => AddJob(Job.Dry.WithToolchain(new FailingBuildToolchain())); + } + + private sealed class FailingBuildToolchain() + : Toolchain("FailingBuild", UnknownRuntime.Instance, new NoopGenerator(), new FailingBuilder(), new UnreachableExecutor()) + { + } + + private sealed class NoopGenerator : IGenerator + { + public ValueTask GenerateProjectAsync(BuildPartition buildPartition, ILogger logger, string rootArtifactsFolderPath, CancellationToken cancellationToken) + => new(GenerateResult.Success(ArtifactsPaths.Empty, [])); + } + + private sealed class FailingBuilder : IBuilder + { + public ValueTask BuildAsync(GenerateResult generateResult, BuildPartition buildPartition, ILogger logger, CancellationToken cancellationToken) + => new(BuildResult.Failure(generateResult, ErrorMessage)); + } + + private sealed class UnreachableExecutor : IExecutor + { + // A benchmark that failed to build is never executed. + public ValueTask ExecuteAsync(ExecuteParameters executeParameters, CancellationToken cancellationToken) + => throw new InvalidOperationException("The benchmark should never have been executed."); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/CollisionProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/CollisionProbe.cs new file mode 100644 index 0000000000..f40a600703 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/CollisionProbe.cs @@ -0,0 +1,36 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures +{ + /// + /// Two benchmark cases that BenchmarkDotNet identifies as one, because a benchmark is identified by the string + /// representation of its parameters and both values stringify the same way. The platform cannot tell the two apart + /// either, so the adapter is expected to report the collision instead of running them. + /// + [Config(typeof(FastConfig))] + public class CollisionProbe + { + public IEnumerable Values => [new Ambiguous(1), new Ambiguous(2)]; + + [ParamsSource(nameof(Values))] + public Ambiguous? Value { get; set; } + + [Benchmark] + public int Identity() => Value!.Number; + + public class Ambiguous(int number) + { + public int Number { get; } = number; + + public override string ToString() => "ambiguous"; + } + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/DescriptionCollisionProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/DescriptionCollisionProbe.cs new file mode 100644 index 0000000000..f79a582aa0 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/DescriptionCollisionProbe.cs @@ -0,0 +1,27 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures +{ + /// + /// Two benchmarks without any parameter that BenchmarkDotNet still identifies as one, because the description of + /// the first is the method name of the second and the identity of a benchmark is built from the name it displays. + /// The reported collision has to point at that rather than at the parameters. + /// + [Config(typeof(FastConfig))] + public class DescriptionCollisionProbe + { + [Benchmark(Description = "Twin")] + public int Described() => 1; + + [Benchmark] + public int Twin() => 2; + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/README.md b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/README.md new file mode 100644 index 0000000000..1905d04e12 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures/README.md @@ -0,0 +1,9 @@ +# BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures + +Every benchmark in this Microsoft.Testing.Platform application is expected to be reported as failed, so a plain +`dotnet test` over it fails by design. It exists for the paths of `BenchmarkDotNet.TestAdapter` that only a broken +benchmark reaches: the uid collision report and the mapping of a build failure onto failed tests. + +`TestingPlatformAdapterTests` in `BenchmarkDotNet.IntegrationTests` drives it, one probe at a time, and asserts on +what the platform reports. The benchmarks that are expected to pass live in +`BenchmarkDotNet.IntegrationTests.TestingPlatform` instead. diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/AbandonedBenchmarks.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/AbandonedBenchmarks.cs new file mode 100644 index 0000000000..ed14864ba3 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/AbandonedBenchmarks.cs @@ -0,0 +1,46 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals +{ + /// + /// Benchmarks whose parameter values are disposable, used to check what becomes of the values of a request that + /// is still in flight when the application ends. + /// + [Config(typeof(FastConfig))] + public class AbandonedBenchmarks + { + // Created once, so that re-reading the source cannot change the count. + private static readonly Abandoned[] Instances = [new Abandoned(1), new Abandoned(2)]; + + public IEnumerable Values => Instances; + + [ParamsSource(nameof(Values))] + public Abandoned? Value { get; set; } + + [Benchmark] + public int Identity() => Value!.Number; + + public class Abandoned : IDisposable + { + public Abandoned(int number) => Number = number; + + public static int Created => Instances.Length; + + public static int Disposed { get; private set; } + + public int Number { get; } + + public void Dispose() => Disposed++; + + public override string ToString() => $"abandoned-{Number}"; + } + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals.csproj b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals.csproj new file mode 100644 index 0000000000..e848a7ef77 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals.csproj @@ -0,0 +1,25 @@ + + + + net10.0 + Exe + BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals + BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals + BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals + + + false + + + true + + + + + + + + diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/FilteredBenchmarks.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/FilteredBenchmarks.cs new file mode 100644 index 0000000000..9850bcf8d5 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/FilteredBenchmarks.cs @@ -0,0 +1,30 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals +{ + /// + /// The benchmarks this application's requests enumerate. They are never executed - the run request under test is + /// refused before anything is handed to BenchmarkDotNet - so what they do does not matter, only that there are + /// several of them to be reported about. + /// + [Config(typeof(FastConfig))] + public class FilteredBenchmarks + { + [Params(1, 2)] + public int Size { get; set; } + + [Benchmark] + public int Add() => Size + Size; + + [Benchmark] + public int Multiply() => Size * Size; + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/PlatformStub.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/PlatformStub.cs new file mode 100644 index 0000000000..4232021bac --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/PlatformStub.cs @@ -0,0 +1,115 @@ +using Microsoft.Testing.Platform.Extensions.Messages; +using Microsoft.Testing.Platform.Extensions.OutputDevice; +using Microsoft.Testing.Platform.Messages; +using Microsoft.Testing.Platform.OutputDevice; +using Microsoft.Testing.Platform.Requests; +using Microsoft.Testing.Platform.TestHost; +using System.Reflection; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals +{ + /// + /// The parts of Microsoft.Testing.Platform that surround a test framework while it serves one request, recorded + /// rather than acted on. + /// +#pragma warning disable TPEXP // Completing a request is still marked as experimental by the platform. + internal sealed class PlatformStub : IMessageBus, IOutputDevice, IExecuteRequestCompletionNotifier +#pragma warning restore TPEXP + { + private readonly List messages = []; + + /// + /// Gets the node states the framework published, as "<state> <display name>" lines, and the + /// output device data it displayed, as "output <text>" lines. + /// + public IReadOnlyList Messages => messages; + + /// + /// Gets whether the framework completed the request. + /// + public bool IsComplete { get; private set; } + + public Task PublishAsync(IDataProducer dataProducer, IData data) + { + if (data is TestNodeUpdateMessage update) + { + var state = update.TestNode.Properties.SingleOrDefault(); + + lock (messages) + messages.Add($"{StateName(state)} {update.TestNode.DisplayName}"); + } + + return Task.CompletedTask; + } + + public Task DisplayAsync(IOutputDeviceDataProducer producer, IOutputDeviceData data, CancellationToken cancellationToken) + { + var text = data switch + { + WarningMessageOutputDeviceData warning => warning.Message, + ErrorMessageOutputDeviceData error => error.Message, + TextOutputDeviceData plain => plain.Text, + _ => data.ToString() ?? string.Empty + }; + + lock (messages) + messages.Add($"output {text}"); + + return Task.CompletedTask; + } + + public void Complete() => IsComplete = true; + + /// + /// Builds the service provider the framework resolves the output device from. + /// + /// + /// GetOutputDevice casts to the platform's own provider rather than asking any + /// for the service, so this has to be that type - which the platform keeps to + /// itself - rather than something implemented here. + /// + /// The output device to resolve. + /// The created service provider. + public static IServiceProvider CreateServiceProvider(IOutputDevice outputDevice) + { + var type = typeof(IOutputDevice).Assembly.GetType("Microsoft.Testing.Platform.Services.ServiceProvider", throwOnError: true)!; + var provider = (IServiceProvider)Activator.CreateInstance(type)!; + var addService = type.GetMethod("AddService", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic) + ?? throw new InvalidOperationException("The platform's service provider no longer has an AddService method."); + + addService.Invoke(provider, [outputDevice, false]); + + return provider; + } + + /// + /// Creates the session a request belongs to. The platform keeps the constructor to itself, and there is no + /// other way to build one of these from outside it. + /// + /// The created session context. + public static TestSessionContext CreateSessionContext() + { + var constructor = typeof(TestSessionContext).GetConstructor( + BindingFlags.Instance | BindingFlags.NonPublic, + binder: null, + [typeof(SessionUid)], + modifiers: null) + ?? throw new InvalidOperationException( + $"{nameof(TestSessionContext)} no longer has a constructor taking a {nameof(SessionUid)}."); + + return (TestSessionContext)constructor.Invoke([new SessionUid(Guid.NewGuid().ToString())]); + } + + private static string StateName(TestNodeStateProperty? state) => state switch + { + null => "none", + DiscoveredTestNodeStateProperty => "discovered", + InProgressTestNodeStateProperty => "in-progress", + PassedTestNodeStateProperty => "passed", + FailedTestNodeStateProperty failed => $"failed({failed.Explanation})", + ErrorTestNodeStateProperty error => $"error({error.Explanation})", + SkippedTestNodeStateProperty => "skipped", + _ => state.GetType().Name + }; + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/Program.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/Program.cs new file mode 100644 index 0000000000..3328433481 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/Program.cs @@ -0,0 +1,100 @@ +using BenchmarkDotNet.Running; +using BenchmarkDotNet.TestAdapter.TestingPlatform; +using Microsoft.Testing.Platform.Capabilities.TestFramework; +using Microsoft.Testing.Platform.Extensions.TestFramework; +using Microsoft.Testing.Platform.Requests; +using System.Reflection; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals +{ + /// + /// Drives the adapter's Microsoft.Testing.Platform types the way a real test host cannot, and prints what they + /// did. See the README next to this file. + /// + internal static class Program + { + private static async Task Main() + { + // First, so that the values it counts have not already been disposed by the requests below - they are + // benchmarks of this assembly too, so those enumerate and dispose them as well. + Console.WriteLine("== abandoned"); + Console.WriteLine(await ReportAbandonedRequestAsync().ConfigureAwait(false)); + + Console.WriteLine("== discover"); + foreach (var line in await ExecuteAsync(session => new DiscoverTestExecutionRequest(session, new UnrecognisedFilter())).ConfigureAwait(false)) + Console.WriteLine(line); + + Console.WriteLine("== run"); + foreach (var line in await ExecuteAsync(session => new RunTestExecutionRequest(session, new UnrecognisedFilter())).ConfigureAwait(false)) + Console.WriteLine(line); + + Console.WriteLine("== done"); + + return 0; + } + + /// + /// Ends an application while a request is still in flight, which is what the client sending `exit` or an IDE + /// cancelling looks like from in here, and reports what became of the values that request had enumerated. + /// + /// + /// Nothing else holds them: the request hands its values over by completing, and this one never does. Left + /// undisposed they reach the finalizer, which is the dotnet/BenchmarkDotNet#1383 hang. + /// + /// The counts, as a line. + private static async Task ReportAbandonedRequestAsync() + { + var lifetime = new ParameterValueLifetime(); + var request = lifetime.BeginRequest(); + + request.Track(BenchmarkConverter.TypeToBenchmarks(typeof(AbandonedBenchmarks)).BenchmarksCases); + + // Deliberately no CompleteAsync: that is the point of this one. + await lifetime.AfterRunAsync(0, CancellationToken.None).ConfigureAwait(false); + + return $"created={AbandonedBenchmarks.Abandoned.Created} disposed={AbandonedBenchmarks.Abandoned.Disposed}"; + } + + private static async Task> ExecuteAsync(Func createRequest) + { + var platform = new PlatformStub(); + var lifetime = new ParameterValueLifetime(); + var framework = new BenchmarkTestFramework( + new TestFrameworkCapabilities(), + PlatformStub.CreateServiceProvider(platform), + Assembly.GetExecutingAssembly(), + lifetime); + + var request = createRequest(PlatformStub.CreateSessionContext()); +#pragma warning disable TPEXP // Building a request context is still marked as experimental by the platform. + var context = new ExecuteRequestContext(request, platform, platform, CancellationToken.None); +#pragma warning restore TPEXP + + // The framework is expected to deal with the filter rather than throw at the caller, which is the whole + // point: an exception here escapes after the request has already been completed, so the platform reports + // a finished request that said nothing about any benchmark. + try + { + await framework.ExecuteRequestAsync(context).ConfigureAwait(false); + } + catch (Exception exception) + { + return [.. platform.Messages, $"threw {exception.GetType().Name}: {exception.Message}"]; + } + finally + { + await lifetime.AfterRunAsync(0, CancellationToken.None).ConfigureAwait(false); + } + + return [.. platform.Messages, $"complete {platform.IsComplete}"]; + } + + /// + /// A filter of a kind the adapter has never been told about, which is what a newer Microsoft.Testing.Platform + /// or an extension registering its own filter factory would hand it. + /// + private sealed class UnrecognisedFilter : ITestExecutionFilter + { + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/README.md b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/README.md new file mode 100644 index 0000000000..90bc6cdec1 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals/README.md @@ -0,0 +1,26 @@ +# BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals + +Drives the adapter's Microsoft.Testing.Platform types directly, to reach what a real test host cannot. + +The first of those is `BenchmarkTestFramework` with a test execution filter that `BenchmarkDotNet.TestAdapter` does not recognise, +which is the one thing the other probe applications cannot do: Microsoft.Testing.Platform 2.3.3 ships `NopFilter`, +`TestNodeUidListFilter` and `TreeNodeFilter`, the adapter handles all three, and `ITestExecutionFilterFactory` — the +extension point a consumer would register another one through — is internal to the platform. So the branch that +handles an unknown filter is unreachable from a real test host, and a regression in it would be silent. + +`ITestExecutionFilter` itself is public, so this application implements one, hands it to the framework through a +discovery request and a run request, and prints what the framework did with each. It is not a Microsoft.Testing.Platform +application: it has its own entry point, and stands in for the platform around the framework. + +The two requests are deliberately treated differently, which is what this pins: + +* **discovery** lists every benchmark and warns on the output device — a wrong list costs nothing to correct; +* **a run** refuses, reporting every benchmark it could have selected as failed — running the whole assembly instead + of the subset that was asked for would cost the machine the next hour. + +It also ends an application while a request is still in flight - the client sending `exit`, or an IDE cancelling - +which no driven test host can be made to do on cue either. The values that request had enumerated are reachable from +nothing else, so `ParameterValueLifetime` has to dispose them on its way out; left to the finalizer they are the +dotnet/BenchmarkDotNet#1383 hang. + +`TestingPlatformAdapterTests` in `BenchmarkDotNet.IntegrationTests` runs it and asserts on the report. diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized.csproj b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized.csproj new file mode 100644 index 0000000000..fcd02ada6e --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized.csproj @@ -0,0 +1,33 @@ + + + + net10.0 + Exe + BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized + BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized + BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized + + + false + + + + + + + + + + + + diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/DroppedProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/DroppedProbe.cs new file mode 100644 index 0000000000..55a40e47b3 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/DroppedProbe.cs @@ -0,0 +1,33 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized +{ + /// + /// A benchmark that only runs out of process, so an unoptimized assembly hides every one of its cases and nothing + /// downstream can reach the parameter values they own. This is the leak: they are disposed by nobody unless the + /// enumeration disposes them itself. + /// + [Config(typeof(OutOfProcessConfig))] + public class DroppedProbe + { + // Created once, so that re-reading the source cannot change the count. + private static readonly TrackedValue[] Instances = [new TrackedValue("dropped-1"), new TrackedValue("dropped-2")]; + + public IEnumerable Values => Instances; + + [ParamsSource(nameof(Values))] + public TrackedValue? Value { get; set; } + + [Benchmark] + public int Length() => Value!.Name.Length; + + private class OutOfProcessConfig : ManualConfig + { + // A dry job on the default toolchain, which generates, builds and runs a separate executable - except + // that it is never reached here, because the enumeration hides it before anything is built. + public OutOfProcessConfig() => AddJob(Job.Dry.WithId("OutOfProcess")); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/README.md b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/README.md new file mode 100644 index 0000000000..27127548b5 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/README.md @@ -0,0 +1,12 @@ +# BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized + +A Microsoft.Testing.Platform application built with `false`, which is the one thing that makes it +different from `BenchmarkDotNet.IntegrationTests.TestingPlatform`. + +`BenchmarkEnumerator` hides the benchmarks that would run out of process when the assembly is not optimized, so that +they can be debugged from a test runner. Those benchmarks are enumerated first and BenchmarkDotNet never sees them +afterwards, so the parameter values they own are the adapter's to dispose - a value with a locking finalizer hangs the +runtime otherwise, see dotnet/BenchmarkDotNet#1383. Every other project in this repository is optimized, deliberately, +which leaves that path unreachable in a Release CI run. + +`TestingPlatformAdapterTests` in `BenchmarkDotNet.IntegrationTests` drives it. diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/SharedValueProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/SharedValueProbe.cs new file mode 100644 index 0000000000..5901939dfc --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/SharedValueProbe.cs @@ -0,0 +1,38 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized +{ + /// + /// A benchmark that runs under an in-process and an out-of-process job over the same parameter values. An + /// unoptimized assembly hides the out-of-process cases only, and BenchmarkConverter hands the very same + /// ParameterInstance to both jobs, so disposing what is hidden case by case would take down values the surviving + /// benchmarks still own. + /// + [Config(typeof(BothToolchainsConfig))] + public class SharedValueProbe + { + // Created once, so that re-reading the source cannot change the count. + private static readonly TrackedValue[] Instances = [new TrackedValue("shared-1"), new TrackedValue("shared-2")]; + + public IEnumerable Values => Instances; + + [ParamsSource(nameof(Values))] + public TrackedValue? Value { get; set; } + + [Benchmark] + public int Length() => Value!.Name.Length; + + private class BothToolchainsConfig : ManualConfig + { + // The ids are set explicitly, so that the two jobs stay distinguishable by name. + public BothToolchainsConfig() + { + AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default).WithId("InProcess")); + AddJob(Job.Dry.WithId("OutOfProcess")); + } + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/TrackedValue.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/TrackedValue.cs new file mode 100644 index 0000000000..eefb07a368 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized/TrackedValue.cs @@ -0,0 +1,34 @@ +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized +{ + /// + /// A disposable parameter value that keeps count of how many of its kind were created and disposed. What is left + /// undisposed only shows at the end, so the counts are written out when the process exits. + /// + public sealed class TrackedValue : IDisposable + { + /// + /// The name of the file the counts are written to, next to the probe application. + /// + public const string ReportFileName = "unoptimized-probe.txt"; + + private static int created; + private static int disposed; + + static TrackedValue() => + AppDomain.CurrentDomain.ProcessExit += (_, _) => File.WriteAllText( + Path.Combine(AppContext.BaseDirectory, ReportFileName), + $"created={Volatile.Read(ref created)} disposed={Volatile.Read(ref disposed)}"); + + public TrackedValue(string name) + { + Name = name; + Interlocked.Increment(ref created); + } + + public string Name { get; } + + public void Dispose() => Interlocked.Increment(ref disposed); + + public override string ToString() => Name; + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/AsyncDisposableProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/AsyncDisposableProbe.cs new file mode 100644 index 0000000000..ddb8bf03e4 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/AsyncDisposableProbe.cs @@ -0,0 +1,69 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A benchmark whose parameter values come from an async source and are only . They + /// are disposed by exactly the same rules as a synchronously disposable one, and casting a value to + /// would drop them on the floor instead - which the finalizer of a value holding a lock + /// then turns into the hang of dotnet/BenchmarkDotNet#1383. + /// + [Config(typeof(FastConfig))] + public class AsyncDisposableProbe + { + /// + /// The name of the file the counts are written to, next to the probe application. + /// + public const string ReportFileName = "async-disposable-probe.txt"; + + // Created once, so that re-reading the source cannot change the count. + private static readonly AsyncTracked[] Instances = [new AsyncTracked(1), new AsyncTracked(2)]; + + public static async IAsyncEnumerable GetValues() + { + await Task.Yield(); + + foreach (var instance in Instances) + yield return instance; + } + + [ParamsSource(nameof(GetValues))] + public AsyncTracked? Value { get; set; } + + [Benchmark] + public int Identity() => Value!.Number; + + /// + /// Deliberately not : the whole point of the probe. + /// + public class AsyncTracked : IAsyncDisposable + { + private static int disposed; + + static AsyncTracked() => + AppDomain.CurrentDomain.ProcessExit += (_, _) => File.WriteAllText( + Path.Combine(AppContext.BaseDirectory, ReportFileName), + $"created={Instances.Length} disposed={Volatile.Read(ref disposed)}"); + + public AsyncTracked(int number) => Number = number; + + public int Number { get; } + + public ValueTask DisposeAsync() + { + Interlocked.Increment(ref disposed); + return default; + } + + public override string ToString() => $"async-{Number}"; + } + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/BenchmarkDotNet.IntegrationTests.TestingPlatform.csproj b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/BenchmarkDotNet.IntegrationTests.TestingPlatform.csproj new file mode 100644 index 0000000000..cdbdbc7edc --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/BenchmarkDotNet.IntegrationTests.TestingPlatform.csproj @@ -0,0 +1,32 @@ + + + + net10.0 + Exe + BenchmarkDotNet.IntegrationTests.TestingPlatform + BenchmarkDotNet.IntegrationTests.TestingPlatform + BenchmarkDotNet.IntegrationTests.TestingPlatform + + + true + + + + + + + + + + + + diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/BracketProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/BracketProbe.cs new file mode 100644 index 0000000000..dac2397d2d --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/BracketProbe.cs @@ -0,0 +1,27 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A benchmark whose parameter contains the characters a --treenode-filter uses to delimit a property + /// filter. The leaf of every path ends in the job between those same characters, so a filter has to be able to + /// spell them out rather than have them parsed. + /// + [Config(typeof(FastConfig))] + public class BracketProbe + { + [Params("[Dry]")] + public string Value { get; set; } = ""; + + [Benchmark] + public int Length() => Value.Length; + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/CategoryProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/CategoryProbe.cs new file mode 100644 index 0000000000..44c4e3bd80 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/CategoryProbe.cs @@ -0,0 +1,37 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Running; +using BenchmarkDotNet.Toolchains.InProcess.Emit; +using System.Reflection; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A benchmark whose category comes from a custom rather than from a + /// [BenchmarkCategory]. The adapter has to publish the categories the config resolved: rediscovering them with the + /// default discoverer would leave a --treenode-filter on a custom category matching nothing, even though + /// BenchmarkDotNet's own --anyCategories and the summary do see it. + /// + [Config(typeof(DiscoveredCategoryConfig))] + public class CategoryProbe + { + [Benchmark] + public int Identity() => 1; + + private class CategoryFromMethodName : ICategoryDiscoverer + { + // The default discoverer only reads [BenchmarkCategory], so this category exists nowhere else. + public string[] GetCategories(MethodInfo method) => [$"Discovered{method.Name}"]; + } + + private class DiscoveredCategoryConfig : ManualConfig + { + public DiscoveredCategoryConfig() + { + AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + WithCategoryDiscoverer(new CategoryFromMethodName()); + } + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/DescribedProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/DescribedProbe.cs new file mode 100644 index 0000000000..72922bc2a7 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/DescribedProbe.cs @@ -0,0 +1,28 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// Benchmarks whose display name comes from the description rather than the method name. + /// + [Config(typeof(FastConfig))] + public class DescribedProbe + { + [Params(1)] + public int Size { get; set; } + + [Benchmark(Description = "A described benchmark")] + public int Described() => Size; + + [Benchmark] + public int Undescribed() => Size; + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/DisposableProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/DisposableProbe.cs new file mode 100644 index 0000000000..7749a1c0c3 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/DisposableProbe.cs @@ -0,0 +1,68 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A benchmark whose parameter values are disposable. Enumerating the assembly creates every one of them, whether + /// the case it belongs to is going to run or not, and BenchmarkDotNet only disposes the ones it was handed - a + /// parameter with a locking finalizer hangs the runtime otherwise, see dotnet/BenchmarkDotNet#1383. What is left + /// undisposed only shows at the end, so the counts are written out when the process exits. + /// + [Config(typeof(FastConfig))] + public class DisposableProbe + { + /// + /// The name of the file the counts are written to, next to the probe application. + /// + public const string ReportFileName = "disposable-probe.txt"; + + // Created once, so that re-reading the source cannot change the count. + private static readonly Tracked[] Instances = [new Tracked(1), new Tracked(2), new Tracked(3)]; + + public IEnumerable Values => Instances; + + [ParamsSource(nameof(Values))] + public Tracked? Value { get; set; } + + [Benchmark] + public int Identity() => Value!.Number; + + public class Tracked : IDisposable + { + private static int disposed; + + private readonly int number; + + private bool isDisposed; + + static Tracked() => + AppDomain.CurrentDomain.ProcessExit += (_, _) => File.WriteAllText( + Path.Combine(AppContext.BaseDirectory, ReportFileName), + $"created={Instances.Length} disposed={Volatile.Read(ref disposed)}"); + + public Tracked(int number) => this.number = number; + + /// + /// Reading this after the value was disposed is the failure a run that executes against the values a + /// discovery already disposed would otherwise get away with, so it is made loud rather than counted. + /// + public int Number => isDisposed ? throw new ObjectDisposedException(ToString()) : number; + + public void Dispose() + { + isDisposed = true; + Interlocked.Increment(ref disposed); + } + + public override string ToString() => $"tracked-{number}"; + } + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/FreshValueProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/FreshValueProbe.cs new file mode 100644 index 0000000000..be82e6d48a --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/FreshValueProbe.cs @@ -0,0 +1,76 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A benchmark whose parameter source constructs its values on every read - the common shape of a source over + /// resources, yield return new FileStream(...) - as opposed to , which hands + /// back the same instances every time. Under server mode every request reads the source again, and nothing a + /// later request could reuse comes of it, so the values of one request must not be held for the whole session. + /// + /// + /// The report is a line per read, written as the read happens, so that a test driving several requests from one + /// process can see what had been disposed by the time each request enumerated - and a last line at exit. + /// + [Config(typeof(FastConfig))] + public class FreshValueProbe + { + /// + /// The name of the file the counts are written to, next to the probe application. + /// + public const string ReportFileName = "fresh-value-probe.txt"; + + private static int reads; + private static int created; + private static int disposed; + + static FreshValueProbe() => + AppDomain.CurrentDomain.ProcessExit += (_, _) => Report($"exit created={Volatile.Read(ref created)} disposed={Volatile.Read(ref disposed)}"); + + public static IEnumerable Values + { + get + { + var read = Interlocked.Increment(ref reads); + Fresh[] values = [new Fresh(1), new Fresh(2)]; + + Report($"read={read} created={Volatile.Read(ref created)} disposed={Volatile.Read(ref disposed)}"); + + return values; + } + } + + [ParamsSource(nameof(Values))] + public Fresh? Value { get; set; } + + [Benchmark] + public int Identity() => Value!.Number; + + private static void Report(string line) + => File.AppendAllText(Path.Combine(AppContext.BaseDirectory, ReportFileName), line + Environment.NewLine); + + public class Fresh : IDisposable + { + public Fresh(int number) + { + Number = number; + Interlocked.Increment(ref created); + } + + public int Number { get; } + + public void Dispose() => Interlocked.Increment(ref disposed); + + // The same name on every read, so that a benchmark keeps its identity across the requests of a session. + public override string ToString() => $"fresh-{Number}"; + } + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/GenericProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/GenericProbe.cs new file mode 100644 index 0000000000..24ffa27b9c --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/GenericProbe.cs @@ -0,0 +1,25 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A generic benchmark, used to check how closed generic types are named and grouped by test runners. + /// + [Config(typeof(GenericProbeConfig))] + [GenericTypeArguments(typeof(int))] + [GenericTypeArguments(typeof(char))] + [GenericTypeArguments(typeof(System.Collections.Generic.List))] + public class GenericProbe where T : new() + { + [Benchmark] + public T Create() => new T(); + } + + internal class GenericProbeConfig : ManualConfig + { + public GenericProbeConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/InvalidConfigProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/InvalidConfigProbe.cs new file mode 100644 index 0000000000..3bd8daccff --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/InvalidConfigProbe.cs @@ -0,0 +1,39 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// Benchmarks whose [Config] cannot be constructed. Reflection builds every attribute of a type in order to hand + /// any of them back, so reading the attributes of these types throws, and it throws while the list of types is + /// being built - before any benchmark of the assembly has been converted. They have to be dropped rather than + /// take the discovery of every other benchmark down with them. + /// + public static class InvalidConfigProbe + { + /// + /// ConfigAttribute instantiates the type it is given, and an abstract one cannot be instantiated. + /// + [Config(typeof(DebugConfig))] + public class WithAbstractConfig + { + [Benchmark] + public int Identity() => 1; + } + + /// + /// Same read, a different reason: the config has no public parameterless constructor. + /// + [Config(typeof(NoPublicConstructorConfig))] + public class WithInaccessibleConfig + { + [Benchmark] + public int Identity() => 1; + + private class NoPublicConstructorConfig : ManualConfig + { + private NoPublicConstructorConfig() { } + } + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/NestedProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/NestedProbe.cs new file mode 100644 index 0000000000..d1e372bb90 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/NestedProbe.cs @@ -0,0 +1,26 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A benchmark declared inside another type. ECMA-335 qualifies a nested type by its declaring types rather than + /// by its namespace, which the identity a test runner reads has to follow. + /// + public static class NestedProbe + { + [Config(typeof(FastConfig))] + public class Inner + { + [Benchmark] + public int Identity() => 1; + } + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/OutOfProcessProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/OutOfProcessProbe.cs new file mode 100644 index 0000000000..cba4a1748b --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/OutOfProcessProbe.cs @@ -0,0 +1,25 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A benchmark that runs out of process, on the default toolchain. The other probes stay in process to keep the run + /// fast, which skips the generate/build/execute cycle entirely, so this one is what makes the adapter see a real + /// . + /// + [Config(typeof(OutOfProcessConfig))] + public class OutOfProcessProbe + { + [Benchmark] + public int Add() => 1 + 1; + + private class OutOfProcessConfig : ManualConfig + { + // A dry job on the default toolchain: one iteration, but a separate executable is still generated, built + // and run. + public OutOfProcessConfig() => AddJob(Job.Dry); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/SampleBenchmarks.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/SampleBenchmarks.cs new file mode 100644 index 0000000000..27bce02ef5 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/SampleBenchmarks.cs @@ -0,0 +1,31 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// Benchmarks used to exercise the Microsoft.Testing.Platform adapter end to end. They run in-process with a + /// single iteration so that a full run stays fast. + /// + [Config(typeof(FastConfig))] + public class SampleBenchmarks + { + [Params(1, 2)] + public int Size { get; set; } + + [Benchmark] + [BenchmarkCategory("Fast")] + public int Add() => Size + Size; + + [Benchmark] + [BenchmarkCategory("Slow")] + public int Multiply() => Size * Size; + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/SeparatorProbe.cs b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/SeparatorProbe.cs new file mode 100644 index 0000000000..ab01cae901 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/SeparatorProbe.cs @@ -0,0 +1,26 @@ +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Toolchains.InProcess.Emit; + +namespace BenchmarkDotNet.IntegrationTests.TestingPlatform +{ + /// + /// A benchmark whose parameter contains the character Microsoft.Testing.Platform uses to separate the levels of the + /// tree a --treenode-filter walks. It has to stay at the same level of that tree as every other benchmark. + /// + [Config(typeof(FastConfig))] + public class SeparatorProbe + { + [Params("a/b")] + public string Value { get; set; } = ""; + + [Benchmark] + public int Length() => Value.Length; + + private class FastConfig : ManualConfig + { + public FastConfig() => AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.Default)); + } + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/global.json b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/global.json new file mode 100644 index 0000000000..3140116df3 --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests.TestingPlatform/global.json @@ -0,0 +1,5 @@ +{ + "test": { + "runner": "Microsoft.Testing.Platform" + } +} diff --git a/tests/BenchmarkDotNet.IntegrationTests/BenchmarkDotNet.IntegrationTests.csproj b/tests/BenchmarkDotNet.IntegrationTests/BenchmarkDotNet.IntegrationTests.csproj index 9fde386d87..bf5a148547 100644 --- a/tests/BenchmarkDotNet.IntegrationTests/BenchmarkDotNet.IntegrationTests.csproj +++ b/tests/BenchmarkDotNet.IntegrationTests/BenchmarkDotNet.IntegrationTests.csproj @@ -40,6 +40,17 @@ + + + + + + + diff --git a/tests/BenchmarkDotNet.IntegrationTests/TestingPlatformAdapterTests.cs b/tests/BenchmarkDotNet.IntegrationTests/TestingPlatformAdapterTests.cs new file mode 100644 index 0000000000..3caecaaf2a --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests/TestingPlatformAdapterTests.cs @@ -0,0 +1,645 @@ +#if NETCOREAPP +using BenchmarkDotNet.Detectors; +using System.Diagnostics; +using System.Text; +using System.Text.Json; +using System.Text.RegularExpressions; + +namespace BenchmarkDotNet.IntegrationTests +{ + /// + /// Drives the two Microsoft.Testing.Platform probe applications through their command line and asserts on what + /// BenchmarkDotNet.TestAdapter reports back. Everything here goes through a separate process on purpose: the + /// adapter's job is to keep a benchmark identifiable and addressable from the outside, and discovery and execution + /// are two different processes when a test runner drives it. + /// + public class TestingPlatformAdapterTests(ITestOutputHelper output) + { + private const string PassingProbes = "BenchmarkDotNet.IntegrationTests.TestingPlatform"; + private const string FailingProbes = "BenchmarkDotNet.IntegrationTests.TestingPlatform.Failures"; + private const string UnoptimizedProbes = "BenchmarkDotNet.IntegrationTests.TestingPlatform.Unoptimized"; + private const string InternalsProbe = "BenchmarkDotNet.IntegrationTests.TestingPlatform.Internals"; + + // Every probe project is single targeted, see their .csproj files. + private const string ProbeTargetFramework = "net10.0"; + + // A run that has to build a benchmark pays for a restore and a build of the generated project. + private static readonly TimeSpan Timeout = TimeSpan.FromMinutes(10); + + [Fact] + public void EveryBenchmarkIsDiscoveredUnderItsOwnName() + { + string[] expected = + [ + // The values of this one are IAsyncDisposable and not IDisposable, which is what an async + // [ParamsSource] can produce since #3248. + "AsyncDisposableProbe.Identity(Value: async-1)", + "AsyncDisposableProbe.Identity(Value: async-2)", + + "BracketProbe.Length(Value: \"[Dry]\")", + "CategoryProbe.Identity", + + // The description of a [Benchmark(Description = ...)] is what a user recognises it by, so it is + // used instead of the method name, and spelled the way it was written. Descriptor quotes a + // description containing a space so that BenchmarkDotNet's own --filter can delimit it, which an IDE + // label has no use for. Without a description the method name is used, and the parameters are + // appended to both. + "DescribedProbe.A described benchmark(Size: 1)", + "DescribedProbe.Undescribed(Size: 1)", + + "DisposableProbe.Identity(Value: tracked-1)", + "DisposableProbe.Identity(Value: tracked-2)", + "DisposableProbe.Identity(Value: tracked-3)", + + "FreshValueProbe.Identity(Value: fresh-1)", + "FreshValueProbe.Identity(Value: fresh-2)", + + // A generic benchmark is named after the type arguments it was closed over. + "GenericProbe.Create", + "GenericProbe>.Create", + "GenericProbe.Create", + + "NestedProbe.Inner.Identity", + "OutOfProcessProbe.Add", + "SampleBenchmarks.Add(Size: 1)", + "SampleBenchmarks.Add(Size: 2)", + "SampleBenchmarks.Multiply(Size: 1)", + "SampleBenchmarks.Multiply(Size: 2)", + "SeparatorProbe.Length(Value: \"a/b\")", + ]; + + var discovered = Discover(PassingProbes); + + // InvalidConfigProbe's benchmarks are deliberately absent: their [Config] cannot be constructed, and a + // type whose attributes cannot be read is dropped rather than allowed to abort the whole discovery. + Assert.Equal( + expected, + discovered.Select(test => test.DisplayName.Substring(PassingProbes.Length + 1)).OrderBy(name => name, StringComparer.Ordinal)); + + // The platform identifies a node by its uid, so two benchmarks sharing one cannot be told apart. + Assert.Equal(discovered.Count, discovered.Select(test => test.Uid).Distinct().Count()); + } + + [Fact] + public void TheTypeOfABenchmarkIsIdentifiedByItsEcmaName() + { + // Microsoft.Testing.Platform documents TestMethodIdentifierProperty as ECMA-335, which is the form a + // test runner - Visual Studio's Test Explorer above all - matches a type by. A generic type is named + // after its arity there, and its arguments are no part of it. + var generic = Discover(PassingProbes, "--treenode-filter", "/*/*/GenericProbe*/*"); + + Assert.Equal(3, generic.Count); + Assert.All(generic, test => Assert.Equal("GenericProbe`1", test.TypeName)); + + // The arguments are still what tells one closed generic from another, in the name the user reads. + Assert.Equal(3, generic.Select(test => test.DisplayName).Distinct(StringComparer.Ordinal).Count()); + + // A nested type is qualified by its declaring types rather than by its namespace, which the property + // carries separately. + var nested = Discover(PassingProbes, "--treenode-filter", "/*/*/NestedProbe*/*"); + + Assert.Equal("NestedProbe+Inner", Assert.Single(nested).TypeName); + } + + [Fact] + public void TheUidOfABenchmarkIsTheSameInEveryProcess() + { + var first = Discover(PassingProbes).ToDictionary(test => test.Uid, test => test.DisplayName); + var second = Discover(PassingProbes).ToDictionary(test => test.Uid, test => test.DisplayName); + + Assert.Equal(first, second); + } + + [Fact] + public void ABenchmarkCanBeRunByTheUidItWasDiscoveredWith() + { + // This is the contract a test runner relies on: it discovers in one process and asks for a uid in another. + var uid = Discover(PassingProbes) + .Single(test => test.DisplayName.EndsWith("SampleBenchmarks.Add(Size: 2)", StringComparison.Ordinal)) + .Uid; + + var summary = RunAndSummarize(PassingProbes, "--filter-uid", uid); + + Assert.Equal(1, summary.Total); + Assert.Equal(1, summary.Succeeded); + Assert.Equal(0, summary.Failed); + } + + [Fact] + public void ATreeNodeFilterMatchesTheCategoriesOfABenchmark() + { + // The categories are published as filterable properties, which is what makes this expression work. + var discovered = Discover(PassingProbes, "--treenode-filter", "/*/*/*/*[Category=Fast]"); + + Assert.Equal( + new[] { "SampleBenchmarks.Add(Size: 1)", "SampleBenchmarks.Add(Size: 2)" }, + discovered.Select(test => test.DisplayName.Substring(PassingProbes.Length + 1)).OrderBy(name => name, StringComparer.Ordinal)); + } + + [Fact] + public void ATreeNodeFilterMatchesTheCategoriesOfACustomCategoryDiscoverer() + { + // The node has to carry the categories the config resolved, not the ones the default discoverer finds: + // this category is produced by an ICategoryDiscoverer and exists on no [BenchmarkCategory] anywhere. + var discovered = Discover(PassingProbes, "--treenode-filter", "/*/*/*/*[Category=DiscoveredIdentity]"); + + Assert.Equal( + new[] { "CategoryProbe.Identity" }, + discovered.Select(test => test.DisplayName.Substring(PassingProbes.Length + 1))); + } + + [Fact] + public void ParameterValuesAreDisposedWhenBenchmarksAreOnlyListed() + { + // Listing runs nothing, so BenchmarkDotNet disposes nothing: every value the enumeration created is the + // adapter's to dispose. + Assert.Equal( + "created=3 disposed=3", + ReadDisposalReport(PassingProbes, "disposable-probe.txt", () => Discover(PassingProbes))); + } + + [Fact] + public void ParameterValuesThatAreOnlyAsyncDisposableAreDisposedToo() + { + // An async [ParamsSource] can hand back a value that implements IAsyncDisposable and not IDisposable. + // Disposal goes through ParameterInstance, which knows both, rather than casting the value to + // IDisposable - which would drop these on the floor and leave them to the finalizer. + Assert.Equal( + "created=2 disposed=2", + ReadDisposalReport(PassingProbes, "async-disposable-probe.txt", () => Discover(PassingProbes))); + } + + [Fact] + public void ParameterValuesSurviveADiscoveryWhenTheSameProcessRunsThemAfterwards() + { + // Server mode is how Visual Studio and the Visual Studio Code Test Explorer drive the platform: one + // process serves the discovery and then the runs. Every request enumerates the assembly again, and a + // source backed by a cached collection hands back the very same values, so disposing them when the + // discovery request ends would leave the run executing against disposed objects - which DisposableProbe + // turns into an ObjectDisposedException rather than letting it pass unnoticed. + IReadOnlyList discovered = []; + IReadOnlyList ran = []; + + var report = ReadDisposalReport( + PassingProbes, + "disposable-probe.txt", + () => (discovered, ran) = TestingPlatformServerModeSession.DiscoverThenRun( + GetProbeApplication(PassingProbes), + "DisposableProbe.Identity(Value: tracked", + Timeout)); + + Assert.NotEmpty(discovered); + Assert.Equal(3, ran.Count); + Assert.All(ran, node => Assert.Equal("passed", node.ExecutionState)); + + // Once each: BenchmarkDotNet disposes what it ran, and the adapter must not have done so beforehand. + Assert.Equal("created=3 disposed=3", report); + } + + [Fact] + public void ParameterValuesOfBenchmarksNoRequestRanAreDisposedWhenTheApplicationEnds() + { + // The mirror image of the test above: the values of every benchmark that neither request ran are the + // adapter's to dispose, and holding them for the application rather than for the request must not turn + // into either a leak or a second disposal. + var report = ReadDisposalReport( + PassingProbes, + "async-disposable-probe.txt", + () => TestingPlatformServerModeSession.DiscoverThenRun( + GetProbeApplication(PassingProbes), + "DisposableProbe.Identity(Value: tracked", + Timeout)); + + Assert.Equal("created=2 disposed=2", report); + } + + [Fact] + public void ParameterValuesOfASourceThatConstructsPerReadAreDisposedAsRequestsGoBy() + { + // FreshValueProbe's source hands back new values on every read, so nothing one request enumerated can + // ever be reused by the next. Holding them for the whole session would turn a long Test Explorer session + // into the handle leak the disposal exists to prevent: they have to go as soon as the following request + // shows they did not come back. The probe reports what had been disposed by the time each read happened. + var report = ReadDisposalReport( + PassingProbes, + "fresh-value-probe.txt", + () => TestingPlatformServerModeSession.DiscoverThenRun( + GetProbeApplication(PassingProbes), + "DisposableProbe.Identity(Value: tracked", + Timeout, + discoverAgain: true)); + + Assert.Equal( + [ + "read=1 created=2 disposed=0", + + // The values of the first request are only known to be unreusable once this read did not hand + // them back, which is after it. + "read=2 created=4 disposed=0", + + // By the third request they are gone, and so on: the session holds one request's worth. + "read=3 created=6 disposed=2", + "exit created=6 disposed=6", + ], + report.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries)); + } + + [Fact] + public void AnAssemblyWideValidationWarningIsReportedOncePerNode() + { + // GenericBenchmarksValidator looks at the whole assembly, however BenchmarkDotNet runs the validators once + // per benchmark type, so an unreadable type is reported again for every type that runs. An error that + // names no benchmark case is put on every node, so without deduplication N types leave N copies of the + // same warning on each of the N types' nodes. + var (_, ran) = TestingPlatformServerModeSession.DiscoverThenRun( + GetProbeApplication(PassingProbes), + "Probe.Identity", + Timeout); + + // The dedup only does anything when more than one BenchmarkRunInfo is validated, so the benchmarks that + // ran have to span several types for this to be exercising it at all - which counting nodes would not say. + var types = ran + .Select(node => node.DisplayName.Substring(PassingProbes.Length + 1).Split('.')[0]) + .Distinct(StringComparer.Ordinal) + .ToArray(); + + Assert.True(types.Length >= 2, $"Expected several benchmark types to run, but only {string.Join(", ", types)} did."); + Assert.All(ran, node => Assert.Equal("passed", node.ExecutionState)); + Assert.All( + ran, + node => Assert.Single(Regex.Matches(node.StandardOutput, "WithAbstractConfig was ignored"))); + } + + [Fact] + public void ParameterValuesAreDisposedWhenBenchmarkDotNetBailsOutOnValidation() + { + // The unoptimized probe application fails JitOptimizationsValidator, which is critical: BenchmarkRunnerClean + // returns before the try whose finally disposes the values it was handed, so nothing disposes them. The + // adapter must not take "handed to BenchmarkDotNet" for "disposed by BenchmarkDotNet" - that assumption + // would leave exactly these values, of a run that never started, to the finalizer for good. + TestRunSummary? summary = null; + + var report = ReadDisposalReport( + UnoptimizedProbes, + "unoptimized-probe.txt", + () => summary = RunAndSummarize(UnoptimizedProbes, "--treenode-filter", "/*/*/SharedValueProbe/*")); + + Assert.Equal(2, summary!.Total); + Assert.Equal(2, summary.Failed); + Assert.Equal("created=4 disposed=4", report); + } + + [Fact] + public void ParameterValuesAreDisposedWhenOnlyOneBenchmarkOfASetIsRun() + { + // BenchmarkDotNet only disposes the case it was handed, so the two that were filtered out would leak. A + // value shared with the case that runs must not be disposed early either, which the count would catch as + // a disposal too many. + var uid = Discover(PassingProbes) + .Single(test => test.DisplayName.EndsWith("DisposableProbe.Identity(Value: tracked-1)", StringComparison.Ordinal)) + .Uid; + + var report = ReadDisposalReport( + PassingProbes, + "disposable-probe.txt", + () => RunAndSummarize(PassingProbes, "--filter-uid", uid)); + + Assert.Equal("created=3 disposed=3", report); + } + + [Fact] + public void OutOfProcessBenchmarksAreHiddenWhenTheAssemblyIsNotOptimized() + { + // The point of the unoptimized probe application: a benchmark that would leave the process is hidden, so + // that it can be debugged from a test runner. DroppedProbe has no other job and disappears entirely, + // SharedValueProbe keeps its in-process cases - which is also why the job is no part of their names. + var discovered = Discover(UnoptimizedProbes); + + Assert.Equal( + new[] { "SharedValueProbe.Length(Value: shared-1)", "SharedValueProbe.Length(Value: shared-2)" }, + discovered.Select(test => test.DisplayName.Substring(UnoptimizedProbes.Length + 1)).OrderBy(name => name, StringComparer.Ordinal)); + } + + [Fact] + public void ParameterValuesAreDisposedWhenBenchmarksAreHiddenByAnUnoptimizedAssembly() + { + // The benchmarks hidden above are never handed to BenchmarkDotNet by either adapter, so the values they + // own are the enumeration's to dispose: the two of DroppedProbe are unreachable from anything that + // survives. The two of SharedValueProbe are shared with cases that do survive, so disposing them here + // would be a disposal too many, which the count catches just as well as a leak. + var report = ReadDisposalReport(UnoptimizedProbes, "unoptimized-probe.txt", () => Discover(UnoptimizedProbes)); + + Assert.Equal("created=4 disposed=4", report); + } + + [Fact] + public void ATreeNodeFilterMatchesTheClassAndTheMethodOfABenchmark() + { + var discovered = Discover(PassingProbes, "--treenode-filter", "/*/*/SampleBenchmarks/Multiply*"); + + Assert.Equal(2, discovered.Count); + Assert.All(discovered, test => Assert.Contains("SampleBenchmarks.Multiply", test.DisplayName, StringComparison.Ordinal)); + } + + [Fact] + public void ABenchmarkStaysAtTheSameLevelOfTheTreeWhenAParameterContainsTheSeparator() + { + // The platform splits the tree path on every '/' and never unescapes it, so a parameter containing one has + // to be encoded rather than escaped: otherwise the benchmark sits one level deeper and this filter, which + // matches every other benchmark, would miss it. + var discovered = Discover(PassingProbes, "--treenode-filter", "/*/*/SeparatorProbe/*"); + + Assert.Single(discovered); + Assert.Contains("SeparatorProbe.Length(Value: \"a/b\")", discovered[0].DisplayName, StringComparison.Ordinal); + } + + [Fact] + public void ABenchmarkIsAddressableWhenItsPathContainsAPropertyFilterDelimiter() + { + // A TreeNodeFilter reads '[' and ']' as the delimiters of a property filter, so a segment carrying them + // has to be encoded rather than left to be parsed - unlike the parentheses around the parameters, which a + // filter escapes with a backslash. That is true of a parameter that contains them... + var byParameter = Discover(PassingProbes, "--treenode-filter", @"/*/*/BracketProbe/Length\(Value: ""%5BDry%5D""\)*"); + + Assert.Single(byParameter); + Assert.Contains("BracketProbe.Length(Value: \"[Dry]\")", byParameter[0].DisplayName, StringComparison.Ordinal); + + // ...and of the job that every leaf ends in, which is what an exact path would otherwise trip over. The + // trailing wildcard stands in for the job name, so that this does not pin how a job is displayed. + var byJob = Discover(PassingProbes, "--treenode-filter", @"/*/*/BracketProbe/Length\(Value: ""%5BDry%5D""\) %5B*"); + + Assert.Single(byJob); + Assert.Equal(byParameter[0].Uid, byJob[0].Uid); + } + + [Fact] + public void ADiscoveryWithAnUnrecognisedFilterListsEveryBenchmarkAndSaysSo() + { + // Microsoft.Testing.Platform 2.3.3 has no filter the adapter does not handle, and the extension point for + // adding one is internal to it, so this branch is unreachable from a real test host - FilterProbe drives + // the framework itself to reach it. Discovery runs nothing, so listing too much is the cheap mistake and + // reporting nothing is the expensive one; the warning is what makes the wrong list visible. + var report = RunInternalsProbe(); + + Assert.Contains( + report.Discover, + line => line.StartsWith("output ", StringComparison.Ordinal) + && line.Contains("does not recognise", StringComparison.Ordinal) + && line.Contains("UnrecognisedFilter", StringComparison.Ordinal)); + + // Everything the probe assembly declares, rather than a count that a benchmark added to it would break. + Assert.NotEmpty(report.Discovered); + Assert.Contains("complete True", report.Discover); + } + + [Fact] + public void ARunWithAnUnrecognisedFilterIsRefusedWithAFailedNodePerBenchmark() + { + // The other half of the same branch: a run cannot list too much, because it would spend the machine's next + // hour on it. Refusing by throwing would be invisible - the request is completed before the exception is + // observed - so every benchmark it could have selected is reported failed instead, which is where an IDE + // shows it. + var report = RunInternalsProbe(); + + var failed = report.Run + .Where(line => line.StartsWith("failed(", StringComparison.Ordinal)) + .ToArray(); + + // Every benchmark the same filter listed during discovery is reported, so that none of them is left + // looking like it was quietly skipped. + Assert.Equal(report.Discovered.Length, failed.Length); + Assert.All(failed, line => Assert.Contains("does not support", line, StringComparison.Ordinal)); + Assert.All(failed, line => Assert.Contains("UnrecognisedFilter", line, StringComparison.Ordinal)); + + // Every one of them was reported as started too, and the request finished rather than throwing. + Assert.Equal(failed.Length, report.Run.Count(line => line.StartsWith("in-progress ", StringComparison.Ordinal))); + Assert.DoesNotContain(report.Run, line => line.StartsWith("threw ", StringComparison.Ordinal)); + Assert.Contains("complete True", report.Run); + } + + [Fact] + public void ParameterValuesOfARequestStillInFlightAreDisposedWhenTheApplicationEnds() + { + // A request hands its values over by completing. One that never gets there - the client sent `exit`, or + // the IDE cancelled, while it was still in flight - leaves them reachable from nothing else, and a value + // left to the finalizer instead is the dotnet/BenchmarkDotNet#1383 hang this disposal exists to prevent. + var report = RunInternalsProbe(); + + Assert.Equal("created=2 disposed=2", Assert.Single(report.Abandoned)); + } + + /// + /// Runs the application that drives the adapter's platform types directly, and splits what it reported into + /// its sections. + /// + /// The lines of each section, and the benchmarks the discovery request listed. + private InternalsReport RunInternalsProbe() + { + var (exitCode, standardOutput) = Execute(InternalsProbe, []); + + Assert.Equal(0, exitCode); + + var lines = standardOutput.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries); + + var abandonedStart = Array.IndexOf(lines, "== abandoned"); + var discoverStart = Array.IndexOf(lines, "== discover"); + var runStart = Array.IndexOf(lines, "== run"); + var end = Array.IndexOf(lines, "== done"); + + Assert.True( + abandonedStart >= 0 && discoverStart > abandonedStart && runStart > discoverStart && end > runStart, + $"The internals probe did not report every section:{Environment.NewLine}{standardOutput}"); + + var discover = lines[(discoverStart + 1)..runStart]; + + return new InternalsReport( + lines[(abandonedStart + 1)..discoverStart], + discover, + lines[(runStart + 1)..end], + discover.Where(line => line.StartsWith("discovered ", StringComparison.Ordinal)).ToArray()); + } + + private sealed record InternalsReport(string[] Abandoned, string[] Discover, string[] Run, string[] Discovered); + + [Fact] + public void AnOutOfProcessBenchmarkIsBuiltAndRun() + { + // The only probe that is not pinned to an in-process toolchain, so the only one that makes the adapter see + // a real generate/build/execute cycle. + var summary = RunAndSummarize(PassingProbes, "--treenode-filter", "/*/*/OutOfProcessProbe/*"); + + Assert.Equal(1, summary.Total); + Assert.Equal(1, summary.Succeeded); + Assert.Equal(0, summary.Failed); + } + + [Fact] + public void ABuildFailureIsReportedAsAFailedTest() + { + var (summary, standardOutput) = Run(FailingProbes, "--treenode-filter", "/*/*/BuildFailureProbe/*"); + + Assert.Equal(1, summary.Total); + Assert.Equal(1, summary.Failed); + Assert.Contains("// Build Error: The build of this benchmark always fails, on purpose.", standardOutput, StringComparison.Ordinal); + } + + [Fact] + public void BenchmarksSharingAUidAreReportedAsOneFailedTest() + { + // Two benchmarks the platform cannot tell apart are published as a single node during discovery, and the + // collision is reported when they are asked to run. + Assert.Single(Discover(FailingProbes, "--treenode-filter", "/*/*/CollisionProbe/*")); + + var (summary, standardOutput) = Run(FailingProbes, "--treenode-filter", "/*/*/CollisionProbe/*"); + + Assert.Equal(1, summary.Total); + Assert.Equal(1, summary.Failed); + Assert.Contains("2 benchmarks are identified as", standardOutput, StringComparison.Ordinal); + } + + [Fact] + public void BenchmarksSharingAUidThroughTheirDescriptionAreReportedWithTheirMethodNames() + { + // Nothing here is parameterized: what collides is the description of one benchmark against the method + // name of the other, so the message has to name the two methods and point at the description. + var (summary, standardOutput) = Run(FailingProbes, "--treenode-filter", "/*/*/DescriptionCollisionProbe/*"); + + Assert.Equal(1, summary.Total); + Assert.Equal(1, summary.Failed); + Assert.Contains("none of them were run: Described, Twin.", standardOutput, StringComparison.Ordinal); + Assert.Contains("[Benchmark(Description = \"...\")]", standardOutput, StringComparison.Ordinal); + } + + /// + /// Runs the probe application and reads back what it reported about the disposal of its parameter values. + /// + /// + /// The counts are written to a file rather than to the output, because the discovery output is parsed as json. + /// + /// The probe application that writes the counts. + /// The name of the file the probe writes them to. + /// The way the probe application is driven. + /// The counts the probe reported when it exited. + private static string ReadDisposalReport(string project, string reportFileName, Action execute) + { + // The probe projects are referenced with ReferenceOutputAssembly="false", so the names are repeated here + // rather than taken from the ReportFileName constants of the probes themselves. + var report = Path.Combine(Path.GetDirectoryName(GetProbeApplication(project))!, reportFileName); + + File.Delete(report); + execute(); + + Assert.True(File.Exists(report), $"The probe application did not write '{report}'."); + + return File.ReadAllText(report); + } + + private IReadOnlyList Discover(string project, params string[] arguments) + { + var (exitCode, standardOutput) = Execute(project, ["--list-tests", "json", .. arguments]); + + Assert.Equal(0, exitCode); + + using var document = JsonDocument.Parse(standardOutput); + + return document.RootElement.GetProperty("tests") + .EnumerateArray() + .Select(test => new DiscoveredTest( + test.GetProperty("uid").GetString()!, + test.GetProperty("displayName").GetString()!, + test.GetProperty("type").GetProperty("typeName").GetString()!)) + .ToArray(); + } + + private TestRunSummary RunAndSummarize(string project, params string[] arguments) => Run(project, arguments).Summary; + + private (TestRunSummary Summary, string StandardOutput) Run(string project, params string[] arguments) + { + var (_, standardOutput) = Execute(project, arguments); + + return (TestRunSummary.Parse(standardOutput), standardOutput); + } + + private (int ExitCode, string StandardOutput) Execute(string project, string[] arguments) + { + var application = GetProbeApplication(project); + var startInfo = new ProcessStartInfo(application) + { + WorkingDirectory = Path.GetDirectoryName(application), + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false + }; + + // Progress reporting redraws the screen in place, which is noise once the output is redirected. + foreach (var argument in arguments.Concat(["--no-ansi", "--progress", "off"])) + startInfo.ArgumentList.Add(argument); + + var standardOutput = new StringBuilder(); + var standardError = new StringBuilder(); + + using var process = new Process { StartInfo = startInfo }; + process.OutputDataReceived += (_, e) => { if (e.Data != null) lock (standardOutput) standardOutput.AppendLine(e.Data); }; + process.ErrorDataReceived += (_, e) => { if (e.Data != null) lock (standardError) standardError.AppendLine(e.Data); }; + + process.Start(); + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + + if (!process.WaitForExit((int)Timeout.TotalMilliseconds)) + { + process.Kill(entireProcessTree: true); + throw new TimeoutException($"'{Path.GetFileName(application)} {string.Join(" ", arguments)}' did not finish within {Timeout}."); + } + + // Lets the redirected output be flushed before it is read. + process.WaitForExit(); + + output.WriteLine($"$ {application} {string.Join(" ", startInfo.ArgumentList)}"); + output.WriteLine(standardOutput.ToString()); + + if (standardError.Length > 0) + output.WriteLine($"stderr:{Environment.NewLine}{standardError}"); + + return (process.ExitCode, standardOutput.ToString()); + } + + private static string GetProbeApplication(string project) + { + // The tests run from /tests/BenchmarkDotNet.IntegrationTests/bin///, and + // the probes are built next to them, by the ProjectReferences of this project. + var binaries = new DirectoryInfo(AppContext.BaseDirectory); + var configuration = binaries.Parent!.Name; + var testsFolder = binaries.Parent!.Parent!.Parent!.Parent!.FullName; + + var fileName = OsDetector.IsWindows() ? $"{project}.exe" : project; + var path = Path.Combine(testsFolder, project, "bin", configuration, ProbeTargetFramework, fileName); + + if (!File.Exists(path)) + throw new FileNotFoundException($"The probe application was not built. Expected it at '{path}'.", path); + + return path; + } + + private sealed record DiscoveredTest(string Uid, string DisplayName, string TypeName); + + private sealed record TestRunSummary(int Total, int Failed, int Succeeded, int Skipped) + { + public static TestRunSummary Parse(string standardOutput) + { + // The platform ends a run with a block of " : " lines under "Test run summary:". + int Read(string name) + { + var match = Regex.Match(standardOutput, $@"^\s*{name}:\s*(?\d+)\s*$", RegexOptions.Multiline); + + return match.Success + ? int.Parse(match.Groups["count"].Value) + : throw new InvalidOperationException($"The test run did not report a '{name}' count.{Environment.NewLine}{standardOutput}"); + } + + return new TestRunSummary(Read("total"), Read("failed"), Read("succeeded"), Read("skipped")); + } + } + } +} +#endif diff --git a/tests/BenchmarkDotNet.IntegrationTests/TestingPlatformServerModeSession.cs b/tests/BenchmarkDotNet.IntegrationTests/TestingPlatformServerModeSession.cs new file mode 100644 index 0000000000..8a1dda83fd --- /dev/null +++ b/tests/BenchmarkDotNet.IntegrationTests/TestingPlatformServerModeSession.cs @@ -0,0 +1,314 @@ +#if NETCOREAPP +using System.Diagnostics; +using System.Net; +using System.Net.Sockets; +using System.Text; +using System.Text.Json; + +namespace BenchmarkDotNet.IntegrationTests +{ + /// + /// Drives a Microsoft.Testing.Platform application through its server mode, which is how Visual Studio and the + /// Visual Studio Code Test Explorer run tests: one process serves a discovery request and then the run requests + /// that follow it, rather than being started again for each of them. + /// + /// + /// The platform speaks JSON-RPC framed the way the language server protocol frames it, over a socket that the + /// client listens on and the test host connects back to. Only the handful of messages this needs are implemented: + /// `initialize`, `testing/discoverTests`, `testing/runTests` and `exit`. + /// + internal sealed class TestingPlatformServerModeSession : IDisposable + { + private readonly Process process; + private readonly TcpClient client; + private readonly NetworkStream stream; + private readonly Dictionary> pendingRequests = []; + private readonly Dictionary> pendingRuns = []; + private readonly List nodes = []; + private readonly TimeSpan timeout; + private int lastRequestId; + + private TestingPlatformServerModeSession(Process process, TcpClient client, TimeSpan timeout) + { + this.process = process; + this.client = client; + this.timeout = timeout; + stream = client.GetStream(); + + Task.Run(ReadLoop); + } + + /// + /// Discovers every benchmark of the application and then runs the ones whose display name contains the given + /// text, from the one process. + /// + /// The probe application to drive. + /// The text the display name of a benchmark has to contain to be run. + /// How long any one step may take. + /// Whether to discover a second time once the run is over, as an IDE refreshing does. + /// The nodes the discovery reported, and the last state each ran node was reported in. + public static (IReadOnlyList Discovered, IReadOnlyList Ran) DiscoverThenRun( + string application, + string runFilter, + TimeSpan timeout, + bool discoverAgain = false) + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + + try + { + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + var startInfo = new ProcessStartInfo(application) + { + WorkingDirectory = Path.GetDirectoryName(application), + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false + }; + + foreach (var argument in new[] { "--server", "--client-host", "127.0.0.1", "--client-port", port.ToString(), "--no-ansi" }) + startInfo.ArgumentList.Add(argument); + + using var process = Process.Start(startInfo)!; + process.OutputDataReceived += (_, _) => { }; + process.ErrorDataReceived += (_, _) => { }; + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + + using var client = listener.AcceptTcpClient(); + using var session = new TestingPlatformServerModeSession(process, client, timeout); + + return session.Run(runFilter, discoverAgain); + } + finally + { + listener.Stop(); + } + } + + private (IReadOnlyList, IReadOnlyList) Run(string runFilter, bool discoverAgain) + { + SendRequest("initialize", new + { + processId = Environment.ProcessId, + clientInfo = new { name = "BenchmarkDotNet.IntegrationTests", version = "1.0.0" }, + capabilities = new { testing = new { debuggerProvider = false } } + }); + Send(new { jsonrpc = "2.0", method = "initialized", @params = new { } }); + + var discovered = Exchange("testing/discoverTests", runId => new { runId }); + + var selected = discovered + .Where(node => node.DisplayName.Contains(runFilter, StringComparison.Ordinal)) + .ToArray(); + + if (selected.Length == 0) + throw new InvalidOperationException($"No discovered benchmark matched '{runFilter}'."); + + var ran = Exchange("testing/runTests", runId => new + { + runId, + tests = selected + .Select(node => new Dictionary { ["uid"] = node.Uid, ["display-name"] = node.DisplayName }) + .ToArray() + }); + + if (discoverAgain) + Exchange("testing/discoverTests", runId => new { runId }); + + Send(new { jsonrpc = "2.0", method = "exit", @params = new { } }); + + if (!process.WaitForExit((int)timeout.TotalMilliseconds)) + { + process.Kill(entireProcessTree: true); + throw new TimeoutException("The test host did not exit after the session was closed."); + } + + // Lets the probe write the report file its process exit handler produces. + process.WaitForExit(); + + return (discovered, ran); + } + + /// + /// Sends one request and collects the node updates the platform reports for it. + /// + private ServerNode[] Exchange(string method, Func parameters) + { + var runId = Guid.NewGuid().ToString(); + var completion = new TaskCompletionSource(); + + lock (pendingRuns) + pendingRuns[runId] = completion; + + lock (nodes) + nodes.Clear(); + + SendRequest(method, parameters(runId)); + + if (!completion.Task.Wait(timeout)) + throw new TimeoutException($"'{method}' did not complete within {timeout}."); + + lock (nodes) + { + // The platform reports a node again whenever its state changes, and the last one is the outcome. + return nodes + .GroupBy(node => node.Uid, StringComparer.Ordinal) + .Select(group => group.Last()) + .ToArray(); + } + } + + private void SendRequest(string method, object parameters) + { + var id = Interlocked.Increment(ref lastRequestId); + var completion = new TaskCompletionSource(); + + lock (pendingRequests) + pendingRequests[id] = completion; + + Send(new { jsonrpc = "2.0", id, method, @params = parameters }); + + if (!completion.Task.Wait(timeout)) + throw new TimeoutException($"'{method}' was not answered within {timeout}."); + } + + private void Send(object message) + { + var body = Encoding.UTF8.GetBytes(JsonSerializer.Serialize(message)); + var header = Encoding.UTF8.GetBytes($"Content-Length: {body.Length}\r\n\r\n"); + + lock (stream) + { + stream.Write(header, 0, header.Length); + stream.Write(body, 0, body.Length); + stream.Flush(); + } + } + + private void ReadLoop() + { + var buffer = new List(); + var chunk = new byte[8192]; + + while (true) + { + int read; + + try + { + read = stream.Read(chunk, 0, chunk.Length); + } + catch + { + return; + } + + if (read == 0) + return; + + buffer.AddRange(chunk.Take(read)); + + while (TryReadMessage(buffer, out var json)) + Handle(json); + } + } + + private static bool TryReadMessage(List buffer, out string json) + { + json = string.Empty; + + // The header is ASCII, so the byte offsets of the separator and of the character offsets agree. + var text = Encoding.ASCII.GetString(buffer.ToArray()); + var headerEnd = text.IndexOf("\r\n\r\n", StringComparison.Ordinal); + if (headerEnd < 0) + return false; + + var lengthHeader = text.Substring(0, headerEnd) + .Split(["\r\n"], StringSplitOptions.None) + .First(header => header.StartsWith("Content-Length", StringComparison.OrdinalIgnoreCase)); + var length = int.Parse(lengthHeader.Split(':')[1].Trim()); + var bodyStart = headerEnd + 4; + + if (buffer.Count < bodyStart + length) + return false; + + json = Encoding.UTF8.GetString(buffer.GetRange(bodyStart, length).ToArray()); + buffer.RemoveRange(0, bodyStart + length); + + return true; + } + + private void Handle(string json) + { + using var document = JsonDocument.Parse(json); + var root = document.RootElement; + + if (root.TryGetProperty("id", out var id) && id.ValueKind == JsonValueKind.Number) + { + lock (pendingRequests) + { + if (pendingRequests.TryGetValue(id.GetInt32(), out var request)) + request.TrySetResult(true); + } + } + + if (!root.TryGetProperty("method", out var method) + || method.GetString() != "testing/testUpdates/tests" + || !root.TryGetProperty("params", out var parameters)) + { + return; + } + + if (parameters.TryGetProperty("changes", out var changes) && changes.ValueKind == JsonValueKind.Array) + { + lock (nodes) + { + foreach (var change in changes.EnumerateArray()) + { + var node = change.GetProperty("node"); + if (node.TryGetProperty("node-type", out var nodeType) && nodeType.GetString() == "action") + { + nodes.Add(new ServerNode( + node.GetProperty("uid").GetString()!, + node.GetProperty("display-name").GetString()!, + node.TryGetProperty("execution-state", out var state) ? state.GetString()! : "", + node.TryGetProperty("standardOutput", out var output) ? output.GetString() ?? "" : "")); + } + } + } + + return; + } + + // A null "changes" is how the platform says the request is over. + if (parameters.TryGetProperty("runId", out var runId)) + { + lock (pendingRuns) + { + if (pendingRuns.TryGetValue(runId.GetString()!, out var run)) + run.TrySetResult(true); + } + } + } + + public void Dispose() + { + try + { + if (!process.HasExited) + process.Kill(entireProcessTree: true); + } + catch + { + // The process is gone, which is what was wanted. + } + + client.Dispose(); + } + + internal sealed record ServerNode(string Uid, string DisplayName, string ExecutionState, string StandardOutput); + } +} +#endif diff --git a/tests/BenchmarkDotNet.Tests/GenericBuilderTests.cs b/tests/BenchmarkDotNet.Tests/GenericBuilderTests.cs index dc41b9e426..7d1ac5c363 100644 --- a/tests/BenchmarkDotNet.Tests/GenericBuilderTests.cs +++ b/tests/BenchmarkDotNet.Tests/GenericBuilderTests.cs @@ -1,4 +1,5 @@ using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; using BenchmarkDotNet.Helpers; namespace BenchmarkDotNet.Tests @@ -122,5 +123,61 @@ public class GenericBenchmarkWithConstraintsWrongArgs where T1 : struct [Benchmark] public T2 CreateT2() => Activator.CreateInstance(); } + + [Fact] + public void TestTypeWithUnreadableAttributesIsDropped() + { + // Reflection constructs every attribute of a type in order to hand any of them back, so a [Config] that + // cannot be instantiated makes the read of the [GenericTypeArguments] throw. The type is unusable at that + // point - BenchmarkConverter would throw on the very same read - so it is dropped, and the benchmarks + // that were listed next to it are still returned. + var types = GenericBenchmarksBuilder.GetRunnableBenchmarks( + [typeof(BenchmarkWithAbstractConfig), typeof(BenchmarkWithInaccessibleConfig), typeof(OneArgGenericBenchmark<>)]); + + Assert.Equal(2, types.Length); + Assert.Single(types, typeof(OneArgGenericBenchmark)); + Assert.Single(types, typeof(OneArgGenericBenchmark)); + } + + [Fact] + public void TestTypeWithUnreadableAttributesIsReportedAsAFailure() + { + var built = GenericBenchmarksBuilder.BuildGenericsIfNeeded(typeof(BenchmarkWithAbstractConfig)).ToArray(); + + var failure = Assert.Single(built); + Assert.False(failure.IsSuccess); + Assert.Contains(nameof(BenchmarkWithAbstractConfig), failure.Error); + + // Told apart from a [GenericTypeArguments] that did not fit, because only this kind has to be reported + // by whoever drops it: GenericBenchmarksValidator needs a surviving benchmark before it ever runs. + Assert.True(failure.IsUnreadable); + } + + [Fact] + public void TestGenericTypeThatFailedToBuildIsNotReportedAsUnreadable() + { + var built = GenericBenchmarksBuilder.BuildGenericsIfNeeded(typeof(GenericBenchmarkWithConstraintsWrongArgs<,>)).ToArray(); + + var failure = Assert.Single(built, candidate => !candidate.IsSuccess); + Assert.False(failure.IsUnreadable); + Assert.Contains("wrong type argument", failure.Error); + } + + [Config(typeof(DebugConfig))] // abstract, so ConfigAttribute's constructor throws + public class BenchmarkWithAbstractConfig + { + [Benchmark] public int Identity() => 1; + } + + [Config(typeof(NoPublicConstructorConfig))] + public class BenchmarkWithInaccessibleConfig + { + [Benchmark] public int Identity() => 1; + + private class NoPublicConstructorConfig : ManualConfig + { + private NoPublicConstructorConfig() { } + } + } } } \ No newline at end of file diff --git a/tests/BenchmarkDotNet.Tests/TypeFilterTests.cs b/tests/BenchmarkDotNet.Tests/TypeFilterTests.cs index 1f3e6e7918..0f1e4c4186 100644 --- a/tests/BenchmarkDotNet.Tests/TypeFilterTests.cs +++ b/tests/BenchmarkDotNet.Tests/TypeFilterTests.cs @@ -1,4 +1,5 @@ using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; using BenchmarkDotNet.ConsoleArguments; using BenchmarkDotNet.Extensions; using BenchmarkDotNet.Loggers; @@ -207,6 +208,85 @@ public void GenericTypesCanBeFilteredByDisplayName() Assert.Contains("SomeGeneric.Create", benchmarks); } + [Fact] + public void ReportsATypeWhoseAttributesCannotBeRead() + { + // The [Config] of this type throws while reflection constructs it, so the type is dropped rather than + // allowed to abort the whole run - but it has to be said out loud, because GenericBenchmarksValidator + // never gets to report it when nothing of the assembly survives, and "No benchmarks were found" on its + // own sends the user looking in the wrong place. + var logger = new AccumulationLogger(); + + var benchmarks = Filter([typeof(ClassWithUnreadableConfig), typeof(ClassA)], ["--filter", "*"], logger); + + Assert.Equal(2, benchmarks.Count); + Assert.Contains("ClassA.Method1", benchmarks); + Assert.DoesNotContain("ClassWithUnreadableConfig.Method1", benchmarks); + Assert.Contains(nameof(ClassWithUnreadableConfig), logger.GetLog(), StringComparison.Ordinal); + } + + [Fact] + public void ReportsATypeWhoseAttributesCannotBeReadWhenItIsTheOnlyOne() + { + // With nothing else to run, the validator never gets a chance to say anything, so this is the one place + // left to explain why nothing was found - and the message must not be that no [Benchmark] was found. + var logger = new AccumulationLogger(); + + var benchmarks = Filter([typeof(ClassWithUnreadableConfig)], ["--filter", "*"], logger); + + Assert.Empty(benchmarks); + Assert.Contains(nameof(ClassWithUnreadableConfig), logger.GetLog(), StringComparison.Ordinal); + Assert.DoesNotContain("No [Benchmark] attribute found", logger.GetLog(), StringComparison.Ordinal); + } + +#if NETCOREAPP + [Fact] + public void ReportsAnAssemblyWhoseOnlyBenchmarkTypeCannotBeRead() + { + // BenchmarkSwitcher.FromAssembly(assembly).Run(args) comes in through the assembly rather than through + // types, where "does this assembly declare benchmarks" used to be answered from the readable types only: + // an assembly whose only benchmark class is unreadable was then told it has no [Benchmark] at all, which + // is both silent about the cause and wrong. Emitted rather than compiled, because every real assembly + // of this repository declares readable benchmarks too. + var assembly = EmitAssemblyWithAnUnreadableBenchmarkType(); + var logger = new AccumulationLogger(); + + var (allTypesValid, runnable) = TypeFilter.GetTypesWithRunnableBenchmarks([], [assembly], logger); + + Assert.True(allTypesValid); + Assert.Empty(runnable); + Assert.Contains("Unreadable was ignored because its attributes could not be read", logger.GetLog(), StringComparison.Ordinal); + Assert.DoesNotContain("No [Benchmark] attribute found", logger.GetLog(), StringComparison.Ordinal); + } + + private static System.Reflection.Assembly EmitAssemblyWithAnUnreadableBenchmarkType() + { + var assembly = System.Reflection.Emit.AssemblyBuilder.DefineDynamicAssembly( + new System.Reflection.AssemblyName("UnreadableBenchmarks"), + System.Reflection.Emit.AssemblyBuilderAccess.Run); + var type = assembly.DefineDynamicModule("UnreadableBenchmarks").DefineType( + "Unreadable", + System.Reflection.TypeAttributes.Public | System.Reflection.TypeAttributes.Class); + + // [Config(typeof(AbstractConfig))]: reflection constructs the attribute in order to hand it back, and + // ConfigAttribute instantiates the config it is given, which an abstract one cannot be. + type.SetCustomAttribute(new System.Reflection.Emit.CustomAttributeBuilder( + typeof(ConfigAttribute).GetConstructor([typeof(Type)])!, + [typeof(AbstractConfig)])); + + // [Benchmark] public void Method1() { } + var method = type.DefineMethod("Method1", System.Reflection.MethodAttributes.Public, typeof(void), Type.EmptyTypes); + method.SetCustomAttribute(new System.Reflection.Emit.CustomAttributeBuilder( + typeof(BenchmarkAttribute).GetConstructor([typeof(int), typeof(string)])!, + [0, ""])); + method.GetILGenerator().Emit(System.Reflection.Emit.OpCodes.Ret); + + type.CreateType(); + + return assembly; + } +#endif + private HashSet Filter(Type[] types, string[] args, ILogger? logger = null) { var nonNullLogger = logger ?? new OutputLogger(Output); @@ -251,6 +331,18 @@ public void Method2() { } public void Method3() { } } + [Config(typeof(AbstractConfig))] + public class ClassWithUnreadableConfig + { + [Benchmark] + public void Method1() { } + } + + // ConfigAttribute instantiates the type it is given, and an abstract one cannot be instantiated. + public abstract class AbstractConfig : ManualConfig + { + } + public class ClassC { // None of these methods are actually Benchmarks!!