Add Configurations.props files for test projects, include tests in build.proj #14663

mellinoe · 2016-12-21T21:18:30Z

There were a large number of changes that needed to be made to the test projects and test code in order to get everything working in dev/eng. This gets all of the tests building and running against netcoreapp (minus one project). There is still no support for different targeting packs (netstandard1.3, etc.), so everything is still compiling against the netcoreapp targeting pack (built live).

Issues remaining:

There are some workarounds necessary because of the TypeInfo inheritance issue ( #14334 ). These are present in the tests for System.Runtime, System.Linq.Expressions, and System.Reflection.Emit.
System.IO.IsolatedStorage is failing sometimes on my machine; no idea why. It usually succeeds when re-tried. I get UnauthorizedAccessExceptions when it fails.
System.Security.AccessControl.Tests always fails for me because of a debug assertion in the product code. No idea why yet, so I disabled the test project.
System.Linq.Expressions references the source project because it uses types not exposed in the contract. This is happening in master, too, so it's not a change from before.

@joperezr @weshaggard @ericstj @karajas

joperezr · 2016-12-26T16:15:10Z

build.proj

@@ -23,6 +23,7 @@
    <Project Include="external\dir.proj" />
    <Project Include="src\ref.builds" />
    <Project Include="src\src.builds" />
+    <Project Include="src\tests.builds" />


I thought we had decided not to add tests.builds as part of the main build any longer. We should either have a separate project for the tests, or at least condition it on a variable so that it is not built by default, and only built when build-tests.cmd/sh is called.

agreed I would like to make the test build and running a separate step from build.cmd.

I forgot about that. I will add a separate project, and a separate build-tests script.

joperezr · 2016-12-26T16:17:14Z

dir.traversal.targets

@@ -65,7 +65,7 @@
  <!-- FilterProjectsToTest will filter the project list according to the FilterToTestTFM value -->
  <Target Name="FilterProjectsToTest"
          BeforeTargets="TestAllProjects"
-          Condition="$(MSBuildProjectName.EndsWith('.Tests'))">
+          Condition="$(MSBuildProjectName.EndsWith('tests'))">


We did we needed this change? looks like it would have the same effect to me.

joperezr · 2016-12-26T16:23:40Z

dir.traversal.targets

@@ -78,7 +78,7 @@
      </Project>

      <!-- Include all projects which have TestTFM from the supported set -->
-      <ProjectsToTest Include="@(Project)" Condition="'%(Project.Extension)'=='.csproj' And $([System.String]::new('%(Project.TestTFMs)').Contains(';$(FilterToTestTFM);'))">
+      <ProjectsToTest Include="@(Project)" Condition="'%(Project.Extension)'=='.csproj' And $([System.String]::new('%(Project.Identity)').Contains('.Tests'))">


I think we will still need some sort of filtering in here, as there are some verticals where we don't want to run some test projects.

Why are you doing filtering to ".Tests" but "tests" above?

As for filtering I think the filtering should be handled by build filtering. While doing a vertical build we should run the tests for any build we do. There isn't going to be the same type of multiplexing on a single test project that we had in the past. We will essentially to the matching build and always run based on the BuildConfiguration provided or defaulted.

joperezr · 2016-12-26T17:19:35Z

external/coreclr/project.json.template

@@ -3,7 +3,8 @@
    "{TFM}": {
      "dependencies": {
        "Microsoft.NETCore.Platforms": "1.2.0-beta-24721-02",
-        "Microsoft.NETCore.Runtime.CoreCLR": "1.2.0-beta-24728-02"
+        "Microsoft.NETCore.Runtime.CoreCLR": "1.2.0-beta-24728-02",
+        "runtime.native.System.Data.SqlClient.sni": "4.4.0-beta-24814-01",


how come do we need a runtime-specific package? shouldn't we depend on System.Data.SqlClient.sni instead and let the RID add this dependency?

I would like to understand why it is needed at all. Is it just for the native shim? Why wouldn't this just be the live built shim?

Because this shim isn't built here; it's still built from our internal TFS stuff.

joperezr · 2016-12-26T17:22:35Z

src/Common/tests/Configurations.props

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <PropertyGroup>


How did you generate all of these? was it done by @chcosta 's tool?

It was done with a modified version of the tool. I needed to modify the tool because the test .builds files do not map 1-to-1 with what we want in these Configurations.props files.

weshaggard · 2016-12-27T21:54:07Z

dir.targets

-    add the test-runtime project.json to the list of supplemental test data so it will get included
-    in the test archive; the stress test build system depends on it when building test mixes.
-  -->
-  <ItemGroup Condition="'$(IsTestProject)'=='true'">


@MattGal or @schaabs can correct me but I think this will end up causing some issues for the stress testing.

weshaggard · 2016-12-27T21:59:00Z

...mmon/tests/System/Net/Prerequisites/Servers/CoreFxNetCloudService/WebServer/WebServer.csproj

@@ -206,4 +206,6 @@
  <Target Name="AfterBuild">
  </Target>
  -->
+  <Target Name="AnnotateProjectsWithConfiguration" />


Why were these needed here?

Not sure, I think this was part of @joperezr 's change that I picked up.

Yeah, I added this here because this project doesn't include the dir.targets where those targets are defined, and given it is under a **\tests***.csproj path it is being considered as a test project that is required to build.

why doesn't it include dir.targets?

weshaggard · 2016-12-27T21:59:29Z

src/System.Collections.Immutable/tests/System.Collections.Immutable.Tests.csproj

@@ -3,6 +3,7 @@
  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
  <PropertyGroup>
    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <NoWarn>0436</NoWarn>


Why is this needed?

Most likely due to the ExcludeFromCodeCoverageAttribute ambiguity.

Yes, I added it everywhere that attribute was being used.

weshaggard · 2016-12-27T22:00:19Z

src/System.Collections/ref/System.Collections.csproj

@@ -2,7 +2,7 @@
 <Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
  <PropertyGroup>
-    <DefineConstants Condition="'$(TargetGroup)' == ''">$(DefineConstants);netcoreapp11</DefineConstants>
+    <DefineConstants Condition="'$(TargetGroup)' == 'netcoreapp'">$(DefineConstants);netcoreapp</DefineConstants>


We probably haven't done this yet but I would expect this to be taken care of when we add the configurations for the refs.

weshaggard · 2016-12-27T22:01:28Z

src/System.Data.SqlClient/tests/StressTests/IMonitorLoader/IMonitorLoader.csproj

@@ -5,8 +5,7 @@
    <ProjectGuid>{AF78BA88-6428-47EA-8682-442DAF8E9656}</ProjectGuid>
    <RootNamespace>Monitoring</RootNamespace>
    <AssemblyName>Monitoring</AssemblyName>
-    <OutputType>Library</OutputType>
-    <NugetTargetMoniker>.NETStandard,Version=v1.3</NugetTargetMoniker>
+    <IsTestProject>true</IsTestProject>


We should probably still change IsTestProject to look for the "/tests/" in the path.

Yeah, we should do that.

* Move test-runtime under external. * Rename RuntimeDir->RuntimePath in targetingpacks.props and Xunit.Runtime.depproj. * Remove test-runtime project.json from the SupplementalTestData list in dir.targets.

This reverts commit 1efc4d9a02d1c2eced0f20a544abcbcd0092f40c.

chcosta · 2016-12-30T22:59:49Z

src/System.Collections/tests/System.Collections.Tests.csproj

@@ -3,7 +3,7 @@
  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
  <PropertyGroup>
    <ProjectGuid>{9574CEEC-5554-411B-B44C-6CA9EC1CEB08}</ProjectGuid>
-    <DefineConstants Condition="'$(TargetGroup)'==''">$(DefineConstants);netstandard17</DefineConstants>
+    <DefineConstants Condition="'$(TargetGroup)'=='netstandard1.7'">$(DefineConstants);netstandard17</DefineConstants>


In the ref project, netcoreapp11 -> netcoreapp. Any reason not to follow a similar pattern and make this netstandard?

chcosta · 2016-12-30T23:02:54Z

src/System.IO.Compression/tests/Configurations.props

+      netstandard1.3-Windows_NT;
+      netstandard1.7-Unix;
+      netstandard1.7-Windows_NT;
+      netstandard1.7;


is netstandard entry needed? I don't think it will ever get chosen.

Hm, seems there's still some unnecessary configurations from the conversion I ran. I tried to clean up most of them.

I think I've identified that this is happening from .builds files that include a Performance test project, because it's usually included with default targets. The modified version of your conversion targets was not filtering those out, and it makes sense that it would only be a problem in test projects because src/ref projects don't have that same issue. I'm pretty sure all the files you mentioned this on have an extra configuration because of that.

chcosta · 2016-12-30T23:03:16Z

src/System.IO.Compression/tests/Performance/System.IO.Compression.Performance.Tests.csproj

@@ -1,24 +1,13 @@
 <?xml version="1.0" encoding="utf-8"?>
 <Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" DefaultTargets="Build">
-  <PropertyGroup>
-    <Configuration Condition="'$(Configuration)'==''">Windows_Debug</Configuration>
-  </PropertyGroup>
  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
  <PropertyGroup>
    <AssemblyName>System.IO.Compression.Performance.Tests</AssemblyName>


chcosta · 2016-12-30T23:04:17Z

src/System.IO.FileSystem/tests/Configurations.props

+      netstandard1.3-Windows_NT;
+      netstandard1.7-Unix;
+      netstandard1.7-Windows_NT;
+      netstandard1.7;


netstandard1.7 not needed

chcosta · 2016-12-30T23:04:34Z

src/System.IO.MemoryMappedFiles/tests/Configurations.props

+    <BuildConfigurations>
+      netstandard1.3-Unix;
+      netstandard1.3-Windows_NT;
+      netstandard1.3;


netstandard1.3 not needed

chcosta · 2016-12-30T23:04:50Z

src/System.IO.Pipes/tests/Configurations.props

+      netstandard1.3-Windows_NT;
+      netstandard1.7-Unix;
+      netstandard1.7-Windows_NT;
+      netstandard1.7;


chcosta · 2016-12-30T23:08:53Z

src/System.Runtime.Extensions/tests/Configurations.props

+      netcoreapp1.1-Unix;
+      netstandard1.5-Unix;
+      netstandard1.5-Windows_NT;
+      netstandard1.5;


netstandard1.5 not needed

chcosta · 2016-12-30T23:10:05Z

src/System.Runtime.InteropServices/tests/Configurations.props

+<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <PropertyGroup>
+    <BuildConfigurations>
+      netstandard1.7-Windows_NT;


I don't think netstandard1.7-Windows_NT is needed

Yeah, the .builds file here was already messed up (at least it looks like it to me), which is what caused this one. I'll manually clean this up.

chcosta · 2016-12-30T23:11:41Z

tests.msbuild

@@ -0,0 +1,60 @@
+<?xml version="1.0" encoding="utf-8"?>


Why is this file necessary?

It's essentially build.proj except with just the stuff related to testing. I didn't want to name it tests.proj because then there would be two .proj files in the root. We can rename it if that doesn't matter.

Why can't it just be tests.builds?

It can just be tests.builds. I was probably overthinking this because it's not exactly like the other .builds files, but that doesn't really matter. I'll rename.

…pped. Configuration still defaults to 'Debug' even if there is no valid configuration.

mellinoe · 2017-01-03T21:26:52Z

I'm going to merge this in now. CI still doesn't build tests by default, but you can do it locally with build-tests.cmd/sh after running build.cmd/sh.

…ild.proj (dotnet/corefx#14663) * [WIP]Created src\tests.proj which is mostly building all tests * Convert test project .builds to Configuration.props * Fix test paths in targetingpack.props, move test-runtime. * Move test-runtime under external. * Rename RuntimeDir->RuntimePath in targetingpacks.props and Xunit.Runtime.depproj. * Remove test-runtime project.json from the SupplementalTestData list in dir.targets. * Remove XUnit.Runtime.depproj from tests.proj * Convert test .builds files to Configuration.props. * Revert "Convert test project .builds to Configuration.props" This reverts commit dotnet/corefx@1efc4d9. * Fix compilation error in System.Collections.Concurrent.Tests * Fix condition in System.Collections ref project * Fix two more Collections test project issues * Add Configuration.props for System.Collections ref * Fix TargetGroup conditions in System.Collections.Tests.csproj. * Fix compilation problems with NameResoltion.Pal.Tests. * Fix compilation errors in SqlClient Stress tests * Fix IO.Compression.Performance tests compilation * Fix compilation and configurations of System.Net.Http.Unit.Tests.csproj. * Fix compilation issues in System.Private.Xml.Linq tests * Disable XsltScenarios.Tests project, like it was before. * Fix missing DefineConstants for System.Linq ref project. * Fix compilation of System.Net.Primitives.Pal.Tests.csproj * Fix compilation of System.Net.Primitives.UnitTests.Tests.csproj * Fix compilation of System.Security.Cryptography.Cng.Tests.csproj. * Fix compilation of System.Net.Primitives reference project * Fix compilation of System.Net.Security ref project * Fix compilation of SYstem.Net.Security.Tests.csproj * Fix configuration of X509Certificates ref project * Fix compilation errors in System.Net.Http.Unit.Tests.csproj * Fix Common.Tests.csproj * Fix System.Diagnostics.Debug.Tests.csproj * Fix Microsoft.Win32.Registry.Tests.csproj * Add runtime.native.System.Data.SqlClient.sni to external project * Fix System.Diagnostics.TraceSource.Tests.csproj * Fix System.IO.Compression.Performance.Tests.csproj * Move supplemental test data into test-runtime/project.json * Fix System.IO.FileSystem.Watcher.Tests.csproj * Fix System.IO.FileSystem.Tests.csproj * Fix System.IO.Pipes.AccessControl.Tests.csproj * Fix System.IO.MemoryMappedFiles.Tests.csproj * Fix System.IO.Pipes.Tests.csproj * Fix System.Json.Tests.csproj * Add missing supplemental test data for net security tests * Fix resource names in System.Xml.XPath.XDocument.Tests.csproj * Fix Assembly.Location test case * Fix assembly loading paths in DefaultLoadContextTest.cs * Fix issues in System.Runtime.Loader.Tests.csproj * Fix RootNamespace in ResourceManager tests * Mark OpenSSL tests as unsupported on Windows * Fix RootNamespace in System.Xml.XPath.Tests.csproj * Fix RootNamespace in System.Xml.XPath.XmlDocument.Tests.csproj * Fix System.IO.Compression.Tests.csproj * Add missing Configurations.props files for System.Net projects * Fix compilation issues in WinHttpHandler tests * Add fixes and a workaround for Linq.Expressions tests * Add workaround for System.Runtime.Tests * Use extension methods explicitly in Ref.Emit tests * Hook up test projects into build * Fix build configuration of Crypto.Primitives for netcoreapp * Enable VB tests outside Windows * Enable Mail tests outside windows * Don't run Unsafe tests outside windows * Don't build Unsafe tests outside Windows * Don't build WebServer project outside of Windows. * Fix corerun path for Unix tests. * Fix casing of 'Native' directory in NETCoreApp private pkgproj * Remove runtime-specific stuff from Xunit.Runtime.depproj * Move tests into a separate project ('tests.msbuild') which isn't built in build.proj * Remove duplicate types from NameResolution.Pal tests * Skip tests in a project if there is no compatible configuration. * Include internal socket types only on Windows in NameResolution tests * Manually fix S.R.InteropServices configurations * Rename tests.msbuild -> tests.builds * Manually fix System.Text.Encoding.Tests.csproj configuration * Manually fix System.Threading.Tests configuration * Manually clean up System.Runtime.Tests.csproj * Fix System.IO.FileSystem.Tests configurations * Manually fix System.Globalization.Tests configurations * Manually fix System.IO.Pipes.Tests configurations * Manually fix System.IO.Compression.Tests configurations * Manually fix System.Runtime.Extensions.Tests configurations * Manually fix System.IO.MemoryMappedFiles.Tests configurations * Manually fix S.Diagnostics.Process.Tests configurations * Manually fix System.ComponentModel.TypeConverter.Tests configurations * Misc PR feedback. * Use HostRunnerName instead of HostRunner in Process tests * Fix configurations for S.ServiceProcess.ServiceController.Tests * Revert to using ConfigurationErrorMsg to check if tests should be skipped. Configuration still defaults to 'Debug' even if there is no valid configuration. Commit migrated from dotnet/corefx@2c0352e

dnfclas added the cla-already-signed label Dec 21, 2016

mellinoe force-pushed the test-configurations-4 branch from a1165d8 to 003abd8 Compare December 21, 2016 22:31

karelz added area-Infrastructure-libraries dev-eng labels Dec 21, 2016

karelz assigned mellinoe Dec 21, 2016

karelz added this to the 2.0.0 milestone Dec 21, 2016

joperezr reviewed Dec 26, 2016

View reviewed changes

weshaggard reviewed Dec 27, 2016

View reviewed changes

mellinoe force-pushed the test-configurations-4 branch 3 times, most recently from 4387b3d to 3998d61 Compare December 30, 2016 20:56

mellinoe and others added 11 commits December 30, 2016 13:15

Convert test project .builds to Configuration.props

db0a98c

[WIP]Created src\tests.proj which is mostly building all tests

ce367e1

Fix test paths in targetingpack.props, move test-runtime.

9e9df0a

* Move test-runtime under external. * Rename RuntimeDir->RuntimePath in targetingpacks.props and Xunit.Runtime.depproj. * Remove test-runtime project.json from the SupplementalTestData list in dir.targets.

Remove XUnit.Runtime.depproj from tests.proj

e7cdd4e

Revert "Convert test project .builds to Configuration.props"

3a3ffc7

This reverts commit 1efc4d9a02d1c2eced0f20a544abcbcd0092f40c.

Convert test .builds files to Configuration.props.

5e36dff

Fix compilation error in System.Collections.Concurrent.Tests

4a2a19d

Fix condition in System.Collections ref project

45890cd

Fix two more Collections test project issues

5641b23

Add Configuration.props for System.Collections ref

c5c0a7e

Fix TargetGroup conditions in System.Collections.Tests.csproj.

15d8573

chcosta reviewed Dec 30, 2016

View reviewed changes

mellinoe added 17 commits January 3, 2017 10:44

Manually fix S.R.InteropServices configurations

4ea808c

Rename tests.msbuild -> tests.builds

f364a64

Manually fix System.Text.Encoding.Tests.csproj configuration

936b154

Manually fix System.Threading.Tests configuration

b3eeec8

Manually clean up System.Runtime.Tests.csproj

b1da1b8

Fix System.IO.FileSystem.Tests configurations

28d501e

Manually fix System.Globalization.Tests configurations

271a0f0

Manually fix System.IO.Pipes.Tests configurations

14c99a4

Manually fix System.IO.Compression.Tests configurations

44db323

Manually fix System.Runtime.Extensions.Tests configurations

fd1b7a5

Manually fix System.IO.MemoryMappedFiles.Tests configurations

9475c41

Manually fix S.Diagnostics.Process.Tests configurations

f59e67f

Manually fix System.ComponentModel.TypeConverter.Tests configurations

e01d14e

Misc PR feedback.

b06eae9

Use HostRunnerName instead of HostRunner in Process tests

cfd2222

Fix configurations for S.ServiceProcess.ServiceController.Tests

089a1e6

Revert to using ConfigurationErrorMsg to check if tests should be ski…

842618d

…pped. Configuration still defaults to 'Debug' even if there is no valid configuration.

mellinoe merged commit 2c0352e into dotnet:dev/eng Jan 3, 2017

Add Configurations.props files for test projects, include tests in build.proj #14663

Add Configurations.props files for test projects, include tests in build.proj #14663

Conversation

mellinoe commented Dec 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mellinoe Jan 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mellinoe commented Jan 3, 2017

mellinoe Jan 3, 2017 •

edited

Loading