From 63baea9922c48954b9286ab99470fae94f4a4d08 Mon Sep 17 00:00:00 2001 From: Brian Robbins Date: Sat, 21 Feb 2026 20:21:36 -0800 Subject: [PATCH] Fix Debug.Assert failures exposed by ThrowingTraceListener in SpeedScope tests Two issues caused Debug.Assert failures when ThrowingTraceListener was active: 1. DynamicTraceEventParser.EnumerateTemplates produced duplicate templates when both registeredParser and eventPipeTraceEventParser knew about the same event, causing Subscribe to assert on the duplicate with mayHaveExistedBefore=false. Fix: filter eventPipeTraceEventParser templates against registeredParser. 2. ApplicationServerTraceEventParser is auto-generated and has many events sharing the same task name, producing duplicate computed event names that ConfirmAllEventsAreInEnumeration cannot handle. Fix: skip this parser from the validation. Also remove the listener-clearing hack in SpeedScope tests since the underlying asserts are now fixed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DebugAssertionTests.cs | 3 +++ .../ThrowingTraceListener.cs | 12 +++++++++++- src/TraceEvent/DynamicTraceEventParser.cs | 12 ++++++++++-- .../Speedscope/SpeedScopeExporterTests.cs | 16 ---------------- src/TraceEvent/TraceEvent.cs | 7 ++++++- 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/PerfView.TestUtilities/DebugAssertionTests.cs b/src/PerfView.TestUtilities/DebugAssertionTests.cs index c561d9f4b..710abc8e7 100644 --- a/src/PerfView.TestUtilities/DebugAssertionTests.cs +++ b/src/PerfView.TestUtilities/DebugAssertionTests.cs @@ -11,6 +11,9 @@ /// /// This file can be linked into any project which needs to validate that assertions are behaving correctly /// for the purpose of unit testing. + /// On .NET Framework, assertion failures throw via registered in + /// app.config. On .NET 5+, the DefaultTraceListener already throws on assertion failures, so no + /// additional listener configuration is needed. /// public class DebugAssertionTests { diff --git a/src/PerfView.TestUtilities/ThrowingTraceListener.cs b/src/PerfView.TestUtilities/ThrowingTraceListener.cs index b57b69fed..52eb976ee 100644 --- a/src/PerfView.TestUtilities/ThrowingTraceListener.cs +++ b/src/PerfView.TestUtilities/ThrowingTraceListener.cs @@ -5,7 +5,10 @@ namespace PerfView.TestUtilities { - // To enable this for a process, add the following to the app.config for the project: + // This listener converts Debug.Assert/Trace.Assert failures into xUnit test failures + // by throwing from the Fail() method. + // + // On .NET Framework (net462), this must be registered via app.config : // // // @@ -17,6 +20,13 @@ namespace PerfView.TestUtilities // // // + // + // On .NET 5+, the app.config section is NOT + // processed, so this listener is never registered. However, the DefaultTraceListener + // on .NET 5+ already throws on assert failures, so no additional configuration is + // needed — Debug.Assert and Trace.Assert will throw without this listener. + // Should this behavior change, the ThrowingTraceListener tests will fail, which will tell us we + // need to do something to re-enable this listener. public sealed class ThrowingTraceListener : TraceListener { public override void Fail(string message, string detailMessage) diff --git a/src/TraceEvent/DynamicTraceEventParser.cs b/src/TraceEvent/DynamicTraceEventParser.cs index 34a60e7a7..7415c4637 100644 --- a/src/TraceEvent/DynamicTraceEventParser.cs +++ b/src/TraceEvent/DynamicTraceEventParser.cs @@ -279,8 +279,16 @@ protected internal override void EnumerateTemplates(Func 0) - { - Trace.Listeners.AddRange(debugListenersCopy); - } } } diff --git a/src/TraceEvent/TraceEvent.cs b/src/TraceEvent/TraceEvent.cs index e6045231b..921f3747c 100644 --- a/src/TraceEvent/TraceEvent.cs +++ b/src/TraceEvent/TraceEvent.cs @@ -2882,7 +2882,12 @@ protected TraceEventParser(TraceEventSource source, bool dontRegister = false) } #if DEBUG - if (GetProviderName() != null && !m_ConfirmedAllEventsAreInEnumeration && !(this is PredefinedDynamicTraceEventParser)) + // ApplicationServerTraceEventParser is auto-generated and has many events sharing the same + // task name, producing duplicate computed event names that ConfirmAllEventsAreInEnumeration + // cannot handle. + if (GetProviderName() != null && !m_ConfirmedAllEventsAreInEnumeration && + !(this is PredefinedDynamicTraceEventParser) && + !(this is Parsers.ApplicationServerTraceEventParser)) { ConfirmAllEventsAreInEnumeration(); m_ConfirmedAllEventsAreInEnumeration = true;