Skip to content

Commit 41b812a

Browse files
Copilottimcassell
andcommitted
Fix handler timing verification to use serialized results
Co-authored-by: timcassell <35501420+timcassell@users.noreply.github.com>
1 parent 709242a commit 41b812a

File tree

2 files changed

+69
-18
lines changed

2 files changed

+69
-18
lines changed

tests/BenchmarkDotNet.IntegrationTests/Diagnosers/MockInProcessDiagnoser.cs

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,17 @@
66
using BenchmarkDotNet.Reports;
77
using BenchmarkDotNet.Running;
88
using BenchmarkDotNet.Validators;
9-
using System.Collections.Generic;
109
using System;
10+
using System.Collections.Generic;
11+
using System.Linq;
1112

1213
namespace BenchmarkDotNet.IntegrationTests.Diagnosers;
1314

1415
public abstract class BaseMockInProcessDiagnoser : IInProcessDiagnoser
1516
{
1617
public Dictionary<BenchmarkCase, string> Results { get; } = [];
1718
public Dictionary<BenchmarkCase, List<BenchmarkSignal>> HandlerSignals { get; } = [];
19+
public Dictionary<BenchmarkCase, DateTime> FirstSignalTimes { get; } = [];
1820

1921
public abstract string DiagnoserName { get; }
2022
public abstract RunMode DiagnoserRunMode { get; }
@@ -43,29 +45,44 @@ public void Handle(HostSignal signal, DiagnoserActionParameters parameters) { }
4345
var (handlerType, serializedConfig) = GetSeparateProcessHandlerTypeAndSerializedConfig(benchmarkCase);
4446
if (handlerType == null)
4547
return null;
46-
var handler = (BaseMockInProcessDiagnoserHandler)Activator.CreateInstance(handlerType);
48+
var handler = (IInProcessDiagnoserHandler)Activator.CreateInstance(handlerType);
4749
handler.Initialize(serializedConfig);
48-
handler.SetDiagnoser(this, benchmarkCase);
4950
return handler;
5051
}
5152

52-
public void DeserializeResults(BenchmarkCase benchmarkCase, string results) => Results.Add(benchmarkCase, results);
53-
54-
internal void RecordSignal(BenchmarkCase benchmarkCase, BenchmarkSignal signal)
53+
public void DeserializeResults(BenchmarkCase benchmarkCase, string results)
5554
{
56-
if (!HandlerSignals.ContainsKey(benchmarkCase))
55+
// Parse the serialized results: "result|signals|timestamp"
56+
var parts = results.Split('|');
57+
var actualResult = parts[0];
58+
Results.Add(benchmarkCase, actualResult);
59+
60+
if (parts.Length >= 3)
5761
{
58-
HandlerSignals[benchmarkCase] = [];
62+
// Parse signals
63+
var signalsString = parts[1];
64+
if (!string.IsNullOrEmpty(signalsString))
65+
{
66+
var signals = signalsString.Split(',')
67+
.Select(s => Enum.Parse<BenchmarkSignal>(s))
68+
.ToList();
69+
HandlerSignals[benchmarkCase] = signals;
70+
}
71+
72+
// Parse timestamp
73+
if (long.TryParse(parts[2], out var ticks))
74+
{
75+
FirstSignalTimes[benchmarkCase] = new DateTime(ticks, DateTimeKind.Utc);
76+
}
5977
}
60-
HandlerSignals[benchmarkCase].Add(signal);
6178
}
6279
}
6380

6481
public abstract class BaseMockInProcessDiagnoserHandler : IInProcessDiagnoserHandler
6582
{
6683
private string _result;
67-
private BaseMockInProcessDiagnoser _diagnoser;
68-
private BenchmarkCase _benchmarkCase;
84+
private readonly List<BenchmarkSignal> _signals = [];
85+
private DateTime _firstSignalTime;
6986

7087
protected BaseMockInProcessDiagnoserHandler() { }
7188

@@ -74,18 +91,22 @@ public void Initialize(string? serializedConfig)
7491
_result = serializedConfig ?? string.Empty;
7592
}
7693

77-
internal void SetDiagnoser(BaseMockInProcessDiagnoser diagnoser, BenchmarkCase benchmarkCase)
94+
public void Handle(BenchmarkSignal signal, InProcessDiagnoserActionArgs args)
7895
{
79-
_diagnoser = diagnoser;
80-
_benchmarkCase = benchmarkCase;
96+
if (_signals.Count == 0)
97+
{
98+
_firstSignalTime = DateTime.UtcNow;
99+
}
100+
_signals.Add(signal);
81101
}
82102

83-
public void Handle(BenchmarkSignal signal, InProcessDiagnoserActionArgs args)
103+
public string SerializeResults()
84104
{
85-
_diagnoser?.RecordSignal(_benchmarkCase, signal);
105+
// Encode the result with timing and signal information
106+
var signalsString = string.Join(",", _signals);
107+
var timestamp = _firstSignalTime.Ticks;
108+
return $"{_result}|{signalsString}|{timestamp}";
86109
}
87-
88-
public string SerializeResults() => _result;
89110
}
90111

91112
public sealed class MockInProcessDiagnoser : BaseMockInProcessDiagnoser

tests/BenchmarkDotNet.IntegrationTests/InProcessDiagnoserTests.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,36 @@ public void MultipleInProcessDiagnosersWork(BaseMockInProcessDiagnoser[] diagnos
142142
Assert.Empty(diagnoser.HandlerSignals); // None should not have any signals
143143
}
144144
}
145+
146+
// Verify timing: NoOverhead diagnosers should complete before ExtraRun diagnosers
147+
var noOverheadDiagnosers = diagnosers.Where(d => d.DiagnoserRunMode == RunMode.NoOverhead).ToList();
148+
var extraRunDiagnosers = diagnosers.Where(d => d.DiagnoserRunMode == RunMode.ExtraRun).ToList();
149+
150+
if (noOverheadDiagnosers.Any() && extraRunDiagnosers.Any())
151+
{
152+
foreach (var benchmarkCase in summary.BenchmarksCases)
153+
{
154+
var noOverheadTimes = noOverheadDiagnosers
155+
.Where(d => d.FirstSignalTimes.ContainsKey(benchmarkCase))
156+
.Select(d => d.FirstSignalTimes[benchmarkCase])
157+
.ToList();
158+
159+
var extraRunTimes = extraRunDiagnosers
160+
.Where(d => d.FirstSignalTimes.ContainsKey(benchmarkCase))
161+
.Select(d => d.FirstSignalTimes[benchmarkCase])
162+
.ToList();
163+
164+
if (noOverheadTimes.Any() && extraRunTimes.Any())
165+
{
166+
var latestNoOverhead = noOverheadTimes.Max();
167+
var earliestExtraRun = extraRunTimes.Min();
168+
169+
Assert.True(latestNoOverhead <= earliestExtraRun,
170+
$"NoOverhead diagnosers should complete before ExtraRun diagnosers. " +
171+
$"Latest NoOverhead: {latestNoOverhead:O}, Earliest ExtraRun: {earliestExtraRun:O}");
172+
}
173+
}
174+
}
145175
}
146176

147177
private IConfig CreateConfig(OutputLogger logger, IToolchain toolchain)

0 commit comments

Comments
 (0)