From 115118e75ab8b42252b9eb1683eea31ed8772b76 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Thu, 1 Nov 2018 11:09:00 -0700 Subject: [PATCH 1/2] Allows Debug.Fail to go through Trace Listeners --- .../tests/DebugTests.cs | 34 ++++------- .../tests/DebugTestsUsingListeners.cs | 57 +++++++++++++++++++ .../Diagnostics/DefaultTraceListener.cs | 10 +--- .../src/System/Diagnostics/TraceInternal.cs | 1 + 4 files changed, 70 insertions(+), 32 deletions(-) diff --git a/src/System.Diagnostics.Debug/tests/DebugTests.cs b/src/System.Diagnostics.Debug/tests/DebugTests.cs index da41ae897700..b1632083ec97 100644 --- a/src/System.Diagnostics.Debug/tests/DebugTests.cs +++ b/src/System.Diagnostics.Debug/tests/DebugTests.cs @@ -41,7 +41,7 @@ protected void VerifyLogged(Action test, string expectedOutput) // First use our test logger to verify the output var originalWriteCoreHook = writeCoreHook.GetValue(null); - writeCoreHook.SetValue(null, new Action(WriteLogger.s_instance.MockWrite)); + writeCoreHook.SetValue(null, new Action(WriteLogger.s_instance.WriteCore)); try { @@ -62,11 +62,12 @@ protected void VerifyLogged(Action test, string expectedOutput) protected void VerifyAssert(Action test, params string[] expectedOutputStrings) { FieldInfo writeCoreHook = typeof(DebugProvider).GetField("s_WriteCore", BindingFlags.Static | BindingFlags.NonPublic); - s_defaultProvider = Debug.SetProvider(WriteLogger.s_instance); - WriteLogger.s_instance.OriginalProvider = s_defaultProvider; - var originalWriteCoreHook = writeCoreHook.GetValue(null); - writeCoreHook.SetValue(null, new Action(WriteLogger.s_instance.MockWrite)); + writeCoreHook.SetValue(null, new Action(WriteLogger.s_instance.WriteCore)); + + FieldInfo showDialogHook = typeof(DebugProvider).GetField("s_ShowDialog", BindingFlags.Static | BindingFlags.Public); + var originalShowDialogHook = showDialogHook.GetValue(null); + showDialogHook.SetValue(null, new Action(WriteLogger.s_instance.ShowDialog)); try { @@ -82,19 +83,16 @@ protected void VerifyAssert(Action test, params string[] expectedOutputStrings) finally { writeCoreHook.SetValue(null, originalWriteCoreHook); - Debug.SetProvider(s_defaultProvider); + showDialogHook.SetValue(null, originalShowDialogHook); } } - private static DebugProvider s_defaultProvider; - private class WriteLogger : DebugProvider + internal class WriteLogger { public static readonly WriteLogger s_instance = new WriteLogger(); private WriteLogger() { } - public DebugProvider OriginalProvider { get; set; } - public string LoggedOutput { get; private set; } public string AssertUIOutput { get; private set; } @@ -105,24 +103,12 @@ public void Clear() AssertUIOutput = string.Empty; } - public override void ShowDialog(string stackTrace, string message, string detailMessage, string errorSource) + public void ShowDialog(string stackTrace, string message, string detailMessage, string errorSource) { AssertUIOutput += stackTrace + message + detailMessage + errorSource; } - public override void OnIndentLevelChanged(int indentLevel) - { - OriginalProvider.OnIndentLevelChanged(indentLevel); - } - - public override void OnIndentSizeChanged(int indentSize) - { - OriginalProvider.OnIndentLevelChanged(indentSize); - } - public override void Write(string message) { OriginalProvider.Write(message); } - public override void WriteLine(string message) { OriginalProvider.WriteLine(message); } - - public void MockWrite(string message) + public void WriteCore(string message) { Assert.NotNull(message); LoggedOutput += message; diff --git a/src/System.Diagnostics.Debug/tests/DebugTestsUsingListeners.cs b/src/System.Diagnostics.Debug/tests/DebugTestsUsingListeners.cs index 992a861d741b..442e3b4a732f 100644 --- a/src/System.Diagnostics.Debug/tests/DebugTestsUsingListeners.cs +++ b/src/System.Diagnostics.Debug/tests/DebugTestsUsingListeners.cs @@ -244,5 +244,62 @@ public void TraceWriteLineIf() VerifyLogged(() => Trace.WriteLineIf(true, "logged", "category"), "category: logged" + Environment.NewLine); VerifyLogged(() => Trace.WriteLineIf(false, "logged", "category"), ""); } + + [Fact] + public void Trace_AssertUiEnabledFalse_SkipsShowDialog() + { + var initialListener = (DefaultTraceListener)Trace.Listeners[0]; + Trace.Listeners.Clear(); + Trace.Fail("Fail won't show dialog"); + Debug.Fail("Fail won't show dialog"); + + initialListener.AssertUiEnabled = false; + Trace.Listeners.Add(initialListener); + Debug.Fail("Fail won't show dialog"); + Trace.Fail("Fail won't show dialog"); + + var myListener = new MyTraceListener(); + string expectedDialog = $"Mock dialog - message: short, detailed message: long"; + Trace.Listeners.Clear(); + Trace.Listeners.Add(myListener); + + try + { + myListener.AssertUiEnabled = false; + Debug.Fail("short", "long"); + Assert.Equal("", myListener.OutputString); + Trace.Fail("short", "long"); + Assert.Equal("", myListener.OutputString); + + myListener.AssertUiEnabled = true; + Debug.Fail("short", "long"); + Assert.Equal(expectedDialog, myListener.OutputString); + Trace.Fail("short", "long"); + Assert.Equal(expectedDialog + expectedDialog, myListener.OutputString); + } + finally + { + Trace.Listeners.Clear(); + Trace.Listeners.Add(initialListener); + } + } + + class MyTraceListener : DefaultTraceListener + { + private void ShowDialog(string stackTrace, string message, string detailMessage, string errorSource) + { + OutputString += $"Mock dialog - message: {message}, detailed message: {detailMessage}"; + } + public string OutputString { get; private set; } = string.Empty; + + public override void Fail(string message, string detailMessage) + { + WriteLine(message, detailMessage); + if (AssertUiEnabled) + { + ShowDialog(string.Empty, message, detailMessage, "Assertion Failed"); + } + } + } } } diff --git a/src/System.Diagnostics.TraceSource/src/System/Diagnostics/DefaultTraceListener.cs b/src/System.Diagnostics.TraceSource/src/System/Diagnostics/DefaultTraceListener.cs index 0c653310c02b..daac05a4ed01 100644 --- a/src/System.Diagnostics.TraceSource/src/System/Diagnostics/DefaultTraceListener.cs +++ b/src/System.Diagnostics.TraceSource/src/System/Diagnostics/DefaultTraceListener.cs @@ -89,16 +89,10 @@ public override void Fail(string message, string detailMessage) { stackTrace = ""; } - // Tracked by #32955: WriteAssert should write "stackTrace" rather than string.Empty. - WriteAssert(string.Empty, message, detailMessage); + WriteAssert(stackTrace, message, detailMessage); if (AssertUiEnabled) { - // Tracked by #32955: Currently AssertUiEnabled is true by default but we are not calling Enviroment.FailFast as Debug.Fail does - // s_provider.ShowDialog(stackTrace, message, detailMessage, "Assertion Failed"); - } - if (Debugger.IsAttached) - { - Debugger.Break(); + DebugProvider.ShowDialog(stackTrace, message, detailMessage, "Assertion Failed"); } } diff --git a/src/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceInternal.cs b/src/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceInternal.cs index 832701d669ae..7ef82101f2c8 100644 --- a/src/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceInternal.cs +++ b/src/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceInternal.cs @@ -13,6 +13,7 @@ internal static class TraceInternal { private class TraceProvider : DebugProvider { + public override void Fail(string message, string detailMessage) { TraceInternal.Fail(message, detailMessage); } public override void OnIndentLevelChanged(int indentLevel) { lock (TraceInternal.critSec) From 9d96b21f77bd541f9e81d7cf103621d82f5d8c5a Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 2 Nov 2018 08:47:05 -0700 Subject: [PATCH 2/2] Rename --- src/System.Diagnostics.Debug/tests/DebugTests.cs | 10 +++++----- .../tests/DebugTestsUsingListeners.cs | 14 +++++++------- .../src/System/Diagnostics/DefaultTraceListener.cs | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/System.Diagnostics.Debug/tests/DebugTests.cs b/src/System.Diagnostics.Debug/tests/DebugTests.cs index b1632083ec97..c0abd37323dc 100644 --- a/src/System.Diagnostics.Debug/tests/DebugTests.cs +++ b/src/System.Diagnostics.Debug/tests/DebugTests.cs @@ -65,9 +65,9 @@ protected void VerifyAssert(Action test, params string[] expectedOutputStrings) var originalWriteCoreHook = writeCoreHook.GetValue(null); writeCoreHook.SetValue(null, new Action(WriteLogger.s_instance.WriteCore)); - FieldInfo showDialogHook = typeof(DebugProvider).GetField("s_ShowDialog", BindingFlags.Static | BindingFlags.Public); - var originalShowDialogHook = showDialogHook.GetValue(null); - showDialogHook.SetValue(null, new Action(WriteLogger.s_instance.ShowDialog)); + FieldInfo failCoreHook = typeof(DebugProvider).GetField("s_FailCore", BindingFlags.Static | BindingFlags.NonPublic); + var originalFailCoreHook = failCoreHook.GetValue(null); + failCoreHook.SetValue(null, new Action(WriteLogger.s_instance.FailCore)); try { @@ -83,7 +83,7 @@ protected void VerifyAssert(Action test, params string[] expectedOutputStrings) finally { writeCoreHook.SetValue(null, originalWriteCoreHook); - showDialogHook.SetValue(null, originalShowDialogHook); + failCoreHook.SetValue(null, originalFailCoreHook); } } @@ -103,7 +103,7 @@ public void Clear() AssertUIOutput = string.Empty; } - public void ShowDialog(string stackTrace, string message, string detailMessage, string errorSource) + public void FailCore(string stackTrace, string message, string detailMessage, string errorSource) { AssertUIOutput += stackTrace + message + detailMessage + errorSource; } diff --git a/src/System.Diagnostics.Debug/tests/DebugTestsUsingListeners.cs b/src/System.Diagnostics.Debug/tests/DebugTestsUsingListeners.cs index 442e3b4a732f..2e46f7a7c84d 100644 --- a/src/System.Diagnostics.Debug/tests/DebugTestsUsingListeners.cs +++ b/src/System.Diagnostics.Debug/tests/DebugTestsUsingListeners.cs @@ -246,17 +246,17 @@ public void TraceWriteLineIf() } [Fact] - public void Trace_AssertUiEnabledFalse_SkipsShowDialog() + public void Trace_AssertUiEnabledFalse_SkipsFail() { var initialListener = (DefaultTraceListener)Trace.Listeners[0]; Trace.Listeners.Clear(); - Trace.Fail("Fail won't show dialog"); - Debug.Fail("Fail won't show dialog"); + Trace.Fail("Skips fail fast"); + Debug.Fail("Skips fail fast"); initialListener.AssertUiEnabled = false; Trace.Listeners.Add(initialListener); - Debug.Fail("Fail won't show dialog"); - Trace.Fail("Fail won't show dialog"); + Debug.Fail("Skips fail fast"); + Trace.Fail("Skips fail fast"); var myListener = new MyTraceListener(); string expectedDialog = $"Mock dialog - message: short, detailed message: long"; @@ -286,7 +286,7 @@ public void Trace_AssertUiEnabledFalse_SkipsShowDialog() class MyTraceListener : DefaultTraceListener { - private void ShowDialog(string stackTrace, string message, string detailMessage, string errorSource) + private void FailCore(string stackTrace, string message, string detailMessage, string errorSource) { OutputString += $"Mock dialog - message: {message}, detailed message: {detailMessage}"; } @@ -297,7 +297,7 @@ public override void Fail(string message, string detailMessage) WriteLine(message, detailMessage); if (AssertUiEnabled) { - ShowDialog(string.Empty, message, detailMessage, "Assertion Failed"); + FailCore(string.Empty, message, detailMessage, "Assertion Failed"); } } } diff --git a/src/System.Diagnostics.TraceSource/src/System/Diagnostics/DefaultTraceListener.cs b/src/System.Diagnostics.TraceSource/src/System/Diagnostics/DefaultTraceListener.cs index daac05a4ed01..7207bc77ce1d 100644 --- a/src/System.Diagnostics.TraceSource/src/System/Diagnostics/DefaultTraceListener.cs +++ b/src/System.Diagnostics.TraceSource/src/System/Diagnostics/DefaultTraceListener.cs @@ -92,7 +92,7 @@ public override void Fail(string message, string detailMessage) WriteAssert(stackTrace, message, detailMessage); if (AssertUiEnabled) { - DebugProvider.ShowDialog(stackTrace, message, detailMessage, "Assertion Failed"); + DebugProvider.FailCore(stackTrace, message, detailMessage, "Assertion Failed"); } }