Skip to content

Commit 2a9c805

Browse files
Copilottimcassell
Copilot
and
timcassell
committed
Fix handler contract and use loops for all run mode combinations
Co-authored-by: timcassell <[email protected]>
1 parent 6b21e82 commit 2a9c805

File tree

2 files changed

+98
-63
lines changed

2 files changed

+98
-63
lines changed

‎tests/BenchmarkDotNet.IntegrationTests/Diagnosers/MockInProcessDiagnoser.cs‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ public abstract class BaseMockInProcessDiagnoser : IInProcessDiagnoser
1717

1818
public abstract string DiagnoserName { get; }
1919
public abstract RunMode DiagnoserRunMode { get; }
20-
public abstract Type HandlerType { get; }
2120
public abstract string ExpectedResult { get; }
2221

2322
public IEnumerable<string> Ids => [DiagnoserName];
@@ -36,22 +35,31 @@ public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { }
3635

3736
public IEnumerable<ValidationError> Validate(ValidationParameters validationParameters) => [];
3837

39-
public virtual (Type? handlerType, string? serializedConfig) GetSeparateProcessHandlerTypeAndSerializedConfig(BenchmarkCase benchmarkCase)
40-
=> (HandlerType, null);
38+
public abstract (Type? handlerType, string? serializedConfig) GetSeparateProcessHandlerTypeAndSerializedConfig(BenchmarkCase benchmarkCase);
4139

4240
public virtual IInProcessDiagnoserHandler? GetSameProcessHandler(BenchmarkCase benchmarkCase)
43-
=> (IInProcessDiagnoserHandler)Activator.CreateInstance(HandlerType, ExpectedResult);
41+
{
42+
var (handlerType, serializedConfig) = GetSeparateProcessHandlerTypeAndSerializedConfig(benchmarkCase);
43+
if (handlerType == null)
44+
return null;
45+
var handler = (IInProcessDiagnoserHandler)Activator.CreateInstance(handlerType);
46+
handler.Initialize(serializedConfig);
47+
return handler;
48+
}
4449

4550
public void DeserializeResults(BenchmarkCase benchmarkCase, string results) => Results.Add(benchmarkCase, results);
4651
}
4752

