From 7710d66d14676eccbdaa85a65484b8da5660d934 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Wed, 27 Nov 2019 23:52:52 -0800 Subject: [PATCH 1/4] Make test libraries configuration agnostic --- .../System/AssertExtensions.cs | 6 +++ src/libraries/Directory.Build.props | 22 +++++----- .../ConnectionPoolTest/ConnectionPoolTest.cs | 4 +- ....Data.SqlClient.ManualTesting.Tests.csproj | 1 + .../System.Data.StressRunner.csproj | 1 + .../tests/AssertTests.cs | 35 ++++++++++------ .../tests/AssumeTests.cs | 35 ++++++++++------ .../tests/ContractFailedTests.cs | 3 -- .../System.Diagnostics.Contracts.Tests.csproj | 1 + .../tests/Utilities.cs | 3 ++ .../tests/ActivityTests.cs | 35 ++++++++-------- .../DiagnosticSourceEventSourceBridgeTests.cs | 7 ---- .../tests/StackTraceTests.cs | 10 +++-- .../BasicEventSourceTest/Harness/Listeners.cs | 2 - .../System/Reflection/MethodBaseTests.cs | 18 +++------ .../System/Reflection/MethodBodyTests.cs | 40 +++++-------------- 16 files changed, 112 insertions(+), 111 deletions(-) diff --git a/src/libraries/Common/tests/CoreFx.Private.TestUtilities/System/AssertExtensions.cs b/src/libraries/Common/tests/CoreFx.Private.TestUtilities/System/AssertExtensions.cs index b57aed2deb2f5..33037de482931 100644 --- a/src/libraries/Common/tests/CoreFx.Private.TestUtilities/System/AssertExtensions.cs +++ b/src/libraries/Common/tests/CoreFx.Private.TestUtilities/System/AssertExtensions.cs @@ -378,6 +378,12 @@ public static void Equal(HashSet expected, HashSet actual) } } + public static void AtLeastOneEquals(T expected1, T expected2, T value) where T : IEquatable + { + if (!(value.Equals(expected1) || value.Equals(expected2))) + throw new XunitException($"Expected: {expected1} || {expected2}{Environment.NewLine}Actual: {value}"); + } + public delegate void AssertThrowsActionReadOnly(ReadOnlySpan span); public delegate void AssertThrowsAction(Span span); diff --git a/src/libraries/Directory.Build.props b/src/libraries/Directory.Build.props index 8c12bd11d493b..27fdfc89d1bc2 100644 --- a/src/libraries/Directory.Build.props +++ b/src/libraries/Directory.Build.props @@ -31,10 +31,10 @@ https://github.com/Microsoft/msbuild/blob/3a9d1d2ae23e41b32a612ea6b0dce531fcf86be7/src/Build/Evaluation/IntrinsicFunctions.cs#L431 --> OSX - FreeBSD - NetBSD - Linux - $(OS) + FreeBSD + NetBSD + Linux + $(OS) @@ -73,7 +73,7 @@ true false - true + true true @@ -153,8 +153,8 @@ - true - true + true + true true @@ -192,12 +192,14 @@ - + true false full - $(DefineConstants),DEBUG,TRACE + + $(DefineConstants),DEBUG + $(DefineConstants),TRACE @@ -216,6 +218,7 @@ true false false + true true @@ -291,7 +294,6 @@ false true - true false diff --git a/src/libraries/System.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs b/src/libraries/System.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs index a6edea508e54f..d55fd8f3c02da 100644 --- a/src/libraries/System.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs +++ b/src/libraries/System.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs @@ -2,6 +2,7 @@ // 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.Diagnostics; using System.Threading.Tasks; using System.Threading; using System.Runtime.ExceptionServices; @@ -76,9 +77,9 @@ private static void BasicConnectionPoolingTest(string connectionString) /// Tests if killing the connection using the InternalConnectionWrapper is working /// /// + [Conditional("DEBUG")] private static void KillConnectionTest(string connectionString) { -#if DEBUG InternalConnectionWrapper wrapper = null; using (SqlConnection connection = new SqlConnection(connectionString)) @@ -103,7 +104,6 @@ private static void KillConnectionTest(string connectionString) DataTestUtility.AssertEqualsWithDescription(5, command.ExecuteScalar(), "Incorrect scalar result."); } } -#endif } /// diff --git a/src/libraries/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj b/src/libraries/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj index ec584d56415ff..aa21e88f0ce37 100644 --- a/src/libraries/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj +++ b/src/libraries/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj @@ -1,6 +1,7 @@  netcoreapp-Debug;netcoreapp-Release + $(DefineConstants);DEBUG diff --git a/src/libraries/System.Data.SqlClient/tests/StressTests/System.Data.StressRunner/System.Data.StressRunner.csproj b/src/libraries/System.Data.SqlClient/tests/StressTests/System.Data.StressRunner/System.Data.StressRunner.csproj index 6d9050afee44c..4463027f6da48 100644 --- a/src/libraries/System.Data.SqlClient/tests/StressTests/System.Data.StressRunner/System.Data.StressRunner.csproj +++ b/src/libraries/System.Data.SqlClient/tests/StressTests/System.Data.StressRunner/System.Data.StressRunner.csproj @@ -4,6 +4,7 @@ Exe 3021 netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netfx-Windows_NT-Debug;netfx-Windows_NT-Release + $(DefineConstants);DEBUG diff --git a/src/libraries/System.Diagnostics.Contracts/tests/AssertTests.cs b/src/libraries/System.Diagnostics.Contracts/tests/AssertTests.cs index b82a6ad7155fb..3ea21e266fbda 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/AssertTests.cs +++ b/src/libraries/System.Diagnostics.Contracts/tests/AssertTests.cs @@ -28,6 +28,7 @@ public static void AssertTrueDoesNotRaiseEvent() public static void AssertFalseRaisesEvent() { bool eventRaised = false; + bool inDebug = false; EventHandler handler = (s, e) => { eventRaised = true; @@ -36,11 +37,16 @@ public static void AssertFalseRaisesEvent() using (Utilities.WithContractFailed(handler)) { Contract.Assert(false); -#if DEBUG - Assert.True(eventRaised, "ContractFailed event not raised"); -#else - Assert.False(eventRaised, "ContractFailed event was raised"); -#endif + inDebug = Utilities.ReturnTrueIfCompiledInDebug; + + if (inDebug) + { + Assert.True(eventRaised, "ContractFailed event not raised"); + } + else + { + Assert.False(eventRaised, "ContractFailed event was raised"); + } } } @@ -48,6 +54,7 @@ public static void AssertFalseRaisesEvent() public static void AssertFalseThrows() { bool eventRaised = false; + bool inDebug = false; EventHandler handler = (s, e) => { eventRaised = true; @@ -55,13 +62,17 @@ public static void AssertFalseThrows() }; using (Utilities.WithContractFailed(handler)) { -#if DEBUG - Utilities.AssertThrowsContractException(() => Contract.Assert(false, "Some kind of user message")); - Assert.True(eventRaised, "ContractFailed was not raised"); -#else - Contract.Assert(false, "Some kind of user message"); - Assert.False(eventRaised, "ContractFailed event was raised"); -#endif + inDebug = Utilities.ReturnTrueIfCompiledInDebug; + if (inDebug) + { + Utilities.AssertThrowsContractException(() => Contract.Assert(false, "Some kind of user message")); + Assert.True(eventRaised, "ContractFailed was not raised"); + } + else + { + Contract.Assert(false, "Some kind of user message"); + Assert.False(eventRaised, "ContractFailed event was raised"); + } } } } diff --git a/src/libraries/System.Diagnostics.Contracts/tests/AssumeTests.cs b/src/libraries/System.Diagnostics.Contracts/tests/AssumeTests.cs index 17c39becf6665..e8f3d7e855661 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/AssumeTests.cs +++ b/src/libraries/System.Diagnostics.Contracts/tests/AssumeTests.cs @@ -31,6 +31,7 @@ public static void AssertTrueDoesNotRaiseEvent() public static void AssertFalseRaisesEvent() { bool eventRaised = false; + bool inDebug = false; EventHandler handler = (s, e) => { eventRaised = true; @@ -39,11 +40,15 @@ public static void AssertFalseRaisesEvent() using (Utilities.WithContractFailed(handler)) { Contract.Assume(false); -#if DEBUG - Assert.True(eventRaised, "ContractFailed event not raised"); -#else - Assert.False(eventRaised, "ContractFailed event was raised"); -#endif + inDebug = Utilities.ReturnTrueIfCompiledInDebug; + if (inDebug) + { + Assert.True(eventRaised, "ContractFailed event not raised"); + } + else + { + Assert.False(eventRaised, "ContractFailed event was raised"); + } } } @@ -51,6 +56,7 @@ public static void AssertFalseRaisesEvent() public static void AssertFalseThrows() { bool eventRaised = false; + bool inDebug = false; EventHandler handler = (s, e) => { eventRaised = true; @@ -58,13 +64,18 @@ public static void AssertFalseThrows() }; using (Utilities.WithContractFailed(handler)) { -#if DEBUG - Utilities.AssertThrowsContractException(() => Contract.Assume(false, "Some kind of user message")); - Assert.True(eventRaised, "ContractFailed was not raised"); -#else - Contract.Assume(false, "Some kind of user message"); - Assert.False(eventRaised, "ContractFailed event was raised"); -#endif + inDebug = Utilities.ReturnTrueIfCompiledInDebug; + + if (inDebug) + { + Utilities.AssertThrowsContractException(() => Contract.Assume(false, "Some kind of user message")); + Assert.True(eventRaised, "ContractFailed was not raised"); + } + else + { + Contract.Assume(false, "Some kind of user message"); + Assert.False(eventRaised, "ContractFailed event was raised"); + } } } } diff --git a/src/libraries/System.Diagnostics.Contracts/tests/ContractFailedTests.cs b/src/libraries/System.Diagnostics.Contracts/tests/ContractFailedTests.cs index e4137691d1004..8ff6cbc55f3cf 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/ContractFailedTests.cs +++ b/src/libraries/System.Diagnostics.Contracts/tests/ContractFailedTests.cs @@ -6,7 +6,6 @@ namespace System.Diagnostics.Contracts.Tests { -#if DEBUG public class ContractFailedTests { [Fact] @@ -84,7 +83,5 @@ public static void FailureKind() Contract.Assume(false); } } - } -#endif } diff --git a/src/libraries/System.Diagnostics.Contracts/tests/System.Diagnostics.Contracts.Tests.csproj b/src/libraries/System.Diagnostics.Contracts/tests/System.Diagnostics.Contracts.Tests.csproj index 78f5b87103169..ff8feb33af087 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/System.Diagnostics.Contracts.Tests.csproj +++ b/src/libraries/System.Diagnostics.Contracts/tests/System.Diagnostics.Contracts.Tests.csproj @@ -1,5 +1,6 @@ + $(DefineConstants);INCLUDE_DEBUG_BEHAVIOR netcoreapp-Debug;netcoreapp-Release true diff --git a/src/libraries/System.Diagnostics.Contracts/tests/Utilities.cs b/src/libraries/System.Diagnostics.Contracts/tests/Utilities.cs index ef97c9208af9f..0d2608275a119 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/Utilities.cs +++ b/src/libraries/System.Diagnostics.Contracts/tests/Utilities.cs @@ -29,5 +29,8 @@ private class UnregisterContractFailed : IDisposable public void Dispose() { Contract.ContractFailed -= _handler; } } + + [Conditional("INCLUDE_DEBUG_BEHAVIOR")] + internal static bool ReturnTrueIfCompiledInDebug => true; } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs index 8a4a8aefb082d..45f2d71fc0c3a 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs @@ -263,33 +263,34 @@ public void IdGenerationInternalParent() //start 2 children in different execution contexts Task.Run(() => child1.Start()).Wait(); Task.Run(() => child2.Start()).Wait(); -#if DEBUG - Assert.Equal($"|{parent.RootId}.{child1.OperationName}-1.", child1.Id); - Assert.Equal($"|{parent.RootId}.{child2.OperationName}-2.", child2.Id); -#else - Assert.Equal($"|{parent.RootId}.1.", child1.Id); - Assert.Equal($"|{parent.RootId}.2.", child2.Id); -#endif + + string child1DebugString = $"|{parent.RootId}.{child1.OperationName}-1."; + string child2DebugString = $"|{parent.RootId}.{child2.OperationName}-2."; + string child1ReleaseString = $"|{parent.RootId}.1."; + string child2ReleaseString = $"|{parent.RootId}.2."; + + AssertExtensions.AtLeastOneEquals(child1DebugString, child1ReleaseString, child1.Id); + AssertExtensions.AtLeastOneEquals(child2DebugString, child2ReleaseString, child2.Id); + Assert.Equal(parent.RootId, child1.RootId); Assert.Equal(parent.RootId, child2.RootId); child1.Stop(); child2.Stop(); var child3 = new Activity("child3"); child3.Start(); -#if DEBUG - Assert.Equal($"|{parent.RootId}.{child3.OperationName}-3.", child3.Id); -#else - Assert.Equal($"|{parent.RootId}.3.", child3.Id); -#endif + + string child3DebugString = $"|{parent.RootId}.{child3.OperationName}-3."; + string child3ReleaseString = $"|{parent.RootId}.3."; + + AssertExtensions.AtLeastOneEquals(child3DebugString, child3ReleaseString, child3.Id); var grandChild = new Activity("grandChild"); grandChild.Start(); -#if DEBUG - Assert.Equal($"{child3.Id}{grandChild.OperationName}-1.", grandChild.Id); -#else - Assert.Equal($"{child3.Id}1.", grandChild.Id); -#endif + child3DebugString = $"{child3.Id}{grandChild.OperationName}-1."; + child3ReleaseString = $"{child3.Id}1."; + + AssertExtensions.AtLeastOneEquals(child3DebugString, child3ReleaseString, grandChild.Id); } /// diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs index f600cbd90f352..dcdb29bafb257 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs @@ -913,11 +913,8 @@ public TestDiagnosticSourceEventListener() public int EventCount; public DiagnosticSourceEvent LastEvent; -#if DEBUG - // Here just for debugging. Lets you see the last 3 events that were sent. public DiagnosticSourceEvent SecondLast; public DiagnosticSourceEvent ThirdLast; -#endif /// /// Sets the EventCount to 0 and LastEvent to null @@ -926,10 +923,8 @@ public void ResetEventCountAndLastEvent() { EventCount = 0; LastEvent = null; -#if DEBUG SecondLast = null; ThirdLast = null; -#endif } /// @@ -943,10 +938,8 @@ private void UpdateLastEvent(DiagnosticSourceEvent anEvent) if (Filter != null && !Filter(anEvent)) return; -#if DEBUG ThirdLast = SecondLast; SecondLast = LastEvent; -#endif EventCount++; LastEvent = anEvent; diff --git a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs index 0399050b8944c..b1327a0857b54 100644 --- a/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs +++ b/src/libraries/System.Diagnostics.StackTrace/tests/StackTraceTests.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using Xunit; namespace System.Diagnostics @@ -247,8 +248,6 @@ public void Ctor_Frame(StackFrame stackFrame) public static IEnumerable ToString_TestData() { - // Debug mode and Release mode give different results. -#if DEBUG yield return new object[] { new StackTrace(InvokeException()), "System.Diagnostics.Tests.StackTraceTests.ThrowException()" }; yield return new object[] { new StackTrace(new Exception()), "" }; yield return new object[] { NoParameters(), "System.Diagnostics.Tests.StackTraceTests.NoParameters()" }; @@ -260,7 +259,6 @@ public static IEnumerable ToString_TestData() // Methods belonging to the System.Diagnostics namespace are ignored. yield return new object[] { InvokeIgnoredMethod(), "System.Diagnostics.Tests.StackTraceTests.InvokeIgnoredMethod()" }; -#endif yield return new object[] { InvokeIgnoredMethodWithException(), "System.Diagnostics.Ignored.MethodWithException()" }; } @@ -300,16 +298,22 @@ public void ToString_NullFrame_ThrowsNullReferenceException() Assert.Equal(Environment.NewLine, stackTrace.ToString()); } + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] private static StackTrace NoParameters() => new StackTrace(); + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] private static StackTrace OneParameter(int x) => new StackTrace(); + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] private static StackTrace TwoParameters(int x, string y) => new StackTrace(); + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] private static StackTrace Generic() => new StackTrace(); + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] private static StackTrace Generic() => new StackTrace(); private static StackTrace InvokeIgnoredMethod() => Ignored.Method(); private static StackTrace InvokeIgnoredMethodWithException() => Ignored.MethodWithException(); + [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)] private static Exception InvokeException() { try diff --git a/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/Harness/Listeners.cs b/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/Harness/Listeners.cs index 7eddf5e76506e..a44330cff1fe9 100644 --- a/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/Harness/Listeners.cs +++ b/src/libraries/System.Diagnostics.Tracing/tests/BasicEventSourceTest/Harness/Listeners.cs @@ -109,7 +109,6 @@ public virtual string PayloadString(int propertyIndex, string propertyName) } public abstract IList PayloadNames { get; } -#if DEBUG /// /// This is a convenience function for the debugger. It is not used typically /// @@ -123,7 +122,6 @@ public List PayloadValues return ret; } } -#endif public override string ToString() { diff --git a/src/libraries/System.Runtime/tests/System/Reflection/MethodBaseTests.cs b/src/libraries/System.Runtime/tests/System/Reflection/MethodBaseTests.cs index 0883838ea23d7..29f86feaeea24 100644 --- a/src/libraries/System.Runtime/tests/System/Reflection/MethodBaseTests.cs +++ b/src/libraries/System.Runtime/tests/System/Reflection/MethodBaseTests.cs @@ -56,26 +56,18 @@ public static void TestMethodBody() MethodBase mbase = typeof(MethodBaseTests).GetMethod("MyOtherMethod", BindingFlags.Static | BindingFlags.Public); MethodBody mb = mbase.GetMethodBody(); Assert.True(mb.InitLocals); // local variables are initialized -#if DEBUG - Assert.Equal(2, mb.MaxStackSize); - Assert.Equal(3, mb.LocalVariables.Count); - foreach (LocalVariableInfo lvi in mb.LocalVariables) - { - if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); } - if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); } - if (lvi.LocalIndex == 2) { Assert.Equal(typeof(bool), lvi.LocalType); } - } -#else - Assert.Equal(1, mb.MaxStackSize); - Assert.Equal(2, mb.LocalVariables.Count); + // Debug code expects 2, Release 1. + AssertExtensions.AtLeastOneEquals(1, 2, mb.MaxStackSize); + // Debug code expects 3, Release 2. + AssertExtensions.AtLeastOneEquals(2, 3, mb.LocalVariables.Count); foreach (LocalVariableInfo lvi in mb.LocalVariables) { if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); } if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); } + if (lvi.LocalIndex == 2) { Assert.Equal(typeof(bool), lvi.LocalType); } } -#endif } private static int MyAnotherMethod(int x) diff --git a/src/libraries/System.Runtime/tests/System/Reflection/MethodBodyTests.cs b/src/libraries/System.Runtime/tests/System/Reflection/MethodBodyTests.cs index a9cc1fc381bb3..45637a0dd594d 100644 --- a/src/libraries/System.Runtime/tests/System/Reflection/MethodBodyTests.cs +++ b/src/libraries/System.Runtime/tests/System/Reflection/MethodBodyTests.cs @@ -19,15 +19,17 @@ public static void Test_MethodBody_ExceptionHandlingClause() MethodBody mb = mi.GetMethodBody(); Assert.True(mb.InitLocals); // local variables are initialized -#if DEBUG + Assert.Equal(2, mb.MaxStackSize); - Assert.Equal(5, mb.LocalVariables.Count); + // Release expects 3, Debug 3. + AssertExtensions.AtLeastOneEquals(3, 5, mb.LocalVariables.Count); foreach (LocalVariableInfo lvi in mb.LocalVariables) { if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); } if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); } - if (lvi.LocalIndex == 2) { Assert.Equal(typeof(bool), lvi.LocalType); } + // Release expects Exception, Debug bool. + if (lvi.LocalIndex == 2) { AssertExtensions.AtLeastOneEquals(typeof(Exception), typeof(bool), lvi.LocalType); } if (lvi.LocalIndex == 3) { Assert.Equal(typeof(bool), lvi.LocalType); } if (lvi.LocalIndex == 4) { Assert.Equal(typeof(Exception), lvi.LocalType); } } @@ -37,37 +39,15 @@ public static void Test_MethodBody_ExceptionHandlingClause() if (ehc.Flags != ExceptionHandlingClauseOptions.Finally && ehc.Flags != ExceptionHandlingClauseOptions.Filter) { Assert.Equal(typeof(Exception), ehc.CatchType); - Assert.Equal(19, ehc.HandlerLength); - Assert.Equal(70, ehc.HandlerOffset); - Assert.Equal(61, ehc.TryLength); - Assert.Equal(9, ehc.TryOffset); - return; - } - } -#else - Assert.Equal(2, mb.MaxStackSize); - Assert.Equal(3, mb.LocalVariables.Count); - - foreach (LocalVariableInfo lvi in mb.LocalVariables) - { - if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); } - if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); } - if (lvi.LocalIndex == 2) { Assert.Equal(typeof(Exception), lvi.LocalType); } - } - foreach (ExceptionHandlingClause ehc in mb.ExceptionHandlingClauses) - { - if (ehc.Flags != ExceptionHandlingClauseOptions.Finally && ehc.Flags != ExceptionHandlingClauseOptions.Filter) - { - Assert.Equal(typeof(Exception), ehc.CatchType); - Assert.Equal(14, ehc.HandlerLength); - Assert.Equal(58, ehc.HandlerOffset); - Assert.Equal(50, ehc.TryLength); - Assert.Equal(8, ehc.TryOffset); + // First arg is for Release, second for Debug. + AssertExtensions.AtLeastOneEquals(14, 19, ehc.HandlerLength); + AssertExtensions.AtLeastOneEquals(58, 70, ehc.HandlerOffset); + AssertExtensions.AtLeastOneEquals(50, 61, ehc.TryLength); + AssertExtensions.AtLeastOneEquals(8, 9, ehc.TryOffset); return; } } -#endif Assert.True(false, "Expected to find CatchType clause."); } From 54aaf0ad1617e4fcb27e0f635f234e3ce993ffa0 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Fri, 13 Dec 2019 11:47:16 -0600 Subject: [PATCH 2/4] PR Feedback, keep Debug define for tests --- src/libraries/Directory.Build.props | 6 ++-- ....Data.SqlClient.ManualTesting.Tests.csproj | 1 - .../System.Data.StressRunner.csproj | 1 - .../tests/AssertTests.cs | 35 +++++++------------ .../tests/AssumeTests.cs | 35 +++++++------------ .../tests/ContractFailedTests.cs | 3 ++ .../System.Diagnostics.Contracts.Tests.csproj | 1 - .../tests/Utilities.cs | 3 -- .../DiagnosticSourceEventSourceBridgeTests.cs | 4 ++- 9 files changed, 32 insertions(+), 57 deletions(-) diff --git a/src/libraries/Directory.Build.props b/src/libraries/Directory.Build.props index 27fdfc89d1bc2..f32844ab24fd3 100644 --- a/src/libraries/Directory.Build.props +++ b/src/libraries/Directory.Build.props @@ -73,7 +73,7 @@ true false - true + true true @@ -197,9 +197,7 @@ true false full - - $(DefineConstants),DEBUG - $(DefineConstants),TRACE + $(DefineConstants),TRACE, DEBUG diff --git a/src/libraries/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj b/src/libraries/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj index aa21e88f0ce37..ec584d56415ff 100644 --- a/src/libraries/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj +++ b/src/libraries/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj @@ -1,7 +1,6 @@  netcoreapp-Debug;netcoreapp-Release - $(DefineConstants);DEBUG diff --git a/src/libraries/System.Data.SqlClient/tests/StressTests/System.Data.StressRunner/System.Data.StressRunner.csproj b/src/libraries/System.Data.SqlClient/tests/StressTests/System.Data.StressRunner/System.Data.StressRunner.csproj index 4463027f6da48..6d9050afee44c 100644 --- a/src/libraries/System.Data.SqlClient/tests/StressTests/System.Data.StressRunner/System.Data.StressRunner.csproj +++ b/src/libraries/System.Data.SqlClient/tests/StressTests/System.Data.StressRunner/System.Data.StressRunner.csproj @@ -4,7 +4,6 @@ Exe 3021 netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netfx-Windows_NT-Debug;netfx-Windows_NT-Release - $(DefineConstants);DEBUG diff --git a/src/libraries/System.Diagnostics.Contracts/tests/AssertTests.cs b/src/libraries/System.Diagnostics.Contracts/tests/AssertTests.cs index 3ea21e266fbda..b82a6ad7155fb 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/AssertTests.cs +++ b/src/libraries/System.Diagnostics.Contracts/tests/AssertTests.cs @@ -28,7 +28,6 @@ public static void AssertTrueDoesNotRaiseEvent() public static void AssertFalseRaisesEvent() { bool eventRaised = false; - bool inDebug = false; EventHandler handler = (s, e) => { eventRaised = true; @@ -37,16 +36,11 @@ public static void AssertFalseRaisesEvent() using (Utilities.WithContractFailed(handler)) { Contract.Assert(false); - inDebug = Utilities.ReturnTrueIfCompiledInDebug; - - if (inDebug) - { - Assert.True(eventRaised, "ContractFailed event not raised"); - } - else - { - Assert.False(eventRaised, "ContractFailed event was raised"); - } +#if DEBUG + Assert.True(eventRaised, "ContractFailed event not raised"); +#else + Assert.False(eventRaised, "ContractFailed event was raised"); +#endif } } @@ -54,7 +48,6 @@ public static void AssertFalseRaisesEvent() public static void AssertFalseThrows() { bool eventRaised = false; - bool inDebug = false; EventHandler handler = (s, e) => { eventRaised = true; @@ -62,17 +55,13 @@ public static void AssertFalseThrows() }; using (Utilities.WithContractFailed(handler)) { - inDebug = Utilities.ReturnTrueIfCompiledInDebug; - if (inDebug) - { - Utilities.AssertThrowsContractException(() => Contract.Assert(false, "Some kind of user message")); - Assert.True(eventRaised, "ContractFailed was not raised"); - } - else - { - Contract.Assert(false, "Some kind of user message"); - Assert.False(eventRaised, "ContractFailed event was raised"); - } +#if DEBUG + Utilities.AssertThrowsContractException(() => Contract.Assert(false, "Some kind of user message")); + Assert.True(eventRaised, "ContractFailed was not raised"); +#else + Contract.Assert(false, "Some kind of user message"); + Assert.False(eventRaised, "ContractFailed event was raised"); +#endif } } } diff --git a/src/libraries/System.Diagnostics.Contracts/tests/AssumeTests.cs b/src/libraries/System.Diagnostics.Contracts/tests/AssumeTests.cs index e8f3d7e855661..17c39becf6665 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/AssumeTests.cs +++ b/src/libraries/System.Diagnostics.Contracts/tests/AssumeTests.cs @@ -31,7 +31,6 @@ public static void AssertTrueDoesNotRaiseEvent() public static void AssertFalseRaisesEvent() { bool eventRaised = false; - bool inDebug = false; EventHandler handler = (s, e) => { eventRaised = true; @@ -40,15 +39,11 @@ public static void AssertFalseRaisesEvent() using (Utilities.WithContractFailed(handler)) { Contract.Assume(false); - inDebug = Utilities.ReturnTrueIfCompiledInDebug; - if (inDebug) - { - Assert.True(eventRaised, "ContractFailed event not raised"); - } - else - { - Assert.False(eventRaised, "ContractFailed event was raised"); - } +#if DEBUG + Assert.True(eventRaised, "ContractFailed event not raised"); +#else + Assert.False(eventRaised, "ContractFailed event was raised"); +#endif } } @@ -56,7 +51,6 @@ public static void AssertFalseRaisesEvent() public static void AssertFalseThrows() { bool eventRaised = false; - bool inDebug = false; EventHandler handler = (s, e) => { eventRaised = true; @@ -64,18 +58,13 @@ public static void AssertFalseThrows() }; using (Utilities.WithContractFailed(handler)) { - inDebug = Utilities.ReturnTrueIfCompiledInDebug; - - if (inDebug) - { - Utilities.AssertThrowsContractException(() => Contract.Assume(false, "Some kind of user message")); - Assert.True(eventRaised, "ContractFailed was not raised"); - } - else - { - Contract.Assume(false, "Some kind of user message"); - Assert.False(eventRaised, "ContractFailed event was raised"); - } +#if DEBUG + Utilities.AssertThrowsContractException(() => Contract.Assume(false, "Some kind of user message")); + Assert.True(eventRaised, "ContractFailed was not raised"); +#else + Contract.Assume(false, "Some kind of user message"); + Assert.False(eventRaised, "ContractFailed event was raised"); +#endif } } } diff --git a/src/libraries/System.Diagnostics.Contracts/tests/ContractFailedTests.cs b/src/libraries/System.Diagnostics.Contracts/tests/ContractFailedTests.cs index 8ff6cbc55f3cf..e4137691d1004 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/ContractFailedTests.cs +++ b/src/libraries/System.Diagnostics.Contracts/tests/ContractFailedTests.cs @@ -6,6 +6,7 @@ namespace System.Diagnostics.Contracts.Tests { +#if DEBUG public class ContractFailedTests { [Fact] @@ -83,5 +84,7 @@ public static void FailureKind() Contract.Assume(false); } } + } +#endif } diff --git a/src/libraries/System.Diagnostics.Contracts/tests/System.Diagnostics.Contracts.Tests.csproj b/src/libraries/System.Diagnostics.Contracts/tests/System.Diagnostics.Contracts.Tests.csproj index ff8feb33af087..78f5b87103169 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/System.Diagnostics.Contracts.Tests.csproj +++ b/src/libraries/System.Diagnostics.Contracts/tests/System.Diagnostics.Contracts.Tests.csproj @@ -1,6 +1,5 @@ - $(DefineConstants);INCLUDE_DEBUG_BEHAVIOR netcoreapp-Debug;netcoreapp-Release true diff --git a/src/libraries/System.Diagnostics.Contracts/tests/Utilities.cs b/src/libraries/System.Diagnostics.Contracts/tests/Utilities.cs index 0d2608275a119..ef97c9208af9f 100644 --- a/src/libraries/System.Diagnostics.Contracts/tests/Utilities.cs +++ b/src/libraries/System.Diagnostics.Contracts/tests/Utilities.cs @@ -29,8 +29,5 @@ private class UnregisterContractFailed : IDisposable public void Dispose() { Contract.ContractFailed -= _handler; } } - - [Conditional("INCLUDE_DEBUG_BEHAVIOR")] - internal static bool ReturnTrueIfCompiledInDebug => true; } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs index dcdb29bafb257..46978a958e65a 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs @@ -913,11 +913,13 @@ public TestDiagnosticSourceEventListener() public int EventCount; public DiagnosticSourceEvent LastEvent; + + // Here just for debugging. Lets you see the last 3 events that were sent. public DiagnosticSourceEvent SecondLast; public DiagnosticSourceEvent ThirdLast; /// - /// Sets the EventCount to 0 and LastEvent to null + /// Sets the EventCount to 0 and LastEvents to null /// public void ResetEventCountAndLastEvent() { From e0007421ecb11c53699f7fbddca35475e03ebc3a Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Wed, 18 Dec 2019 17:35:52 -0600 Subject: [PATCH 3/4] More PR Feedback --- src/libraries/Directory.Build.props | 2 +- .../ConnectionPoolTest/ConnectionPoolTest.cs | 4 +- .../tests/ActivityTests.cs | 7 ++++ .../System/Reflection/MethodBaseTests.cs | 18 ++++++--- .../System/Reflection/MethodBodyTests.cs | 40 ++++++++++++++----- 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/libraries/Directory.Build.props b/src/libraries/Directory.Build.props index 128fd51ee0534..e0a2a43c9a838 100644 --- a/src/libraries/Directory.Build.props +++ b/src/libraries/Directory.Build.props @@ -197,7 +197,7 @@ true false full - $(DefineConstants),TRACE, DEBUG + $(DefineConstants),TRACE,DEBUG diff --git a/src/libraries/System.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs b/src/libraries/System.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs index d55fd8f3c02da..a6edea508e54f 100644 --- a/src/libraries/System.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs +++ b/src/libraries/System.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs @@ -2,7 +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 System.Diagnostics; using System.Threading.Tasks; using System.Threading; using System.Runtime.ExceptionServices; @@ -77,9 +76,9 @@ private static void BasicConnectionPoolingTest(string connectionString) /// Tests if killing the connection using the InternalConnectionWrapper is working /// /// - [Conditional("DEBUG")] private static void KillConnectionTest(string connectionString) { +#if DEBUG InternalConnectionWrapper wrapper = null; using (SqlConnection connection = new SqlConnection(connectionString)) @@ -104,6 +103,7 @@ private static void KillConnectionTest(string connectionString) DataTestUtility.AssertEqualsWithDescription(5, command.ExecuteScalar(), "Incorrect scalar result."); } } +#endif } /// diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs index 45f2d71fc0c3a..f185679a53ad5 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs @@ -264,6 +264,13 @@ public void IdGenerationInternalParent() Task.Run(() => child1.Start()).Wait(); Task.Run(() => child2.Start()).Wait(); + // When running in a Debug runtime, the child operation Id will be constructed as follows + // "|parent.RootId.-childCount.". + // This is for debugging purposes to know which operation the child Id is comming from. + // + // In a Release runtime, it will not contain the operation name to keep it simple and it will be as + // "|parent.RootId.childCount.". + string child1DebugString = $"|{parent.RootId}.{child1.OperationName}-1."; string child2DebugString = $"|{parent.RootId}.{child2.OperationName}-2."; string child1ReleaseString = $"|{parent.RootId}.1."; diff --git a/src/libraries/System.Runtime/tests/System/Reflection/MethodBaseTests.cs b/src/libraries/System.Runtime/tests/System/Reflection/MethodBaseTests.cs index 29f86feaeea24..0883838ea23d7 100644 --- a/src/libraries/System.Runtime/tests/System/Reflection/MethodBaseTests.cs +++ b/src/libraries/System.Runtime/tests/System/Reflection/MethodBaseTests.cs @@ -56,11 +56,9 @@ public static void TestMethodBody() MethodBase mbase = typeof(MethodBaseTests).GetMethod("MyOtherMethod", BindingFlags.Static | BindingFlags.Public); MethodBody mb = mbase.GetMethodBody(); Assert.True(mb.InitLocals); // local variables are initialized - - // Debug code expects 2, Release 1. - AssertExtensions.AtLeastOneEquals(1, 2, mb.MaxStackSize); - // Debug code expects 3, Release 2. - AssertExtensions.AtLeastOneEquals(2, 3, mb.LocalVariables.Count); +#if DEBUG + Assert.Equal(2, mb.MaxStackSize); + Assert.Equal(3, mb.LocalVariables.Count); foreach (LocalVariableInfo lvi in mb.LocalVariables) { @@ -68,6 +66,16 @@ public static void TestMethodBody() if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); } if (lvi.LocalIndex == 2) { Assert.Equal(typeof(bool), lvi.LocalType); } } +#else + Assert.Equal(1, mb.MaxStackSize); + Assert.Equal(2, mb.LocalVariables.Count); + + foreach (LocalVariableInfo lvi in mb.LocalVariables) + { + if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); } + if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); } + } +#endif } private static int MyAnotherMethod(int x) diff --git a/src/libraries/System.Runtime/tests/System/Reflection/MethodBodyTests.cs b/src/libraries/System.Runtime/tests/System/Reflection/MethodBodyTests.cs index 45637a0dd594d..a9cc1fc381bb3 100644 --- a/src/libraries/System.Runtime/tests/System/Reflection/MethodBodyTests.cs +++ b/src/libraries/System.Runtime/tests/System/Reflection/MethodBodyTests.cs @@ -19,17 +19,15 @@ public static void Test_MethodBody_ExceptionHandlingClause() MethodBody mb = mi.GetMethodBody(); Assert.True(mb.InitLocals); // local variables are initialized - +#if DEBUG Assert.Equal(2, mb.MaxStackSize); - // Release expects 3, Debug 3. - AssertExtensions.AtLeastOneEquals(3, 5, mb.LocalVariables.Count); + Assert.Equal(5, mb.LocalVariables.Count); foreach (LocalVariableInfo lvi in mb.LocalVariables) { if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); } if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); } - // Release expects Exception, Debug bool. - if (lvi.LocalIndex == 2) { AssertExtensions.AtLeastOneEquals(typeof(Exception), typeof(bool), lvi.LocalType); } + if (lvi.LocalIndex == 2) { Assert.Equal(typeof(bool), lvi.LocalType); } if (lvi.LocalIndex == 3) { Assert.Equal(typeof(bool), lvi.LocalType); } if (lvi.LocalIndex == 4) { Assert.Equal(typeof(Exception), lvi.LocalType); } } @@ -39,15 +37,37 @@ public static void Test_MethodBody_ExceptionHandlingClause() if (ehc.Flags != ExceptionHandlingClauseOptions.Finally && ehc.Flags != ExceptionHandlingClauseOptions.Filter) { Assert.Equal(typeof(Exception), ehc.CatchType); + Assert.Equal(19, ehc.HandlerLength); + Assert.Equal(70, ehc.HandlerOffset); + Assert.Equal(61, ehc.TryLength); + Assert.Equal(9, ehc.TryOffset); + return; + } + } +#else + Assert.Equal(2, mb.MaxStackSize); + Assert.Equal(3, mb.LocalVariables.Count); + + foreach (LocalVariableInfo lvi in mb.LocalVariables) + { + if (lvi.LocalIndex == 0) { Assert.Equal(typeof(int), lvi.LocalType); } + if (lvi.LocalIndex == 1) { Assert.Equal(typeof(string), lvi.LocalType); } + if (lvi.LocalIndex == 2) { Assert.Equal(typeof(Exception), lvi.LocalType); } + } - // First arg is for Release, second for Debug. - AssertExtensions.AtLeastOneEquals(14, 19, ehc.HandlerLength); - AssertExtensions.AtLeastOneEquals(58, 70, ehc.HandlerOffset); - AssertExtensions.AtLeastOneEquals(50, 61, ehc.TryLength); - AssertExtensions.AtLeastOneEquals(8, 9, ehc.TryOffset); + foreach (ExceptionHandlingClause ehc in mb.ExceptionHandlingClauses) + { + if (ehc.Flags != ExceptionHandlingClauseOptions.Finally && ehc.Flags != ExceptionHandlingClauseOptions.Filter) + { + Assert.Equal(typeof(Exception), ehc.CatchType); + Assert.Equal(14, ehc.HandlerLength); + Assert.Equal(58, ehc.HandlerOffset); + Assert.Equal(50, ehc.TryLength); + Assert.Equal(8, ehc.TryOffset); return; } } +#endif Assert.True(false, "Expected to find CatchType clause."); } From 851e71432d8ef927e512aa5ca875000f4975a270 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Thu, 19 Dec 2019 13:28:05 -0600 Subject: [PATCH 4/4] Last pr feedback --- .../tests/ActivityTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs index f185679a53ad5..7ad41bca64d9d 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs @@ -264,11 +264,11 @@ public void IdGenerationInternalParent() Task.Run(() => child1.Start()).Wait(); Task.Run(() => child2.Start()).Wait(); - // When running in a Debug runtime, the child operation Id will be constructed as follows + // In Debug builds of System.Diagnostics.DiagnosticSource, the child operation Id will be constructed as follows // "|parent.RootId.-childCount.". // This is for debugging purposes to know which operation the child Id is comming from. // - // In a Release runtime, it will not contain the operation name to keep it simple and it will be as + // In Release builds of System.Diagnostics.DiagnosticSource, it will not contain the operation name to keep it simple and it will be as // "|parent.RootId.childCount.". string child1DebugString = $"|{parent.RootId}.{child1.OperationName}-1.";