Address msbuild issues with cross compiled tests in helix (dotnet#19901)

* Address msbuild issues with cross compiled tests in helix

* Fix test warning about duplicate types

build-test.cmd

@@ -121,6 +121,16 @@ if defined __BuildAgainstPackagesArg (
     )
 )
 
+set TargetsWindowsArg=
+set TargetsWindowsMsbuildArg=
+if "%__TargetsWindows%"=="1" (
+    set TargetsWindowsArg=-TargetsWindows=true
+    set TargetsWindowsMsbuildArg=/p:TargetsWindows=true
+) else if "%__TargetsWindows%"=="0" (
+    set TargetsWindowsArg=-TargetsWindows=false
+    set TargetsWindowsMsbuildArg=/p:TargetsWindows=false
+)
+
 @if defined _echo @echo on
 
 set __RunArgs=-BuildOS=%__BuildOS% -BuildType=%__BuildType% -BuildArch=%__BuildArch%
@@ -324,8 +334,9 @@ for /l %%G in (1, 1, %__BuildLoopCount%) do (
     set __MsbuildErr=/flp2:ErrorsOnly;LogFile="%__BuildErr%";Append=!__AppendToLog!
 
     set TestBuildSlice=%%G
-    echo Running: msbuild %__ProjectDir%\tests\build.proj !__MsbuildLog! !__MsbuildWrn! !__MsbuildErr! %__msbuildArgs% %__BuildAgainstPackagesMsbuildArg% !__PriorityMsbuildArg! %__UnprocessedBuildArgs%
-    call msbuild %__ProjectDir%\tests\build.proj !__MsbuildLog! !__MsbuildWrn! !__MsbuildErr! %__msbuildArgs% %__BuildAgainstPackagesMsbuildArg% !__PriorityMsbuildArg! %__UnprocessedBuildArgs%
+    echo Running: msbuild %__ProjectDir%\tests\build.proj !__MsbuildLog! !__MsbuildWrn! !__MsbuildErr! %TargetsWindowsMsbuildArg% %__msbuildArgs% %__BuildAgainstPackagesMsbuildArg% !__PriorityMsbuildArg! %__UnprocessedBuildArgs%
+
+    call msbuild %__ProjectDir%\tests\build.proj !__MsbuildLog! !__MsbuildWrn! !__MsbuildErr! %TargetsWindowsMsbuildArg% %__msbuildArgs% %__BuildAgainstPackagesMsbuildArg% !__PriorityMsbuildArg! %__UnprocessedBuildArgs%
 
     if errorlevel 1 (
         echo %__MsgPrefix%Error: build failed. Refer to the build log files for details:
@@ -398,16 +409,6 @@ if defined __RuntimeId (
     set RuntimeIdArg=-RuntimeID="%__RuntimeId%"
 )
 
-set TargetsWindowsArg=
-set TargetsWindowsMsbuildArg=
-if "%__TargetsWindows%"=="1" (
-    set TargetsWindowsArg=-TargetsWindows=true
-    set TargetsWindowsMsbuildArg=/p:TargetsWindows=true
-) else if "%__TargetsWindows%"=="0" (
-    set TargetsWindowsArg=-TargetsWindows=false
-    set TargetsWindowsMsbuildArg=/p:TargetsWindows=false
-)
-
 set __BuildLogRootName=Tests_Overlay_Managed
 set __BuildLog=%__LogsDir%\%__BuildLogRootName%_%__BuildOS%__%__BuildArch%__%__BuildType%.log
 set __BuildWrn=%__LogsDir%\%__BuildLogRootName%_%__BuildOS%__%__BuildArch%__%__BuildType%.wrn

tests/runtest.proj

@@ -57,9 +57,7 @@
   <!-- Target to check the test build, to see if it looks ok. We've had several cases where a change inadvertently and drastically changes
        the set of tests that are built, and that change is unnoticed. The most common case is for a build of the Priority 1 tests
        to only build the Priority 0 tests. This target is run after a test build to verify that the basic number of tests that were
-       built is basically what was expected. When this was written, there were about 2500 Priority 0 tests and about 12270 Priority 1
-       tests (differing slightly based on platform). We currently check that the number of Priority 0 tests is greater than 2000 and
-       less than 3000, and the number of Priority 1 tests is greater than 12000.
+       built is basically what was expected.
   -->
   <Target Name="CheckTestBuild" DependsOnTargets="GetListOfTestCmds">
     <Error Condition="!Exists('$(XunitTestBinBase)')"

tests/src/Interop/COM/Activator/Activator.csproj

@@ -12,7 +12,7 @@
 
     <!-- Test unsupported outside of windows -->
     <TestUnsupportedOutsideWindows>true</TestUnsupportedOutsideWindows>
-    <DisableProjectBuild Condition="'$(TargetsUnix)' == 'true'">true</DisableProjectBuild>
+    <DisableProjectBuild Condition="'$(TargetsWindows)' != 'true'">true</DisableProjectBuild>
   </PropertyGroup>
   <!-- Default configurations to help VS understand the configurations -->
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|x64'">

tests/src/Interop/common/Assertion.cs

@@ -15,7 +15,7 @@ namespace CoreFXTestLibrary
     /// unit tests. If the condition being tested is not met, an exception
     /// is thrown.
     /// </summary>
-    public static class Assert
+    internal static class Assert
     {
         /// <summary>
         ///     Asserts that the given delegate throws an <see cref="ArgumentNullException"/> with the given parameter name.
@@ -771,7 +771,7 @@ private static async Task<Exception> RunWithCatchAsync(Func<Task> action)
     /// <summary>
     /// Exception raised by the Assert on Fail
     /// </summary>
-    public class AssertTestException : Exception
+    internal class AssertTestException : Exception
     {
         public AssertTestException(string message)
             : base(message)
@@ -784,7 +784,7 @@ public AssertTestException()
         }
     }
 
-    public static class ExceptionAssert
+    internal static class ExceptionAssert
     {
         public static void Throws<T>(String message, Action a) where T : Exception
         {
@@ -796,7 +796,7 @@ public static void Throws<T>(String message, Action a) where T : Exception
     ///     Specifies whether <see cref="Assert.Throws{T}"/> should require an exact type match when comparing the expected exception type with the thrown exception.
     /// </summary>
     [Flags]
-    public enum AssertThrowsOptions
+    internal enum AssertThrowsOptions
     {
         /// <summary>
         ///     Specifies that <see cref="Assert.Throws{T}"/> should require an exact type 

tests/src/dir.props

@@ -78,7 +78,10 @@
     </When>
     <When Condition="'$(OSGroup)'=='Windows_NT'">
       <PropertyGroup>
-        <TargetsWindows>true</TargetsWindows>
+        <!-- Since cross compilation of test builds on Windows is possible, the
+             TargetsWindows property may already be set. Only set the property if
+             it is not already defined -->
+        <TargetsWindows Condition="'$(TargetsWindows)' == ''" >true</TargetsWindows>
         <TestNugetRuntimeId>win-$(__BuildArch)</TestNugetRuntimeId>
       </PropertyGroup>
     </When>
@@ -111,8 +114,8 @@
   </Choose>
 
   <PropertyGroup>
-    <TargetRid Condition="'$(__RuntimeId)' != ''">$(__RuntimeId)</TargetRid>
-    <TargetRid Condition="'$(__RuntimeId)' == ''">$(TestNugetRuntimeId)</TargetRid>
+    <TargetRid>$(__RuntimeId)</TargetRid>
+    <TargetRid Condition="'$(TargetRid)' == ''">$(TestNugetRuntimeId)</TargetRid>
   </PropertyGroup>
 
   <PropertyGroup>

tests/src/dir.targets

@@ -76,11 +76,11 @@
   <Import Project="..\dir.targets" />
 
   <!-- Determine if this project should be built or not -->
-  <PropertyGroup> 
-    <BuildAllProjects Condition=" '$(BuildAllProjects)' == ''">false</BuildAllProjects>
+  <PropertyGroup>
+    <BuildAllProjects Condition="'$(BuildAllProjects)' == ''">false</BuildAllProjects>
     <_WillCLRTestProjectBuild Condition="'$(_WillCLRTestProjectBuild)' == ''">false</_WillCLRTestProjectBuild>
-    <_WillCLRTestProjectBuild Condition="'$(BuildAllProjects)' != true">true</_WillCLRTestProjectBuild>
-    <_WillCLRTestProjectBuild Condition="'$(DisableProjectBuild)' != true And '$(BuildAllProjects)' == true And '$(CLRTestPriority)' &lt;= '$(CLRTestPriorityToBuild)'">true</_WillCLRTestProjectBuild>
+    <_WillCLRTestProjectBuild Condition="'$(BuildAllProjects)' != 'true'">true</_WillCLRTestProjectBuild>
+    <_WillCLRTestProjectBuild Condition="'$(DisableProjectBuild)' != 'true' And '$(BuildAllProjects)' == 'true' And '$(CLRTestPriority)' &lt;= '$(CLRTestPriorityToBuild)'">true</_WillCLRTestProjectBuild>
   </PropertyGroup>
 
   <!-- if we have determined that there is nothing to build, overwrite the build targets so that nothing happens -->
@@ -91,7 +91,7 @@
 
   <!-- We will use an imported build here in the instance that we're a non-sdk style project, have source that we need to build, and we are the correct priority...OR if we are being asked to build for
   a test with a higher priority. -->
-  <Import Project="$(ToolsDir)Build.Common.targets" Condition="('$(UsingMicrosoftNETSdk)' != 'true') And ('$(CLRTestKind)'!='RunOnly') And $(_CLRTestCompilesSource) And ('$(_WillCLRTestProjectBuild)')"/>
+  <Import Project="$(ToolsDir)Build.Common.targets" Condition="('$(UsingMicrosoftNETSdk)' != 'true') And ('$(CLRTestKind)'!='RunOnly') And $(_CLRTestCompilesSource) And ('$(_WillCLRTestProjectBuild)' == 'true')"/>
 
 
   <Import Project="..\override.targets" Condition="Exists('..\override.targets')"/>
@@ -114,7 +114,7 @@
   <Import Project="CLRTest.Execute.targets" />
   <Target Name="CreateExecuteScript" 
           AfterTargets="Build"
-          Condition="'$(GenerateRunScript)' != 'false' And ('$(_WillCLRTestProjectBuild)')"
+          Condition="'$(GenerateRunScript)' != 'false' And ('$(_WillCLRTestProjectBuild)' == 'true')"
           DependsOnTargets="GenerateExecutionScriptsInternal" />
 
   <Target Name="CopyNativeProjectBinaries">

tests/src/runonly.targets

@@ -5,7 +5,7 @@
   <Target Name="CoreCompile" />
 
   <!-- if this is a RunOnly project that has been scheduled to build, make the output directory and build any dependent projects -->
-  <Target Name="Build" Condition="('$(CLRTestKind)'=='RunOnly') And '$(_WillCLRTestProjectBuild)'">
+  <Target Name="Build" Condition="('$(CLRTestKind)'=='RunOnly') And '$(_WillCLRTestProjectBuild)' == 'true'">
     <MSBuild Projects="@(ProjectReference)" />
     <MakeDir ContinueOnError="false" Directories="$(OutputPath)" />
   </Target>