4853
public abstract class BaseMockInProcessDiagnoserHandler : IInProcessDiagnoserHandler
4954
{
50-
private readonly string _result;
55+
private string _result;
5156

52-
protected BaseMockInProcessDiagnoserHandler(string result) => _result = result;
57+
protected BaseMockInProcessDiagnoserHandler() { }
5358

54-
public void Initialize(string? serializedConfig) { }
59+
public void Initialize(string? serializedConfig)
60+
{
61+
_result = serializedConfig ?? string.Empty;
62+
}
5563

5664
public void Handle(BenchmarkSignal signal, InProcessDiagnoserActionArgs args) { }
5765

@@ -62,46 +70,48 @@ public sealed class MockInProcessDiagnoser : BaseMockInProcessDiagnoser
6270
{
6371
public override string DiagnoserName => nameof(MockInProcessDiagnoser);
6472
public override RunMode DiagnoserRunMode => RunMode.NoOverhead;
65-
public override Type HandlerType => typeof(MockInProcessDiagnoserHandler);
6673
public override string ExpectedResult => "MockResult";
74+
75+
public override (Type? handlerType, string? serializedConfig) GetSeparateProcessHandlerTypeAndSerializedConfig(BenchmarkCase benchmarkCase)
76+
=> (typeof(MockInProcessDiagnoserHandler), ExpectedResult);
6777
}
6878

6979
public sealed class MockInProcessDiagnoserHandler : BaseMockInProcessDiagnoserHandler
7080
{
71-
public MockInProcessDiagnoserHandler(string result) : base(result) { }
7281
}
7382

7483
public sealed class MockInProcessDiagnoserNoOverhead : BaseMockInProcessDiagnoser
7584
{
7685
public override string DiagnoserName => nameof(MockInProcessDiagnoserNoOverhead);
7786
public override RunMode DiagnoserRunMode => RunMode.NoOverhead;
78-
public override Type HandlerType => typeof(MockInProcessDiagnoserNoOverheadHandler);
7987
public override string ExpectedResult => "NoOverheadResult";
88+
89+
public override (Type? handlerType, string? serializedConfig) GetSeparateProcessHandlerTypeAndSerializedConfig(BenchmarkCase benchmarkCase)
90+
=> (typeof(MockInProcessDiagnoserNoOverheadHandler), ExpectedResult);
8091
}
8192

8293
public sealed class MockInProcessDiagnoserNoOverheadHandler : BaseMockInProcessDiagnoserHandler
8394
{
84-
public MockInProcessDiagnoserNoOverheadHandler(string result) : base(result) { }
8595
}
8696

8797
public sealed class MockInProcessDiagnoserExtraRun : BaseMockInProcessDiagnoser
8898
{
8999
public override string DiagnoserName => nameof(MockInProcessDiagnoserExtraRun);
90100
public override RunMode DiagnoserRunMode => RunMode.ExtraRun;
91-
public override Type HandlerType => typeof(MockInProcessDiagnoserExtraRunHandler);
92101
public override string ExpectedResult => "ExtraRunResult";
102+
103+
public override (Type? handlerType, string? serializedConfig) GetSeparateProcessHandlerTypeAndSerializedConfig(BenchmarkCase benchmarkCase)
104+
=> (typeof(MockInProcessDiagnoserExtraRunHandler), ExpectedResult);
93105
}
94106

95107
public sealed class MockInProcessDiagnoserExtraRunHandler : BaseMockInProcessDiagnoserHandler
96108
{
97-
public MockInProcessDiagnoserExtraRunHandler(string result) : base(result) { }
98109
}
99110

100111
public sealed class MockInProcessDiagnoserNone : BaseMockInProcessDiagnoser
101112
{
102113
public override string DiagnoserName => nameof(MockInProcessDiagnoserNone);
103114
public override RunMode DiagnoserRunMode => RunMode.None;
104-
public override Type HandlerType => typeof(MockInProcessDiagnoserNoneHandler);
105115
public override string ExpectedResult => "NoneResult";
106116

107117
public override (Type? handlerType, string? serializedConfig) GetSeparateProcessHandlerTypeAndSerializedConfig(BenchmarkCase benchmarkCase)
@@ -113,5 +123,4 @@ public override (Type? handlerType, string? serializedConfig) GetSeparateProcess
113123

114124
public sealed class MockInProcessDiagnoserNoneHandler : BaseMockInProcessDiagnoserHandler
115125
{
116-
public MockInProcessDiagnoserNoneHandler(string result) : base(result) { }
117126
}

‎tests/BenchmarkDotNet.IntegrationTests/MultipleInProcessDiagnosersTests.cs‎

Lines changed: 74 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,79 +8,108 @@
88
using BenchmarkDotNet.IntegrationTests.Diagnosers;
99
using BenchmarkDotNet.Jobs;
1010
using BenchmarkDotNet.Tests.Loggers;
11+
using BenchmarkDotNet.Toolchains;
1112
using BenchmarkDotNet.Toolchains.InProcess.Emit;
13+
using BenchmarkDotNet.Toolchains.InProcess.NoEmit;
1214
using Xunit;
1315
using Xunit.Abstractions;
16+
using RunMode = BenchmarkDotNet.Diagnosers.RunMode;
1417

1518
namespace BenchmarkDotNet.IntegrationTests;
1619

1720
public class MultipleInProcessDiagnosersTests : BenchmarkTestExecutor
1821
{
1922
public MultipleInProcessDiagnosersTests(ITestOutputHelper output) : base(output) { }
2023

21-
public static IEnumerable<object[]> GetDiagnoserCombinations()
24+
private static readonly RunMode[] AllRunModes = { RunMode.NoOverhead, RunMode.ExtraRun, RunMode.None };
25+
26+
private static IEnumerable<BaseMockInProcessDiagnoser[]> GetDiagnoserCombinations(int count)
2227
{
23-
// Two diagnosers with NoOverhead
24-
yield return new object[]
28+
if (count == 1)
2529
{
26-
new BaseMockInProcessDiagnoser[] { new MockInProcessDiagnoserNoOverhead(), new MockInProcessDiagnoser() },
27-
typeof(SimpleBenchmark),
28-
new[] { true, true }
29-
};
30-
31-
// Two diagnosers with ExtraRun and NoOverhead
32-
yield return new object[]
30+
foreach (var runMode in AllRunModes)
31+
{
32+
yield return [CreateDiagnoser(runMode, 0)];
33+
}
34+
}
35+
else if (count == 2)
3336
{
34-
new BaseMockInProcessDiagnoser[] { new MockInProcessDiagnoserExtraRun(), new MockInProcessDiagnoserNoOverhead() },
35-
typeof(SimpleBenchmark),
36-
new[] { true, true }
37-
};
37+
foreach (var runMode1 in AllRunModes)
38+
{
39+
foreach (var runMode2 in AllRunModes)
40+
{
41+
yield return [CreateDiagnoser(runMode1, 0), CreateDiagnoser(runMode2, 1)];
42+
}
43+
}
44+
}
45+
else if (count == 3)
46+
{
47+
foreach (var runMode1 in AllRunModes)
48+
{
49+
foreach (var runMode2 in AllRunModes)
50+
{
51+
foreach (var runMode3 in AllRunModes)
52+
{
53+
yield return [CreateDiagnoser(runMode1, 0), CreateDiagnoser(runMode2, 1), CreateDiagnoser(runMode3, 2)];
54+
}
55+
}
56+
}
57+
}
58+
}
3859

39-
// Three diagnosers with varying run modes (None should not collect results)
40-
yield return new object[]
60+
private static BaseMockInProcessDiagnoser CreateDiagnoser(RunMode runMode, int index)
61+
{
62+
return runMode switch
4163
{
42-
new BaseMockInProcessDiagnoser[] { new MockInProcessDiagnoserNoOverhead(), new MockInProcessDiagnoserExtraRun(), new MockInProcessDiagnoserNone() },
43-
typeof(SimpleBenchmark),
44-
new[] { true, true, false }
64+
RunMode.NoOverhead => index == 0 ? new MockInProcessDiagnoserNoOverhead() : new MockInProcessDiagnoser(),
65+
RunMode.ExtraRun => new MockInProcessDiagnoserExtraRun(),
66+
RunMode.None => new MockInProcessDiagnoserNone(),
67+
_ => throw new ArgumentException($"Unsupported run mode: {runMode}")
4568
};
69+
}
4670

47-
// Three different types
48-
yield return new object[]
71+
public static IEnumerable<object[]> GetTestCombinations()
72+
{
73+
var toolchains = new IToolchain[]
4974
{
50-
new BaseMockInProcessDiagnoser[] { new MockInProcessDiagnoserNoOverhead(), new MockInProcessDiagnoser(), new MockInProcessDiagnoserExtraRun() },
51-
typeof(SimpleBenchmark),
52-
new[] { true, true, true }
75+
InProcessEmitToolchain.DontLogOutput,
76+
new InProcessNoEmitToolchain(TimeSpan.Zero, true),
77+
null // Default toolchain
5378
};
5479

55-
// Multiple benchmarks
56-
yield return new object[]
80+
var counts = new[] { 1, 2, 3 };
81+
82+
foreach (var toolchain in toolchains)
5783
{
58-
new BaseMockInProcessDiagnoser[] { new MockInProcessDiagnoserNoOverhead(), new MockInProcessDiagnoserExtraRun() },
59-
typeof(MultipleBenchmarks),
60-
new[] { true, true }
61-
};
84+
foreach (var count in counts)
85+
{
86+
foreach (var diagnosers in GetDiagnoserCombinations(count))
87+
{
88+
yield return new object[] { diagnosers, toolchain };
89+
}
90+
}
91+
}
6292
}
6393

