From 7d8daf212d8705b34133dc60f4fa17e5fb2956af Mon Sep 17 00:00:00 2001 From: Drew Scoggins Date: Wed, 19 Aug 2026 11:34:11 -0700 Subject: [PATCH 1/4] Add PerfLab counter metadata contract Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Reporting.Tests/ReporterTests.cs | 200 +++++++++++++++++- src/tools/Reporting/Reporting/Counter.cs | 76 ++++++- src/tools/Reporting/Reporting/Reporter.cs | 19 +- src/tools/Reporting/Reporting/Test.cs | 39 ++++ 4 files changed, 329 insertions(+), 5 deletions(-) diff --git a/src/tools/Reporting/Reporting.Tests/ReporterTests.cs b/src/tools/Reporting/Reporting.Tests/ReporterTests.cs index ee649e5df84..68d27be2b1b 100644 --- a/src/tools/Reporting/Reporting.Tests/ReporterTests.cs +++ b/src/tools/Reporting/Reporting.Tests/ReporterTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using System; using System.Runtime.InteropServices; using Xunit; @@ -57,6 +58,7 @@ public void WriteReportTableWithEmptyResults() new Counter { DefaultCounter = true, + TopCounter = true, MetricName = "ns", Name = "CounterName", Results = [] @@ -79,6 +81,7 @@ public void WriteReportTableWithNullResults() new Counter { DefaultCounter = true, + TopCounter = true, MetricName = "ns", Name = "CounterName", Results = null @@ -168,11 +171,201 @@ public void JsonCanBeGenerated() Assert.Equal(1.1, retCounter.Results[0]); } + [Fact] + public void LegacyCounterJsonRemainsCompatible() + { + var reporter = GetReporterWithSpecifiedEnvironment(new PerfLabEnvironmentProviderMock()); + var jsonString = reporter.GetJson(); + var jsonCounter = JObject.Parse(jsonString)["tests"][0]["counters"][0]; + + Assert.Equal(JTokenType.Boolean, jsonCounter["higherIsBetter"].Type); + Assert.False(jsonCounter["higherIsBetter"].Value()); + Assert.Null(jsonCounter["regressionThreshold"]); + Assert.Null(jsonCounter["direction"]); + + var deserialized = JsonConvert.DeserializeObject(jsonString); + Assert.False(deserialized.Tests[0].Counters[0].HigherIsBetter); + Assert.Equal(CounterDirection.LowerIsBetter, deserialized.Tests[0].Counters[0].Direction); + Assert.Null(deserialized.Tests[0].Counters[0].RegressionThreshold); + } + + [Fact] + public void RegressionThresholdIsSerialized() + { + var reporter = GetReporterWithSpecifiedEnvironment(new PerfLabEnvironmentProviderMock()); + reporter.Tests[0].Counters[0].RegressionThreshold = 0.02; + + var jsonString = reporter.GetJson(); + var jsonCounter = JObject.Parse(jsonString)["tests"][0]["counters"][0]; + Assert.Equal(0.02, jsonCounter["regressionThreshold"].Value()); + + var deserialized = JsonConvert.DeserializeObject(jsonString); + Assert.Equal(0.02, deserialized.Tests[0].Counters[0].RegressionThreshold); + } + + [Fact] + public void UnknownDirectionIsSerializedForNonTopCounter() + { + var reporter = GetReporterWithSpecifiedEnvironment(new PerfLabEnvironmentProviderMock()); + reporter.Tests[0].AddCounter(new Counter + { + Name = "Storage only", + Direction = CounterDirection.Unknown, + MetricName = "value", + Results = [2.0] + }); + + var jsonString = reporter.GetJson(); + var jsonCounter = JObject.Parse(jsonString)["tests"][0]["counters"][1]; + Assert.Equal(JTokenType.Null, jsonCounter["higherIsBetter"].Type); + + var deserialized = JsonConvert.DeserializeObject(jsonString); + var counter = deserialized.Tests[0].Counters[1]; + Assert.Equal(CounterDirection.Unknown, counter.Direction); + Assert.Throws(() => counter.HigherIsBetter); + } + + [Theory] + [InlineData(0)] + [InlineData(-0.01)] + [InlineData(1.01)] + [InlineData(double.NaN)] + [InlineData(double.PositiveInfinity)] + [InlineData(double.NegativeInfinity)] + public void InvalidRegressionThresholdIsRejected(double threshold) + { + Assert.Throws(() => new Counter { RegressionThreshold = threshold }); + } + + [Theory] + [InlineData(true, true)] + [InlineData(false, true)] + public void UnknownDirectionIsRejectedForDefaultOrTopCounter(bool defaultCounter, bool topCounter) + { + var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); + var test = new Test + { + Counters = [ + new Counter + { + DefaultCounter = defaultCounter, + TopCounter = topCounter, + Direction = CounterDirection.Unknown + } + ] + }; + + reporter.AddTest(test); + Assert.Throws(() => reporter.GetJson()); + } + + [Fact] + public void SerializationRejectsTestWithoutDefaultCounter() + { + var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); + reporter.AddTest(new Test + { + Counters = [ + new Counter + { + Name = "Not default" + } + ] + }); + + Assert.Throws(() => reporter.GetJson()); + } + + [Fact] + public void SerializationRejectsDirectlyAssignedMultipleDefaultCounters() + { + var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); + reporter.AddTest(new Test + { + Counters = [ + new Counter + { + Name = "First", + DefaultCounter = true, + TopCounter = true + }, + new Counter + { + Name = "Second", + DefaultCounter = true, + TopCounter = true + } + ] + }); + + Assert.Throws(() => reporter.GetJson()); + } + + [Fact] + public void DeserializationRejectsMultipleDefaultCounters() + { + const string json = +@"{ + ""name"": ""Test"", + ""counters"": [ + { + ""name"": ""First"", + ""topCounter"": true, + ""defaultCounter"": true, + ""higherIsBetter"": false + }, + { + ""name"": ""Second"", + ""topCounter"": true, + ""defaultCounter"": true, + ""higherIsBetter"": false + } + ] +}"; + + var exception = Assert.ThrowsAny(() => JsonConvert.DeserializeObject(json)); + Assert.IsType(exception.GetBaseException()); + } + + [Fact] + public void AddCounterRejectsDefaultCounterThatIsNotTop() + { + var test = new Test(); + + Assert.Throws(() => test.AddCounter(new Counter + { + DefaultCounter = true + })); + } + + [Fact] + public void SerializationRejectsDirectlyAssignedDuplicateCounterNames() + { + var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); + reporter.AddTest(new Test + { + Counters = [ + new Counter + { + Name = "Duplicate", + DefaultCounter = true, + TopCounter = true + }, + new Counter + { + Name = "Duplicate" + } + ] + }); + + Assert.Throws(() => reporter.GetJson()); + } + [Fact] public void EnforceDefaultCounterConstraint() { var t = new Test(); - var c = new Counter { DefaultCounter = true }; + var c = new Counter { DefaultCounter = true, TopCounter = true }; t.AddCounter(c); Assert.Throws(() => t.AddCounter(c)); } @@ -199,9 +392,9 @@ public void EnforceUniqueCounterName() public void AddCountersEnumerable() { var t = new Test(); - var c1 = new Counter { Name = "Counter1", DefaultCounter = true }; + var c1 = new Counter { Name = "Counter1", DefaultCounter = true, TopCounter = true }; var c2 = new Counter { Name = "Counter2" }; - t.AddCounters([c1, c2]); + t.AddCounters([c2, c1]); Assert.Equal(2, t.Counters.Count); } @@ -216,6 +409,7 @@ private static Reporter GetReporterWithSpecifiedEnvironment(PerfLabEnvironmentPr new Counter { DefaultCounter = true, + TopCounter = true, HigherIsBetter = false, MetricName = "ns", Name = counterName ?? "CounterName", diff --git a/src/tools/Reporting/Reporting/Counter.cs b/src/tools/Reporting/Reporting/Counter.cs index 2a7b8b686f3..2097e7d21dd 100644 --- a/src/tools/Reporting/Reporting/Counter.cs +++ b/src/tools/Reporting/Reporting/Counter.cs @@ -2,23 +2,97 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using Newtonsoft.Json; +using System; using System.Collections.Generic; namespace Reporting; +public enum CounterDirection +{ + Unknown, + LowerIsBetter, + HigherIsBetter +} + public class Counter { + private const double MaximumRegressionThreshold = 1.0; + private CounterDirection _direction = CounterDirection.LowerIsBetter; + private double? _regressionThreshold; + public string Name { get; set; } = "Counter"; public bool TopCounter { get; set; } public bool DefaultCounter { get; set; } - public bool HigherIsBetter { get; set; } + [JsonIgnore] + public bool HigherIsBetter + { + get => _direction switch + { + CounterDirection.HigherIsBetter => true, + CounterDirection.LowerIsBetter => false, + _ => throw new InvalidOperationException($"Counter '{Name}' has an unknown direction.") + }; + set => _direction = value ? CounterDirection.HigherIsBetter : CounterDirection.LowerIsBetter; + } + + [JsonIgnore] + public CounterDirection Direction + { + get => _direction; + set + { + if (value is not CounterDirection.Unknown and not CounterDirection.LowerIsBetter and not CounterDirection.HigherIsBetter) + { + throw new ArgumentOutOfRangeException(nameof(value), value, "Unknown counter direction."); + } + + _direction = value; + } + } + + [JsonProperty("higherIsBetter", NullValueHandling = NullValueHandling.Include)] + private bool? SerializedHigherIsBetter + { + get => _direction == CounterDirection.Unknown ? null : _direction == CounterDirection.HigherIsBetter; + set => _direction = value switch + { + true => CounterDirection.HigherIsBetter, + false => CounterDirection.LowerIsBetter, + null => CounterDirection.Unknown + }; + } public string MetricName { get; set; } = "Count"; + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] + public double? RegressionThreshold + { + get => _regressionThreshold; + set + { + if (value is double threshold && + (double.IsNaN(threshold) || double.IsInfinity(threshold) || threshold <= 0 || threshold > MaximumRegressionThreshold)) + { + throw new ArgumentOutOfRangeException(nameof(value), value, $"Regression threshold must be finite and in the range (0, {MaximumRegressionThreshold}]."); + } + + _regressionThreshold = value; + } + } + public IList? Results { get; set; } + internal void Validate() + { + if (_direction == CounterDirection.Unknown && (DefaultCounter || TopCounter)) + { + throw new InvalidOperationException($"Counter '{Name}' must have a known direction when it is a default or top counter."); + } + } + public override string ToString() => $"{nameof(Name)}: {Name}, {nameof(TopCounter)}: {TopCounter}, {nameof(DefaultCounter)}: {DefaultCounter}, {nameof(MetricName)}: {MetricName}"; } diff --git a/src/tools/Reporting/Reporting/Reporter.cs b/src/tools/Reporting/Reporting/Reporter.cs index a405e6e5096..1a2346fedb4 100644 --- a/src/tools/Reporting/Reporting/Reporter.cs +++ b/src/tools/Reporting/Reporting/Reporter.cs @@ -51,6 +51,7 @@ public Reporter(Build build, Os os, Run run, List tests) Os = os; Run = run; Tests = tests; + ValidateTests(); InLab = new EnvironmentProvider().IsLabEnvironment(); } @@ -65,7 +66,15 @@ public void AddTest(Test test) } public string? GetJson() - => InLab ? JsonConvert.SerializeObject(this, Formatting.Indented, _jsonSerializerSettings) : null; + { + if (!InLab) + { + return null; + } + + ValidateTests(); + return JsonConvert.SerializeObject(this, Formatting.Indented, _jsonSerializerSettings); + } public string WriteResultTable() { @@ -258,4 +267,12 @@ private static string PrintCounter(Counter counter, int counterWidth, int result var min = ((FormattableString)$"{counter.Results.Min():F3} {counter.MetricName}").ToString(_culture); return $"{LeftJustify(counter.Name, counterWidth)}|{LeftJustify(average, resultWidth)}|{LeftJustify(min, resultWidth)}|{LeftJustify(max, resultWidth)}"; } + + private void ValidateTests() + { + foreach (var test in Tests) + { + test.Validate(); + } + } } diff --git a/src/tools/Reporting/Reporting/Test.cs b/src/tools/Reporting/Reporting/Test.cs index 44464b06eee..7d01ed08807 100644 --- a/src/tools/Reporting/Reporting/Test.cs +++ b/src/tools/Reporting/Reporting/Test.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.Serialization; namespace Reporting; @@ -20,6 +21,13 @@ public class Test public void AddCounter(Counter counter) { + counter.Validate(); + + if (counter.DefaultCounter && !counter.TopCounter) + { + throw new InvalidOperationException($"Default counter '{counter.Name}' must also be a top counter."); + } + if (counter.DefaultCounter && Counters.Any(c => c.DefaultCounter)) { throw new Exception($"Duplicate default counter, name: ${counter.Name}"); @@ -40,4 +48,35 @@ public void AddCounters(IEnumerable counters) AddCounter(counter); } } + + internal void Validate() + { + var defaultCounters = Counters.Where(c => c.DefaultCounter).ToList(); + if (defaultCounters.Count != 1) + { + throw new InvalidOperationException($"Test '{Name}' must have exactly one default counter, but found {defaultCounters.Count}."); + } + + if (!defaultCounters[0].TopCounter) + { + throw new InvalidOperationException($"Default counter '{defaultCounters[0].Name}' must also be a top counter."); + } + + var duplicateCounter = Counters.GroupBy(c => c.Name).FirstOrDefault(group => group.Count() > 1); + if (duplicateCounter is not null) + { + throw new InvalidOperationException($"Duplicate counter name, name: ${duplicateCounter.Key}"); + } + + foreach (var counter in Counters) + { + counter.Validate(); + } + } + + [OnSerializing] + private void OnSerializing(StreamingContext _) => Validate(); + + [OnDeserialized] + private void OnDeserialized(StreamingContext _) => Validate(); } From ed56fdefd6e25409ceebda4f961f110889dab173 Mon Sep 17 00:00:00 2001 From: Drew Scoggins Date: Wed, 19 Aug 2026 15:04:10 -0700 Subject: [PATCH 2/4] Validate PerfLab counter metric names Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Reporting.Tests/ReporterTests.cs | 111 +++++++++++++++++- src/tools/Reporting/Reporting/Counter.cs | 7 +- src/tools/Reporting/Reporting/Reporter.cs | 6 +- 3 files changed, 117 insertions(+), 7 deletions(-) diff --git a/src/tools/Reporting/Reporting.Tests/ReporterTests.cs b/src/tools/Reporting/Reporting.Tests/ReporterTests.cs index 68d27be2b1b..f30a8ade4a0 100644 --- a/src/tools/Reporting/Reporting.Tests/ReporterTests.cs +++ b/src/tools/Reporting/Reporting.Tests/ReporterTests.cs @@ -2,10 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; using System; using System.Runtime.InteropServices; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using Xunit; namespace Reporting.Tests; @@ -122,7 +122,7 @@ public void WriteReportTableWithoutEnvironment() public void WriteReportWithLongNameTableWithoutEnvironment() { PerfLabEnvironmentProviderMock environment = new NonPerfLabEnvironmentProviderMock(); - var reporter = GetReporterWithSpecifiedEnvironment(environment, counterName:"ThisIsALongerCounterName"); + var reporter = GetReporterWithSpecifiedEnvironment(environment, counterName: "ThisIsALongerCounterName"); var table = reporter.WriteResultTable(); Assert.Equal(LongCounterNameTable, table); } @@ -259,6 +259,111 @@ public void UnknownDirectionIsRejectedForDefaultOrTopCounter(bool defaultCounter Assert.Throws(() => reporter.GetJson()); } + [Theory] + [InlineData(null, true)] + [InlineData("", true)] + [InlineData(" ", true)] + [InlineData(null, false)] + [InlineData("", false)] + [InlineData(" ", false)] + public void AddCounterRejectsBlankMetricNameForDefaultOrTopCounter(string metricName, bool defaultCounter) + { + var test = new Test(); + + Assert.Throws(() => test.AddCounter(new Counter + { + DefaultCounter = defaultCounter, + TopCounter = true, + MetricName = metricName + })); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void SerializationRejectsDirectlyAssignedBlankMetricName(string metricName) + { + var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); + reporter.AddTest(new Test + { + Counters = [ + new Counter + { + DefaultCounter = true, + TopCounter = true, + MetricName = metricName + } + ] + }); + + Assert.Throws(() => reporter.GetJson()); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void DeserializationRejectsBlankMetricName(string metricName) + { + var json = new JObject + { + ["name"] = "Test", + ["counters"] = new JArray + { + new JObject + { + ["name"] = "Default", + ["topCounter"] = true, + ["defaultCounter"] = true, + ["higherIsBetter"] = false, + ["metricName"] = metricName is null ? JValue.CreateNull() : new JValue(metricName) + } + } + }.ToString(); + + var exception = Assert.ThrowsAny(() => JsonConvert.DeserializeObject(json)); + Assert.IsType(exception.GetBaseException()); + } + + [Fact] + public void ValidMetricNameRoundTrips() + { + var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); + var test = new Test(); + test.AddCounter(new Counter + { + Name = "Default", + DefaultCounter = true, + TopCounter = true, + MetricName = "ms" + }); + reporter.AddTest(test); + + var deserialized = JsonConvert.DeserializeObject(reporter.GetJson()); + Assert.Equal("ms", deserialized.Tests[0].Counters[0].MetricName); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void NonTopCounterAllowsBlankMetricName(string metricName) + { + var reporter = GetReporterWithSpecifiedEnvironment(new PerfLabEnvironmentProviderMock()); + reporter.Tests[0].AddCounter(new Counter + { + Name = "Storage only", + MetricName = metricName, + Results = [2.0] + }); + + var json = reporter.GetJson(); + var deserialized = JsonConvert.DeserializeObject(json); + Assert.Equal(metricName, deserialized.Tests[0].Counters[1].MetricName); + Assert.Contains("Storage only", reporter.WriteResultTable()); + } + [Fact] public void SerializationRejectsTestWithoutDefaultCounter() { diff --git a/src/tools/Reporting/Reporting/Counter.cs b/src/tools/Reporting/Reporting/Counter.cs index 2097e7d21dd..cc10053be2a 100644 --- a/src/tools/Reporting/Reporting/Counter.cs +++ b/src/tools/Reporting/Reporting/Counter.cs @@ -2,9 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Newtonsoft.Json; using System; using System.Collections.Generic; +using Newtonsoft.Json; namespace Reporting; @@ -92,6 +92,11 @@ internal void Validate() { throw new InvalidOperationException($"Counter '{Name}' must have a known direction when it is a default or top counter."); } + + if ((DefaultCounter || TopCounter) && string.IsNullOrWhiteSpace(MetricName)) + { + throw new InvalidOperationException($"Counter '{Name}' must have a nonblank metric name when it is a default or top counter."); + } } public override string ToString() => $"{nameof(Name)}: {Name}, {nameof(TopCounter)}: {TopCounter}, {nameof(DefaultCounter)}: {DefaultCounter}, {nameof(MetricName)}: {MetricName}"; diff --git a/src/tools/Reporting/Reporting/Reporter.cs b/src/tools/Reporting/Reporting/Reporter.cs index 1a2346fedb4..718c2bd40ee 100644 --- a/src/tools/Reporting/Reporting/Reporter.cs +++ b/src/tools/Reporting/Reporting/Reporter.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Newtonsoft.Json; -using Newtonsoft.Json.Serialization; using System; using System.Collections; using System.Collections.Generic; @@ -12,6 +10,8 @@ using System.Linq; using System.Runtime.InteropServices; using System.Text; +using Newtonsoft.Json; +using Newtonsoft.Json.Serialization; using RuntimeEnvironment = Microsoft.DotNet.PlatformAbstractions.RuntimeEnvironment; namespace Reporting; @@ -90,7 +90,7 @@ public string WriteResultTable() var countersWithResults = test.Counters.Where(c => c.Results != null && c.Results.Count > 0); var counterWidth = Math.Max(test.Counters.Max(c => c.Name.Length) + 1, 15); - var resultWidth = Math.Max(countersWithResults.Max(c => c.Results.Max().ToString("F3", _culture).Length + c.MetricName.Length) + 2, 15); + var resultWidth = Math.Max(countersWithResults.Max(c => c.Results.Max().ToString("F3", _culture).Length + (c.MetricName?.Length ?? 0)) + 2, 15); ret.AppendLine(test.Name); ret.AppendLine($"{LeftJustify("Metric", counterWidth)}|{LeftJustify("Average", resultWidth)}|{LeftJustify("Min", resultWidth)}|{LeftJustify("Max", resultWidth)}"); ret.AppendLine($"{new string('-', counterWidth)}|{new string('-', resultWidth)}|{new string('-', resultWidth)}|{new string('-', resultWidth)}"); From a04ce13733fca6053112ecd05c832bf3180f200f Mon Sep 17 00:00:00 2001 From: Drew Scoggins Date: Mon, 31 Aug 2026 10:39:40 -0700 Subject: [PATCH 3/4] Simplify PerfLab counter metadata Keep HigherIsBetter as the existing boolean contract, retain only the optional per-counter regression threshold, and migrate Reporting JSON serialization to System.Text.Json. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f713269-e30d-4770-8a91-3fc53109abea --- src/tools/Reporting/Directory.Packages.props | 2 +- .../Reporting.Tests/ReporterTests.cs | 313 ++---------------- src/tools/Reporting/Reporting/Counter.cs | 83 +---- src/tools/Reporting/Reporting/Reporter.cs | 24 +- .../Reporting/Reporting/Reporting.csproj | 2 +- src/tools/Reporting/Reporting/Test.cs | 39 --- 6 files changed, 44 insertions(+), 419 deletions(-) diff --git a/src/tools/Reporting/Directory.Packages.props b/src/tools/Reporting/Directory.Packages.props index 0fde0cf995d..234b485b92a 100644 --- a/src/tools/Reporting/Directory.Packages.props +++ b/src/tools/Reporting/Directory.Packages.props @@ -4,7 +4,7 @@ true - + diff --git a/src/tools/Reporting/Reporting.Tests/ReporterTests.cs b/src/tools/Reporting/Reporting.Tests/ReporterTests.cs index f30a8ade4a0..83d0097c8e5 100644 --- a/src/tools/Reporting/Reporting.Tests/ReporterTests.cs +++ b/src/tools/Reporting/Reporting.Tests/ReporterTests.cs @@ -4,14 +4,18 @@ using System; using System.Runtime.InteropServices; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; +using System.Text.Json; using Xunit; namespace Reporting.Tests; public class ReporterTests { + private static readonly JsonSerializerOptions s_jsonSerializerOptions = new() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase + }; + // this matches the output from the reporter made in GetReporterWithSpecifiedEnvironment private const string ExpectedTestTable = @"TestName @@ -31,7 +35,6 @@ public class ReporterTests ---------------|-------------------------|-------------------------|------------------------- CounterName |10000000000000000.000 ns |10000000000000000.000 ns |10000000000000000.000 ns "; - private const string NoResultsTable = @"TestName No results in file. @@ -142,8 +145,9 @@ public void JsonCanBeGenerated() var environment = new PerfLabEnvironmentProviderMock(); var reporter = GetReporterWithSpecifiedEnvironment(environment); var jsonString = reporter.GetJson(); + Assert.NotNull(jsonString); - var jsonObj = JsonConvert.DeserializeObject(jsonString); + var jsonObj = DeserializeReporter(jsonString); Assert.Equal(environment.GetEnvironmentVariable("HELIX_CORRELATION_ID"), jsonObj.Run.CorrelationId); Assert.Equal(environment.GetEnvironmentVariable("HELIX_WORKITEM_FRIENDLYNAME"), jsonObj.Run.WorkItemName); @@ -176,16 +180,17 @@ public void LegacyCounterJsonRemainsCompatible() { var reporter = GetReporterWithSpecifiedEnvironment(new PerfLabEnvironmentProviderMock()); var jsonString = reporter.GetJson(); - var jsonCounter = JObject.Parse(jsonString)["tests"][0]["counters"][0]; + Assert.NotNull(jsonString); + + using var document = JsonDocument.Parse(jsonString); + var jsonCounter = document.RootElement.GetProperty("tests")[0].GetProperty("counters")[0]; - Assert.Equal(JTokenType.Boolean, jsonCounter["higherIsBetter"].Type); - Assert.False(jsonCounter["higherIsBetter"].Value()); - Assert.Null(jsonCounter["regressionThreshold"]); - Assert.Null(jsonCounter["direction"]); + Assert.Equal(JsonValueKind.False, jsonCounter.GetProperty("higherIsBetter").ValueKind); + Assert.False(jsonCounter.TryGetProperty("regressionThreshold", out _)); + Assert.False(jsonCounter.TryGetProperty("direction", out _)); - var deserialized = JsonConvert.DeserializeObject(jsonString); + var deserialized = DeserializeReporter(jsonString); Assert.False(deserialized.Tests[0].Counters[0].HigherIsBetter); - Assert.Equal(CounterDirection.LowerIsBetter, deserialized.Tests[0].Counters[0].Direction); Assert.Null(deserialized.Tests[0].Counters[0].RegressionThreshold); } @@ -196,281 +201,21 @@ public void RegressionThresholdIsSerialized() reporter.Tests[0].Counters[0].RegressionThreshold = 0.02; var jsonString = reporter.GetJson(); - var jsonCounter = JObject.Parse(jsonString)["tests"][0]["counters"][0]; - Assert.Equal(0.02, jsonCounter["regressionThreshold"].Value()); - - var deserialized = JsonConvert.DeserializeObject(jsonString); - Assert.Equal(0.02, deserialized.Tests[0].Counters[0].RegressionThreshold); - } - - [Fact] - public void UnknownDirectionIsSerializedForNonTopCounter() - { - var reporter = GetReporterWithSpecifiedEnvironment(new PerfLabEnvironmentProviderMock()); - reporter.Tests[0].AddCounter(new Counter - { - Name = "Storage only", - Direction = CounterDirection.Unknown, - MetricName = "value", - Results = [2.0] - }); - - var jsonString = reporter.GetJson(); - var jsonCounter = JObject.Parse(jsonString)["tests"][0]["counters"][1]; - Assert.Equal(JTokenType.Null, jsonCounter["higherIsBetter"].Type); - - var deserialized = JsonConvert.DeserializeObject(jsonString); - var counter = deserialized.Tests[0].Counters[1]; - Assert.Equal(CounterDirection.Unknown, counter.Direction); - Assert.Throws(() => counter.HigherIsBetter); - } - - [Theory] - [InlineData(0)] - [InlineData(-0.01)] - [InlineData(1.01)] - [InlineData(double.NaN)] - [InlineData(double.PositiveInfinity)] - [InlineData(double.NegativeInfinity)] - public void InvalidRegressionThresholdIsRejected(double threshold) - { - Assert.Throws(() => new Counter { RegressionThreshold = threshold }); - } - - [Theory] - [InlineData(true, true)] - [InlineData(false, true)] - public void UnknownDirectionIsRejectedForDefaultOrTopCounter(bool defaultCounter, bool topCounter) - { - var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); - var test = new Test - { - Counters = [ - new Counter - { - DefaultCounter = defaultCounter, - TopCounter = topCounter, - Direction = CounterDirection.Unknown - } - ] - }; - - reporter.AddTest(test); - Assert.Throws(() => reporter.GetJson()); - } - - [Theory] - [InlineData(null, true)] - [InlineData("", true)] - [InlineData(" ", true)] - [InlineData(null, false)] - [InlineData("", false)] - [InlineData(" ", false)] - public void AddCounterRejectsBlankMetricNameForDefaultOrTopCounter(string metricName, bool defaultCounter) - { - var test = new Test(); - - Assert.Throws(() => test.AddCounter(new Counter - { - DefaultCounter = defaultCounter, - TopCounter = true, - MetricName = metricName - })); - } - - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - public void SerializationRejectsDirectlyAssignedBlankMetricName(string metricName) - { - var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); - reporter.AddTest(new Test - { - Counters = [ - new Counter - { - DefaultCounter = true, - TopCounter = true, - MetricName = metricName - } - ] - }); - - Assert.Throws(() => reporter.GetJson()); - } - - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - public void DeserializationRejectsBlankMetricName(string metricName) - { - var json = new JObject - { - ["name"] = "Test", - ["counters"] = new JArray - { - new JObject - { - ["name"] = "Default", - ["topCounter"] = true, - ["defaultCounter"] = true, - ["higherIsBetter"] = false, - ["metricName"] = metricName is null ? JValue.CreateNull() : new JValue(metricName) - } - } - }.ToString(); - - var exception = Assert.ThrowsAny(() => JsonConvert.DeserializeObject(json)); - Assert.IsType(exception.GetBaseException()); - } - - [Fact] - public void ValidMetricNameRoundTrips() - { - var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); - var test = new Test(); - test.AddCounter(new Counter - { - Name = "Default", - DefaultCounter = true, - TopCounter = true, - MetricName = "ms" - }); - reporter.AddTest(test); - - var deserialized = JsonConvert.DeserializeObject(reporter.GetJson()); - Assert.Equal("ms", deserialized.Tests[0].Counters[0].MetricName); - } + Assert.NotNull(jsonString); - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - public void NonTopCounterAllowsBlankMetricName(string metricName) - { - var reporter = GetReporterWithSpecifiedEnvironment(new PerfLabEnvironmentProviderMock()); - reporter.Tests[0].AddCounter(new Counter - { - Name = "Storage only", - MetricName = metricName, - Results = [2.0] - }); - - var json = reporter.GetJson(); - var deserialized = JsonConvert.DeserializeObject(json); - Assert.Equal(metricName, deserialized.Tests[0].Counters[1].MetricName); - Assert.Contains("Storage only", reporter.WriteResultTable()); - } - - [Fact] - public void SerializationRejectsTestWithoutDefaultCounter() - { - var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); - reporter.AddTest(new Test - { - Counters = [ - new Counter - { - Name = "Not default" - } - ] - }); - - Assert.Throws(() => reporter.GetJson()); - } + using var document = JsonDocument.Parse(jsonString); + var jsonCounter = document.RootElement.GetProperty("tests")[0].GetProperty("counters")[0]; + Assert.Equal(0.02, jsonCounter.GetProperty("regressionThreshold").GetDouble()); - [Fact] - public void SerializationRejectsDirectlyAssignedMultipleDefaultCounters() - { - var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); - reporter.AddTest(new Test - { - Counters = [ - new Counter - { - Name = "First", - DefaultCounter = true, - TopCounter = true - }, - new Counter - { - Name = "Second", - DefaultCounter = true, - TopCounter = true - } - ] - }); - - Assert.Throws(() => reporter.GetJson()); - } - - [Fact] - public void DeserializationRejectsMultipleDefaultCounters() - { - const string json = -@"{ - ""name"": ""Test"", - ""counters"": [ - { - ""name"": ""First"", - ""topCounter"": true, - ""defaultCounter"": true, - ""higherIsBetter"": false - }, - { - ""name"": ""Second"", - ""topCounter"": true, - ""defaultCounter"": true, - ""higherIsBetter"": false - } - ] -}"; - - var exception = Assert.ThrowsAny(() => JsonConvert.DeserializeObject(json)); - Assert.IsType(exception.GetBaseException()); - } - - [Fact] - public void AddCounterRejectsDefaultCounterThatIsNotTop() - { - var test = new Test(); - - Assert.Throws(() => test.AddCounter(new Counter - { - DefaultCounter = true - })); - } - - [Fact] - public void SerializationRejectsDirectlyAssignedDuplicateCounterNames() - { - var reporter = new Reporter(new PerfLabEnvironmentProviderMock()); - reporter.AddTest(new Test - { - Counters = [ - new Counter - { - Name = "Duplicate", - DefaultCounter = true, - TopCounter = true - }, - new Counter - { - Name = "Duplicate" - } - ] - }); - - Assert.Throws(() => reporter.GetJson()); + var deserialized = DeserializeReporter(jsonString); + Assert.Equal(0.02, deserialized.Tests[0].Counters[0].RegressionThreshold); } [Fact] public void EnforceDefaultCounterConstraint() { var t = new Test(); - var c = new Counter { DefaultCounter = true, TopCounter = true }; + var c = new Counter { DefaultCounter = true }; t.AddCounter(c); Assert.Throws(() => t.AddCounter(c)); } @@ -497,12 +242,19 @@ public void EnforceUniqueCounterName() public void AddCountersEnumerable() { var t = new Test(); - var c1 = new Counter { Name = "Counter1", DefaultCounter = true, TopCounter = true }; + var c1 = new Counter { Name = "Counter1", DefaultCounter = true }; var c2 = new Counter { Name = "Counter2" }; - t.AddCounters([c2, c1]); + t.AddCounters([c1, c2]); Assert.Equal(2, t.Counters.Count); } + private static Reporter DeserializeReporter(string json) + { + var reporter = JsonSerializer.Deserialize(json, s_jsonSerializerOptions); + Assert.NotNull(reporter); + return reporter; + } + private static Reporter GetReporterWithSpecifiedEnvironment(PerfLabEnvironmentProviderMock enviroment, string counterName = null, double result = 1.1) { var reporter = new Reporter(enviroment); @@ -514,7 +266,6 @@ private static Reporter GetReporterWithSpecifiedEnvironment(PerfLabEnvironmentPr new Counter { DefaultCounter = true, - TopCounter = true, HigherIsBetter = false, MetricName = "ns", Name = counterName ?? "CounterName", diff --git a/src/tools/Reporting/Reporting/Counter.cs b/src/tools/Reporting/Reporting/Counter.cs index cc10053be2a..0abc689e925 100644 --- a/src/tools/Reporting/Reporting/Counter.cs +++ b/src/tools/Reporting/Reporting/Counter.cs @@ -2,102 +2,27 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Collections.Generic; -using Newtonsoft.Json; +using System.Text.Json.Serialization; namespace Reporting; -public enum CounterDirection -{ - Unknown, - LowerIsBetter, - HigherIsBetter -} - public class Counter { - private const double MaximumRegressionThreshold = 1.0; - private CounterDirection _direction = CounterDirection.LowerIsBetter; - private double? _regressionThreshold; - public string Name { get; set; } = "Counter"; public bool TopCounter { get; set; } public bool DefaultCounter { get; set; } - [JsonIgnore] - public bool HigherIsBetter - { - get => _direction switch - { - CounterDirection.HigherIsBetter => true, - CounterDirection.LowerIsBetter => false, - _ => throw new InvalidOperationException($"Counter '{Name}' has an unknown direction.") - }; - set => _direction = value ? CounterDirection.HigherIsBetter : CounterDirection.LowerIsBetter; - } - - [JsonIgnore] - public CounterDirection Direction - { - get => _direction; - set - { - if (value is not CounterDirection.Unknown and not CounterDirection.LowerIsBetter and not CounterDirection.HigherIsBetter) - { - throw new ArgumentOutOfRangeException(nameof(value), value, "Unknown counter direction."); - } - - _direction = value; - } - } - - [JsonProperty("higherIsBetter", NullValueHandling = NullValueHandling.Include)] - private bool? SerializedHigherIsBetter - { - get => _direction == CounterDirection.Unknown ? null : _direction == CounterDirection.HigherIsBetter; - set => _direction = value switch - { - true => CounterDirection.HigherIsBetter, - false => CounterDirection.LowerIsBetter, - null => CounterDirection.Unknown - }; - } + public bool HigherIsBetter { get; set; } public string MetricName { get; set; } = "Count"; - [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] - public double? RegressionThreshold - { - get => _regressionThreshold; - set - { - if (value is double threshold && - (double.IsNaN(threshold) || double.IsInfinity(threshold) || threshold <= 0 || threshold > MaximumRegressionThreshold)) - { - throw new ArgumentOutOfRangeException(nameof(value), value, $"Regression threshold must be finite and in the range (0, {MaximumRegressionThreshold}]."); - } - - _regressionThreshold = value; - } - } + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public double? RegressionThreshold { get; set; } public IList? Results { get; set; } - internal void Validate() - { - if (_direction == CounterDirection.Unknown && (DefaultCounter || TopCounter)) - { - throw new InvalidOperationException($"Counter '{Name}' must have a known direction when it is a default or top counter."); - } - - if ((DefaultCounter || TopCounter) && string.IsNullOrWhiteSpace(MetricName)) - { - throw new InvalidOperationException($"Counter '{Name}' must have a nonblank metric name when it is a default or top counter."); - } - } - public override string ToString() => $"{nameof(Name)}: {Name}, {nameof(TopCounter)}: {TopCounter}, {nameof(DefaultCounter)}: {DefaultCounter}, {nameof(MetricName)}: {MetricName}"; } diff --git a/src/tools/Reporting/Reporting/Reporter.cs b/src/tools/Reporting/Reporting/Reporter.cs index 718c2bd40ee..207f228af37 100644 --- a/src/tools/Reporting/Reporting/Reporter.cs +++ b/src/tools/Reporting/Reporting/Reporter.cs @@ -10,8 +10,8 @@ using System.Linq; using System.Runtime.InteropServices; using System.Text; -using Newtonsoft.Json; -using Newtonsoft.Json.Serialization; +using System.Text.Json; +using System.Text.Json.Serialization; using RuntimeEnvironment = Microsoft.DotNet.PlatformAbstractions.RuntimeEnvironment; namespace Reporting; @@ -19,13 +19,10 @@ namespace Reporting; public class Reporter { private readonly static CultureInfo _culture = CultureInfo.InvariantCulture; - private readonly static JsonSerializerSettings _jsonSerializerSettings = new() + private readonly static JsonSerializerOptions _jsonSerializerOptions = new() { - ContractResolver = new DefaultContractResolver - { - NamingStrategy = new CamelCaseNamingStrategy() { ProcessDictionaryKeys = false } - }, - Culture = _culture + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + WriteIndented = true }; public List Tests { get; private set; } = []; @@ -51,7 +48,6 @@ public Reporter(Build build, Os os, Run run, List tests) Os = os; Run = run; Tests = tests; - ValidateTests(); InLab = new EnvironmentProvider().IsLabEnvironment(); } @@ -72,8 +68,7 @@ public void AddTest(Test test) return null; } - ValidateTests(); - return JsonConvert.SerializeObject(this, Formatting.Indented, _jsonSerializerSettings); + return JsonSerializer.Serialize(this, _jsonSerializerOptions); } public string WriteResultTable() @@ -268,11 +263,4 @@ private static string PrintCounter(Counter counter, int counterWidth, int result return $"{LeftJustify(counter.Name, counterWidth)}|{LeftJustify(average, resultWidth)}|{LeftJustify(min, resultWidth)}|{LeftJustify(max, resultWidth)}"; } - private void ValidateTests() - { - foreach (var test in Tests) - { - test.Validate(); - } - } } diff --git a/src/tools/Reporting/Reporting/Reporting.csproj b/src/tools/Reporting/Reporting/Reporting.csproj index 7f9c470cb41..af9642877bf 100644 --- a/src/tools/Reporting/Reporting/Reporting.csproj +++ b/src/tools/Reporting/Reporting/Reporting.csproj @@ -8,7 +8,7 @@ - + diff --git a/src/tools/Reporting/Reporting/Test.cs b/src/tools/Reporting/Reporting/Test.cs index 7d01ed08807..44464b06eee 100644 --- a/src/tools/Reporting/Reporting/Test.cs +++ b/src/tools/Reporting/Reporting/Test.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Runtime.Serialization; namespace Reporting; @@ -21,13 +20,6 @@ public class Test public void AddCounter(Counter counter) { - counter.Validate(); - - if (counter.DefaultCounter && !counter.TopCounter) - { - throw new InvalidOperationException($"Default counter '{counter.Name}' must also be a top counter."); - } - if (counter.DefaultCounter && Counters.Any(c => c.DefaultCounter)) { throw new Exception($"Duplicate default counter, name: ${counter.Name}"); @@ -48,35 +40,4 @@ public void AddCounters(IEnumerable counters) AddCounter(counter); } } - - internal void Validate() - { - var defaultCounters = Counters.Where(c => c.DefaultCounter).ToList(); - if (defaultCounters.Count != 1) - { - throw new InvalidOperationException($"Test '{Name}' must have exactly one default counter, but found {defaultCounters.Count}."); - } - - if (!defaultCounters[0].TopCounter) - { - throw new InvalidOperationException($"Default counter '{defaultCounters[0].Name}' must also be a top counter."); - } - - var duplicateCounter = Counters.GroupBy(c => c.Name).FirstOrDefault(group => group.Count() > 1); - if (duplicateCounter is not null) - { - throw new InvalidOperationException($"Duplicate counter name, name: ${duplicateCounter.Key}"); - } - - foreach (var counter in Counters) - { - counter.Validate(); - } - } - - [OnSerializing] - private void OnSerializing(StreamingContext _) => Validate(); - - [OnDeserialized] - private void OnDeserialized(StreamingContext _) => Validate(); } From a10db34fed9c18f73ba12d4510fded78f0f1811d Mon Sep 17 00:00:00 2001 From: Drew Scoggins Date: Mon, 31 Aug 2026 10:50:51 -0700 Subject: [PATCH 4/4] Preserve Reporting JSON compatibility Provide a System.Text.Json round-trip API and retain named floating-point literal handling for existing counter results. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6f713269-e30d-4770-8a91-3fc53109abea --- .../Reporting.Tests/ReporterTests.cs | 24 +++++++++++-------- src/tools/Reporting/Reporting/Reporter.cs | 6 ++++- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/tools/Reporting/Reporting.Tests/ReporterTests.cs b/src/tools/Reporting/Reporting.Tests/ReporterTests.cs index 83d0097c8e5..bee589682e4 100644 --- a/src/tools/Reporting/Reporting.Tests/ReporterTests.cs +++ b/src/tools/Reporting/Reporting.Tests/ReporterTests.cs @@ -11,11 +11,6 @@ namespace Reporting.Tests; public class ReporterTests { - private static readonly JsonSerializerOptions s_jsonSerializerOptions = new() - { - PropertyNamingPolicy = JsonNamingPolicy.CamelCase - }; - // this matches the output from the reporter made in GetReporterWithSpecifiedEnvironment private const string ExpectedTestTable = @"TestName @@ -211,6 +206,19 @@ public void RegressionThresholdIsSerialized() Assert.Equal(0.02, deserialized.Tests[0].Counters[0].RegressionThreshold); } + [Fact] + public void NonFiniteResultsRoundTrip() + { + var reporter = GetReporterWithSpecifiedEnvironment(new PerfLabEnvironmentProviderMock(), result: double.NaN); + + var jsonString = reporter.GetJson(); + Assert.NotNull(jsonString); + Assert.Contains("\"NaN\"", jsonString); + + var deserialized = DeserializeReporter(jsonString); + Assert.True(double.IsNaN(deserialized.Tests[0].Counters[0].Results[0])); + } + [Fact] public void EnforceDefaultCounterConstraint() { @@ -249,11 +257,7 @@ public void AddCountersEnumerable() } private static Reporter DeserializeReporter(string json) - { - var reporter = JsonSerializer.Deserialize(json, s_jsonSerializerOptions); - Assert.NotNull(reporter); - return reporter; - } + => Reporter.FromJson(json); private static Reporter GetReporterWithSpecifiedEnvironment(PerfLabEnvironmentProviderMock enviroment, string counterName = null, double result = 1.1) { diff --git a/src/tools/Reporting/Reporting/Reporter.cs b/src/tools/Reporting/Reporting/Reporter.cs index 207f228af37..f0ef0b3941c 100644 --- a/src/tools/Reporting/Reporting/Reporter.cs +++ b/src/tools/Reporting/Reporting/Reporter.cs @@ -21,6 +21,7 @@ public class Reporter private readonly static CultureInfo _culture = CultureInfo.InvariantCulture; private readonly static JsonSerializerOptions _jsonSerializerOptions = new() { + NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals, PropertyNamingPolicy = JsonNamingPolicy.CamelCase, WriteIndented = true }; @@ -71,6 +72,10 @@ public void AddTest(Test test) return JsonSerializer.Serialize(this, _jsonSerializerOptions); } + public static Reporter FromJson(string json) + => JsonSerializer.Deserialize(json, _jsonSerializerOptions) + ?? throw new JsonException("The JSON payload did not contain a reporter."); + public string WriteResultTable() { var ret = new StringBuilder(); @@ -262,5 +267,4 @@ private static string PrintCounter(Counter counter, int counterWidth, int result var min = ((FormattableString)$"{counter.Results.Min():F3} {counter.MetricName}").ToString(_culture); return $"{LeftJustify(counter.Name, counterWidth)}|{LeftJustify(average, resultWidth)}|{LeftJustify(min, resultWidth)}|{LeftJustify(max, resultWidth)}"; } - }