Make test libraries configuration agnostic (#378)

* Make test libraries configuration agnostic * PR Feedback, keep Debug define for tests * More PR Feedback * Last pr feedback
dotnet · Dec 20, 2019 · 0d596d2 · 0d596d2
1 parent 423c1eb
commit 0d596d2
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 39 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
@@ -378,6 +378,13 @@ public static void Equal<T>(HashSet<T> expected, HashSet<T> actual)
             }
         }
 
+        public static void AtLeastOneEquals<T>(T expected1, T expected2, T value)
+        {
+            EqualityComparer<T> comparer = EqualityComparer<T>.Default;
+            if (!(comparer.Equals(value, expected1) || comparer.Equals(value, expected2)))
+                throw new XunitException($"Expected: {expected1} || {expected2}{Environment.NewLine}Actual: {value}");
+        }
+
         public delegate void AssertThrowsActionReadOnly<T>(ReadOnlySpan<T> span);
 
         public delegate void AssertThrowsAction<T>(Span<T> span);

diff --git a/src/libraries/Directory.Build.props b/src/libraries/Directory.Build.props
@@ -27,10 +27,10 @@
       https://github.com/Microsoft/msbuild/blob/3a9d1d2ae23e41b32a612ea6b0dce531fcf86be7/src/Build/Evaluation/IntrinsicFunctions.cs#L431
     -->
     <DefaultOSGroup Condition="$([MSBuild]::IsOSPlatform('OSX'))">OSX</DefaultOSGroup>
-    <DefaultOSGroup Condition="'$(DefaultOSGroup)'=='' and $([MSBuild]::IsOSPlatform('FREEBSD'))">FreeBSD</DefaultOSGroup>
-    <DefaultOSGroup Condition="'$(DefaultOSGroup)'=='' and $([MSBuild]::IsOSPlatform('NETBSD'))">NetBSD</DefaultOSGroup>
-    <DefaultOSGroup Condition="'$(DefaultOSGroup)'=='' and $([MSBuild]::IsOSUnixLike())">Linux</DefaultOSGroup>
-    <DefaultOSGroup Condition="'$(DefaultOSGroup)'==''">$(OS)</DefaultOSGroup>
+    <DefaultOSGroup Condition="'$(DefaultOSGroup)' == '' and $([MSBuild]::IsOSPlatform('FREEBSD'))">FreeBSD</DefaultOSGroup>
+    <DefaultOSGroup Condition="'$(DefaultOSGroup)' == '' and $([MSBuild]::IsOSPlatform('NETBSD'))">NetBSD</DefaultOSGroup>
+    <DefaultOSGroup Condition="'$(DefaultOSGroup)' == '' and $([MSBuild]::IsOSUnixLike())">Linux</DefaultOSGroup>
+    <DefaultOSGroup Condition="'$(DefaultOSGroup)' == ''">$(OS)</DefaultOSGroup>
   </PropertyGroup>
 
   <PropertyGroup>
@@ -72,7 +72,7 @@
     <IsTestProject Condition="$(MSBuildProjectName.EndsWith('.UnitTests')) or $(MSBuildProjectName.EndsWith('.Tests'))">true</IsTestProject>
 
     <IsTestSupportProject>false</IsTestSupportProject>
-    <IsTestSupportProject Condition="($(MSBuildProjectFullPath.Contains('\tests\')) OR $(MSBuildProjectFullPath.Contains('/tests/'))) AND '$(IsTestProject)' != 'true'">true</IsTestSupportProject>
+    <IsTestSupportProject Condition="($(MSBuildProjectFullPath.Contains('\tests\')) or $(MSBuildProjectFullPath.Contains('/tests/'))) and '$(IsTestProject)' != 'true'">true</IsTestSupportProject>
     <IsTestProject Condition="'$(IsTestSupportProject)' == 'true'">true</IsTestProject>
 
     <!-- Treat test assemblies as non-shipping (do not publish or sign them). -->
@@ -153,8 +153,8 @@
 
   <!-- set properties for each vertical -->
   <PropertyGroup>
