Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/PerfView.TestUtilities/DebugAssertionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
/// <remarks>
/// <para>This file can be linked into any project which needs to validate that assertions are behaving correctly
/// for the purpose of unit testing.</para>
/// <para>On .NET Framework, assertion failures throw via <see cref="ThrowingTraceListener"/> registered in
/// app.config. On .NET 5+, the DefaultTraceListener already throws on assertion failures, so no
/// additional listener configuration is needed.</para>
/// </remarks>
public class DebugAssertionTests
{
Expand Down
12 changes: 11 additions & 1 deletion src/PerfView.TestUtilities/ThrowingTraceListener.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <system.diagnostics>:
//
// <configuration>
// <system.diagnostics>
Expand All @@ -17,6 +20,13 @@ namespace PerfView.TestUtilities
// </trace>
// </system.diagnostics>
//</configuration>
//
// On .NET 5+, the app.config <system.diagnostics> 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)
Expand Down
12 changes: 10 additions & 2 deletions src/TraceEvent/DynamicTraceEventParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,16 @@ protected internal override void EnumerateTemplates(Func<string, string, EventFi
// also enumerate any events from the registeredParser.
registeredParser.EnumerateTemplates(eventsToObserve, callback);

// also enumerate any events from the eventPipeTraceEventParser
eventPipeTraceEventParser.EnumerateTemplates(eventsToObserve, callback);
// also enumerate any events from the eventPipeTraceEventParser.
// Filter out any duplicates that the registeredParser already knows about,
// similar to how manifest-based templates are filtered above.
eventPipeTraceEventParser.EnumerateTemplates(eventsToObserve, delegate (TraceEvent template)
{
if (!registeredParser.HasDefinitionForTemplate(template))
{
callback(template);
}
});
}

private class PartialManifestInfo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,13 @@
using Microsoft.Diagnostics.Tracing.Stacks;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.IO.Compression;
using System.Linq;
using Xunit;

using static Microsoft.Diagnostics.Tracing.Stacks.StackSourceWriterHelper;

// For Debug.Listeners
#if NETCOREAPP3_0_OR_GREATER
using Trace = System.Diagnostics.Trace;
#else
using Trace = System.Diagnostics.Debug;
#endif

namespace TraceEventTests
{
public class SpeedScopeStackSourceWriterTests
Expand Down Expand Up @@ -343,10 +335,6 @@ public void ValidationAllowsForCompleteResults()
[InlineData("only_managed_samples.nettrace.zip")]
public void CanConvertProvidedTraceFiles(string zippedTraceFileName)
{
var debugListenersCopy = new TraceListener[Trace.Listeners.Count];
Trace.Listeners.CopyTo(debugListenersCopy, index: 0);
Trace.Listeners.Clear();

string fileToUnzip = Path.Combine("inputs", "speedscope", zippedTraceFileName);
string unzippedFile = Path.ChangeExtension(fileToUnzip, string.Empty);

Expand Down Expand Up @@ -402,10 +390,6 @@ public void CanConvertProvidedTraceFiles(string zippedTraceFileName)
{
File.Delete(unzippedFile);
}
if (debugListenersCopy.Length > 0)
{
Trace.Listeners.AddRange(debugListenersCopy);
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/TraceEvent/TraceEvent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fragile. Fine for now, but if you find you need another class it might be time to think through the design.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I agree that if this gets bigger we should look to make this a real pattern.

{
ConfirmAllEventsAreInEnumeration();
m_ConfirmedAllEventsAreInEnumeration = true;
Expand Down
Loading