6494
[Theory]
65-
[MemberData(nameof(GetDiagnoserCombinations))]
66-
public void MultipleInProcessDiagnosersWork(BaseMockInProcessDiagnoser[] diagnosers, Type benchmarkType, bool[] shouldHaveResults)
95+
[MemberData(nameof(GetTestCombinations))]
96+
public void MultipleInProcessDiagnosersWork(BaseMockInProcessDiagnoser[] diagnosers, IToolchain toolchain)
6797
{
6898
var logger = new OutputLogger(Output);
69-
var config = CreateInProcessConfig(logger);
99+
var config = CreateConfig(logger, toolchain);
70100

71101
foreach (var diagnoser in diagnosers)
72102
{
73103
config = config.AddDiagnoser(diagnoser);
74104
}
75105

76-
var summary = CanExecute(benchmarkType, config);
106+
var summary = CanExecute<SimpleBenchmark>(config);
77107

78-
for (int i = 0; i < diagnosers.Length; i++)
108+
foreach (var diagnoser in diagnosers)
79109
{
80-
var diagnoser = diagnosers[i];
81-
var shouldHaveResult = shouldHaveResults[i];
110+
bool shouldHaveResults = diagnoser.DiagnoserRunMode != RunMode.None;
82111

83-
if (shouldHaveResult)
112+
if (shouldHaveResults)
84113
{
85114
Assert.NotEmpty(diagnoser.Results);
86115
Assert.Equal(summary.BenchmarksCases.Length, diagnoser.Results.Count);
@@ -91,21 +120,18 @@ public void MultipleInProcessDiagnosersWork(BaseMockInProcessDiagnoser[] diagnos
91120
Assert.Empty(diagnoser.Results);
92121
}
93122
}
123+
}
94124

95-
// For multiple benchmarks, verify all benchmark methods are present
96-
if (benchmarkType == typeof(MultipleBenchmarks))
125+
private IConfig CreateConfig(OutputLogger logger, IToolchain toolchain)
126+
{
127+
var job = Job.Dry;
128+
if (toolchain != null)
97129
{
98-
var benchmarkMethods = summary.BenchmarksCases.Select(bc => bc.Descriptor.WorkloadMethod.Name).ToList();
99-
Assert.Contains("Benchmark1", benchmarkMethods);
100-
Assert.Contains("Benchmark2", benchmarkMethods);
101-
Assert.Contains("Benchmark3", benchmarkMethods);
130+
job = job.WithToolchain(toolchain);
102131
}
103-
}
104132

105-
private IConfig CreateInProcessConfig(OutputLogger logger)
106-
{
107133
return new ManualConfig()
108-
.AddJob(Job.Dry.WithToolchain(InProcessEmitToolchain.DontLogOutput))
134+
.AddJob(job)
109135
.AddLogger(logger)
110136
.AddColumnProvider(DefaultColumnProviders.Instance);
111137
}

0 commit comments

Comments
 (0)