-    <BuildingNETCoreAppVertical Condition="'$(BuildingNETCoreAppVertical)' == '' and ('$(_bc_TargetGroup)'=='$(NetCoreAppCurrent)' or '$(BuildAllConfigurations)' == 'true')">true</BuildingNETCoreAppVertical>
-    <BuildingNETFxVertical Condition="'$(BuildingNETFxVertical)' == '' and ('$(_bc_TargetGroup)'=='$(NetFrameworkCurrent)' or '$(BuildAllConfigurations)' == 'true')">true</BuildingNETFxVertical>
+    <BuildingNETCoreAppVertical Condition="'$(BuildingNETCoreAppVertical)' == '' and ('$(_bc_TargetGroup)' == '$(NetCoreAppCurrent)' or '$(BuildAllConfigurations)' == 'true')">true</BuildingNETCoreAppVertical>
+    <BuildingNETFxVertical Condition="'$(BuildingNETFxVertical)' == '' and ('$(_bc_TargetGroup)' == '$(NetFrameworkCurrent)' or '$(BuildAllConfigurations)' == 'true')">true</BuildingNETFxVertical>
     <BinPlaceNETCoreAppPackage Condition="'$(BuildingNETCoreAppVertical)' == 'true'">true</BinPlaceNETCoreAppPackage>
   </PropertyGroup>
 
@@ -192,12 +192,12 @@
 
   <!-- Set up Default symbol and optimization for Configuration -->
   <Choose>
-    <When Condition="'$(ConfigurationGroup)'=='Debug'">
+    <When Condition="'$(ConfigurationGroup)' == 'Debug'">
       <PropertyGroup>
         <DebugSymbols Condition="'$(DebugSymbols)' == ''">true</DebugSymbols>
         <Optimize Condition="'$(Optimize)' == ''">false</Optimize>
         <DebugType Condition="'$(DebugType)' == ''">full</DebugType>
-        <DefineConstants>$(DefineConstants),DEBUG,TRACE</DefineConstants>
+        <DefineConstants>$(DefineConstants),TRACE,DEBUG</DefineConstants>
       </PropertyGroup>
     </When>
     <When Condition="'$(ConfigurationGroup)' == 'Release'">
@@ -216,6 +216,7 @@
     <NoExplicitReferenceToStdLib>true</NoExplicitReferenceToStdLib>
     <AddAdditionalExplicitAssemblyReferences>false</AddAdditionalExplicitAssemblyReferences>
     <CopyNuGetImplementations>false</CopyNuGetImplementations>
+    <DisableImplicitConfigurationDefines>true</DisableImplicitConfigurationDefines>
     <!-- Don't reference implicit framework packages, all projects in this repo must be explicit -->
     <DisableImplicitFrameworkReferences>true</DisableImplicitFrameworkReferences>
     <!-- We can't generate an apphost without restoring the targeting pack. -->
@@ -288,7 +289,6 @@
   <PropertyGroup>
     <EnableDefaultItems>false</EnableDefaultItems>
     <EmbedUntrackedSources>true</EmbedUntrackedSources>
-    <DisableImplicitConfigurationDefines>true</DisableImplicitConfigurationDefines>
 
     <!-- Workaround for https://github.com/microsoft/msbuild/issues/4474 -->
     <GenerateResourceUsePreserializedResources>false</GenerateResourceUsePreserializedResources>

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs
@@ -263,33 +263,41 @@ 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
+
+            // In Debug builds of System.Diagnostics.DiagnosticSource, the child operation Id will be constructed as follows
+            // "|parent.RootId.<child.OperationName.Replace(., -)>-childCount.".
+            // This is for debugging purposes to know which operation the child Id is comming from.
+            //
+            // 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.";
+            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);
         }
 
         /// <summary>

diff --git a/...aries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs b/...aries/System.Diagnostics.DiagnosticSource/tests/DiagnosticSourceEventSourceBridgeTests.cs
@@ -913,23 +913,20 @@ 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
 
         /// <summary>
-        /// Sets the EventCount to 0 and LastEvent to null
+        /// Sets the EventCount to 0 and LastEvents to null
         /// </summary>
         public void ResetEventCountAndLastEvent()
         {
             EventCount = 0;
             LastEvent = null;
-#if DEBUG
             SecondLast = null;
             ThirdLast = null;
-#endif
         }
 
         /// <summary>
@@ -943,10 +940,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
@@ -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<object[]> 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<object[]> 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<T>() => new StackTrace();
+        [MethodImpl(MethodImplOptions.NoOptimization | MethodImplOptions.NoInlining)]
         private static StackTrace Generic<T, U>() => 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
@@ -109,7 +109,6 @@ public virtual string PayloadString(int propertyIndex, string propertyName)
         }
         public abstract IList<string> PayloadNames { get; }
 
-#if DEBUG
         /// <summary>
         /// This is a convenience function for the debugger.   It is not used typically
         /// </summary>
@@ -123,7 +122,6 @@ public List<object> PayloadValues
                 return ret;
             }
         }
-#endif
 
         public override string ToString()
         {